[U-Boot] [PATCH 2/6] serial: add UniPhier serial driver

[Marek Vasut]

On Thursday, July 10, 2014 at 12:06:50 PM, Masahiro Yamada wrote:
> Hi Marek,
> 
> 
> On Fri, 4 Jul 2014 16:40:29 +0200
> 
> Marek Vasut <marex at denx.de> wrote:
> > > +static void uniphier_serial_init(struct uniphier_serial *port)
> > > +{
> > > +	writeb(UART_LCR_WLS_8, &port->lcr);
> > > +
> > > +#define MODE_X_DIV 16
> > 
> > You can use just const unsigned here instead of #define.
> 
> I adjusted drivers/serial/serial_ns16550.c for my own.
> 
> Is using a macro here so bad?
> If so, should I also fix the line 138 of drivers/serial/serial_ns16550.c ?

You're loosing the typechecking, that's all.

> > > +static void uniphier_serial_putc(struct uniphier_serial *port, const
> > > char c) +{
> > > +	if (c == '\n')
> > > +		uniphier_serial_putc(port, '\r');
> > > +
> > > +	while (!(readb(&port->lsr) & UART_LSR_THRE))
> > > +		;
> > 
> > I think in this function, you can avoid such completely unbounded loop.
> > 
> > [...]
> 
> Why?
> Could you give me more detailed explanation?

In case the LSR_THRE bit would never get cleared, this loop would keep spinning 
forever. On the other hand, if there was some kind of a timeout, U-Boot would be 
able to continue doing at least something when exiting this loop by timing out. 
Though you're right that something would most likely have gone really bonkers if 
this LSR_THRE never got cleared.

Best regards,
Marek Vasut