Re: [U-Boot] [PATCH 2/2] nios2: convert altera_jtag_uart to driver model

[Marek Vasut]

On Sunday, September 13, 2015 at 10:32:10 AM, Thomas Chou wrote:
> Convert altera_jtag_uart to driver model.
> 
> Signed-off-by: Thomas Chou <tho...@wytron.com.tw>

[...]

> -static int altera_jtag_serial_init(void)
> +static int altera_jtaguart_putc(struct udevice *dev, const char c)
>  {
> -     return 0;
> -}
> +     struct altera_jtaguart_platdata *plat = dev->platdata;
> +     struct altera_jtaguart_regs *const regs = plat->reg;
> 
> -static void altera_jtag_serial_putc(char c)
> -{
>       while (1) {
> -             unsigned st = readl(&jtag->control);
> +             unsigned st = readl(&regs->control);
>               if (NIOS_JTAG_WSPACE(st))
>                       break;
>  #ifdef CONFIG_ALTERA_JTAG_UART_BYPASS
>               if (!(st & NIOS_JTAG_AC)) /* no connection */
> -                     return;
> +                     return 0;
>  #endif
> -             WATCHDOG_RESET();

Please keep those WATCHDOG_RESET() calls, they're needed in case you
do have a WDT on your board.

>       }
> -     writel ((unsigned char)c, &jtag->data);
> +     writel((unsigned char)c, &regs->data);
> +
> +     return 0;
>  }

[...]

> -static int altera_jtag_serial_getc(void)
> +static int altera_jtaguart_getc(struct udevice *dev)
>  {
> -     int c;
> +     struct altera_jtaguart_platdata *plat = dev->platdata;
> +     struct altera_jtaguart_regs *const regs = plat->reg;
>       unsigned val;
> 
> -     while (1) {
> -             WATCHDOG_RESET ();
> -             val = readl (&jtag->data);
> -             if (val & NIOS_JTAG_RVALID)
> -                     break;
> -     }
> -     c = val & 0x0ff;
> -     return (c);
> +     while (!((val = readl(&regs->data)) & NIOS_JTAG_RVALID))

Ewww, this is ugly. Please expand it into something like the following,
for the sake of readability.

while (1) {
        val = readl()
        if (val & cond)
                break;
}

> +             ;
> +
> +     return (val & 0xff);
>  }

Looks good otherwise, thanks ! :)

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