[PATCH v1 0/6] serial: Some yak shaving

[PATCH v1 0/6] serial: Some yak shaving

[Uwe Kleine-König]

Hello,

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

> 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
-- 
2.55.0.11.g153666a7d9bb

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

[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 <ukleinek@kernel.org>
---
 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(-)

diff --git a/Documentation/driver-api/serial/driver.rst b/Documentation/driver-api/serial/driver.rst
index c1db6a1a67c4..975b0ba53e98 100644
--- a/Documentation/driver-api/serial/driver.rst
+++ b/Documentation/driver-api/serial/driver.rst
@@ -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
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index c0e8a4efbdcc..6059b84d8364 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -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 */
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index edd1e7be2a5c..8a6c53a9996e 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -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)
 {
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index c4cc4f66af4b..1659805e2233 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -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
-- 
2.55.0.11.g153666a7d9bb