[PATCH tty/serial v2 1/1] tty: serial: imx: enable imx serial console port as module

[PATCH tty/serial v2 1/1] tty: serial: imx: enable imx serial console port as module

[Fugang Duan]

GKI kernel requires drivers built as modules including
console driver, so add the support that imx serial console
driver can be built as module.

The changes of the patch:
- imx console driver can be built as module.
- move out earlycon code to separated driver like imx_earlycon.c,
  and imx earlycon driver only support build-in.

v2:
 - fix kbuild error

Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
 drivers/tty/serial/Kconfig        | 18 +++++++----
 drivers/tty/serial/Makefile       |  1 +
 drivers/tty/serial/imx.c          | 37 ++---------------------
 drivers/tty/serial/imx_earlycon.c | 50 +++++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 40 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 780908d43557..dabd06ca9b66 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -502,20 +502,28 @@ config SERIAL_IMX
 	  can enable its onboard serial port by enabling this option.
 
 config SERIAL_IMX_CONSOLE
-	bool "Console on IMX serial port"
-	depends on SERIAL_IMX=y
+	tristate "Console on IMX serial port"
+	depends on SERIAL_IMX
 	select SERIAL_CORE_CONSOLE
-	select SERIAL_EARLYCON if OF
 	help
 	  If you have enabled the serial port on the Freescale IMX
-	  CPU you can make it the console by answering Y to this option.
+	  CPU you can make it the console by answering Y/M to this option.
 
-	  Even if you say Y here, the currently visible virtual console
+	  Even if you say Y/M here, the currently visible virtual console
 	  (/dev/tty0) will still be used as the system console by default, but
 	  you can alter that using a kernel command line option such as
 	  "console=ttymxc0". (Try "man bootparam" or see the documentation of
 	  your bootloader about how to pass options to the kernel at boot time.)
 
+config SERIAL_IMX_EARLYCON
+	bool "Earlycon on IMX serial port"
+	depends on OF
+	select SERIAL_EARLYCON
+	default y
+	help
+	  If you have enabled the earlycon on the Freescale IMX
+	  CPU you can make it the earlycon by answering Y to this option.
+
 config SERIAL_UARTLITE
 	tristate "Xilinx uartlite serial port support"
 	depends on HAS_IOMEM
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index d056ee6cca33..caf167f0c10a 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_SERIAL_ZS) += zs.o
 obj-$(CONFIG_SERIAL_SH_SCI) += sh-sci.o
 obj-$(CONFIG_SERIAL_CPM) += cpm_uart/
 obj-$(CONFIG_SERIAL_IMX) += imx.o
+obj-$(CONFIG_SERIAL_IMX_EARLYCON) += imx_earlycon.o
 obj-$(CONFIG_SERIAL_MPC52xx) += mpc52xx_uart.o
 obj-$(CONFIG_SERIAL_ICOM) += icom.o
 obj-$(CONFIG_SERIAL_MESON) += meson_uart.o
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1265e8d86d8a..6814485cf255 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -361,7 +361,7 @@ static inline int imx_uart_is_imx6q(struct imx_port *sport)
 /*
  * Save and restore functions for UCR1, UCR2 and UCR3 registers
  */
-#if defined(CONFIG_SERIAL_IMX_CONSOLE)
+#if IS_ENABLED(CONFIG_SERIAL_IMX_CONSOLE)
 static void imx_uart_ucrs_save(struct imx_port *sport,
 			       struct imx_port_ucrs *ucr)
 {
@@ -1915,7 +1915,7 @@ static const struct uart_ops imx_uart_pops = {
 
 static struct imx_port *imx_uart_ports[UART_NR];
 
-#ifdef CONFIG_SERIAL_IMX_CONSOLE
+#if IS_ENABLED(CONFIG_SERIAL_IMX_CONSOLE)
 static void imx_uart_console_putchar(struct uart_port *port, int ch)
 {
 	struct imx_port *sport = (struct imx_port *)port;
@@ -2112,39 +2112,6 @@ static struct console imx_uart_console = {
 
 #define IMX_CONSOLE	&imx_uart_console
 
-#ifdef CONFIG_OF
-static void imx_uart_console_early_putchar(struct uart_port *port, int ch)
-{
-	struct imx_port *sport = (struct imx_port *)port;
-
-	while (imx_uart_readl(sport, IMX21_UTS) & UTS_TXFULL)
-		cpu_relax();
-
-	imx_uart_writel(sport, ch, URTX0);
-}
-
-static void imx_uart_console_early_write(struct console *con, const char *s,
-					 unsigned count)
-{
-	struct earlycon_device *dev = con->data;
-
-	uart_console_write(&dev->port, s, count, imx_uart_console_early_putchar);
-}
-
-static int __init
-imx_console_early_setup(struct earlycon_device *dev, const char *opt)
-{
-	if (!dev->port.membase)
-		return -ENODEV;
-
-	dev->con->write = imx_uart_console_early_write;
-
-	return 0;
-}
-OF_EARLYCON_DECLARE(ec_imx6q, "fsl,imx6q-uart", imx_console_early_setup);
-OF_EARLYCON_DECLARE(ec_imx21, "fsl,imx21-uart", imx_console_early_setup);
-#endif
-
 #else
 #define IMX_CONSOLE	NULL
 #endif
diff --git a/drivers/tty/serial/imx_earlycon.c b/drivers/tty/serial/imx_earlycon.c
new file mode 100644
index 000000000000..795606e1a22f
--- /dev/null
+++ b/drivers/tty/serial/imx_earlycon.c
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2020 NXP
+ */
+
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/init.h>
+#include <linux/serial_core.h>
+#include <linux/serial.h>
+#include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/io.h>
+
+#define URTX0 0x40 /* Transmitter Register */
+#define UTS_TXFULL (1<<4) /* TxFIFO full */
+#define IMX21_UTS 0xb4 /* UART Test Register on all other i.mx*/
+
+static void imx_uart_console_early_putchar(struct uart_port *port, int ch)
+{
+	while (readl_relaxed(port->membase + IMX21_UTS) & UTS_TXFULL)
+		cpu_relax();
+
+	writel_relaxed(ch, port->membase + URTX0);
+}
+
+static void imx_uart_console_early_write(struct console *con, const char *s,
+					 unsigned count)
+{
+	struct earlycon_device *dev = con->data;
+
+	uart_console_write(&dev->port, s, count, imx_uart_console_early_putchar);
+}
+
+static int __init
+imx_console_early_setup(struct earlycon_device *dev, const char *opt)
+{
+	if (!dev->port.membase)
+		return -ENODEV;
+
+	dev->con->write = imx_uart_console_early_write;
+
+	return 0;
+}
+OF_EARLYCON_DECLARE(ec_imx6q, "fsl,imx6q-uart", imx_console_early_setup);
+OF_EARLYCON_DECLARE(ec_imx21, "fsl,imx21-uart", imx_console_early_setup);
+
+MODULE_AUTHOR("NXP");
+MODULE_DESCRIPTION("IMX earlycon driver");
+MODULE_LICENSE("GPL");
-- 
2.17.1

Re: [PATCH tty/serial v2 1/1] tty: serial: imx: enable imx serial console port as module

[Greg KH]

On Fri, Jul 17, 2020 at 04:18:27PM +0800, Fugang Duan wrote:
> GKI kernel requires drivers built as modules including
> console driver, so add the support that imx serial console
> driver can be built as module.
> 
> The changes of the patch:
> - imx console driver can be built as module.
> - move out earlycon code to separated driver like imx_earlycon.c,
>   and imx earlycon driver only support build-in.
> 
> v2:
>  - fix kbuild error
> 
> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> ---
>  drivers/tty/serial/Kconfig        | 18 +++++++----
>  drivers/tty/serial/Makefile       |  1 +
>  drivers/tty/serial/imx.c          | 37 ++---------------------
>  drivers/tty/serial/imx_earlycon.c | 50 +++++++++++++++++++++++++++++++
>  4 files changed, 66 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 780908d43557..dabd06ca9b66 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -502,20 +502,28 @@ config SERIAL_IMX
>  	  can enable its onboard serial port by enabling this option.
>  
>  config SERIAL_IMX_CONSOLE
> -	bool "Console on IMX serial port"
> -	depends on SERIAL_IMX=y
> +	tristate "Console on IMX serial port"
> +	depends on SERIAL_IMX
>  	select SERIAL_CORE_CONSOLE
> -	select SERIAL_EARLYCON if OF
>  	help
>  	  If you have enabled the serial port on the Freescale IMX
> -	  CPU you can make it the console by answering Y to this option.
> +	  CPU you can make it the console by answering Y/M to this option.
>  
> -	  Even if you say Y here, the currently visible virtual console
> +	  Even if you say Y/M here, the currently visible virtual console
>  	  (/dev/tty0) will still be used as the system console by default, but
>  	  you can alter that using a kernel command line option such as
>  	  "console=ttymxc0". (Try "man bootparam" or see the documentation of
>  	  your bootloader about how to pass options to the kernel at boot time.)
>  
> +config SERIAL_IMX_EARLYCON
> +	bool "Earlycon on IMX serial port"
> +	depends on OF
> +	select SERIAL_EARLYCON
> +	default y

Only put "default y" if you can not boot a machine without this.
Otherwise leave the default line off, it will be 'n' and can be selected
if a user wants that.

That should be the case here, please change this.

thanks,

greg k-h

Re: [PATCH tty/serial v2 1/1] tty: serial: imx: enable imx serial console port as module

[Greg KH]

On Fri, Jul 17, 2020 at 04:18:27PM +0800, Fugang Duan wrote:
> GKI kernel requires drivers built as modules including
> console driver, so add the support that imx serial console
> driver can be built as module.

No one knows what "GKI" is, and it isn't relevant here.

> 
> The changes of the patch:
> - imx console driver can be built as module.
> - move out earlycon code to separated driver like imx_earlycon.c,
>   and imx earlycon driver only support build-in.

Why is this not 2 separate patches?

> v2:
>  - fix kbuild error

These v2: stuff should go below the --- line.

thanks,

greg k-h

RE: [EXT] Re: [PATCH tty/serial v2 1/1] tty: serial: imx: enable imx serial console port as module

[Andy Duan]

From: Greg KH <gregkh@linuxfoundation.org> Sent: Friday, July 17, 2020 4:58 PM
> On Fri, Jul 17, 2020 at 04:18:27PM +0800, Fugang Duan wrote:
> > GKI kernel requires drivers built as modules including console driver,
> > so add the support that imx serial console driver can be built as
> > module.
> >
> > The changes of the patch:
> > - imx console driver can be built as module.
> > - move out earlycon code to separated driver like imx_earlycon.c,
> >   and imx earlycon driver only support build-in.
> >
> > v2:
> >  - fix kbuild error
> >
> > Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> > ---
> >  drivers/tty/serial/Kconfig        | 18 +++++++----
> >  drivers/tty/serial/Makefile       |  1 +
> >  drivers/tty/serial/imx.c          | 37 ++---------------------
> >  drivers/tty/serial/imx_earlycon.c | 50
> > +++++++++++++++++++++++++++++++
> >  4 files changed, 66 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> > index 780908d43557..dabd06ca9b66 100644
> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -502,20 +502,28 @@ config SERIAL_IMX
> >         can enable its onboard serial port by enabling this option.
> >
> >  config SERIAL_IMX_CONSOLE
> > -     bool "Console on IMX serial port"
> > -     depends on SERIAL_IMX=y
> > +     tristate "Console on IMX serial port"
> > +     depends on SERIAL_IMX
> >       select SERIAL_CORE_CONSOLE
> > -     select SERIAL_EARLYCON if OF
> >       help
> >         If you have enabled the serial port on the Freescale IMX
> > -       CPU you can make it the console by answering Y to this option.
> > +       CPU you can make it the console by answering Y/M to this option.
> >
> > -       Even if you say Y here, the currently visible virtual console
> > +       Even if you say Y/M here, the currently visible virtual
> > + console
> >         (/dev/tty0) will still be used as the system console by default, but
> >         you can alter that using a kernel command line option such as
> >         "console=ttymxc0". (Try "man bootparam" or see the
> documentation of
> >         your bootloader about how to pass options to the kernel at
> > boot time.)
> >
> > +config SERIAL_IMX_EARLYCON
> > +     bool "Earlycon on IMX serial port"
> > +     depends on OF
> > +     select SERIAL_EARLYCON
> > +     default y
> 
> Only put "default y" if you can not boot a machine without this.
> Otherwise leave the default line off, it will be 'n' and can be selected if a user
> wants that.
> 
> That should be the case here, please change this.

I will change it to "default n" in next version, thanks.
> 
> thanks,
> 
> greg k-h

RE: [EXT] Re: [PATCH tty/serial v2 1/1] tty: serial: imx: enable imx serial console port as module

[Andy Duan]

From: Greg KH <gregkh@linuxfoundation.org> Sent: Friday, July 17, 2020 4:59 PM
> On Fri, Jul 17, 2020 at 04:18:27PM +0800, Fugang Duan wrote:
> > GKI kernel requires drivers built as modules including console driver,
> > so add the support that imx serial console driver can be built as
> > module.
> 
> No one knows what "GKI" is, and it isn't relevant here.
The Next Generation Android Device kernel will consist of generic
Kernel Image (GKI) and several GKI modules, and drivers for specific
hardware (possibly closed-source drivers) will be loaded as kernel modules. 
In short, in this scenario, everything soc vendor driver is modularized. 

Anyway, it isn't relevant here. Will delete the information in v3.
> 
> >
> > The changes of the patch:
> > - imx console driver can be built as module.
> > - move out earlycon code to separated driver like imx_earlycon.c,
> >   and imx earlycon driver only support build-in.
> 
> Why is this not 2 separate patches?
It can be split into 2 separated patches, will update it in v3.

> 
> > v2:
> >  - fix kbuild error
> 
> These v2: stuff should go below the --- line.

Got it, thanks for your comments.
> 
> thanks,
> 
> greg k-h

Re: [EXT] Re: [PATCH tty/serial v2 1/1] tty: serial: imx: enable imx serial console port as module

[Greg KH]

On Fri, Jul 17, 2020 at 09:33:20AM +0000, Andy Duan wrote:
> From: Greg KH <gregkh@linuxfoundation.org> Sent: Friday, July 17, 2020 4:58 PM
> > On Fri, Jul 17, 2020 at 04:18:27PM +0800, Fugang Duan wrote:
> > > GKI kernel requires drivers built as modules including console driver,
> > > so add the support that imx serial console driver can be built as
> > > module.
> > >
> > > The changes of the patch:
> > > - imx console driver can be built as module.
> > > - move out earlycon code to separated driver like imx_earlycon.c,
> > >   and imx earlycon driver only support build-in.
> > >
> > > v2:
> > >  - fix kbuild error
> > >
> > > Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> > > ---
> > >  drivers/tty/serial/Kconfig        | 18 +++++++----
> > >  drivers/tty/serial/Makefile       |  1 +
> > >  drivers/tty/serial/imx.c          | 37 ++---------------------
> > >  drivers/tty/serial/imx_earlycon.c | 50
> > > +++++++++++++++++++++++++++++++
> > >  4 files changed, 66 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> > > index 780908d43557..dabd06ca9b66 100644
> > > --- a/drivers/tty/serial/Kconfig
> > > +++ b/drivers/tty/serial/Kconfig
> > > @@ -502,20 +502,28 @@ config SERIAL_IMX
> > >         can enable its onboard serial port by enabling this option.
> > >
> > >  config SERIAL_IMX_CONSOLE
> > > -     bool "Console on IMX serial port"
> > > -     depends on SERIAL_IMX=y
> > > +     tristate "Console on IMX serial port"
> > > +     depends on SERIAL_IMX
> > >       select SERIAL_CORE_CONSOLE
> > > -     select SERIAL_EARLYCON if OF
> > >       help
> > >         If you have enabled the serial port on the Freescale IMX
> > > -       CPU you can make it the console by answering Y to this option.
> > > +       CPU you can make it the console by answering Y/M to this option.
> > >
> > > -       Even if you say Y here, the currently visible virtual console
> > > +       Even if you say Y/M here, the currently visible virtual
> > > + console
> > >         (/dev/tty0) will still be used as the system console by default, but
> > >         you can alter that using a kernel command line option such as
> > >         "console=ttymxc0". (Try "man bootparam" or see the
> > documentation of
> > >         your bootloader about how to pass options to the kernel at
> > > boot time.)
> > >
> > > +config SERIAL_IMX_EARLYCON
> > > +     bool "Earlycon on IMX serial port"
> > > +     depends on OF
> > > +     select SERIAL_EARLYCON
> > > +     default y
> > 
> > Only put "default y" if you can not boot a machine without this.
> > Otherwise leave the default line off, it will be 'n' and can be selected if a user
> > wants that.
> > 
> > That should be the case here, please change this.
> 
> I will change it to "default n" in next version, thanks.

No, just remove the line, the default is 'n' with no default specified.

thanks,

greg k-h

RE: [EXT] Re: [PATCH tty/serial v2 1/1] tty: serial: imx: enable imx serial console port as module

[Andy Duan]

From: Greg KH <gregkh@linuxfoundation.org> Sent: Friday, July 17, 2020 5:42 PM
> On Fri, Jul 17, 2020 at 09:33:20AM +0000, Andy Duan wrote:
> > From: Greg KH <gregkh@linuxfoundation.org> Sent: Friday, July 17, 2020
> > 4:58 PM
> > > On Fri, Jul 17, 2020 at 04:18:27PM +0800, Fugang Duan wrote:
> > > > GKI kernel requires drivers built as modules including console
> > > > driver, so add the support that imx serial console driver can be
> > > > built as module.
> > > >
> > > > The changes of the patch:
> > > > - imx console driver can be built as module.
> > > > - move out earlycon code to separated driver like imx_earlycon.c,
> > > >   and imx earlycon driver only support build-in.
> > > >
> > > > v2:
> > > >  - fix kbuild error
> > > >
> > > > Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> > > > ---
> > > >  drivers/tty/serial/Kconfig        | 18 +++++++----
> > > >  drivers/tty/serial/Makefile       |  1 +
> > > >  drivers/tty/serial/imx.c          | 37 ++---------------------
> > > >  drivers/tty/serial/imx_earlycon.c | 50
> > > > +++++++++++++++++++++++++++++++
> > > >  4 files changed, 66 insertions(+), 40 deletions(-)
> > > >
> > > > diff --git a/drivers/tty/serial/Kconfig
> > > > b/drivers/tty/serial/Kconfig index 780908d43557..dabd06ca9b66
> > > > 100644
> > > > --- a/drivers/tty/serial/Kconfig
> > > > +++ b/drivers/tty/serial/Kconfig
> > > > @@ -502,20 +502,28 @@ config SERIAL_IMX
> > > >         can enable its onboard serial port by enabling this option.
> > > >
> > > >  config SERIAL_IMX_CONSOLE
> > > > -     bool "Console on IMX serial port"
> > > > -     depends on SERIAL_IMX=y
> > > > +     tristate "Console on IMX serial port"
> > > > +     depends on SERIAL_IMX
> > > >       select SERIAL_CORE_CONSOLE
> > > > -     select SERIAL_EARLYCON if OF
> > > >       help
> > > >         If you have enabled the serial port on the Freescale IMX
> > > > -       CPU you can make it the console by answering Y to this option.
> > > > +       CPU you can make it the console by answering Y/M to this
> option.
> > > >
> > > > -       Even if you say Y here, the currently visible virtual console
> > > > +       Even if you say Y/M here, the currently visible virtual
> > > > + console
> > > >         (/dev/tty0) will still be used as the system console by default,
> but
> > > >         you can alter that using a kernel command line option such
> as
> > > >         "console=ttymxc0". (Try "man bootparam" or see the
> > > documentation of
> > > >         your bootloader about how to pass options to the kernel at
> > > > boot time.)
> > > >
> > > > +config SERIAL_IMX_EARLYCON
> > > > +     bool "Earlycon on IMX serial port"
> > > > +     depends on OF
> > > > +     select SERIAL_EARLYCON
> > > > +     default y
> > >
> > > Only put "default y" if you can not boot a machine without this.
> > > Otherwise leave the default line off, it will be 'n' and can be
> > > selected if a user wants that.
> > >
> > > That should be the case here, please change this.
> >
> > I will change it to "default n" in next version, thanks.
> 
> No, just remove the line, the default is 'n' with no default specified.

Got it, it makes sense. Thanks!
> 
> thanks,
> 
> greg k-h