[v4,0/4] Add support for DEVNAME:0.0 style hardware based addressing

Message ID 20231205073255.20562-1-tony@atomide.com
Headers
Series Add support for DEVNAME:0.0 style hardware based addressing |

Message

Tony Lindgren Dec. 5, 2023, 7:32 a.m. UTC
  Hi all,

With the recent serial core changes, we can now add DEVNAME:0.0 style
addressing for the serial ports. When using DEVNAME:0.0 naming, we don't
need to care which ttyS instance number is allocated depending on HSUART
settings or if the devicetree has added aliases for all the ports.

We also prepare the serial core to handle the ttyS related quirks done
in console_setup() to prepare things for eventually dropping the parsing
from console_setup(). This can only happen after further changes to
register_console().

Regards,

Tony

Changes since v3:
- Fix handling of brl_options by saving them too in console_opt_save()

- Drop changes to remove console_setup() parsing, further changes to
  register_console() are still needed before can leave out the early
  parsing

- Added a patch for serial8250_isa_init_preferred_console(), otherwise
  the console gets initialized later for x86 when the console_setup()
  parsing is dropped as pointed out by Petr

- Initialize __free() variables to NULL as noted by Dan

- Return handled in console_setup() if saving console options fails

- Simplify serial_base_add_preferred_console() and add a helper for
  serial_base_add_one_prefcon() as suggested by Andy

- Header include changes to kernel/printk/conopt.c as suggested by Andy

Changes since v2:

- Console name got constified and already applied as suggested by Ilpo
  and Andy

- Add printk/conopt.c to save console command line options

- Add a patch to drop old console_setup() character device name parsing

- Use cleanup.h to simplify freeing as suggested by Andy

- Use types.h instead of kernel.h as suggested by Andy

- Use strcspn() as suggested by Andy

- Various coding improvments suggested by Andy

Changes since v1:

- Constify printk add_preferred_console() as suggested by Jiri

- Use proper kernel command line helpers for parsing console as
  suggested by Jiri

- Update description for HSUART based on Andy's comments

- Standardize on DEVNAME:0.0 style naming as suggested by Andy

- Added missing put_device() calls paired with device_find_child()

Tony Lindgren (4):
  printk: Save console options for add_preferred_console_match()
  serial: core: Add support for DEVNAME:0.0 style naming for kernel
    console
  serial: core: Handle serial console options
  serial: 8250: Add preferred console in serial8250_isa_init_ports()

 drivers/tty/serial/8250/8250_core.c  |  32 +++++++
 drivers/tty/serial/serial_base.h     |  14 +++
 drivers/tty/serial/serial_base_bus.c | 115 ++++++++++++++++++++++++
 drivers/tty/serial/serial_core.c     |   4 +
 include/linux/printk.h               |   3 +
 kernel/printk/Makefile               |   2 +-
 kernel/printk/conopt.c               | 128 +++++++++++++++++++++++++++
 kernel/printk/console_cmdline.h      |   6 ++
 kernel/printk/printk.c               |  14 ++-
 9 files changed, 314 insertions(+), 4 deletions(-)
 create mode 100644 kernel/printk/conopt.c
  

Comments

Tony Lindgren Dec. 5, 2023, 7:45 a.m. UTC | #1
* Tony Lindgren <tony@atomide.com> [700101 02:00]:
> We also prepare the serial core to handle the ttyS related quirks done
> in console_setup() to prepare things for eventually dropping the parsing
> from console_setup(). This can only happen after further changes to
> register_console().

Petr FYI, so for dropping the console_setup() parsing, below is a hack
patch to see what goes wrong in register_console() if you have some ideas
on how to handle this.

We end up with the console device backed up seria8250 instead of ttyS0,
and earlycon won't get properly disabled. And of course other consoles
beyond ttyS need to be also considered.

Regards,

Tony

8< ----------------------
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2438,9 +2438,7 @@ __setup("console_msg_format=", console_msg_format_setup);
  */
 static int __init console_setup(char *str)
 {
-	char buf[sizeof(console_cmdline[0].name) + 4]; /* 4 for "ttyS" */
-	char *s, *options, *brl_options = NULL;
-	int idx;
+	char *brl_options = NULL;
 
 	/*
 	 * console="" or console=null have been suggested as a way to
@@ -2459,32 +2457,9 @@ static int __init console_setup(char *str)
 	if (console_opt_save(str, brl_options))
 		return 1;
 
-	/*
-	 * Decode str into name, index, options.
-	 */
-	if (str[0] >= '0' && str[0] <= '9') {
-		strcpy(buf, "ttyS");
-		strncpy(buf + 4, str, sizeof(buf) - 5);
-	} else {
-		strncpy(buf, str, sizeof(buf) - 1);
-	}
-	buf[sizeof(buf) - 1] = 0;
-	options = strchr(str, ',');
-	if (options)
-		*(options++) = 0;
-#ifdef __sparc__
-	if (!strcmp(str, "ttya"))
-		strcpy(buf, "ttyS0");
-	if (!strcmp(str, "ttyb"))
-		strcpy(buf, "ttyS1");
-#endif
-	for (s = buf; *s; s++)
-		if (isdigit(*s) || *s == ',')
-			break;
-	idx = simple_strtoul(s, NULL, 10);
-	*s = 0;
+	/* Indicate register_console() a console was specified */
+	console_set_on_cmdline = 1;
 
-	__add_preferred_console(buf, idx, options, brl_options, true);
 	return 1;
 }
 __setup("console=", console_setup);
@@ -3476,7 +3451,7 @@ void register_console(struct console *newcon)
 	 * Note that a console with tty binding will have CON_CONSDEV
 	 * flag set and will be first in the list.
 	 */
-	if (preferred_console < 0) {
+	if (preferred_console < 0 && !console_set_on_cmdline) {
 		if (hlist_empty(&console_list) || !console_first()->device ||
 		    console_first()->flags & CON_BOOT) {
 			try_enable_default_console(newcon);
  
Tony Lindgren Dec. 8, 2023, 8:28 a.m. UTC | #2
* Tony Lindgren <tony@atomide.com> [700101 02:00]:
> * Tony Lindgren <tony@atomide.com> [700101 02:00]:
> > We also prepare the serial core to handle the ttyS related quirks done
> > in console_setup() to prepare things for eventually dropping the parsing
> > from console_setup(). This can only happen after further changes to
> > register_console().
> 
> Petr FYI, so for dropping the console_setup() parsing, below is a hack
> patch to see what goes wrong in register_console() if you have some ideas
> on how to handle this.
> 
> We end up with the console device backed up seria8250 instead of ttyS0,
> and earlycon won't get properly disabled. And of course other consoles
> beyond ttyS need to be also considered.

Hmm so the following extra patch seems to fix the issues based on light
testing. But is it safe to assume that if CON_PRINTBUFFER is set we can
disable the bootconsole?

I also noticed that the bootconsole not getting disabled issue is there
also for console=DEVNAME:0.0 usage even before we start dropping handling
in console_setup().

Regards,

Tony

8< ----------------------
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3549,7 +3549,8 @@ void register_console(struct console *newcon)
 	 */
 	con_printk(KERN_INFO, newcon, "enabled\n");
 	if (bootcon_registered &&
-	    ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV) &&
+	    !(newcon->flags & CON_BOOT) &&
+	    (newcon->flags & (CON_CONSDEV | CON_PRINTBUFFER)) &&
 	    !keep_bootcon) {
 		struct hlist_node *tmp;
  
Tony Lindgren Dec. 18, 2023, 6:38 a.m. UTC | #3
* Tony Lindgren <tony@atomide.com> [231208 10:29]:
> * Tony Lindgren <tony@atomide.com> [700101 02:00]:
> > * Tony Lindgren <tony@atomide.com> [700101 02:00]:
> > > We also prepare the serial core to handle the ttyS related quirks done
> > > in console_setup() to prepare things for eventually dropping the parsing
> > > from console_setup(). This can only happen after further changes to
> > > register_console().
> > 
> > Petr FYI, so for dropping the console_setup() parsing, below is a hack
> > patch to see what goes wrong in register_console() if you have some ideas
> > on how to handle this.
> > 
> > We end up with the console device backed up seria8250 instead of ttyS0,
> > and earlycon won't get properly disabled. And of course other consoles
> > beyond ttyS need to be also considered.
> 
> Hmm so the following extra patch seems to fix the issues based on light
> testing. But is it safe to assume that if CON_PRINTBUFFER is set we can
> disable the bootconsole?

OK so no need for the CON_PRINTBUFFER change, it's wrong. I found a few
bugs causing this issue and a lot of other confusion while testing:

- In console_setup(), a DEVNAME:0.0 style console can get added with the
  IO address turned into a ttyS console with some crazy index :) So we
  need to bail out early on consoles with ':' in the name.

- The brl_opts can be empty or NULL, but we need to pass NULL to
  __add_preferred_console() to get CON_CONSDEV flag set for DEVNAME:0.0
  console. Otherwise the preferred_console won't get set and the boot
  console won't get disabled.

- The console_set_on_cmdline flag needs to be set if console_setup()
  does not call __add_preferred_console() for DEVNAME:0.0 style console
  as otherwise try_enable_default_console() may get called before the
  console handling driver has added the preferred console.

I think with these the remaining issues are sorted out :) I'll post a
v5 set with as RFC as it's getting close to the merge window.

Regards,

Tony