Re: [U-Boot] [PATCH 6/8] serial: lpuart: Prepare the driver for DM conversion

[Stefan Agner]

On 2015-12-31 00:53, Bin Meng wrote:
> Create internal routines which take lpuart's register base as
> a parameter, in preparation for driver model conversion.
> 
> Signed-off-by: Bin Meng <bmeng...@gmail.com>
> ---
> 
>  drivers/serial/serial_lpuart.c | 146 
> +++++++++++++++++++++++++++--------------
>  1 file changed, 95 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/serial/serial_lpuart.c b/drivers/serial/serial_lpuart.c
> index 0c0ab87..fed83a6 100644
> --- a/drivers/serial/serial_lpuart.c
> +++ b/drivers/serial/serial_lpuart.c
> @@ -50,46 +50,43 @@ DECLARE_GLOBAL_DATA_PTR;
>  struct lpuart_fsl *base = (struct lpuart_fsl *)LPUART_BASE;
>  
>  #ifndef CONFIG_LPUART_32B_REG
> -static void lpuart_serial_setbrg(void)
> +static void _lpuart_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
>  {
>       u32 clk = mxc_get_clock(MXC_UART_CLK);
>       u16 sbr;
>  
> -     if (!gd->baudrate)
> -             gd->baudrate = CONFIG_BAUDRATE;
> -
> -     sbr = (u16)(clk / (16 * gd->baudrate));
> +     sbr = (u16)(clk / (16 * baudrate));
>  
>       /* place adjustment later - n/32 BRFA */
> -     __raw_writeb(sbr >> 8, &base->ubdh);
> -     __raw_writeb(sbr & 0xff, &base->ubdl);
> +     __raw_writeb(sbr >> 8, &reg->ubdh);
> +     __raw_writeb(sbr & 0xff, &reg->ubdl);
>  }
>  
> -static int lpuart_serial_getc(void)
> +static int _lpuart_serial_getc(struct lpuart_fsl *reg)
>  {
> -     while (!(__raw_readb(&base->us1) & (US1_RDRF | US1_OR)))
> +     while (!(__raw_readb(&reg->us1) & (US1_RDRF | US1_OR)))
>               WATCHDOG_RESET();
>  
>       barrier();
>  
> -     return __raw_readb(&base->ud);
> +     return __raw_readb(&reg->ud);
>  }
>  
> -static void lpuart_serial_putc(const char c)
> +static void _lpuart_serial_putc(struct lpuart_fsl *reg, const char c)
>  {
>       if (c == '\n')
> -             lpuart_serial_putc('\r');
> +             _lpuart_serial_putc(reg, '\r');
>  
> -     while (!(__raw_readb(&base->us1) & US1_TDRE))
> +     while (!(__raw_readb(&reg->us1) & US1_TDRE))
>               WATCHDOG_RESET();
>  
> -     __raw_writeb(c, &base->ud);
> +     __raw_writeb(c, &reg->ud);
>  }
>  
>  /* Test whether a character is in the RX buffer */
> -static int lpuart_serial_tstc(void)
> +static int _lpuart_serial_tstc(struct lpuart_fsl *reg)
>  {
> -     if (__raw_readb(&base->urcfifo) == 0)
> +     if (__raw_readb(&reg->urcfifo) == 0)
>               return 0;
>  
>       return 1;
> @@ -99,32 +96,57 @@ static int lpuart_serial_tstc(void)
>   * Initialise the serial port with the given baudrate. The settings
>   * are always 8 data bits, no parity, 1 stop bit, no start bits.
>   */
> -static int lpuart_serial_init(void)
> +static int _lpuart_serial_init(struct lpuart_fsl *reg)

Couldn't you just name the parameter base and get rid of the changes
below? IMHO, renaming variables is only really necessary if the current
name is misleading, which I don't think it is...

>  {
>       u8 ctrl;
>  
> -     ctrl = __raw_readb(&base->uc2);
> +     ctrl = __raw_readb(&reg->uc2);
>       ctrl &= ~UC2_RE;
>       ctrl &= ~UC2_TE;
> -     __raw_writeb(ctrl, &base->uc2);
> +     __raw_writeb(ctrl, &reg->uc2);
>  
> -     __raw_writeb(0, &base->umodem);
> -     __raw_writeb(0, &base->uc1);
> +     __raw_writeb(0, &reg->umodem);
> +     __raw_writeb(0, &reg->uc1);
>  
>       /* Disable FIFO and flush buffer */
> -     __raw_writeb(0x0, &base->upfifo);
> -     __raw_writeb(0x0, &base->utwfifo);
> -     __raw_writeb(0x1, &base->urwfifo);
> -     __raw_writeb(CFIFO_TXFLUSH | CFIFO_RXFLUSH, &base->ucfifo);
> +     __raw_writeb(0x0, &reg->upfifo);
> +     __raw_writeb(0x0, &reg->utwfifo);
> +     __raw_writeb(0x1, &reg->urwfifo);
> +     __raw_writeb(CFIFO_TXFLUSH | CFIFO_RXFLUSH, &reg->ucfifo);
>  
>       /* provide data bits, parity, stop bit, etc */
> -     lpuart_serial_setbrg();
> +     _lpuart_serial_setbrg(reg, gd->baudrate);
>  
> -     __raw_writeb(UC2_RE | UC2_TE, &base->uc2);
> +     __raw_writeb(UC2_RE | UC2_TE, &reg->uc2);
>  
>       return 0;
>  }
>  
> +static void lpuart_serial_setbrg(void)
> +{
> +     _lpuart_serial_setbrg(base, gd->baudrate);
> +}
> +
> +static int lpuart_serial_getc(void)
> +{
> +     return _lpuart_serial_getc(base);
> +}
> +
> +static void lpuart_serial_putc(const char c)
> +{
> +     _lpuart_serial_putc(base, c);
> +}
> +
> +static int lpuart_serial_tstc(void)
> +{
> +     return _lpuart_serial_tstc();
> +}
> +
> +static int lpuart_serial_init(void)
> +{
> +     return _lpuart_serial_init(base);
> +}
> +
>  static struct serial_device lpuart_serial_drv = {
>       .name = "lpuart_serial",
>       .start = lpuart_serial_init,
> @@ -136,47 +158,44 @@ static struct serial_device lpuart_serial_drv = {
>       .tstc = lpuart_serial_tstc,
>  };
>  #else
> -static void lpuart32_serial_setbrg(void)
> +static void _lpuart32_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
>  {
>       u32 clk = CONFIG_SYS_CLK_FREQ;
>       u32 sbr;
>  
> -     if (!gd->baudrate)
> -             gd->baudrate = CONFIG_BAUDRATE;
> -
> -     sbr = (clk / (16 * gd->baudrate));
> +     sbr = (clk / (16 * baudrate));
>  
>       /* place adjustment later - n/32 BRFA */
> -     out_be32(&base->baud, sbr);
> +     out_be32(&reg->baud, sbr);
>  }
>  
> -static int lpuart32_serial_getc(void)
> +static int _lpuart32_serial_getc(struct lpuart_fsl *reg)
>  {
>       u32 stat;
>  
> -     while (((stat = in_be32(&base->stat)) & STAT_RDRF) == 0) {
> -             out_be32(&base->stat, STAT_FLAGS);
> +     while (((stat = in_be32(&reg->stat)) & STAT_RDRF) == 0) {
> +             out_be32(&reg->stat, STAT_FLAGS);
>               WATCHDOG_RESET();
>       }
>  
> -     return in_be32(&base->data) & 0x3ff;
> +     return in_be32(&reg->data) & 0x3ff;
>  }
>  
> -static void lpuart32_serial_putc(const char c)
> +static void _lpuart32_serial_putc(struct lpuart_fsl *reg, const char c)
>  {
>       if (c == '\n')
> -             lpuart32_serial_putc('\r');
> +             _lpuart32_serial_putc(reg, '\r');
>  
> -     while (!(in_be32(&base->stat) & STAT_TDRE))
> +     while (!(in_be32(&reg->stat) & STAT_TDRE))
>               WATCHDOG_RESET();
>  
> -     out_be32(&base->data, c);
> +     out_be32(&reg->data, c);
>  }
>  
>  /* Test whether a character is in the RX buffer */
> -static int lpuart32_serial_tstc(void)
> +static int _lpuart32_serial_tstc(struct lpuart_fsl *reg)
>  {
> -     if ((in_be32(&base->water) >> 24) == 0)
> +     if ((in_be32(&reg->water) >> 24) == 0)
>               return 0;
>  
>       return 1;
> @@ -186,28 +205,53 @@ static int lpuart32_serial_tstc(void)
>   * Initialise the serial port with the given baudrate. The settings
>   * are always 8 data bits, no parity, 1 stop bit, no start bits.
>   */
> -static int lpuart32_serial_init(void)
> +static int _lpuart32_serial_init(struct lpuart_fsl *reg)
>  {
>       u8 ctrl;
>  
> -     ctrl = in_be32(&base->ctrl);
> +     ctrl = in_be32(&reg->ctrl);
>       ctrl &= ~CTRL_RE;
>       ctrl &= ~CTRL_TE;
> -     out_be32(&base->ctrl, ctrl);
> +     out_be32(&reg->ctrl, ctrl);
>  
> -     out_be32(&base->modir, 0);
> -     out_be32(&base->fifo, ~(FIFO_TXFE | FIFO_RXFE));
> +     out_be32(&reg->modir, 0);
> +     out_be32(&reg->fifo, ~(FIFO_TXFE | FIFO_RXFE));
>  
> -     out_be32(&base->match, 0);
> +     out_be32(&reg->match, 0);
>  
>       /* provide data bits, parity, stop bit, etc */
> -     lpuart32_serial_setbrg();
> +     _lpuart32_serial_setbrg(reg, gd->baudrate);
>  
> -     out_be32(&base->ctrl, CTRL_RE | CTRL_TE);
> +     out_be32(&reg->ctrl, CTRL_RE | CTRL_TE);
>  
>       return 0;
>  }
>  
> +static void lpuart32_serial_setbrg(void)
> +{
> +     _lpuart32_serial_setbrg(base, gd->baudrate);
> +}
> +
> +static int lpuart32_serial_getc(void)
> +{
> +     return _lpuart32_serial_getc(base);
> +}
> +
> +static void lpuart32_serial_putc(const char c)
> +{
> +     _lpuart32_serial_putc(base, c);
> +}
> +
> +static int lpuart32_serial_tstc(void)
> +{
> +     return _lpuart32_serial_tstc(base);
> +}
> +
> +static int lpuart32_serial_init(void)
> +{
> +     return _lpuart32_serial_init(base);
> +}
> +
>  static struct serial_device lpuart32_serial_drv = {
>       .name = "lpuart32_serial",
>       .start = lpuart32_serial_init,
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot