Re: [PATCH] tty: serial: samsung: support driver modulization

[Krzysztof Kozlowski]

On Sun, 1 Dec 2019 at 09:05, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Sun, Dec 01, 2019 at 04:59:14PM +0900, Hyunki Koo wrote:
> > From: Shinbeom Choi <sbeom.choi@xxxxxxxxxxx>
> >
> > This commit enables modulization of samsung uart driver.
> >
> > There was no way to make use of this driver in other module,
> > because uart functions were static.
> >
> > By exporting required functions, user can use this driver
> > in other module.
> >
> > Signed-off-by: Shinbeom Choi <sbeom.choi@xxxxxxxxxxx>
> > Signed-off-by: Hyunki Koo <hyunki00.koo@xxxxxxxxxxx>
> > ---
> >  drivers/tty/serial/samsung.h     | 32 ++++++++++++
> >  drivers/tty/serial/samsung_tty.c | 85 +++++++++++++++-----------------
> >  2 files changed, 73 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/tty/serial/samsung.h b/drivers/tty/serial/samsung.h
> > index f93022113f59..25be0962284d 100644
> > --- a/drivers/tty/serial/samsung.h
> > +++ b/drivers/tty/serial/samsung.h
> > @@ -144,4 +144,36 @@ static inline void s3c24xx_clear_bit(struct uart_port *port, int idx,
> >       local_irq_restore(flags);
> >  }
> >
> > +#if defined(CONFIG_ARCH_EXYNOS)
> > +#define EXYNOS_COMMON_SERIAL_DRV_DATA                                \
> > +     .info = &(struct s3c24xx_uart_info) {                   \
> > +             .name           = "Samsung Exynos UART",        \
> > +             .type           = PORT_S3C6400,                 \
> > +             .has_divslot    = 1,                            \
> > +             .rx_fifomask    = S5PV210_UFSTAT_RXMASK,        \
> > +             .rx_fifoshift   = S5PV210_UFSTAT_RXSHIFT,       \
> > +             .rx_fifofull    = S5PV210_UFSTAT_RXFULL,        \
> > +             .tx_fifofull    = S5PV210_UFSTAT_TXFULL,        \
> > +             .tx_fifomask    = S5PV210_UFSTAT_TXMASK,        \
> > +             .tx_fifoshift   = S5PV210_UFSTAT_TXSHIFT,       \
> > +             .def_clk_sel    = S3C2410_UCON_CLKSEL0,         \
> > +             .num_clks       = 1,                            \
> > +             .clksel_mask    = 0,                            \
> > +             .clksel_shift   = 0,                            \
> > +     },                                                      \
> > +     .def_cfg = &(struct s3c2410_uartcfg) {                  \
> > +             .ucon           = S5PV210_UCON_DEFAULT,         \
> > +             .ufcon          = S5PV210_UFCON_DEFAULT,        \
> > +             .has_fracval    = 1,                            \
> > +     }                                                       \
> > +
> > +#endif
> > +
> > +int s3c24xx_serial_get_ports(struct s3c24xx_uart_port **ourport, int index);
> > +int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
> > +                                 struct platform_device *platdev);
> > +int s3c24xx_serial_unregister_port(struct platform_device *dev);
> > +int s3c24xx_serial_suspend(struct device *dev);
> > +int s3c24xx_serial_resume(struct device *dev);
> > +int s3c24xx_serial_resume_noirq(struct device *dev);
> >  #endif
> > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
> > index 83fd51607741..15414ecd9008 100644
> > --- a/drivers/tty/serial/samsung_tty.c
> > +++ b/drivers/tty/serial/samsung_tty.c
> > @@ -1735,7 +1735,7 @@ static int s3c24xx_serial_enable_baudclk(struct s3c24xx_uart_port *ourport)
> >   * initialise a single serial port from the platform device given
> >   */
> >
> > -static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
> > +int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
> >                                   struct platform_device *platdev)
> >  {
> >       struct uart_port *port = &ourport->port;
> > @@ -1842,12 +1842,24 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
> >       /* reset the fifos (and setup the uart) */
> >       s3c24xx_serial_resetport(port, cfg);
> >
> > +     if (!s3c24xx_uart_drv.state) {
> > +             ret = uart_register_driver(&s3c24xx_uart_drv);
> > +             if (ret < 0) {
> > +                     dev_err(port->dev, "Failed to register Samsung UART driver\n");
> > +                     return ret;
> > +             }
> > +     }
> > +
> > +     dbg("%s: adding port\n", __func__);
> > +     uart_add_one_port(&s3c24xx_uart_drv, &ourport->port);
> > +
> >       return 0;
> >
> >  err:
> >       port->mapbase = 0;
> >       return ret;
> >  }
> > +EXPORT_SYMBOL_GPL(s3c24xx_serial_init_port);
>
> Why are you exporting all of these functions?  What other code uses
> them?  Why are you converting them all to global functions I don't see
> any other in-kernel callers, so why are those changes needed here?
>
> totally confused,

I cannot find the original email from Hyunki on mailing lists (neither
LKML nor serial) so this was not even public till Greg replied.

Anyway, probably this is for new Android and some out-of-tree usage...
but it is wrong.

Best regards,
Krzysztof