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

On Friday, September 18, 2015 at 05:24:18 AM, Thomas Chou wrote:

Convert altera_uart to driver model.

Signed-off-by: Thomas Chou thomas@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!