Re: [U-Boot] [PATCH] nios2: convert altera_uart to driver model

[Marek Vasut]

On Friday, September 18, 2015 at 05:24:18 AM, Thomas Chou wrote:
> Convert altera_uart to driver model.
> 
> Signed-off-by: Thomas Chou <tho...@wytron.com.tw>
> ---

Hi!

minor nitpicks below :)

> -typedef volatile struct {
> +struct altera_uart_regs {
>       unsigned        rxdata;         /* Rx data reg */
>       unsigned        txdata;         /* Tx data reg */
>       unsigned        status;         /* Status reg */
>       unsigned        control;        /* Control reg */
>       unsigned        divisor;        /* Baud rate divisor reg */
>       unsigned        endofpacket;    /* End-of-packet reg */

Probably make this u32 instead of unsigned, so we're in the safe.

> -} nios_uart_t;
> +};

[...]

> -static int altera_serial_init(void)
> +static int altera_uart_putc(struct udevice *dev, const char c)
>  {
> -     return 0;
> -}
> +     struct altera_uart_platdata *plat = dev->platdata;
> +     struct altera_uart_regs *const regs = plat->reg;
> 
> -#else
> +     if (!(readl(&regs->status) & NIOS_UART_TRDY))
> +             return -EAGAIN;
> 
> -static void altera_serial_setbrg(void)
> -{
> -     unsigned div;
> +     writel((unsigned char)c, &regs->txdata);

Is this type-cast needed ?

> -     div = (CONFIG_SYS_CLK_FREQ/gd->baudrate)-1;
> -     writel (div, &uart->divisor);
> +     return 0;
>  }
> 
> -static int altera_serial_init(void)
> +static int altera_uart_pending(struct udevice *dev, bool input)
>  {
> -     serial_setbrg();
> -     return 0;
> +     struct altera_uart_platdata *plat = dev->platdata;
> +     struct altera_uart_regs *const regs = plat->reg;
> +     unsigned st = readl(&regs->status);
> +
> +     if (input)
> +             return (st & NIOS_UART_RRDY) ? 1 : 0;
> +     else
> +             return (st & NIOS_UART_TMT) ? 0 : 1;

Drop the ternary please, just use return st & flag or return !(st & flag),
it's not necessary to return 0 or 1, you can safely return 0 or non-zero.

>  }
> 
> -#endif /* CONFIG_SYS_NIOS_FIXEDBAUD */
> -
> -/*-----------------------------------------------------------------------
> - * UART CONSOLE
> - *---------------------------------------------------------------------*/
> -static void altera_serial_putc(char c)
> +static int altera_uart_getc(struct udevice *dev)
>  {
> -     if (c == '\n')
> -             serial_putc ('\r');
> -     while ((readl (&uart->status) & NIOS_UART_TRDY) == 0)
> -             WATCHDOG_RESET ();
> -     writel ((unsigned char)c, &uart->txdata);
> +     struct altera_uart_platdata *plat = dev->platdata;
> +     struct altera_uart_regs *const regs = plat->reg;
> +
> +     if (readl(&regs->status) & NIOS_UART_RRDY)
> +             return (readl(&regs->rxdata) & 0xff);

Parenthesis not needed around the readl(...) & 0xff expression.

> +     else
> +             return -EAGAIN;
>  }

[...]

Thanks!
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot