[U-Boot] dm: sh: serial: Add support driver model

This adds driver model support with this driver. This was tested by Koelsch
board and Gose board.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---
 drivers/serial/serial_sh.c           | 299 ++++++++++++++++++++++++-----------
 drivers/serial/serial_sh.h           |  10 +-
 include/dm/platform_data/serial_sh.h |  37 +++++
 3 files changed, 245 insertions(+), 101 deletions(-)
 create mode 100644 include/dm/platform_data/serial_sh.h

Hi Nobuhiro,

On 9 December 2014 at 22:44, Nobuhiro Iwamatsu
<nobuhiro.iwamatsu.yj@renesas.com> wrote:
> This adds driver model support with this driver. This was tested by Koelsch
> board and Gose board.
>
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
> ---
>  drivers/serial/serial_sh.c           | 299 ++++++++++++++++++++++++-----------
>  drivers/serial/serial_sh.h           |  10 +-
>  include/dm/platform_data/serial_sh.h |  37 +++++
>  3 files changed, 245 insertions(+), 101 deletions(-)
>  create mode 100644 include/dm/platform_data/serial_sh.h
>

Nice to see this patch.

> diff --git a/drivers/serial/serial_sh.c b/drivers/serial/serial_sh.c
> index 7c1f271..dd5f9bf 100644
> --- a/drivers/serial/serial_sh.c
> +++ b/drivers/serial/serial_sh.c
> @@ -1,78 +1,20 @@
>  /*
>   * SuperH SCIF device driver.
>   * Copyright (C) 2013  Renesas Electronics Corporation
> - * Copyright (C) 2007,2008,2010 Nobuhiro Iwamatsu
> + * Copyright (C) 2007,2008,2010, 2014 Nobuhiro Iwamatsu
>   * Copyright (C) 2002 - 2008  Paul Mundt
>   *
>   * SPDX-License-Identifier:    GPL-2.0+
>   */
>
>  #include <common.h>
> +#include <dm.h>
>  #include <asm/io.h>
>  #include <asm/processor.h>
> -#include "serial_sh.h"
>  #include <serial.h>
>  #include <linux/compiler.h>
> -
> -#if defined(CONFIG_CONS_SCIF0)
> -# define SCIF_BASE     SCIF0_BASE
> -#elif defined(CONFIG_CONS_SCIF1)
> -# define SCIF_BASE     SCIF1_BASE
> -#elif defined(CONFIG_CONS_SCIF2)
> -# define SCIF_BASE     SCIF2_BASE
> -#elif defined(CONFIG_CONS_SCIF3)
> -# define SCIF_BASE     SCIF3_BASE
> -#elif defined(CONFIG_CONS_SCIF4)
> -# define SCIF_BASE     SCIF4_BASE
> -#elif defined(CONFIG_CONS_SCIF5)
> -# define SCIF_BASE     SCIF5_BASE
> -#elif defined(CONFIG_CONS_SCIF6)
> -# define SCIF_BASE     SCIF6_BASE
> -#elif defined(CONFIG_CONS_SCIF7)
> -# define SCIF_BASE     SCIF7_BASE
> -#else
> -# error "Default SCIF doesn't set....."
> -#endif
> -
> -#if defined(CONFIG_SCIF_A)
> -       #define SCIF_BASE_PORT  PORT_SCIFA
> -#else
> -       #define SCIF_BASE_PORT  PORT_SCIF
> -#endif
> -
> -static struct uart_port sh_sci = {
> -       .membase        = (unsigned char*)SCIF_BASE,
> -       .mapbase        = SCIF_BASE,
> -       .type           = SCIF_BASE_PORT,
> -};
> -
> -static void sh_serial_setbrg(void)
> -{
> -       DECLARE_GLOBAL_DATA_PTR;
> -#ifdef CONFIG_SCIF_USE_EXT_CLK
> -       unsigned short dl = DL_VALUE(gd->baudrate, CONFIG_SH_SCIF_CLK_FREQ);
> -       sci_out(&sh_sci, DL, dl);
> -       /* Need wait: Clock * 1/dl × 1/16 */
> -       udelay((1000000 * dl * 16 / CONFIG_SYS_CLK_FREQ) * 1000 + 1);
> -#else
> -       sci_out(&sh_sci, SCBRR,
> -               SCBRR_VALUE(gd->baudrate, CONFIG_SH_SCIF_CLK_FREQ));
> -#endif
> -}
> -
> -static int sh_serial_init(void)
> -{
> -       sci_out(&sh_sci, SCSCR , SCSCR_INIT(&sh_sci));
> -       sci_out(&sh_sci, SCSCR , SCSCR_INIT(&sh_sci));
> -       sci_out(&sh_sci, SCSMR, 0);
> -       sci_out(&sh_sci, SCSMR, 0);
> -       sci_out(&sh_sci, SCFCR, SCFCR_RFRST|SCFCR_TFRST);
> -       sci_in(&sh_sci, SCFCR);
> -       sci_out(&sh_sci, SCFCR, 0);
> -
> -       serial_setbrg();
> -       return 0;
> -}
> +#include <dm/platform_data/serial_sh.h>
> +#include "serial_sh.h"
>
>  #if defined(CONFIG_CPU_SH7760) || \
>         defined(CONFIG_CPU_SH7780) || \
> @@ -109,83 +51,249 @@ static int scif_rxfill(struct uart_port *port)
>  }
>  #endif
>
> -static int serial_rx_fifo_level(void)
> +static void sh_serial_init_generic(struct uart_port *port)
> +{
> +       sci_out(port, SCSCR , SCSCR_INIT(port));
> +       sci_out(port, SCSCR , SCSCR_INIT(port));
> +       sci_out(port, SCSMR, 0);
> +       sci_out(port, SCSMR, 0);
> +       sci_out(port, SCFCR, SCFCR_RFRST|SCFCR_TFRST);
> +       sci_in(port, SCFCR);
> +       sci_out(port, SCFCR, 0);
> +}
> +
> +static void
> +sh_serial_setbrg_generic(struct uart_port *port, int clk, int baudrate)
>  {
> -       return scif_rxfill(&sh_sci);
> +       if (port->clk_mode == EXT_CLK) {
> +               unsigned short dl = DL_VALUE(baudrate, clk);
> +               sci_out(port, DL, dl);
> +               /* Need wait: Clock * 1/dl × 1/16 */
> +               udelay((1000000 * dl * 16 / clk) * 1000 + 1);
> +       } else {
> +               sci_out(port, SCBRR, SCBRR_VALUE(baudrate, clk));
> +       }
>  }
>
> -static void handle_error(void)
> +static void handle_error(struct uart_port *port)
>  {
> -       sci_in(&sh_sci, SCxSR);
> -       sci_out(&sh_sci, SCxSR, SCxSR_ERROR_CLEAR(&sh_sci));
> -       sci_in(&sh_sci, SCLSR);
> -       sci_out(&sh_sci, SCLSR, 0x00);
> +       sci_in(port, SCxSR);
> +       sci_out(port, SCxSR, SCxSR_ERROR_CLEAR(port));
> +       sci_in(port, SCLSR);
> +       sci_out(port, SCLSR, 0x00);
>  }
>
> -static void serial_raw_putc(const char c)
> +static void serial_raw_putc(struct uart_port *port, const char c)
>  {
>         while (1) {
>                 /* Tx fifo is empty */
> -               if (sci_in(&sh_sci, SCxSR) & SCxSR_TEND(&sh_sci))
> +               if (sci_in(port, SCxSR) & SCxSR_TEND(port))
>                         break;
>         }

There should be no loop. Just return -EAGAIN if you cannot write.

>
> -       sci_out(&sh_sci, SCxTDR, c);
> -       sci_out(&sh_sci, SCxSR, sci_in(&sh_sci, SCxSR) & ~SCxSR_TEND(&sh_sci));
> +       sci_out(port, SCxTDR, c);
> +       sci_out(port, SCxSR, sci_in(port, SCxSR) & ~SCxSR_TEND(port));
>  }
>
> -static void sh_serial_putc(const char c)
> +static void sh_serial_putc_generic(struct uart_port *port, const char c)
>  {
>         if (c == '\n')
> -               serial_raw_putc('\r');
> -       serial_raw_putc(c);
> +               serial_raw_putc(port, '\r');

You don't need to write \r when you see \n. Driver model does this.

> +       serial_raw_putc(port, c);
>  }
>
> -static int sh_serial_tstc(void)
> +static int serial_rx_fifo_level(struct uart_port *port)
>  {
> -       if (sci_in(&sh_sci, SCxSR) & SCIF_ERRORS) {
> -               handle_error();
> +       return scif_rxfill(port);
> +}
> +
> +static int sh_serial_tstc_generic(struct uart_port *port)
> +{
> +       if (sci_in(port, SCxSR) & SCIF_ERRORS) {
> +               handle_error(port);
>                 return 0;
>         }
>
> -       return serial_rx_fifo_level() ? 1 : 0;
> +       return serial_rx_fifo_level(port) ? 1 : 0;
>  }
>
> -
> -static int serial_getc_check(void)
> +static int serial_getc_check(struct uart_port *port)
>  {
>         unsigned short status;
>
> -       status = sci_in(&sh_sci, SCxSR);
> +       status = sci_in(port, SCxSR);
>
>         if (status & SCIF_ERRORS)
> -               handle_error();
> -       if (sci_in(&sh_sci, SCLSR) & SCxSR_ORER(&sh_sci))
> -               handle_error();
> -       return status & (SCIF_DR | SCxSR_RDxF(&sh_sci));
> +               handle_error(port);
> +       if (sci_in(port, SCLSR) & SCxSR_ORER(port))
> +               handle_error(port);
> +       return status & (SCIF_DR | SCxSR_RDxF(port));
>  }
>
> -static int sh_serial_getc(void)
> +static int sh_serial_getc_generic(struct uart_port *port)
>  {
>         unsigned short status;
>         char ch;
>
> -       while (!serial_getc_check())
> +       while (!serial_getc_check(port))
>                 ;

There should be no loop. Just return -EAGAIN if you cannot read.

>
> -       ch = sci_in(&sh_sci, SCxRDR);
> -       status = sci_in(&sh_sci, SCxSR);
> +       ch = sci_in(port, SCxRDR);
> +       status = sci_in(port, SCxSR);
>
> -       sci_out(&sh_sci, SCxSR, SCxSR_RDxF_CLEAR(&sh_sci));
> +       sci_out(port, SCxSR, SCxSR_RDxF_CLEAR(port));
>
>         if (status & SCIF_ERRORS)
> -                       handle_error();
> +               handle_error(port);
> +
> +       if (sci_in(port, SCLSR) & SCxSR_ORER(port))
> +               handle_error(port);
>
> -       if (sci_in(&sh_sci, SCLSR) & SCxSR_ORER(&sh_sci))
> -               handle_error();
>         return ch;
>  }
>
> +#ifdef CONFIG_DM_SERIAL

How about just convert all SH boards to driver model?

> +
> +static int sh_serial_pending(struct udevice *dev, bool input)
> +{
> +       struct uart_port *priv = dev_get_priv(dev);
> +
> +       return sh_serial_tstc_generic(priv);
> +}
> +
> +static int sh_serial_putc(struct udevice *dev, const char ch)
> +{
> +       struct uart_port *priv = dev_get_priv(dev);
> +
> +       sh_serial_putc_generic(priv, ch);
> +
> +       return 0;
> +}
> +
> +static int sh_serial_getc(struct udevice *dev)
> +{
> +       struct uart_port *priv = dev_get_priv(dev);
> +
> +       return sh_serial_getc_generic(priv);
> +}
> +
> +static int sh_serial_setbrg(struct udevice *dev, int baudrate)
> +{
> +       struct sh_serial_platdata *plat = dev_get_platdata(dev);
> +       struct uart_port *priv = dev_get_priv(dev);
> +
> +       sh_serial_setbrg_generic(priv, plat->clk, baudrate);
> +
> +       return 0;
> +}
> +
> +static int sh_serial_probe(struct udevice *dev)
> +{
> +       struct sh_serial_platdata *plat = dev_get_platdata(dev);
> +       struct uart_port *priv = dev_get_priv(dev);
> +
> +       priv->membase   = (unsigned char *)plat->base;
> +       priv->mapbase   = plat->base;
> +       priv->type      = plat->type;
> +       priv->clk_mode  = plat->clk_mode;
> +
> +       sh_serial_init_generic(priv);
> +
> +       return 0;
> +}
> +
> +static const struct dm_serial_ops sh_serial_ops = {
> +       .putc = sh_serial_putc,
> +       .pending = sh_serial_pending,
> +       .getc = sh_serial_getc,
> +       .setbrg = sh_serial_setbrg,
> +};
> +
> +U_BOOT_DRIVER(serial_sh) = {
> +       .name   = "serial_sh",
> +       .id     = UCLASS_SERIAL,
> +       .probe  = sh_serial_probe,
> +       .ops    = &sh_serial_ops,
> +       .flags  = DM_FLAG_PRE_RELOC,
> +       .priv_auto_alloc_size = sizeof(struct uart_port),
> +};
> +
> +#else /* CONFIG_DM_SERIAL */
> +
> +#if defined(CONFIG_CONS_SCIF0)
> +# define SCIF_BASE     SCIF0_BASE
> +#elif defined(CONFIG_CONS_SCIF1)
> +# define SCIF_BASE     SCIF1_BASE
> +#elif defined(CONFIG_CONS_SCIF2)
> +# define SCIF_BASE     SCIF2_BASE
> +#elif defined(CONFIG_CONS_SCIF3)
> +# define SCIF_BASE     SCIF3_BASE
> +#elif defined(CONFIG_CONS_SCIF4)
> +# define SCIF_BASE     SCIF4_BASE
> +#elif defined(CONFIG_CONS_SCIF5)
> +# define SCIF_BASE     SCIF5_BASE
> +#elif defined(CONFIG_CONS_SCIF6)
> +# define SCIF_BASE     SCIF6_BASE
> +#elif defined(CONFIG_CONS_SCIF7)
> +# define SCIF_BASE     SCIF7_BASE
> +#else
> +# error "Default SCIF doesn't set....."
> +#endif
> +
> +#if defined(CONFIG_SCIF_A)
> +       #define SCIF_BASE_PORT  PORT_SCIFA
> +#else
> +       #define SCIF_BASE_PORT  PORT_SCIF
> +#endif
> +
> +static struct uart_port sh_sci = {
> +       .membase        = (unsigned char *)SCIF_BASE,
> +       .mapbase        = SCIF_BASE,
> +       .type           = SCIF_BASE_PORT,
> +#ifdef CONFIG_SCIF_USE_EXT_CLK
> +       .clk_mode =     EXT_CLK,
> +#endif
> +};
> +
> +static void sh_serial_setbrg(void)
> +{
> +       DECLARE_GLOBAL_DATA_PTR;
> +       struct uart_port *port = &sh_sci;
> +
> +       sh_serial_setbrg_generic(port, CONFIG_SH_SCIF_CLK_FREQ, gd->baudrate);
> +}
> +
> +static int sh_serial_init(void)
> +{
> +       struct uart_port *port = &sh_sci;
> +
> +       sh_serial_init_generic(port);
> +       serial_setbrg();
> +
> +       return 0;
> +}
> +
> +static void sh_serial_putc(const char c)
> +{
> +       struct uart_port *port = &sh_sci;
> +
> +       sh_serial_putc_generic(port, c);
> +}
> +
> +static int sh_serial_tstc(void)
> +{
> +       struct uart_port *port = &sh_sci;
> +
> +       return sh_serial_tstc_generic(port);
> +}
> +
> +static int sh_serial_getc(void)
> +{
> +       struct uart_port *port = &sh_sci;
> +
> +       return sh_serial_getc_generic(port);
> +}
> +
>  static struct serial_device sh_serial_drv = {
>         .name   = "sh_serial",
>         .start  = sh_serial_init,
> @@ -206,3 +314,4 @@ __weak struct serial_device *default_serial_console(void)
>  {
>         return &sh_serial_drv;
>  }
> +#endif /* CONFIG_DM_SERIAL */
> diff --git a/drivers/serial/serial_sh.h b/drivers/serial/serial_sh.h
> index ef88c8f..4b6199a 100644
> --- a/drivers/serial/serial_sh.h
> +++ b/drivers/serial/serial_sh.h
> @@ -2,18 +2,16 @@
>   * Copy and modify from linux/drivers/serial/sh-sci.h
>   */
>
> +#include <dm/platform_data/serial_sh.h>
> +
>  struct uart_port {
>         unsigned long   iobase;         /* in/out[bwl] */
>         unsigned char   *membase;       /* read/write[bwl] */
>         unsigned long   mapbase;        /* for ioremap */
> -       unsigned int    type;           /* port type */
> +       enum sh_serial_type type;       /* port type */
> +       enum sh_clk_mode clk_mode;      /* clock mode */
>  };
>
> -#define PORT_SCI       52
> -#define PORT_SCIF      53
> -#define PORT_SCIFA     83
> -#define PORT_SCIFB     93
> -
>  #if defined(CONFIG_H83007) || defined(CONFIG_H83068)
>  #include <asm/regs306x.h>
>  #endif
> diff --git a/include/dm/platform_data/serial_sh.h b/include/dm/platform_data/serial_sh.h
> new file mode 100644
> index 0000000..0271ad6
> --- /dev/null
> +++ b/include/dm/platform_data/serial_sh.h
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (c) 2014  Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
> + * Copyright (c) 2014  Renesas Electronics Corporation
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef __serial_sh_h
> +#define __serial_sh_h
> +
> +enum sh_clk_mode {
> +       INT_CLK,
> +       EXT_CLK,
> +};
> +
> +enum sh_serial_type {
> +       PORT_SCI,
> +       PORT_SCIF,
> +       PORT_SCIFA,
> +       PORT_SCIFB,
> +};
> +
> +/*
> + * Information about SCIF port
> + *
> + * @base:      Register base address
> + * @clk:       Input clock rate, used for calculating the baud rate divisor
> + * @clk_mode:  Clock mode, set internal (INT) or external (EXT)
> + * @type:      Type of SCIF
> + */
> +struct sh_serial_platdata {
> +       unsigned long base;
> +       unsigned int clk;
> +       enum sh_clk_mode clk_mode;
> +       enum sh_serial_type type;
> +};
> +#endif /* __serial_sh_h */
> --
> 2.1.3
>

Regards,
Simon

Hi,

Thanks for your review.

2014-12-11 13:49 GMT+09:00 Simon Glass <sjg@chromium.org>:
> Hi Nobuhiro,
>
> On 9 December 2014 at 22:44, Nobuhiro Iwamatsu
> <nobuhiro.iwamatsu.yj@renesas.com> wrote:
>> This adds driver model support with this driver. This was tested by Koelsch
>> board and Gose board.
>>
>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
>> ---
>>  drivers/serial/serial_sh.c           | 299 ++++++++++++++++++++++++-----------
>>  drivers/serial/serial_sh.h           |  10 +-
>>  include/dm/platform_data/serial_sh.h |  37 +++++
>>  3 files changed, 245 insertions(+), 101 deletions(-)
>>  create mode 100644 include/dm/platform_data/serial_sh.h
>>
>
> Nice to see this patch.
>
>> diff --git a/drivers/serial/serial_sh.c b/drivers/serial/serial_sh.c
>> index 7c1f271..dd5f9bf 100644
>> --- a/drivers/serial/serial_sh.c
>> +++ b/drivers/serial/serial_sh.c
>> @@ -1,78 +1,20 @@
>>  /*
>>   * SuperH SCIF device driver.
>>   * Copyright (C) 2013  Renesas Electronics Corporation
>> - * Copyright (C) 2007,2008,2010 Nobuhiro Iwamatsu
>> + * Copyright (C) 2007,2008,2010, 2014 Nobuhiro Iwamatsu
>>   * Copyright (C) 2002 - 2008  Paul Mundt
>>   *
>>   * SPDX-License-Identifier:    GPL-2.0+
>>   */
>>
>>  #include <common.h>
>> +#include <dm.h>
>>  #include <asm/io.h>
>>  #include <asm/processor.h>
>> -#include "serial_sh.h"
>>  #include <serial.h>
>>  #include <linux/compiler.h>
>> -
>> -#if defined(CONFIG_CONS_SCIF0)
>> -# define SCIF_BASE     SCIF0_BASE
>> -#elif defined(CONFIG_CONS_SCIF1)
>> -# define SCIF_BASE     SCIF1_BASE
>> -#elif defined(CONFIG_CONS_SCIF2)
>> -# define SCIF_BASE     SCIF2_BASE
>> -#elif defined(CONFIG_CONS_SCIF3)
>> -# define SCIF_BASE     SCIF3_BASE
>> -#elif defined(CONFIG_CONS_SCIF4)
>> -# define SCIF_BASE     SCIF4_BASE
>> -#elif defined(CONFIG_CONS_SCIF5)
>> -# define SCIF_BASE     SCIF5_BASE
>> -#elif defined(CONFIG_CONS_SCIF6)
>> -# define SCIF_BASE     SCIF6_BASE
>> -#elif defined(CONFIG_CONS_SCIF7)
>> -# define SCIF_BASE     SCIF7_BASE
>> -#else
>> -# error "Default SCIF doesn't set....."
>> -#endif
>> -
>> -#if defined(CONFIG_SCIF_A)
>> -       #define SCIF_BASE_PORT  PORT_SCIFA
>> -#else
>> -       #define SCIF_BASE_PORT  PORT_SCIF
>> -#endif
>> -
>> -static struct uart_port sh_sci = {
>> -       .membase        = (unsigned char*)SCIF_BASE,
>> -       .mapbase        = SCIF_BASE,
>> -       .type           = SCIF_BASE_PORT,
>> -};
>> -
>> -static void sh_serial_setbrg(void)
>> -{
>> -       DECLARE_GLOBAL_DATA_PTR;
>> -#ifdef CONFIG_SCIF_USE_EXT_CLK
>> -       unsigned short dl = DL_VALUE(gd->baudrate, CONFIG_SH_SCIF_CLK_FREQ);
>> -       sci_out(&sh_sci, DL, dl);
>> -       /* Need wait: Clock * 1/dl × 1/16 */
>> -       udelay((1000000 * dl * 16 / CONFIG_SYS_CLK_FREQ) * 1000 + 1);
>> -#else
>> -       sci_out(&sh_sci, SCBRR,
>> -               SCBRR_VALUE(gd->baudrate, CONFIG_SH_SCIF_CLK_FREQ));
>> -#endif
>> -}
>> -
>> -static int sh_serial_init(void)
>> -{
>> -       sci_out(&sh_sci, SCSCR , SCSCR_INIT(&sh_sci));
>> -       sci_out(&sh_sci, SCSCR , SCSCR_INIT(&sh_sci));
>> -       sci_out(&sh_sci, SCSMR, 0);
>> -       sci_out(&sh_sci, SCSMR, 0);
>> -       sci_out(&sh_sci, SCFCR, SCFCR_RFRST|SCFCR_TFRST);
>> -       sci_in(&sh_sci, SCFCR);
>> -       sci_out(&sh_sci, SCFCR, 0);
>> -
>> -       serial_setbrg();
>> -       return 0;
>> -}
>> +#include <dm/platform_data/serial_sh.h>
>> +#include "serial_sh.h"
>>
>>  #if defined(CONFIG_CPU_SH7760) || \
>>         defined(CONFIG_CPU_SH7780) || \
>> @@ -109,83 +51,249 @@ static int scif_rxfill(struct uart_port *port)
>>  }
>>  #endif
>>
>> -static int serial_rx_fifo_level(void)
>> +static void sh_serial_init_generic(struct uart_port *port)
>> +{
>> +       sci_out(port, SCSCR , SCSCR_INIT(port));
>> +       sci_out(port, SCSCR , SCSCR_INIT(port));
>> +       sci_out(port, SCSMR, 0);
>> +       sci_out(port, SCSMR, 0);
>> +       sci_out(port, SCFCR, SCFCR_RFRST|SCFCR_TFRST);
>> +       sci_in(port, SCFCR);
>> +       sci_out(port, SCFCR, 0);
>> +}
>> +
>> +static void
>> +sh_serial_setbrg_generic(struct uart_port *port, int clk, int baudrate)
>>  {
>> -       return scif_rxfill(&sh_sci);
>> +       if (port->clk_mode == EXT_CLK) {
>> +               unsigned short dl = DL_VALUE(baudrate, clk);
>> +               sci_out(port, DL, dl);
>> +               /* Need wait: Clock * 1/dl × 1/16 */
>> +               udelay((1000000 * dl * 16 / clk) * 1000 + 1);
>> +       } else {
>> +               sci_out(port, SCBRR, SCBRR_VALUE(baudrate, clk));
>> +       }
>>  }
>>
>> -static void handle_error(void)
>> +static void handle_error(struct uart_port *port)
>>  {
>> -       sci_in(&sh_sci, SCxSR);
>> -       sci_out(&sh_sci, SCxSR, SCxSR_ERROR_CLEAR(&sh_sci));
>> -       sci_in(&sh_sci, SCLSR);
>> -       sci_out(&sh_sci, SCLSR, 0x00);
>> +       sci_in(port, SCxSR);
>> +       sci_out(port, SCxSR, SCxSR_ERROR_CLEAR(port));
>> +       sci_in(port, SCLSR);
>> +       sci_out(port, SCLSR, 0x00);
>>  }
>>
>> -static void serial_raw_putc(const char c)
>> +static void serial_raw_putc(struct uart_port *port, const char c)
>>  {
>>         while (1) {
>>                 /* Tx fifo is empty */
>> -               if (sci_in(&sh_sci, SCxSR) & SCxSR_TEND(&sh_sci))
>> +               if (sci_in(port, SCxSR) & SCxSR_TEND(port))
>>                         break;
>>         }
>
> There should be no loop. Just return -EAGAIN if you cannot write.

OK, I will fix this.

>
>>
>> -       sci_out(&sh_sci, SCxTDR, c);
>> -       sci_out(&sh_sci, SCxSR, sci_in(&sh_sci, SCxSR) & ~SCxSR_TEND(&sh_sci));
>> +       sci_out(port, SCxTDR, c);
>> +       sci_out(port, SCxSR, sci_in(port, SCxSR) & ~SCxSR_TEND(port));
>>  }
>>
>> -static void sh_serial_putc(const char c)
>> +static void sh_serial_putc_generic(struct uart_port *port, const char c)
>>  {
>>         if (c == '\n')
>> -               serial_raw_putc('\r');
>> -       serial_raw_putc(c);
>> +               serial_raw_putc(port, '\r');
>
> You don't need to write \r when you see \n. Driver model does this.

likewise.

>
>> +       serial_raw_putc(port, c);
>>  }
>>
>> -static int sh_serial_tstc(void)
>> +static int serial_rx_fifo_level(struct uart_port *port)
>>  {
>> -       if (sci_in(&sh_sci, SCxSR) & SCIF_ERRORS) {
>> -               handle_error();
>> +       return scif_rxfill(port);
>> +}
>> +
>> +static int sh_serial_tstc_generic(struct uart_port *port)
>> +{
>> +       if (sci_in(port, SCxSR) & SCIF_ERRORS) {
>> +               handle_error(port);
>>                 return 0;
>>         }
>>
>> -       return serial_rx_fifo_level() ? 1 : 0;
>> +       return serial_rx_fifo_level(port) ? 1 : 0;
>>  }
>>
>> -
>> -static int serial_getc_check(void)
>> +static int serial_getc_check(struct uart_port *port)
>>  {
>>         unsigned short status;
>>
>> -       status = sci_in(&sh_sci, SCxSR);
>> +       status = sci_in(port, SCxSR);
>>
>>         if (status & SCIF_ERRORS)
>> -               handle_error();
>> -       if (sci_in(&sh_sci, SCLSR) & SCxSR_ORER(&sh_sci))
>> -               handle_error();
>> -       return status & (SCIF_DR | SCxSR_RDxF(&sh_sci));
>> +               handle_error(port);
>> +       if (sci_in(port, SCLSR) & SCxSR_ORER(port))
>> +               handle_error(port);
>> +       return status & (SCIF_DR | SCxSR_RDxF(port));
>>  }
>>
>> -static int sh_serial_getc(void)
>> +static int sh_serial_getc_generic(struct uart_port *port)
>>  {
>>         unsigned short status;
>>         char ch;
>>
>> -       while (!serial_getc_check())
>> +       while (!serial_getc_check(port))
>>                 ;
>
> There should be no loop. Just return -EAGAIN if you cannot read.

likewise.

>
>>
>> -       ch = sci_in(&sh_sci, SCxRDR);
>> -       status = sci_in(&sh_sci, SCxSR);
>> +       ch = sci_in(port, SCxRDR);
>> +       status = sci_in(port, SCxSR);
>>
>> -       sci_out(&sh_sci, SCxSR, SCxSR_RDxF_CLEAR(&sh_sci));
>> +       sci_out(port, SCxSR, SCxSR_RDxF_CLEAR(port));
>>
>>         if (status & SCIF_ERRORS)
>> -                       handle_error();
>> +               handle_error(port);
>> +
>> +       if (sci_in(port, SCLSR) & SCxSR_ORER(port))
>> +               handle_error(port);
>>
>> -       if (sci_in(&sh_sci, SCLSR) & SCxSR_ORER(&sh_sci))
>> -               handle_error();
>>         return ch;
>>  }
>>
>> +#ifdef CONFIG_DM_SERIAL
>
> How about just convert all SH boards to driver model?

I have this plan, of course. But  I think that start form rmobile first.

Best regards,
  Nobuhiro