serial: Some yak shaving

PATCH In review Uwe Kleine-König 4 participants 13 messages Jul 31 – Aug 3

Subsystems (2)

Documentation TTY layer and serial drivers

lore

Share & tools

lore

Uwe Kleine-König

author

2026-07-31 09:39 UTC #0 lore raw

cover serial: Some yak shaving

Hello,

after the discussion with Hugo about hub6_match_port()[1] I looked into implementing my suggestion. Hugo had a doubt there:

Quoted 3 lines

I am not sure to fully understand what you suggest. Copying the same function code to two drivers seems like a waisted opportunity for code reuse?

I don't think this is a wasted opportunity, as uart_match_port() only becomes a single comparison for the txx9 driver. Also having 8250 specific parts in serial_core is a layer violation.

I was unable to unsee some of the stuff I discovered, so there are a few more patches in this series now ...

The first txx9 patch is a preparation for the second to establish the assertion ".iotype is always UPIO_PORT". It's also a fix that is correct on its own (assuming I understood the driver model misuse in this driver correctly).

After that uart_match_port() is only used by the 8250 driver code and thus can be moved to there in the third patch, simplifying the dependencies a bit which enables fixing the breakage I found in the fourth patch.

The two remaining patches are cleanups I found while working on that driver.

Note this is only compile tested as I don't have a machine with any of the three affected serial hardwares.

Best regards
Uwe

[1] https://lore.kernel.org/linux-serial/amn8Hb2dbv_Sah_Y@monoceros/

Uwe Kleine-König (6):
serial: txx9: Drop noop probe function and dangerous remove callback
serial: txx9: Drop usage of uart_match_port()
serial: 8250: Make uart_match_port() a 8250 specific function
serial: 8250: Fix corner case for port matching
serial: 8250: hub6: Add cleanup code
serial: 8250: pnp: Annotate init and exit functions for conditional
discarding

 Documentation/driver-api/serial/driver.rst |  2 +-
 drivers/tty/serial/8250/8250.h             | 10 +++-
 drivers/tty/serial/8250/8250_core.c        | 27 +++++++++-
 drivers/tty/serial/8250/8250_hub6.c        | 11 ++---
 drivers/tty/serial/8250/8250_platform.c    |  9 +++-
 drivers/tty/serial/8250/8250_pnp.c         |  4 +-
 drivers/tty/serial/8250/Kconfig            |  4 +-
 drivers/tty/serial/8250/Makefile           |  2 +-
 drivers/tty/serial/serial_core.c           | 27 ----------
 drivers/tty/serial/serial_txx9.c           | 57 ++--------------------
 include/linux/serial_core.h                |  2 -
 11 files changed, 58 insertions(+), 97 deletions(-)

base-commit: 95d6a9ccef99117115e41e9adb271243bd5e985b

Signature · 1 line

2.55.0.11.g153666a7d9bb

Uwe Kleine-König

author

2026-07-31 09:39 UTC #1 lore raw

PATCH 1/6 serial: txx9: Drop noop probe function and dangerous remove callback

The only platform device that can be bound to the driver is allocated by the driver itself. That device doesn't have platdata, so the probe function only consists of a loop that is never run and thus can be dropped without loss of functionality.

The remove function then should better not undo things that were not done in the first place. That happens because serial_txx9_init() calls serial_txx9_register_ports() with the created platform device and so up->dev == &dev->dev evaluates to true and uart_remove_one_port() is called twice for each port.

So let serial_txx9_exit() undo exactly what serial_txx9_init() does and remove the useless respectively wrong functions.

Fixes 0970769aceb9 ("[SERIAL] serial_txx9 driver update")

Signed-off-by Uwe Kleine-König <[email protected]>

---
 drivers/tty/serial/serial_txx9.c | 55 ++------------------------------
 1 file changed, 3 insertions(+), 52 deletions(-)

drivers/tty/serial/serial_txx9.c +3 ~~-52~~

index 4ae9a45c8e3a..5ed20129b44b 100644
@@ -1009,7 +1009,8 @@ static void serial_txx9_unregister_port(int line)
 	struct uart_port *uart = &serial_txx9_ports[line];
 
 	mutex_lock(&serial_txx9_mutex);
-	uart_remove_one_port(&serial_txx9_reg, uart);
+	if (up->iobase || up->mapbase)
+		uart_remove_one_port(&serial_txx9_reg, uart);
 	uart->flags = 0;
 	uart->type = PORT_UNKNOWN;
 	uart->iobase = 0;

@@ -1019,52 +1020,6 @@ static void serial_txx9_unregister_port(int line)
 	mutex_unlock(&serial_txx9_mutex);
 }
 
-/*
- * Register a set of serial devices attached to a platform device.
- */
-static int serial_txx9_probe(struct platform_device *dev)
-{
-	struct uart_port *p = dev_get_platdata(&dev->dev);
-	struct uart_port port;
-	int ret, i;
-
-	memset(&port, 0, sizeof(struct uart_port));
-	for (i = 0; p && p->uartclk != 0; p++, i++) {
-		port.iobase	= p->iobase;
-		port.membase	= p->membase;
-		port.irq	= p->irq;
-		port.uartclk	= p->uartclk;
-		port.iotype	= p->iotype;
-		port.flags	= p->flags;
-		port.mapbase	= p->mapbase;
-		port.dev	= &dev->dev;
-		port.has_sysrq	= IS_ENABLED(CONFIG_SERIAL_TXX9_CONSOLE);
-		ret = serial_txx9_register_port(&port);
-		if (ret < 0) {
-			dev_err(&dev->dev, "unable to register port at index %d "
-				"(IO%lx MEM%llx IRQ%d): %d\n", i,
-				p->iobase, (unsigned long long)p->mapbase,
-				p->irq, ret);
-		}
-	}
-	return 0;
-}
-
-/*
- * Remove serial ports registered against a platform device.
- */
-static void serial_txx9_remove(struct platform_device *dev)
-{
-	int i;
-
-	for (i = 0; i < UART_NR; i++) {
-		struct uart_port *up = &serial_txx9_ports[i];
-
-		if (up->dev == &dev->dev)
-			serial_txx9_unregister_port(i);
-	}
-}
-
 #ifdef CONFIG_PM
 static int serial_txx9_suspend(struct platform_device *dev, pm_message_t state)
 {

@@ -1096,8 +1051,6 @@ static int serial_txx9_resume(struct platform_device *dev)
 #endif
 
 static struct platform_driver serial_txx9_plat_driver = {
-	.probe		= serial_txx9_probe,
-	.remove		= serial_txx9_remove,
 #ifdef CONFIG_PM
 	.suspend	= serial_txx9_suspend,
 	.resume		= serial_txx9_resume,

@@ -1251,9 +1204,7 @@ static void __exit serial_txx9_exit(void)
 	platform_driver_unregister(&serial_txx9_plat_driver);
 	platform_device_unregister(serial_txx9_plat_devs);
 	for (i = 0; i < UART_NR; i++) {
-		struct uart_port *up = &serial_txx9_ports[i];
-		if (up->iobase || up->mapbase)
-			uart_remove_one_port(&serial_txx9_reg, up);
+		serial_txx9_unregister_port(i);
 	}
 
 	uart_unregister_driver(&serial_txx9_reg);

Signature · 1 line

2.55.0.11.g153666a7d9bb

Uwe Kleine-König

author

2026-07-31 09:39 UTC #2 lore raw

PATCH 2/6 serial: txx9: Drop usage of uart_match_port()

In reply to Uwe Kleine-König

In this driver .iotype is always UPIO_PORT. So uart_match_port() is equivalent to comparing .iobase. Remove the call to uart_match_port() which then only has a single caller that is handled in the next commit.

Signed-off-by Uwe Kleine-König <[email protected]>

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

drivers/tty/serial/serial_txx9.c +1 -1

index 5ed20129b44b..2368081c3742 100644
@@ -965,7 +965,7 @@ static int serial_txx9_register_port(struct uart_port *port)
 	mutex_lock(&serial_txx9_mutex);
 	for (i = 0; i < UART_NR; i++) {
 		uart = &serial_txx9_ports[i];
-		if (uart_match_port(uart, port)) {
+		if (uart->iobase == port->iobase)
 			uart_remove_one_port(&serial_txx9_reg, uart);
 			break;
 		}

Signature · 1 line

2.55.0.11.g153666a7d9bb

Greg Kroah-Hartman

maintainer

2026-08-03 14:46 UTC #3 lore raw

Re: [PATCH v1 2/6] serial: txx9: Drop usage of uart_match_port()

Quoted 14 lines · Uwe Kleine-König

In this driver .iotype is always UPIO_PORT. So uart_match_port() is equivalent to comparing .iobase. Remove the call to uart_match_port() which then only has a single caller that is handled in the next commit.
Signed-off-by Uwe Kleine-König <[email protected]>
---
 drivers/tty/serial/serial_txx9.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/serial_txx9.c +0 -0
index 5ed20129b44b..2368081c3742 100644

drivers/tty/serial/serial_txx9.c static int serial_txx9_register_port(struct uart_port *port) · line 971

Quoted 8 lines

drivers/tty/serial/serial_txx9.c +1 -1

@@ -965,7 +965,7 @@ static int serial_txx9_register_port(struct uart_port *port)
 	mutex_lock(&serial_txx9_mutex);
 	for (i = 0; i < UART_NR; i++) {
 		uart = &serial_txx9_ports[i];
-		if (uart_match_port(uart, port)) {
+		if (uart->iobase == port->iobase)
 			uart_remove_one_port(&serial_txx9_reg, uart);
 			break;
 		}

Does this even build?

thanks,

greg k-h

Uwe Kleine-König

author

2026-08-03 18:57 UTC #4 lore raw

Re: [PATCH v1 2/6] serial: txx9: Drop usage of uart_match_port()

Quoted 10 lines · Greg Kroah-Hartman

Quoted 9 lines · Uwe Kleine-König
In this driver .iotype is always UPIO_PORT. So uart_match_port() is equivalent to comparing .iobase. Remove the call to uart_match_port() which then only has a single caller that is handled in the next commit.
Signed-off-by Uwe Kleine-König <[email protected]>
---
 drivers/tty/serial/serial_txx9.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Quoted 15 lines

drivers/tty/serial/serial_txx9.c static int serial_txx9_register_port(struct uart_port *port) · line 971

Quoted 13 lines

drivers/tty/serial/serial_txx9.c +1 -1

index 5ed20129b44b..2368081c3742 100644
@@ -965,7 +965,7 @@ static int serial_txx9_register_port(struct uart_port *port)
 	mutex_lock(&serial_txx9_mutex);
 	for (i = 0; i < UART_NR; i++) {
 		uart = &serial_txx9_ports[i];
-		if (uart_match_port(uart, port)) {
+		if (uart->iobase == port->iobase)
 			uart_remove_one_port(&serial_txx9_reg, uart);
 			break;
 		}

Does this even build?

Sigh, it passed my build scripts (which is essentially allmodconfig for arc, sh, alpha, arm64, x86_64, s390, riscv, mips, powerpc, loongarch, sparc, m68k and parisc), but this driver is only compiled for certain mips configs and not allmodconfig :-(

Will respin with a better suitable test build.

Best regards
Uwe

Ilpo Järvinen

2026-08-03 17:27 UTC #5 lore raw

Re: [PATCH v1 2/6] serial: txx9: Drop usage of uart_match_port()

In reply to Uwe Kleine-König

Quoted 3 lines · Uwe Kleine-König

In this driver .iotype is always UPIO_PORT. So uart_match_port() is equivalent to comparing .iobase. Remove the call to uart_match_port() which then only has a single caller that is handled in the next commit.

Not directly related to the change. The txx9 driver seems to have some iotype switch/cases as well so some !UPIO_PORT deadcode too.

Signature · 1 line

i.

Quoted 19 lines

Signed-off-by Uwe Kleine-König <[email protected]>

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

drivers/tty/serial/serial_txx9.c +1 -1

index 5ed20129b44b..2368081c3742 100644
@@ -965,7 +965,7 @@ static int serial_txx9_register_port(struct uart_port *port)
 	mutex_lock(&serial_txx9_mutex);
 	for (i = 0; i < UART_NR; i++) {
 		uart = &serial_txx9_ports[i];
-		if (uart_match_port(uart, port)) {
+		if (uart->iobase == port->iobase)
 			uart_remove_one_port(&serial_txx9_reg, uart);
 			break;
 		}

Uwe Kleine-König

author

2026-08-03 18:59 UTC #6 lore raw

Re: [PATCH v1 2/6] serial: txx9: Drop usage of uart_match_port()

Quoted 8 lines · Ilpo Järvinen

Quoted 3 lines · Uwe Kleine-König
In this driver .iotype is always UPIO_PORT. So uart_match_port() is equivalent to comparing .iobase. Remove the call to uart_match_port() which then only has a single caller that is handled in the next commit.
Not directly related to the change. The txx9 driver seems to have some iotype switch/cases as well so some !UPIO_PORT deadcode too.

So patch #1 could get rid of these, too. Will look into that for v2.

Best regards
Uwe

Uwe Kleine-König

author

2026-07-31 09:39 UTC #7 lore raw

PATCH 3/6 serial: 8250: Make uart_match_port() a 8250 specific function

In reply to Uwe Kleine-König

The 8250 driver conglomerate is the only user of this function and in fact it already is 8250 specific as it uses hub6_match_port() which is defined in drivers/tty/serial/8250/8250_hub6.c unless the dummy implementation is in use.

Signed-off-by Uwe Kleine-König <[email protected]>

---
 Documentation/driver-api/serial/driver.rst |  2 +-
 drivers/tty/serial/8250/8250_core.c        | 27 +++++++++++++++++++++-
 drivers/tty/serial/serial_core.c           | 27 ----------------------
 include/linux/serial_core.h                |  2 --
 4 files changed, 27 insertions(+), 31 deletions(-)

Documentation/driver-api/serial/driver.rst +1 -1

index c1db6a1a67c4..975b0ba53e98 100644
@@ -70,7 +70,7 @@ Other functions
 
 .. kernel-doc:: drivers/tty/serial/serial_core.c
    :identifiers: uart_update_timeout uart_get_baud_rate uart_get_divisor
-           uart_match_port uart_write_wakeup uart_register_driver
+           uart_write_wakeup uart_register_driver
            uart_unregister_driver uart_suspend_port uart_resume_port
            uart_add_one_port uart_remove_one_port uart_console_write
            uart_parse_earlycon uart_parse_options uart_set_options

drivers/tty/serial/8250/8250_core.c +26 -1

index c0e8a4efbdcc..6059b84d8364 100644
@@ -630,6 +630,31 @@ EXPORT_SYMBOL(serial8250_resume_port);
  */
 static DEFINE_MUTEX(serial_mutex);
 
+/**
+ * serial8250_match_port - are the two ports equivalent?
+ * @port1: first port
+ * @port2: second port
+ *
+ * This utility function can be used to determine whether two uart_port
+ * structures describe the same port.
+ */
+static bool serial8250_match_port(const struct uart_port *port1,
+				  const struct uart_port *port2)
+{
+	if (port1->iotype != port2->iotype)
+		return false;
+	else if (port1->iotype == UPIO_PORT)
+		return port1->iobase == port2->iobase;
+	else if (port1->iotype == UPIO_HUB6)
+		return hub6_match_port(port1, port2);
+	else if (uart_iotype_mmio(port1->iotype))
+		return port1->mapbase == port2->mapbase;
+	else if (port1->iotype == UPIO_BUS)
+		return true;
+	else
+		return false;
+}
+
 static struct uart_8250_port *serial8250_find_match_or_unused(const struct uart_port *port)
 {
 	int i;

@@ -638,7 +663,7 @@ static struct uart_8250_port *serial8250_find_match_or_unused(const struct uart_
 	 * First, find a port entry which matches.
 	 */
 	for (i = 0; i < nr_uarts; i++)
-		if (uart_match_port(&serial8250_ports[i].port, port))
+		if (serial8250_match_port(&serial8250_ports[i].port, port))
 			return &serial8250_ports[i];
 
 	/* try line number first if still available */

drivers/tty/serial/serial_core.c +0 ~~-27~~

index edd1e7be2a5c..8a6c53a9996e 100644
@@ -33,7 +33,6 @@
 #include <linux/uaccess.h>
 
 #include "serial_base.h"
-#include "8250/8250.h" /* For hub6_match_port() */
 
 /*
  * This is used to lock changes in serial line configuration.

@@ -3229,32 +3228,6 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
 	state->uart_port = NULL;
 }
 
-/**
- * uart_match_port - are the two ports equivalent?
- * @port1: first port
- * @port2: second port
- *
- * This utility function can be used to determine whether two uart_port
- * structures describe the same port.
- */
-bool uart_match_port(const struct uart_port *port1,
-		const struct uart_port *port2)
-{
-	if (port1->iotype != port2->iotype)
-		return false;
-	else if (port1->iotype == UPIO_PORT)
-		return port1->iobase == port2->iobase;
-	else if (port1->iotype == UPIO_HUB6)
-		return hub6_match_port(port1, port2);
-	else if (uart_iotype_mmio(port1->iotype))
-		return port1->mapbase == port2->mapbase;
-	else if (port1->iotype == UPIO_BUS)
-		return true;
-	else
-		return false;
-}
-EXPORT_SYMBOL(uart_match_port);
-
 static struct serial_ctrl_device *
 serial_core_get_ctrl_dev(struct serial_port_device *port_dev)
 {

include/linux/serial_core.h +0 -2

index c4cc4f66af4b..1659805e2233 100644
@@ -1137,8 +1137,6 @@ int uart_add_one_port(struct uart_driver *reg, struct uart_port *port);
 void uart_remove_one_port(struct uart_driver *reg, struct uart_port *port);
 int uart_read_port_properties(struct uart_port *port);
 int uart_read_and_validate_port_properties(struct uart_port *port);
-bool uart_match_port(const struct uart_port *port1,
-		const struct uart_port *port2);
 
 /*
  * Power Management

Signature · 1 line

2.55.0.11.g153666a7d9bb

Ilpo Järvinen

2026-08-03 17:23 UTC #8 lore raw

Re: [PATCH v1 3/6] serial: 8250: Make uart_match_port() a 8250 specific function

Quoted 1 line · Uwe Kleine-König

The 8250 driver conglomerate is the only user of this function and in

Please write these such that they don't depend on the shortlog. Here "this function" lacks any context without assuming one has first read the shortlog.

You have similar problem with "this driver" in patch 2.

Signature · 1 line

i.

Quoted 129 lines

fact it already is 8250 specific as it uses hub6_match_port() which is defined in drivers/tty/serial/8250/8250_hub6.c unless the dummy implementation is in use.

Signed-off-by Uwe Kleine-König <[email protected]>

---
 Documentation/driver-api/serial/driver.rst |  2 +-
 drivers/tty/serial/8250/8250_core.c        | 27 +++++++++++++++++++++-
 drivers/tty/serial/serial_core.c           | 27 ----------------------
 include/linux/serial_core.h                |  2 --
 4 files changed, 27 insertions(+), 31 deletions(-)

Documentation/driver-api/serial/driver.rst +1 -1

index c1db6a1a67c4..975b0ba53e98 100644
@@ -70,7 +70,7 @@ Other functions
 
 .. kernel-doc:: drivers/tty/serial/serial_core.c
    :identifiers: uart_update_timeout uart_get_baud_rate uart_get_divisor
-           uart_match_port uart_write_wakeup uart_register_driver
+           uart_write_wakeup uart_register_driver
            uart_unregister_driver uart_suspend_port uart_resume_port
            uart_add_one_port uart_remove_one_port uart_console_write
            uart_parse_earlycon uart_parse_options uart_set_options

drivers/tty/serial/8250/8250_core.c +26 -1

index c0e8a4efbdcc..6059b84d8364 100644
@@ -630,6 +630,31 @@ EXPORT_SYMBOL(serial8250_resume_port);
  */
 static DEFINE_MUTEX(serial_mutex);
 
+/**
+ * serial8250_match_port - are the two ports equivalent?
+ * @port1: first port
+ * @port2: second port
+ *
+ * This utility function can be used to determine whether two uart_port
+ * structures describe the same port.
+ */
+static bool serial8250_match_port(const struct uart_port *port1,
+				  const struct uart_port *port2)
+{
+	if (port1->iotype != port2->iotype)
+		return false;
+	else if (port1->iotype == UPIO_PORT)
+		return port1->iobase == port2->iobase;
+	else if (port1->iotype == UPIO_HUB6)
+		return hub6_match_port(port1, port2);
+	else if (uart_iotype_mmio(port1->iotype))
+		return port1->mapbase == port2->mapbase;
+	else if (port1->iotype == UPIO_BUS)
+		return true;
+	else
+		return false;
+}
+
 static struct uart_8250_port *serial8250_find_match_or_unused(const struct uart_port *port)
 {
 	int i;

@@ -638,7 +663,7 @@ static struct uart_8250_port *serial8250_find_match_or_unused(const struct uart_
 	 * First, find a port entry which matches.
 	 */
 	for (i = 0; i < nr_uarts; i++)
-		if (uart_match_port(&serial8250_ports[i].port, port))
+		if (serial8250_match_port(&serial8250_ports[i].port, port))
 			return &serial8250_ports[i];
 
 	/* try line number first if still available */

drivers/tty/serial/serial_core.c +0 ~~-27~~

index edd1e7be2a5c..8a6c53a9996e 100644
@@ -33,7 +33,6 @@
 #include <linux/uaccess.h>
 
 #include "serial_base.h"
-#include "8250/8250.h" /* For hub6_match_port() */
 
 /*
  * This is used to lock changes in serial line configuration.

@@ -3229,32 +3228,6 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
 	state->uart_port = NULL;
 }
 
-/**
- * uart_match_port - are the two ports equivalent?
- * @port1: first port
- * @port2: second port
- *
- * This utility function can be used to determine whether two uart_port
- * structures describe the same port.
- */
-bool uart_match_port(const struct uart_port *port1,
-		const struct uart_port *port2)
-{
-	if (port1->iotype != port2->iotype)
-		return false;
-	else if (port1->iotype == UPIO_PORT)
-		return port1->iobase == port2->iobase;
-	else if (port1->iotype == UPIO_HUB6)
-		return hub6_match_port(port1, port2);
-	else if (uart_iotype_mmio(port1->iotype))
-		return port1->mapbase == port2->mapbase;
-	else if (port1->iotype == UPIO_BUS)
-		return true;
-	else
-		return false;
-}
-EXPORT_SYMBOL(uart_match_port);
-
 static struct serial_ctrl_device *
 serial_core_get_ctrl_dev(struct serial_port_device *port_dev)
 {

include/linux/serial_core.h +0 -2

index c4cc4f66af4b..1659805e2233 100644
@@ -1137,8 +1137,6 @@ int uart_add_one_port(struct uart_driver *reg, struct uart_port *port);
 void uart_remove_one_port(struct uart_driver *reg, struct uart_port *port);
 int uart_read_port_properties(struct uart_port *port);
 int uart_read_and_validate_port_properties(struct uart_port *port);
-bool uart_match_port(const struct uart_port *port1,
-		const struct uart_port *port2);
 
 /*
  * Power Management

Hugo Villeneuve

2026-08-03 19:22 UTC #9 lore raw

Re: [PATCH v1 3/6] serial: 8250: Make uart_match_port() a 8250 specific function

In reply to Uwe Kleine-König

Hi Uwe,

Quoted 45 lines · On Fri, 31 Jul 2026 11:39:06 +0200 Uwe Kleine-König <[email protected]> wrote:

Signed-off-by Uwe Kleine-König <[email protected]>

---
 Documentation/driver-api/serial/driver.rst |  2 +-
 drivers/tty/serial/8250/8250_core.c        | 27 +++++++++++++++++++++-
 drivers/tty/serial/serial_core.c           | 27 ----------------------
 include/linux/serial_core.h                |  2 --
 4 files changed, 27 insertions(+), 31 deletions(-)

Documentation/driver-api/serial/driver.rst +1 -1

index c1db6a1a67c4..975b0ba53e98 100644
@@ -70,7 +70,7 @@ Other functions
 
 .. kernel-doc:: drivers/tty/serial/serial_core.c
    :identifiers: uart_update_timeout uart_get_baud_rate uart_get_divisor
-           uart_match_port uart_write_wakeup uart_register_driver
+           uart_write_wakeup uart_register_driver
            uart_unregister_driver uart_suspend_port uart_resume_port
            uart_add_one_port uart_remove_one_port uart_console_write
            uart_parse_earlycon uart_parse_options uart_set_options

drivers/tty/serial/8250/8250_core.c +11 -0

index c0e8a4efbdcc..6059b84d8364 100644
@@ -630,6 +630,31 @@ EXPORT_SYMBOL(serial8250_resume_port);
  */
 static DEFINE_MUTEX(serial_mutex);
 
+/**
+ * serial8250_match_port - are the two ports equivalent?
+ * @port1: first port
+ * @port2: second port
+ *
+ * This utility function can be used to determine whether two uart_port
+ * structures describe the same port.
+ */
+static bool serial8250_match_port(const struct uart_port *port1,
+				  const struct uart_port *port2)
+{

drivers/tty/serial/8250/8250_core.c EXPORT_SYMBOL(serial8250_resume_port); · line 653

Quoted 10 lines

drivers/tty/serial/8250/8250_core.c +10 -0

+	if (port1->iotype != port2->iotype)
+		return false;
+	else if (port1->iotype == UPIO_PORT)
+		return port1->iobase == port2->iobase;
+	else if (port1->iotype == UPIO_HUB6)
+		return hub6_match_port(port1, port2);
+	else if (uart_iotype_mmio(port1->iotype))
+		return port1->mapbase == port2->mapbase;
+	else if (port1->iotype == UPIO_BUS)
+		return true;

Since this is now moved to 8250-only context, UPIO_BUS case can probably be dropped.

Quoted 79 lines

drivers/tty/serial/8250/8250_core.c +5 -1

+	else
+		return false;
+}
+
 static struct uart_8250_port *serial8250_find_match_or_unused(const struct uart_port *port)
 {
 	int i;

@@ -638,7 +663,7 @@ static struct uart_8250_port *serial8250_find_match_or_unused(const struct uart_
 	 * First, find a port entry which matches.
 	 */
 	for (i = 0; i < nr_uarts; i++)
-		if (uart_match_port(&serial8250_ports[i].port, port))
+		if (serial8250_match_port(&serial8250_ports[i].port, port))
 			return &serial8250_ports[i];
 
 	/* try line number first if still available */

drivers/tty/serial/serial_core.c +0 ~~-27~~

index edd1e7be2a5c..8a6c53a9996e 100644
@@ -33,7 +33,6 @@
 #include <linux/uaccess.h>
 
 #include "serial_base.h"
-#include "8250/8250.h" /* For hub6_match_port() */
 
 /*
  * This is used to lock changes in serial line configuration.

@@ -3229,32 +3228,6 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
 	state->uart_port = NULL;
 }
 
-/**
- * uart_match_port - are the two ports equivalent?
- * @port1: first port
- * @port2: second port
- *
- * This utility function can be used to determine whether two uart_port
- * structures describe the same port.
- */
-bool uart_match_port(const struct uart_port *port1,
-		const struct uart_port *port2)
-{
-	if (port1->iotype != port2->iotype)
-		return false;
-	else if (port1->iotype == UPIO_PORT)
-		return port1->iobase == port2->iobase;
-	else if (port1->iotype == UPIO_HUB6)
-		return hub6_match_port(port1, port2);
-	else if (uart_iotype_mmio(port1->iotype))
-		return port1->mapbase == port2->mapbase;
-	else if (port1->iotype == UPIO_BUS)
-		return true;
-	else
-		return false;
-}
-EXPORT_SYMBOL(uart_match_port);
-
 static struct serial_ctrl_device *
 serial_core_get_ctrl_dev(struct serial_port_device *port_dev)
 {

include/linux/serial_core.h +0 -2

index c4cc4f66af4b..1659805e2233 100644
@@ -1137,8 +1137,6 @@ int uart_add_one_port(struct uart_driver *reg, struct uart_port *port);
 void uart_remove_one_port(struct uart_driver *reg, struct uart_port *port);
 int uart_read_port_properties(struct uart_port *port);
 int uart_read_and_validate_port_properties(struct uart_port *port);
-bool uart_match_port(const struct uart_port *port1,
-		const struct uart_port *port2);
 
 /*
  * Power Management

Signature · 1 line

2.55.0.11.g153666a7d9bb

Signature · 1 line

Hugo Villeneuve

Uwe Kleine-König

author

2026-07-31 09:39 UTC #10 lore raw

PATCH 4/6 serial: 8250: Fix corner case for port matching

In reply to Uwe Kleine-König

In the case CONFIG_SERIAL_8250=y + CONFIG_SERIAL_8250_HUB6=m hub6_match_port() evaluates statically to false in serial8250_match_port() which is a bug and a change in behavior compared to before commit 7ab80d1e7243 ("serial: 8250: fix compile error with hub6_match_port() when compiled as a module").

To make the comparison correct, the dummy implementation of hub6_match_port() must only be used if CONFIG_SERIAL_8250_HUB6 is disabled. Then as there is a bidirectional dependency between 8250_hub6.c and 8250_core.c, put both in the same module.

Fixes 7ab80d1e7243 ("serial: 8250: fix compile error with hub6_match_port() when compiled as a module")

Signed-off-by Uwe Kleine-König <[email protected]>

---
 drivers/tty/serial/8250/8250.h          | 8 +++++++-
 drivers/tty/serial/8250/8250_hub6.c     | 8 +-------
 drivers/tty/serial/8250/8250_platform.c | 4 ++++
 drivers/tty/serial/8250/Kconfig         | 4 ++--
 drivers/tty/serial/8250/Makefile        | 2 +-
 5 files changed, 15 insertions(+), 11 deletions(-)

drivers/tty/serial/8250/8250.h +7 -1

index b62f88eec881..39d72f1480c0 100644
@@ -315,6 +315,12 @@ static inline int serial8250_pnp_init(void) { return 0; }
 static inline void serial8250_pnp_exit(void) { }
 #endif
 
+#ifdef CONFIG_SERIAL_8250_HUB6
+int serial8250_hub6_init(void);
+#else
+static inline int serial8250_hub6_init(void) { return 0; }
+#endif
+
 #ifdef CONFIG_SERIAL_8250_RSA
 void univ8250_rsa_support(struct uart_ops *ops, const struct uart_ops *core_ops);
 void rsa_enable(struct uart_8250_port *up);

@@ -335,7 +341,7 @@ int fintek_8250_probe(struct uart_8250_port *uart);
 static inline int fintek_8250_probe(struct uart_8250_port *uart) { return 0; }
 #endif
 
-#if IS_REACHABLE(CONFIG_SERIAL_8250_HUB6)
+#if IS_ENABLED(CONFIG_SERIAL_8250_HUB6)
 bool hub6_match_port(const struct uart_port *port1, const struct uart_port *port2);
 #else
 static inline bool hub6_match_port(const struct uart_port *port1, const struct uart_port *port2)

drivers/tty/serial/8250/8250_hub6.c +1 -7

index b6767633c966..3e548530ec4e 100644
@@ -49,13 +49,7 @@ bool hub6_match_port(const struct uart_port *port1, const struct uart_port *port
 }
 EXPORT_SYMBOL_GPL(hub6_match_port);
 
-static int __init hub6_init(void)
+int __init serial8250_hub6_init(void)
 {
 	return platform_device_register(&hub6_device);
 }
-
-module_init(hub6_init);
-
-MODULE_AUTHOR("Russell King");
-MODULE_DESCRIPTION("8250 serial probe module for Hub6 cards");
-MODULE_LICENSE("GPL");

drivers/tty/serial/8250/8250_platform.c +4 -0

index 5a0bc1fae11d..e8524d545de9 100644
@@ -319,6 +319,10 @@ static int __init serial8250_init(void)
 	if (ret)
 		goto unreg_uart_drv;
 
+	ret = serial8250_hub6_init();
+	if (ret)
+		goto unreg_pnp;
+
 	serial8250_isa_devs = platform_device_alloc("serial8250", PLAT8250_DEV_LEGACY);
 	if (!serial8250_isa_devs) {
 		ret = -ENOMEM;

drivers/tty/serial/8250/Kconfig +2 -2

index fc3e58d62233..00c2ced12717 100644
@@ -294,8 +294,8 @@ config SERIAL_8250_EXAR_ST16C554
 	  will be called 8250_exar_st16c554.
 
 config SERIAL_8250_HUB6
-	tristate "Support Hub6 cards"
-	depends on SERIAL_8250 != n && ISA && SERIAL_8250_MANY_PORTS
+	bool "Support Hub6 cards"
+	depends on SERIAL_8250_MANY_PORTS && ISA
 	help
 	  Say Y here if you have a HUB6 serial board.

drivers/tty/serial/8250/Makefile +1 -1

index 6d21402b4435..416c32e3aba2 100644
@@ -7,6 +7,7 @@ obj-$(CONFIG_SERIAL_8250)		+= 8250.o
 8250-y					:= 8250_core.o
 8250-y					+= 8250_platform.o
 8250-$(CONFIG_SERIAL_8250_PNP)		+= 8250_pnp.o
+8250-$(CONFIG_SERIAL_8250_HUB6)		+= 8250_hub6.o
 
 obj-$(CONFIG_SERIAL_8250)		+= 8250_base.o
 8250_base-y				:= 8250_port.o

@@ -35,7 +36,6 @@ obj-$(CONFIG_SERIAL_8250_EXAR_ST16C554)	+= 8250_exar_st16c554.o
 obj-$(CONFIG_SERIAL_8250_FOURPORT)	+= 8250_fourport.o
 obj-$(CONFIG_SERIAL_8250_FSL)		+= 8250_fsl.o
 obj-$(CONFIG_SERIAL_8250_HP300)		+= 8250_hp300.o
-obj-$(CONFIG_SERIAL_8250_HUB6)		+= 8250_hub6.o
 obj-$(CONFIG_SERIAL_8250_INGENIC)	+= 8250_ingenic.o
 obj-$(CONFIG_SERIAL_8250_IOC3)		+= 8250_ioc3.o
 obj-$(CONFIG_SERIAL_8250_KEBA)		+= 8250_keba.o

Signature · 1 line

2.55.0.11.g153666a7d9bb

Uwe Kleine-König

author

2026-07-31 09:39 UTC #11 lore raw

PATCH 5/6 serial: 8250: hub6: Add cleanup code

In reply to Uwe Kleine-König

At module load time a platform device is registered. That should be undone when the module is unloaded.

Fixes ec9f47cd6a14 ("[PATCH] Serial: Split 8250 port table")

Signed-off-by Uwe Kleine-König <[email protected]>

---
 drivers/tty/serial/8250/8250.h          | 2 ++
 drivers/tty/serial/8250/8250_hub6.c     | 5 +++++
 drivers/tty/serial/8250/8250_platform.c | 5 ++++-
 3 files changed, 11 insertions(+), 1 deletion(-)

drivers/tty/serial/8250/8250.h +2 -0

index 39d72f1480c0..01fed8d44b7b 100644
@@ -317,8 +317,10 @@ static inline void serial8250_pnp_exit(void) { }
 
 #ifdef CONFIG_SERIAL_8250_HUB6
 int serial8250_hub6_init(void);
+void serial8250_hub6_exit(void);
 #else
 static inline int serial8250_hub6_init(void) { return 0; }
+static inline void serial8250_hub6_exit(void) { }
 #endif
 
 #ifdef CONFIG_SERIAL_8250_RSA

drivers/tty/serial/8250/8250_hub6.c +5 -0

index 3e548530ec4e..55320b794165 100644
@@ -53,3 +53,8 @@ int __init serial8250_hub6_init(void)
 {
 	return platform_device_register(&hub6_device);
 }
+
+int __init_or_module serial8250_hub6_exit(void)
+{
+	return platform_device_unregister(&hub6_device);
+}

drivers/tty/serial/8250/8250_platform.c +4 -1

index e8524d545de9..72076ccb5bde 100644
@@ -326,7 +326,7 @@ static int __init serial8250_init(void)
 	serial8250_isa_devs = platform_device_alloc("serial8250", PLAT8250_DEV_LEGACY);
 	if (!serial8250_isa_devs) {
 		ret = -ENOMEM;
-		goto unreg_pnp;
+		goto unreg_hub6;
 	}
 
 	ret = platform_device_add(serial8250_isa_devs);

@@ -342,6 +342,8 @@ static int __init serial8250_init(void)
 	platform_device_del(serial8250_isa_devs);
 put_dev:
 	platform_device_put(serial8250_isa_devs);
+unreg_hub6:
+	serial8250_hub6_exit();
 unreg_pnp:
 	serial8250_pnp_exit();
 unreg_uart_drv:

@@ -369,6 +371,7 @@ static void __exit serial8250_exit(void)
 	platform_driver_unregister(&serial8250_isa_driver);
 	platform_device_unregister(isa_dev);
 
+	serial8250_hub6_exit();
 	serial8250_pnp_exit();
 
 #ifdef CONFIG_SPARC

Signature · 1 line

2.55.0.11.g153666a7d9bb

Uwe Kleine-König

author

2026-07-31 09:39 UTC #12 lore raw

PATCH 6/6 serial: 8250: pnp: Annotate init and exit functions for conditional discarding

In reply to Uwe Kleine-König

serial8250_pnp_init() is only called from serial8250_init() which is marked with __init, so the pnp init function can also get the same marking. serial8250_pnp_exit() is only called from __init and __exit code, so __init_or_module is appropriate for that.

Signed-off-by Uwe Kleine-König <[email protected]>

---
 drivers/tty/serial/8250/8250_pnp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/tty/serial/8250/8250_pnp.c +2 -2

index b39f005f11cf..e142f4296062 100644
@@ -526,12 +526,12 @@ static struct pnp_driver serial_pnp_driver = {
 	.id_table	= pnp_dev_table,
 };
 
-int serial8250_pnp_init(void)
+int __init serial8250_pnp_init(void)
 {
 	return pnp_register_driver(&serial_pnp_driver);
 }
 
-void serial8250_pnp_exit(void)
+void __init_or_module serial8250_pnp_exit(void)
 {
 	pnp_unregister_driver(&serial_pnp_driver);
 }

Signature · 1 line

2.55.0.11.g153666a7d9bb