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

["구현기/HYUN-KI KOO" <hyunki00.koo@samsung.com>]

To support module for Samsung serial driver,
I would like to split the file into 2 files.
Because it cannot be supported in one file both early console and
module driver
Thus some function need to change to EXPORT_SYMBOL to use in module
driver file.
I'm not pushed yet for module driver.

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Friday, December 6, 2019 1:03 AM
> To: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Hyunki Koo <kkoos00@naver.com>; jslaby@suse.com; linux-
> serial@vger.kernel.org; linux-kernel@vger.kernel.org;
> kkoos00@gmail.com; Shinbeom Choi <sbeom.choi@samsung.com>; Hyunki Koo
> <hyunki00.koo@samsung.com>
> Subject: Re: [PATCH] tty: serial: samsung: support driver modulization
> 
> On Thu, Dec 05, 2019 at 04:36:48PM +0100, Krzysztof Kozlowski wrote:
> > On Sun, 1 Dec 2019 at 09:05, Greg KH <gregkh@linuxfoundation.org>
> wrote:
> > >
> > > On Sun, Dec 01, 2019 at 04:59:14PM +0900, Hyunki Koo wrote:
> > > > From: Shinbeom Choi <sbeom.choi@samsung.com>
> > > >
> > > > 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@samsung.com>
> > > > Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com>
> > > > ---
> > > >  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.
> 
> I think it might have been sent in html format.
> 
> > Anyway, probably this is for new Android and some out-of-tree
> usage...
> > but it is wrong.
> 
> Making the driver be able to be built as a module is a good thing, no
> matter what project is causing the work to have happen.
> 
> And yes, it's Android :)
> 
> thanks,
> 
> greg k-h