linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] tty: serial: samsung: support driver modulization
       [not found] <20191201075914.23512-1-kkoos00@naver.com>
@ 2019-12-01  8:03 ` Greg KH
  2019-12-05 15:36   ` Krzysztof Kozlowski
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2019-12-01  8:03 UTC (permalink / raw)
  To: Hyunki Koo
  Cc: jslaby, linux-serial, linux-kernel, kkoos00, Shinbeom Choi, Hyunki Koo

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,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] tty: serial: samsung: support driver modulization
  2019-12-01  8:03 ` [PATCH] tty: serial: samsung: support driver modulization Greg KH
@ 2019-12-05 15:36   ` Krzysztof Kozlowski
  2019-12-05 16:02     ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2019-12-05 15:36 UTC (permalink / raw)
  To: Greg KH
  Cc: Hyunki Koo, jslaby, linux-serial, linux-kernel, kkoos00,
	Shinbeom Choi, Hyunki Koo

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.

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

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] tty: serial: samsung: support driver modulization
  2019-12-05 15:36   ` Krzysztof Kozlowski
@ 2019-12-05 16:02     ` Greg KH
  2019-12-06  1:44       ` 구현기/HYUN-KI KOO
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2019-12-05 16:02 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Hyunki Koo, jslaby, linux-serial, linux-kernel, kkoos00,
	Shinbeom Choi, Hyunki Koo

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: [PATCH] tty: serial: samsung: support driver modulization
  2019-12-05 16:02     ` Greg KH
@ 2019-12-06  1:44       ` 구현기/HYUN-KI KOO
  2019-12-06  6:40         ` 'Greg KH'
  2019-12-06  8:09         ` Krzysztof Kozlowski
  0 siblings, 2 replies; 6+ messages in thread
From: 구현기/HYUN-KI KOO @ 2019-12-06  1:44 UTC (permalink / raw)
  To: 'Greg KH', 'Krzysztof Kozlowski'
  Cc: 'Hyunki Koo',
	jslaby, linux-serial, linux-kernel, kkoos00,
	'Shinbeom Choi'

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


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] tty: serial: samsung: support driver modulization
  2019-12-06  1:44       ` 구현기/HYUN-KI KOO
@ 2019-12-06  6:40         ` 'Greg KH'
  2019-12-06  8:09         ` Krzysztof Kozlowski
  1 sibling, 0 replies; 6+ messages in thread
From: 'Greg KH' @ 2019-12-06  6:40 UTC (permalink / raw)
  To: ������/HYUN-KI KOO
  Cc: 'Krzysztof Kozlowski', 'Hyunki Koo',
	jslaby, linux-serial, linux-kernel, kkoos00,
	'Shinbeom Choi'


A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Fri, Dec 06, 2019 at 10:44:20AM +0900, ������/HYUN-KI KOO wrote:
> To support module for Samsung serial driver,
> I would like to split the file into 2 files.

But you did not do that here in this patch, right?

> 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.

I do not understand, this patch feels wrong and incomplete as-is, right?
Please fix it up to work properly.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] tty: serial: samsung: support driver modulization
  2019-12-06  1:44       ` 구현기/HYUN-KI KOO
  2019-12-06  6:40         ` 'Greg KH'
@ 2019-12-06  8:09         ` Krzysztof Kozlowski
  1 sibling, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2019-12-06  8:09 UTC (permalink / raw)
  To: 구현기/HYUN-KI KOO
  Cc: Greg KH, Hyunki Koo, jslaby, linux-serial, linux-kernel, kkoos00,
	Shinbeom Choi

On Fri, 6 Dec 2019 at 02:44, 구현기/HYUN-KI KOO <hyunki00.koo@samsung.com> wrote:
>
> 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.

Hi,

You sent few patches independently of each other. If we did not ask,
we would not get the idea why you need them. This is not how the Linux
kernel development works. Instead, start preparing a proper patchset
to achieve your goal. This patchset will contain multiple patches. It
should go with a cover letter (see git format-patch) explaining why
you do it. This way reviewers will know the big picture. The entire
work should not break existing systems (see comments for IRQ combiner
or pinctrl). You should consider how your change will affect existing
systems.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-06  8:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20191201075914.23512-1-kkoos00@naver.com>
2019-12-01  8:03 ` [PATCH] tty: serial: samsung: support driver modulization Greg KH
2019-12-05 15:36   ` Krzysztof Kozlowski
2019-12-05 16:02     ` Greg KH
2019-12-06  1:44       ` 구현기/HYUN-KI KOO
2019-12-06  6:40         ` 'Greg KH'
2019-12-06  8:09         ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).