diff mbox series

[v3,3/3] serial: core: Rename preferred console handling for match and update

Message ID 20240618045458.14731-4-tony.lindgren@linux.intel.com
State New
Headers show
Series Fixes for console command line ordering | expand

Commit Message

Tony Lindgren June 18, 2024, 4:54 a.m. UTC 
We are now matching and updating the preferred console, not adding it.
Let's update the naming accordingly to avoid confusion.

Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Tony Lindgren <tony.lindgren@linux.intel.com>
---
 drivers/tty/serial/8250/8250_core.c  |  4 ++--
 drivers/tty/serial/serial_base.h     |  4 ++--
 drivers/tty/serial/serial_base_bus.c | 21 +++++++++++----------
 3 files changed, 15 insertions(+), 14 deletions(-)

Comments

Petr Mladek June 18, 2024, 1:51 p.m. UTC | #1 
On Tue 2024-06-18 07:54:50, Tony Lindgren wrote:
> We are now matching and updating the preferred console, not adding it.
> Let's update the naming accordingly to avoid confusion.
> 
> --- a/drivers/tty/serial/serial_base_bus.c
> +++ b/drivers/tty/serial/serial_base_bus.c
> @@ -304,7 +305,7 @@ int serial_base_add_preferred_console(struct uart_driver *drv,

I was curious whether this patch renamed everything. And it seems
that it did not rename serial_base_add_preferred_console().

>  	const char *port_match __free(kfree) = NULL;
>  	int ret;
>  
> -	ret = serial_base_add_prefcon(drv->dev_name, port->line);
> +	ret = serial_base_match_and_update_prefcon(drv->dev_name, port->line);
>  	if (ret)
>  		return ret;
>  

Honestly, I do not understand what are all these layers for.
Especially, serial_base_match_and_update_prefcon() looks suspicious:

static int serial_base_match_and_update_prefcon(const char *name, int idx)
{
	const char *char_match __free(kfree) = NULL;
	const char *nmbr_match __free(kfree) = NULL;
	int ret;

	/* Handle ttyS specific options */
	if (strstarts(name, "ttyS")) {
		/* No name, just a number */
		nmbr_match = kasprintf(GFP_KERNEL, "%i", idx);
		if (!nmbr_match)
			return -ENODEV;

		ret = serial_base_match_and_update_one_prefcon(nmbr_match, name, idx);
		if (ret)
			return ret;

		/* Sparc ttya and ttyb */
		ret = serial_base_add_sparc_console(name, idx);
		if (ret)
			return ret;
	}

	/* Handle the traditional character device name style console=ttyS0 */
	char_match = kasprintf(GFP_KERNEL, "%s%i", name, idx);
	if (!char_match)
		return -ENOMEM;

	return serial_base_match_and_update_one_prefcon(char_match, name, idx);
}

It seems to try whether c->devname matches a number "X", or "ttySX".
It even tries the sparc-specific transformations in
serial_base_add_sparc_console()

But this is the original format which does _not_ include ":".
It never will be stored in c->devname and will never match.

I think that it has been the case even before this patchset.

I think that we should remove these layers and check just
the "DEVNAME:X.Y" format, aka "%s:%d.%d" [*].


[*] It would be nice to use the same printf format "%s:%d.%d"
    in both serial_base_device_init() and also in the functions
    matching the devname to make it clear that these are
    the same names. Heh, I just guess that these are the same
    names.

Best Regards,
Petr
diff mbox series

Patch

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index ff15022369e4..3556fe42ec65 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -42,7 +42,7 @@ 
 
 #include <asm/irq.h>
 
-#include "../serial_base.h"	/* For serial_base_add_isa_preferred_console() */
+#include "../serial_base.h"	/* For serial_base_match_and_update_isa_preferred_console() */
 
 #include "8250.h"
 
@@ -564,7 +564,7 @@  static void __init serial8250_isa_init_ports(void)
 		if (serial8250_isa_config != NULL)
 			serial8250_isa_config(i, &up->port, &up->capabilities);
 
-		serial_base_add_isa_preferred_console(serial8250_reg.dev_name, i);
+		serial_base_match_and_update_isa_preferred_console(serial8250_reg.dev_name, i);
 	}
 }
 
diff --git a/drivers/tty/serial/serial_base.h b/drivers/tty/serial/serial_base.h
index 743a72ac34f3..a5632eeda250 100644
--- a/drivers/tty/serial/serial_base.h
+++ b/drivers/tty/serial/serial_base.h
@@ -68,12 +68,12 @@  int serial_base_add_preferred_console(struct uart_driver *drv,
 
 #ifdef CONFIG_SERIAL_8250_CONSOLE
 
-int serial_base_add_isa_preferred_console(const char *name, int idx);
+int serial_base_match_and_update_isa_preferred_console(const char *name, int idx);
 
 #else
 
 static inline
-int serial_base_add_isa_preferred_console(const char *name, int idx)
+int serial_base_match_and_update_isa_preferred_console(const char *name, int idx)
 {
 	return 0;
 }
diff --git a/drivers/tty/serial/serial_base_bus.c b/drivers/tty/serial/serial_base_bus.c
index 2cf86f1ff298..391d41ef5942 100644
--- a/drivers/tty/serial/serial_base_bus.c
+++ b/drivers/tty/serial/serial_base_bus.c
@@ -207,8 +207,9 @@  void serial_base_port_device_remove(struct serial_port_device *port_dev)
 
 #ifdef CONFIG_SERIAL_CORE_CONSOLE
 
-static int serial_base_add_one_prefcon(const char *match, const char *dev_name,
-				       int port_id)
+static int serial_base_match_and_update_one_prefcon(const char *match,
+						    const char *dev_name,
+						    int port_id)
 {
 	int ret;
 
@@ -237,7 +238,7 @@  static int serial_base_add_sparc_console(const char *dev_name, int idx)
 		return 0;
 	}
 
-	return serial_base_add_one_prefcon(name, dev_name, idx);
+	return serial_base_match_and_update_one_prefcon(name, dev_name, idx);
 }
 
 #else
@@ -249,7 +250,7 @@  static inline int serial_base_add_sparc_console(const char *dev_name, int idx)
 
 #endif
 
-static int serial_base_add_prefcon(const char *name, int idx)
+static int serial_base_match_and_update_prefcon(const char *name, int idx)
 {
 	const char *char_match __free(kfree) = NULL;
 	const char *nmbr_match __free(kfree) = NULL;
@@ -262,7 +263,7 @@  static int serial_base_add_prefcon(const char *name, int idx)
 		if (!nmbr_match)
 			return -ENODEV;
 
-		ret = serial_base_add_one_prefcon(nmbr_match, name, idx);
+		ret = serial_base_match_and_update_one_prefcon(nmbr_match, name, idx);
 		if (ret)
 			return ret;
 
@@ -277,7 +278,7 @@  static int serial_base_add_prefcon(const char *name, int idx)
 	if (!char_match)
 		return -ENOMEM;
 
-	return serial_base_add_one_prefcon(char_match, name, idx);
+	return serial_base_match_and_update_one_prefcon(char_match, name, idx);
 }
 
 /**
@@ -304,7 +305,7 @@  int serial_base_add_preferred_console(struct uart_driver *drv,
 	const char *port_match __free(kfree) = NULL;
 	int ret;
 
-	ret = serial_base_add_prefcon(drv->dev_name, port->line);
+	ret = serial_base_match_and_update_prefcon(drv->dev_name, port->line);
 	if (ret)
 		return ret;
 
@@ -314,7 +315,7 @@  int serial_base_add_preferred_console(struct uart_driver *drv,
 		return -ENOMEM;
 
 	/* Translate a hardware addressing style console=DEVNAME:0.0 */
-	return serial_base_add_one_prefcon(port_match, drv->dev_name, port->line);
+	return serial_base_match_and_update_one_prefcon(port_match, drv->dev_name, port->line);
 }
 
 #endif
@@ -326,9 +327,9 @@  int serial_base_add_preferred_console(struct uart_driver *drv,
  * This should be only called from serial8250_isa_init_preferred_console(),
  * other callers are likely wrong and should rely on earlycon instead.
  */
-int serial_base_add_isa_preferred_console(const char *name, int idx)
+int serial_base_match_and_update_isa_preferred_console(const char *name, int idx)
 {
-	return serial_base_add_prefcon(name, idx);
+	return serial_base_match_and_update_prefcon(name, idx);
 }
 
 #endif