[PATCH] tty: serial: serial-omap: depend on !8250_omap

[PATCH] tty: serial: serial-omap: depend on !8250_omap

[Sebastian Andrzej Siewior]

Technically speaking this is not required. If both are enabled then the
Maikefile order says that 8250 one wins, the second is never probed.

If we choose to enable 8250_omap via defconfig then one might get supprised
that his console isn't working anymore since nothing says use ttySx
instead ttyOx.
This patch _tries_ to bring this to the users' attention by not showing
the serial-omap driver once the 8250 one is enabled. So the user might
choose to use the help text which says that this driver (8250_omap)
uses ttySx instead ttyOx.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
This is my attempt to warn the defconfig user of the defconfig change
(which did not yet happen). Any suggestions?

 drivers/tty/serial/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index e71a28b4b94e..1b1bdf946fee 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1125,7 +1125,7 @@ config SERIAL_OF_PLATFORM
 
 config SERIAL_OMAP
 	tristate "OMAP serial port support"
-	depends on ARCH_OMAP2PLUS
+	depends on ARCH_OMAP2PLUS && !SERIAL_8250_OMAP
 	select SERIAL_CORE
 	help
 	  If you have a machine based on an Texas Instruments OMAP CPU you
-- 
2.1.3

Re: [PATCH] tty: serial: serial-omap: depend on !8250_omap

[Sebastian Andrzej Siewior]

* Sebastian Andrzej Siewior | 2014-11-26 23:01:46 [+0100]:

>Technically speaking this is not required. If both are enabled then the
>Maikefile order says that 8250 one wins, the second is never probed.
>
>If we choose to enable 8250_omap via defconfig then one might get supprised
>that his console isn't working anymore since nothing says use ttySx
>instead ttyOx.
>This patch _tries_ to bring this to the users' attention by not showing
>the serial-omap driver once the 8250 one is enabled. So the user might
>choose to use the help text which says that this driver (8250_omap)
>uses ttySx instead ttyOx.
>
>Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>---
>This is my attempt to warn the defconfig user of the defconfig change
>(which did not yet happen). Any suggestions?

This is was Uwe Kleine-König suggested off-list:

|Currently there are two drivers for the serial device. Until the new one
|matured enough to drop the old one, let them conflict each other. They
|both handle the same devices, but only one can be responsible for a
|single device. There is no technical need, but having two drivers in the
|same kernel might result in surprises.

I personally don't mind having two drivers enabled since the makefile
order is always the same. My main concern is the ttyOx -> ttySx switch
after the newer driver is enabled by default via defconfig (and the user
does not know it). "make oldconfig" is covered by "default n".
Uwe's has a point about "matured enough to drop the old one". It isn't
mentioned anywhere that the newer driver supports also DMA. Is this
something we want to add to the help text?
Could someone that will probably be dealling with possible fallout comment
on this? This patch shouldn't go in without an ACK.

Sebastian

Re: [PATCH] tty: serial: serial-omap: depend on !8250_omap

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [141129 01:50]:
> * Sebastian Andrzej Siewior | 2014-11-26 23:01:46 [+0100]:
> 
> >Technically speaking this is not required. If both are enabled then the
> >Maikefile order says that 8250 one wins, the second is never probed.
> >
> >If we choose to enable 8250_omap via defconfig then one might get supprised
> >that his console isn't working anymore since nothing says use ttySx
> >instead ttyOx.
> >This patch _tries_ to bring this to the users' attention by not showing
> >the serial-omap driver once the 8250 one is enabled. So the user might
> >choose to use the help text which says that this driver (8250_omap)
> >uses ttySx instead ttyOx.
> >
> >Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> >---
> >This is my attempt to warn the defconfig user of the defconfig change
> >(which did not yet happen). Any suggestions?
> 
> This is was Uwe Kleine-König suggested off-list:
> 
> |Currently there are two drivers for the serial device. Until the new one
> |matured enough to drop the old one, let them conflict each other. They
> |both handle the same devices, but only one can be responsible for a
> |single device. There is no technical need, but having two drivers in the
> |same kernel might result in surprises.
> 
> I personally don't mind having two drivers enabled since the makefile
> order is always the same. My main concern is the ttyOx -> ttySx switch
> after the newer driver is enabled by default via defconfig (and the user
> does not know it). "make oldconfig" is covered by "default n".
> Uwe's has a point about "matured enough to drop the old one". It isn't
> mentioned anywhere that the newer driver supports also DMA. Is this
> something we want to add to the help text?
> Could someone that will probably be dealling with possible fallout comment
> on this? This patch shouldn't go in without an ACK.

Well the nightmare userspace switch from ttyS to ttyO few years ago is
something we want to avoid.. I think the best solution would be to make
serial-omap.c transparently provide support for ttyO using the new 8250
code so both ttyS and ttyO devices would just work. Otherwise it will
be years of "my serial port stopped working" questions again.

Regards,

Tony

Re: [PATCH] tty: serial: serial-omap: depend on !8250_omap

[One Thousand Gnomes]

> Well the nightmare userspace switch from ttyS to ttyO few years ago is
> something we want to avoid.. I think the best solution would be to make
> serial-omap.c transparently provide support for ttyO using the new 8250
> code so both ttyS and ttyO devices would just work. Otherwise it will
> be years of "my serial port stopped working" questions again.

Thata a udev problem not a kernel one surely.

Alan

Re: [PATCH] tty: serial: serial-omap: depend on !8250_omap

[Tony Lindgren]

* One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> [141201 06:11]:
> > Well the nightmare userspace switch from ttyS to ttyO few years ago is
> > something we want to avoid.. I think the best solution would be to make
> > serial-omap.c transparently provide support for ttyO using the new 8250
> > code so both ttyS and ttyO devices would just work. Otherwise it will
> > be years of "my serial port stopped working" questions again.
> 
> Thata a udev problem not a kernel one surely.

How do you suggest we get people to update their kernel command line
and inittab? Udev may not even be installed.

Regards,

Tony

Re: [PATCH] tty: serial: serial-omap: depend on !8250_omap

[Sebastian Andrzej Siewior]

On 12/01/2014 05:38 PM, Tony Lindgren wrote:
> * One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> [141201 06:11]:
>>> Well the nightmare userspace switch from ttyS to ttyO few years ago is
>>> something we want to avoid.. I think the best solution would be to make
>>> serial-omap.c transparently provide support for ttyO using the new 8250
>>> code so both ttyS and ttyO devices would just work. Otherwise it will
>>> be years of "my serial port stopped working" questions again.
>>
>> Thata a udev problem not a kernel one surely.
> 
> How do you suggest we get people to update their kernel command line
> and inittab? Udev may not even be installed.

There are three use cases that I can think of right now:
- people that enable that new driver via oldconfig
  I would expect that they read the help message in Kconfig. No worry
  about them.

- people that get a complete system via magic_build_tool (may yocto or
  whatever)
  If $TOOL decides to use the new driver, then it should update
  commandline in bootloader. Those things create usually bootloader +
  kernel + rootfile system. If the commandline is saved on flash/mmc
  then it won't be reset from default. However udev should help here.
  So not a problem either (udev can't fix the kernel boot output but we
  should see atleast the login console).

- people that build omap2plus_defconfig and we switch to the new driver
  Those people get switched from one driver to the other without
  knowing. This is what I tried to bring to everyone's attention. The
  defconfig hasn't been changed yet so it is not problem for next
  release (yet).

I agree that this is a user problem. We agreed not to introduce a
console proxy in kernel _or_ hack the command line in kernel (to
replace O with S).
So I think the problem boils down to educate the user about this
change. Making the old driver disappear was one way of getting the
user's attention. Another idea would be to introduce a #warning which is
also activated by the defconfig and informs the user about the change.
Ideally this #warning could be switched off by Kconfig once the user
reads & deactivates it. This requires the pay attention to warnings
during build. #error would make sure he does but it breaks auto-builds
so it is not an option.

> Regards,
> 
> Tony
> 

Sebastian

Re: [PATCH] tty: serial: serial-omap: depend on !8250_omap

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [141201 09:27]:
> On 12/01/2014 05:38 PM, Tony Lindgren wrote:
> > * One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> [141201 06:11]:
> >>> Well the nightmare userspace switch from ttyS to ttyO few years ago is
> >>> something we want to avoid.. I think the best solution would be to make
> >>> serial-omap.c transparently provide support for ttyO using the new 8250
> >>> code so both ttyS and ttyO devices would just work. Otherwise it will
> >>> be years of "my serial port stopped working" questions again.
> >>
> >> Thata a udev problem not a kernel one surely.
> > 
> > How do you suggest we get people to update their kernel command line
> > and inittab? Udev may not even be installed.
> 
> There are three use cases that I can think of right now:
> - people that enable that new driver via oldconfig
>   I would expect that they read the help message in Kconfig. No worry
>   about them.
> 
> - people that get a complete system via magic_build_tool (may yocto or
>   whatever)
>   If $TOOL decides to use the new driver, then it should update
>   commandline in bootloader. Those things create usually bootloader +
>   kernel + rootfile system. If the commandline is saved on flash/mmc
>   then it won't be reset from default. However udev should help here.
>   So not a problem either (udev can't fix the kernel boot output but we
>   should see atleast the login console).
> 
> - people that build omap2plus_defconfig and we switch to the new driver
>   Those people get switched from one driver to the other without
>   knowing. This is what I tried to bring to everyone's attention. The
>   defconfig hasn't been changed yet so it is not problem for next
>   release (yet).
> 
> I agree that this is a user problem. We agreed not to introduce a
> console proxy in kernel _or_ hack the command line in kernel (to
> replace O with S).
> So I think the problem boils down to educate the user about this
> change. Making the old driver disappear was one way of getting the
> user's attention. Another idea would be to introduce a #warning which is
> also activated by the defconfig and informs the user about the change.
> Ideally this #warning could be switched off by Kconfig once the user
> reads & deactivates it. This requires the pay attention to warnings
> during build. #error would make sure he does but it breaks auto-builds
> so it is not an option.

The problem is the kernel will just mysteriously stop outputting
anything if we enable the new driver. This is a "flag day" type
problem that needs the user to somehow coordinate the kernel version,
kernel .config, kernel cmdline, dev entries, and the inittab.

Regards,

Tony

Re: [PATCH] tty: serial: serial-omap: depend on !8250_omap

[Aaro Koskinen]

Hi,

On Mon, Dec 01, 2014 at 02:09:14PM +0000, One Thousand Gnomes wrote:
> > Well the nightmare userspace switch from ttyS to ttyO few years ago is
> > something we want to avoid.. I think the best solution would be to make
> > serial-omap.c transparently provide support for ttyO using the new 8250
> > code so both ttyS and ttyO devices would just work. Otherwise it will
> > be years of "my serial port stopped working" questions again.
> 
> Thata a udev problem not a kernel one surely.

People also use serial console to observe the early kernel boot before
the userspace is started.

A.

Re: [PATCH] tty: serial: serial-omap: depend on !8250_omap

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 925 bytes --]

On Tue, Dec 02, 2014 at 01:13:38AM +0200, Aaro Koskinen wrote:
> Hi,
> 
> On Mon, Dec 01, 2014 at 02:09:14PM +0000, One Thousand Gnomes wrote:
> > > Well the nightmare userspace switch from ttyS to ttyO few years ago is
> > > something we want to avoid.. I think the best solution would be to make
> > > serial-omap.c transparently provide support for ttyO using the new 8250
> > > code so both ttyS and ttyO devices would just work. Otherwise it will
> > > be years of "my serial port stopped working" questions again.
> > 
> > Thata a udev problem not a kernel one surely.
> 
> People also use serial console to observe the early kernel boot before
> the userspace is started.

right, we need a way to tell the kernel that ttyO%d and ttyS%d are the
exact same device so that console=ttyO0 and console=ttyS0 mean the same
thing. That maintains backwards compatibility and lets people move on

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] tty: serial: serial-omap: depend on !8250_omap

[One Thousand Gnomes]

On Mon, 1 Dec 2014 19:51:18 -0600
Felipe Balbi <balbi@ti.com> wrote:

> On Tue, Dec 02, 2014 at 01:13:38AM +0200, Aaro Koskinen wrote:
> > Hi,
> > 
> > On Mon, Dec 01, 2014 at 02:09:14PM +0000, One Thousand Gnomes wrote:
> > > > Well the nightmare userspace switch from ttyS to ttyO few years ago is
> > > > something we want to avoid.. I think the best solution would be to make
> > > > serial-omap.c transparently provide support for ttyO using the new 8250
> > > > code so both ttyS and ttyO devices would just work. Otherwise it will
> > > > be years of "my serial port stopped working" questions again.
> > > 
> > > Thata a udev problem not a kernel one surely.
> > 
> > People also use serial console to observe the early kernel boot before
> > the userspace is started.
> 
> right, we need a way to tell the kernel that ttyO%d and ttyS%d are the
> exact same device so that console=ttyO0 and console=ttyS0 mean the same
> thing. That maintains backwards compatibility and lets people move on

Yes.. and that possibly also means a temporary char driver that claims
ttyO* and simply redirects the calls into the tty.

[PATCH] tty: serial: 8250: omap: add ttySx console if the user didn't

[Sebastian Andrzej Siewior]

This patch invokes add_preferred_console() with ttyS based on ttyO
arguments if the user didn't specify it on its own. This ensures that
the user will see the kernel booting on his serial console in case he
forgot to update the command line.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---

While trying to to make a proxy driver like Alan suggested I wasn't sure
how to obtain the ttyS console via "official" API. Then I stumbled upon
update_console_cmdline() and then add_preferred_console(). 

 drivers/tty/serial/8250/8250_omap.c | 40 +++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig     | 19 ++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 336602eb453e..78401fbc823b 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1248,6 +1248,46 @@ static int omap8250_runtime_resume(struct device *dev)
 }
 #endif
 
+#ifdef CONFIG_SERIAL_8250_OMAP_TTYO_FIXUP
+static int __init omap8250_console_fixup(void)
+{
+	char *omap_str;
+	char *options;
+	u8 idx;
+
+	if (strstr(boot_command_line, "console=ttyS"))
+		/* user set a ttyS based name for the console */
+		return 0;
+
+	omap_str = strstr(boot_command_line, "console=ttyO");
+	if (!omap_str)
+		/* user did not set ttyO based console, so we don't care */
+		return 0;
+
+	omap_str += 12;
+	if ('0' <= *omap_str && *omap_str <= '9')
+		idx = *omap_str - '0';
+	else
+		return 0;
+
+	omap_str++;
+	if (omap_str[0] == ',') {
+		omap_str++;
+		options = omap_str;
+	} else {
+		options = NULL;
+	}
+
+	add_preferred_console("ttyS", idx, options);
+	pr_err("WARNING: Your 'console=ttyO%d' has been replaced by 'ttyS%d'\n",
+	       idx, idx);
+	pr_err("This ensures that you still see kernel messages. Please\n");
+	pr_err("update your kernel commandline.\n");
+	return 0;
+}
+console_initcall(omap8250_console_fixup);
+#endif
+
 static const struct dev_pm_ops omap8250_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(omap8250_suspend, omap8250_resume)
 	SET_RUNTIME_PM_OPS(omap8250_runtime_suspend,
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 0fcbcd29502f..87adf08b9b6b 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -308,6 +308,25 @@ config SERIAL_8250_OMAP
 
 	  This driver uses ttyS instead of ttyO.
 
+config SERIAL_8250_OMAP_TTYO_FIXUP
+	bool "Replace ttyO with ttyS"
+	depends on SERIAL_8250_OMAP=y && SERIAL_8250_CONSOLE
+	default y
+	help
+	  This option replaces the "console=ttyO" argument with the matching
+	  ttyS argument if the user did not specified it on the command line.
+	  This ensures that the user can see the kernel output during boot
+	  which he wouldn't see otherwise. The getty has still to be configured
+	  for ttyS instead of ttyO regardless of this option.
+	  This option is intended for people who "automatically" enable this
+	  driver without knowing that this driver requires a different console=
+	  argument. If you read this, please keep this option disabled and
+	  instead update your kernel command line. If you prepare a kernel for a
+	  distribution or other kind of larger user base then you probably want
+	  to keep this option enabled. Otherwise people might complain about a
+	  not booting kernel because the serial console remains silent in case
+	  they forgot to update the command line.
+
 config SERIAL_8250_FINTEK
 	tristate "Support for Fintek F81216A LPC to 4 UART"
 	depends on SERIAL_8250 && PNP
-- 
2.1.3

Re: [PATCH] tty: serial: 8250: omap: add ttySx console if the user didn't

[One Thousand Gnomes]

On Thu, 18 Dec 2014 18:47:12 +0100
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> This patch invokes add_preferred_console() with ttyS based on ttyO
> arguments if the user didn't specify it on its own. This ensures that
> the user will see the kernel booting on his serial console in case he
> forgot to update the command line.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Acked-by: Alan Cox <alan@linux.intel.com>