[PATCH 0/9] serial: add new I/O type for SPI and I2C bus devices

[PATCH 0/9] serial: add new I/O type for SPI and I2C bus devices

[Hugo Villeneuve]

Hello,
this patch series add a new I/O type for serial devices on a SPI/I2C bus.

These changes are based on a suggestion [1] made by Ilpo Järvinen during
past sc16is7xx driver patches review.

I decided to use UPIO_BUS/SERIAL_IO_BUS names but maybe other names would
be more appropriate:
   UPIO_SBUS   / SERIAL_IO_SBUS
   UPIO_SERBUS / SERIAL_IO_SERBUS
   UPIO_EXT    / SERIAL_IO_EXT
   UPIO_CUSTOM / SERIAL_IO_CUSTOM
   UPIO_REGMAP / SERIAL_IO_REGMAP  /* Maybe not always true in future? */
   ...

Tested on a imx6 board with two SC16is752 using SPI mode, and a dummy
device-tree entry for a MAX3100:
  dmesg -t | grep "base_baud"
    2020000.serial: ttymxc0 at MMIO 0x2020000 (irq = 197, base_baud = 5000000) is a IMX
    21e8000.serial: ttymxc1 at MMIO 0x21e8000 (irq = 198, base_baud = 5000000) is a IMX
    spi1.0: ttySC0 (irq = 165, base_baud = 1500000) is a SC16IS752
    spi1.0: ttySC1 (irq = 165, base_baud = 1500000) is a SC16IS752
    spi3.0: ttySC2 (irq = 37, base_baud = 1500000) is a SC16IS752
    spi3.0: ttySC3 (irq = 37, base_baud = 1500000) is a SC16IS752
    spi3.1: ttyMAX0 (irq = 0, base_baud = 0) is a MAX3100
Note that before these patches, max3100 silently failed in uart_configure_port()
because membase/iobase/mapbase were zero.

For max310x and SC16is7xx in i2c mode, tested only that driver is properly
registered by using i2c-stub.

Also tested on a custom board with a Renesas RZ/G2L cpu (sh-sci driver) to
confirm there is no regression:
  dmesg -t | grep "base_baud"
    1004b800.serial: ttySC0 at MMIO 0x1004b800 (irq = 35, base_baud = 0) is a scif
    1004bc00.serial: ttySC1 at MMIO 0x1004bc00 (irq = 40, base_baud = 0) is a scif

Maybe some of the patches could be merged together, but for now I decided
to keep them separate to help the review process.

Thank you.

[1] https://lore.kernel.org/lkml/2936e18f-44ea-faed-9fa0-2ddefe7c3194@linux.intel.com/raw

---
Hugo Villeneuve (9):
      serial: core: add uart_iotype_mmio/legacy_io helper functions
      serial: core: use uart_iotype_*() to simplify code
      serial: 8250: use uart_iotype_*() to simplify code
      serial: core: fix indentation/alignment
      serial: core: add new I/O type for SPI and I2C bus devices
      serial: core: prevent irrelevant I/O infos display for UPIO_BUS
      serial: sc16is7xx: use new UPIO_BUS as iotype
      serial: max310x: use new UPIO_BUS as iotype
      serial: max3100: use new UPIO_BUS as iotype

 drivers/tty/serial/8250/8250_port.c |  33 ++--------
 drivers/tty/serial/max3100.c        |   1 +
 drivers/tty/serial/max310x.c        |   9 +--
 drivers/tty/serial/sc16is7xx.c      |   9 +--
 drivers/tty/serial/serial_core.c    | 121 ++++++++++++++++++++----------------
 include/linux/serial_core.h         |   5 ++
 include/uapi/linux/serial.h         |   1 +
 7 files changed, 83 insertions(+), 96 deletions(-)
---
base-commit: 2e68039281932e6dc37718a1ea7cbb8e2cda42e6
change-id: 20260423-tty-upio-c8b66a0a593e

Best regards,
-- 
Hugo Villeneuve <hvilleneuve@dimonoff.com>

[PATCH 1/9] serial: core: add uart_iotype_mmio/legacy_io helper functions

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

To help simplify code that check on the io type mode of the port.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
Will also be used by future commit that add UPIO_BUS.
---
 drivers/tty/serial/serial_core.c | 28 ++++++++++++++++++++++++++++
 include/linux/serial_core.h      |  4 ++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 89cebdd278410a5a97f3f5ee1b9c171715145067..b1cf45a8fc854cd97e349ff077d83b42e3ef8b16 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1966,6 +1966,34 @@ static const char *uart_type(struct uart_port *port)
 	return str;
 }
 
+bool uart_iotype_mmio(enum uart_iotype iotype)
+{
+	switch (iotype) {
+	case UPIO_MEM:
+	case UPIO_MEM32:
+	case UPIO_AU:
+	case UPIO_TSI:
+	case UPIO_MEM32BE:
+	case UPIO_MEM16:
+		return true;
+	default:
+		return false;
+	}
+}
+EXPORT_SYMBOL_GPL(uart_iotype_mmio);
+
+bool uart_iotype_legacy_io(enum uart_iotype iotype)
+{
+	switch (iotype) {
+	case UPIO_PORT:
+	case UPIO_HUB6:
+		return true;
+	default:
+		return false;
+	}
+}
+EXPORT_SYMBOL_GPL(uart_iotype_legacy_io);
+
 #ifdef CONFIG_PROC_FS
 
 static void uart_line_info(struct seq_file *m, struct uart_state *state)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 666430b4789977ae19641442b369eb1c773233f4..fc3f5ebe389658c197ffc105ce4ac11cacef59bb 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -1306,4 +1306,8 @@ static inline int uart_handle_break(struct uart_port *port)
 					 !((cflag) & CLOCAL))
 
 int uart_get_rs485_mode(struct uart_port *port);
+
+bool uart_iotype_mmio(enum uart_iotype iotype);
+bool uart_iotype_legacy_io(enum uart_iotype iotype);
+
 #endif /* LINUX_SERIAL_CORE_H */

-- 
2.47.3

[PATCH 2/9] serial: core: use uart_iotype_*() to simplify code

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Make use of new functions uart_iotype_mmio() and uart_iotype_legacy_io()
to simplify and improve code readability.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/serial_core.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index b1cf45a8fc854cd97e349ff077d83b42e3ef8b16..fc273f8f9e75de89dca1ac1aca3589567bcf8a18 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3221,23 +3221,17 @@ bool uart_match_port(const struct uart_port *port1,
 {
 	if (port1->iotype != port2->iotype)
 		return false;
-
-	switch (port1->iotype) {
-	case UPIO_PORT:
-		return port1->iobase == port2->iobase;
-	case UPIO_HUB6:
-		return port1->iobase == port2->iobase &&
-		       port1->hub6   == port2->hub6;
-	case UPIO_MEM:
-	case UPIO_MEM16:
-	case UPIO_MEM32:
-	case UPIO_MEM32BE:
-	case UPIO_AU:
-	case UPIO_TSI:
+	else if (uart_iotype_mmio(port1->iotype))
 		return port1->mapbase == port2->mapbase;
-	default:
+	else if (uart_iotype_legacy_io(port1->iotype))
+		if (port1->iobase != port2->iobase)
+			return false;
+		else if (port1->iotype == UPIO_HUB6 && port1->hub6 != port2->hub6)
+			return false;
+		else
+			return true;
+	else
 		return false;
-	}
 }
 EXPORT_SYMBOL(uart_match_port);
 

-- 
2.47.3

[PATCH 3/9] serial: 8250: use uart_iotype_*() to simplify code

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Make use of new functions uart_iotype_mmio() and uart_iotype_legacy_io()
to simplify and improve code readability.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/8250/8250_port.c | 33 +++++----------------------------
 1 file changed, 5 insertions(+), 28 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index af78cc02f38e719573becd0aea226f7790555a3e..48ff4cad028ff0b6a9c7167ea8d793b3cb0f32da 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2863,13 +2863,7 @@ static int serial8250_request_std_resource(struct uart_8250_port *up)
 	unsigned int size = serial8250_port_size(up);
 	struct uart_port *port = &up->port;
 
-	switch (port->iotype) {
-	case UPIO_AU:
-	case UPIO_TSI:
-	case UPIO_MEM32:
-	case UPIO_MEM32BE:
-	case UPIO_MEM16:
-	case UPIO_MEM:
+	if (uart_iotype_mmio(port->iotype)) {
 		if (!port->mapbase)
 			return -EINVAL;
 
@@ -2883,14 +2877,9 @@ static int serial8250_request_std_resource(struct uart_8250_port *up)
 				return -ENOMEM;
 			}
 		}
-		return 0;
-	case UPIO_HUB6:
-	case UPIO_PORT:
+	} else if (uart_iotype_legacy_io(port->iotype)) {
 		if (!request_region(port->iobase, size, "serial"))
 			return -EBUSY;
-		return 0;
-	case UPIO_UNKNOWN:
-		break;
 	}
 
 	return 0;
@@ -2901,15 +2890,9 @@ static void serial8250_release_std_resource(struct uart_8250_port *up)
 	unsigned int size = serial8250_port_size(up);
 	struct uart_port *port = &up->port;
 
-	switch (port->iotype) {
-	case UPIO_AU:
-	case UPIO_TSI:
-	case UPIO_MEM32:
-	case UPIO_MEM32BE:
-	case UPIO_MEM16:
-	case UPIO_MEM:
+	if (uart_iotype_mmio(port->iotype)) {
 		if (!port->mapbase)
-			break;
+			return;
 
 		if (port->flags & UPF_IOREMAP) {
 			iounmap(port->membase);
@@ -2917,14 +2900,8 @@ static void serial8250_release_std_resource(struct uart_8250_port *up)
 		}
 
 		release_mem_region(port->mapbase, size);
-		break;
-
-	case UPIO_HUB6:
-	case UPIO_PORT:
+	} else if (uart_iotype_legacy_io(port->iotype)) {
 		release_region(port->iobase, size);
-		break;
-	case UPIO_UNKNOWN:
-		break;
 	}
 }
 

-- 
2.47.3

[PATCH 4/9] serial: core: fix indentation/alignment

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Fixes the following checkpatch warnings:
    CHECK: Alignment should match open parenthesis

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
Without this, the next patch will have a checkpatch warning.
---
 drivers/tty/serial/serial_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index fc273f8f9e75de89dca1ac1aca3589567bcf8a18..cb5da344305e397b42f47bdbc2c053fc487ed784 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2507,10 +2507,10 @@ uart_report_port(struct uart_driver *drv, struct uart_port *port)
 	}
 
 	pr_info("%s%s%s at %s (irq = %u, base_baud = %u) is a %s\n",
-	       port->dev ? dev_name(port->dev) : "",
-	       port->dev ? ": " : "",
-	       port->name,
-	       address, port->irq, port->uartclk / 16, uart_type(port));
+		port->dev ? dev_name(port->dev) : "",
+		port->dev ? ": " : "",
+		port->name,
+		address, port->irq, port->uartclk / 16, uart_type(port));
 
 	/* The magic multiplier feature is a bit obscure, so report it too.  */
 	if (port->flags & UPF_MAGIC_MULTIPLIER)

-- 
2.47.3

[PATCH 5/9] serial: core: add new I/O type for SPI and I2C bus devices

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

I2C/SPI serial drivers don't use the following struct uart_port variables:
    port->membase
    port->mapbase
    port->iobase

However, they are forced to set membase to a non-zero value so that
uart_configure_port() will succeed because of the following check:

    /* If there isn't a port here, don't do anything further. */
    if (!port->iobase && !port->mapbase && !port->membase)
        return;

Add a new I/O type for SPI and I2C bus devices to remove the need to
implement the kind of above-mentioned ambiguous workarounds to make them
work.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
With this change, uart_match_port() will always return true for UPIO_BUS
types, and this function is not used by any of the SPI/I2C drivers.
---
 drivers/tty/serial/serial_core.c | 11 ++++++-----
 include/linux/serial_core.h      |  1 +
 include/uapi/linux/serial.h      |  1 +
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index cb5da344305e397b42f47bdbc2c053fc487ed784..0bfdb69817e4259681fbc4658c9a68200aa2b65f 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2527,11 +2527,10 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
 {
 	unsigned int flags;
 
-	/*
-	 * If there isn't a port here, don't do anything further.
-	 */
-	if (!port->iobase && !port->mapbase && !port->membase)
-		return;
+	/* If there isn't a port here, don't do anything further. */
+	if (uart_iotype_mmio(port->iotype) || uart_iotype_legacy_io(port->iotype))
+		if (!port->iobase && !port->mapbase && !port->membase)
+			return;
 
 	/*
 	 * Now do the auto configuration stuff.  Note that config_port
@@ -3230,6 +3229,8 @@ bool uart_match_port(const struct uart_port *port1,
 			return false;
 		else
 			return true;
+	else if (port1->iotype == UPIO_BUS)
+		return true;
 	else
 		return false;
 }
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index fc3f5ebe389658c197ffc105ce4ac11cacef59bb..626dd939c53c9f920d71aadd360ec0ea0bacce0d 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -437,6 +437,7 @@ enum uart_iotype {
 	UPIO_TSI	= SERIAL_IO_TSI,	/* Tsi108/109 type IO */
 	UPIO_MEM32BE	= SERIAL_IO_MEM32BE,	/* 32b big endian */
 	UPIO_MEM16	= SERIAL_IO_MEM16,	/* 16b little endian */
+	UPIO_BUS	= SERIAL_IO_BUS,	/* Serial bus I/O access (ex: SPI, I2C) */
 };
 
 struct uart_port {
diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index de9b4733607e6b61b08ff7089ff90070168ff4a2..e6f61538fc2837a264d27942afaaf3f12e743445 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -72,6 +72,7 @@ struct serial_struct {
 #define SERIAL_IO_TSI	  5
 #define SERIAL_IO_MEM32BE 6
 #define SERIAL_IO_MEM16	7
+#define SERIAL_IO_BUS	8
 
 #define UART_CLEAR_FIFO		0x01
 #define UART_USE_FIFO		0x02

-- 
2.47.3

[PATCH 6/9] serial: core: prevent irrelevant I/O infos display for UPIO_BUS

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

It doesn't make sense to display irrelevant MMIO or legacy I/O information
for serial devices on I2C or SPI busses. Now that we have a separate I/O
type for these types of devices, prevent display of I/O information for
them. Using uart_iotype_*() functions to do so also addresses the now
invalid check for "iotype >= UPIO_MEM".

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/serial_core.c | 50 ++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 0bfdb69817e4259681fbc4658c9a68200aa2b65f..42559eda6fc134de77c3a7b850d565ebdc89e216 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2011,13 +2011,18 @@ static void uart_line_info(struct seq_file *m, struct uart_state *state)
 	if (!uport)
 		return;
 
-	mmio = uport->iotype >= UPIO_MEM;
-	seq_printf(m, "%u: uart:%s %s%08llX irq:%u",
-			uport->line, uart_type(uport),
-			mmio ? "mmio:0x" : "port:",
-			mmio ? (unsigned long long)uport->mapbase
-			     : (unsigned long long)uport->iobase,
-			uport->irq);
+	seq_printf(m, "%u: uart:%s", uport->line, uart_type(uport));
+
+	mmio = uart_iotype_mmio(uport->iotype);
+
+	if (mmio || uart_iotype_legacy_io(uport->iotype)) {
+		seq_printf(m, " %s%08llX",
+			   mmio ? "mmio:0x" : "port:",
+			   mmio ? (unsigned long long)uport->mapbase
+			   : (unsigned long long)uport->iobase);
+	}
+
+	seq_printf(m, "irq:%u", uport->irq);
 
 	if (uport->type == PORT_UNKNOWN) {
 		seq_putc(m, '\n');
@@ -2482,31 +2487,20 @@ EXPORT_SYMBOL(uart_resume_port);
 static inline void
 uart_report_port(struct uart_driver *drv, struct uart_port *port)
 {
-	char address[64];
+	char address[64] = "";
 
-	switch (port->iotype) {
-	case UPIO_PORT:
-		snprintf(address, sizeof(address), "I/O 0x%lx", port->iobase);
-		break;
-	case UPIO_HUB6:
+	if (uart_iotype_mmio(port->iotype))
 		snprintf(address, sizeof(address),
-			 "I/O 0x%lx offset 0x%x", port->iobase, port->hub6);
-		break;
-	case UPIO_MEM:
-	case UPIO_MEM16:
-	case UPIO_MEM32:
-	case UPIO_MEM32BE:
-	case UPIO_AU:
-	case UPIO_TSI:
-		snprintf(address, sizeof(address),
-			 "MMIO 0x%llx", (unsigned long long)port->mapbase);
-		break;
-	default:
-		strscpy(address, "*unknown*", sizeof(address));
-		break;
+			 " at MMIO 0x%llx", (unsigned long long)port->mapbase);
+	else if (uart_iotype_legacy_io(port->iotype)) {
+		if (port->iotype == UPIO_PORT)
+			snprintf(address, sizeof(address), " at I/O 0x%lx", port->iobase);
+		else if (port->iotype == UPIO_HUB6)
+			snprintf(address, sizeof(address),
+				 " at I/O 0x%lx offset 0x%x", port->iobase, port->hub6);
 	}
 
-	pr_info("%s%s%s at %s (irq = %u, base_baud = %u) is a %s\n",
+	pr_info("%s%s%s%s (irq = %u, base_baud = %u) is a %s\n",
 		port->dev ? dev_name(port->dev) : "",
 		port->dev ? ": " : "",
 		port->name,

-- 
2.47.3

[PATCH 7/9] serial: sc16is7xx: use new UPIO_BUS as iotype

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Now that we have a new UPIO_BUS I/O type, use it to register our serial
port and remove ambiguous membase/iobase workaround.

Note that commit 5da6b1c079e6 ("sc16is7xx: Set iobase to device index")
used the iobase field as an index within the device to allow infering
the order through sysfs, but this is no longer needed since
commit 1ef2c2df1199 ("serial: core: Fix serial core controller port name
to show controller id").

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
This means that displaying iomem_base will now always be zero:

cat /sys/class/tty/ttySC0/iomem_base
0x0
cat /sys/class/tty/ttySC1/iomem_base
0x0
...

But the index can be properly displayed with this instead (example with two
sc16is7xx devices):
$> ls -al /sys/class/tty/ttySC*/device
... /sys/class/tty/ttySC0/device -> ../../../spi1.0:0.0
... /sys/class/tty/ttySC1/device -> ../../../spi1.0:0.1
... /sys/class/tty/ttySC2/device -> ../../../spi3.0:0.0
... /sys/class/tty/ttySC3/device -> ../../../spi3.0:0.1
---
 drivers/tty/serial/sc16is7xx.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 1fd64a47341d8263c82fcadd90a4c2e549193e19..4b638e69f36f2d20253d8de31f52ecfe349aa39f 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1472,14 +1472,7 @@ static int sc16is7xx_setup_channel(struct sc16is7xx_one *one, int i,
 	port->type	= PORT_SC16IS7XX;
 	port->fifosize	= SC16IS7XX_FIFO_SIZE;
 	port->flags	= UPF_FIXED_TYPE | UPF_LOW_LATENCY;
-	port->iobase	= i;
-	/*
-	 * Use all ones as membase to make sure uart_configure_port() in
-	 * serial_core.c does not abort for SPI/I2C devices where the
-	 * membase address is not applicable.
-	 */
-	port->membase	= (void __iomem *)~0;
-	port->iotype	= UPIO_PORT;
+	port->iotype	= UPIO_BUS;
 	port->rs485_config = sc16is7xx_config_rs485;
 	port->rs485_supported = sc16is7xx_rs485_supported;
 	port->ops	= &sc16is7xx_ops;

-- 
2.47.3

[PATCH 8/9] serial: max310x: use new UPIO_BUS as iotype

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Now that we have a new UPIO_BUS I/O type, use it to register our serial
port and remove obscure membase/iobase workaround.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/max310x.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index ac7d3f197c3a5ce3531d5607f48e21a807314021..cd0496e307091dae1f53911abf86726f52d5770d 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1380,14 +1380,7 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
 		s->p[i].port.type	= PORT_MAX310X;
 		s->p[i].port.fifosize	= MAX310X_FIFO_SIZE;
 		s->p[i].port.flags	= UPF_FIXED_TYPE | UPF_LOW_LATENCY;
-		s->p[i].port.iotype	= UPIO_PORT;
-		s->p[i].port.iobase	= i;
-		/*
-		 * Use all ones as membase to make sure uart_configure_port() in
-		 * serial_core.c does not abort for SPI/I2C devices where the
-		 * membase address is not applicable.
-		 */
-		s->p[i].port.membase	= (void __iomem *)~0;
+		s->p[i].port.iotype	= UPIO_BUS;
 		s->p[i].port.uartclk	= uartclk;
 		s->p[i].port.rs485_config = max310x_rs485_config;
 		s->p[i].port.rs485_supported = max310x_rs485_supported;

-- 
2.47.3

[PATCH 9/9] serial: max3100: use new UPIO_BUS as iotype

[Hugo Villeneuve]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Now that we have a new UPIO_BUS I/O type, use it to register our serial
port. This allows the driver to work properly when using DT where
membase/iobase are not set.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/tty/serial/max3100.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/max3100.c b/drivers/tty/serial/max3100.c
index 475b0a6efce4bec36aa4b94ccff83934dbdf9246..17a2ff410305142798ddbbb22b46f18eadce91aa 100644
--- a/drivers/tty/serial/max3100.c
+++ b/drivers/tty/serial/max3100.c
@@ -725,6 +725,7 @@ static int max3100_probe(struct spi_device *spi)
 	max3100s[i]->port.ops = &max3100_ops;
 	max3100s[i]->port.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF;
 	max3100s[i]->port.line = i;
+	max3100s[i]->port.iotype = UPIO_BUS;
 	max3100s[i]->port.type = PORT_MAX3100;
 	max3100s[i]->port.dev = &spi->dev;
 

-- 
2.47.3

Re: [PATCH 6/9] serial: core: prevent irrelevant I/O infos display for UPIO_BUS

[Ilpo Järvinen]

On Thu, 23 Apr 2026, Hugo Villeneuve wrote:

> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> It doesn't make sense to display irrelevant MMIO or legacy I/O information
> for serial devices on I2C or SPI busses. Now that we have a separate I/O
> type for these types of devices, prevent display of I/O information for
> them. Using uart_iotype_*() functions to do so also addresses the now
> invalid check for "iotype >= UPIO_MEM".
> 
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  drivers/tty/serial/serial_core.c | 50 ++++++++++++++++++----------------------
>  1 file changed, 22 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 0bfdb69817e4259681fbc4658c9a68200aa2b65f..42559eda6fc134de77c3a7b850d565ebdc89e216 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -2011,13 +2011,18 @@ static void uart_line_info(struct seq_file *m, struct uart_state *state)
>  	if (!uport)
>  		return;
>  
> -	mmio = uport->iotype >= UPIO_MEM;
> -	seq_printf(m, "%u: uart:%s %s%08llX irq:%u",
> -			uport->line, uart_type(uport),
> -			mmio ? "mmio:0x" : "port:",
> -			mmio ? (unsigned long long)uport->mapbase
> -			     : (unsigned long long)uport->iobase,
> -			uport->irq);
> +	seq_printf(m, "%u: uart:%s", uport->line, uart_type(uport));
> +
> +	mmio = uart_iotype_mmio(uport->iotype);
> +
> +	if (mmio || uart_iotype_legacy_io(uport->iotype)) {
> +		seq_printf(m, " %s%08llX",
> +			   mmio ? "mmio:0x" : "port:",
> +			   mmio ? (unsigned long long)uport->mapbase
> +			   : (unsigned long long)uport->iobase);

This should align to ?

> +	}
> +
> +	seq_printf(m, "irq:%u", uport->irq);
>  
>  	if (uport->type == PORT_UNKNOWN) {
>  		seq_putc(m, '\n');
> @@ -2482,31 +2487,20 @@ EXPORT_SYMBOL(uart_resume_port);
>  static inline void
>  uart_report_port(struct uart_driver *drv, struct uart_port *port)
>  {
> -	char address[64];
> +	char address[64] = "";
>  
> -	switch (port->iotype) {
> -	case UPIO_PORT:
> -		snprintf(address, sizeof(address), "I/O 0x%lx", port->iobase);
> -		break;
> -	case UPIO_HUB6:
> +	if (uart_iotype_mmio(port->iotype))
>  		snprintf(address, sizeof(address),
> -			 "I/O 0x%lx offset 0x%x", port->iobase, port->hub6);
> -		break;
> -	case UPIO_MEM:
> -	case UPIO_MEM16:
> -	case UPIO_MEM32:
> -	case UPIO_MEM32BE:
> -	case UPIO_AU:
> -	case UPIO_TSI:
> -		snprintf(address, sizeof(address),
> -			 "MMIO 0x%llx", (unsigned long long)port->mapbase);
> -		break;
> -	default:
> -		strscpy(address, "*unknown*", sizeof(address));
> -		break;
> +			 " at MMIO 0x%llx", (unsigned long long)port->mapbase);
> +	else if (uart_iotype_legacy_io(port->iotype)) {
> +		if (port->iotype == UPIO_PORT)
> +			snprintf(address, sizeof(address), " at I/O 0x%lx", port->iobase);
> +		else if (port->iotype == UPIO_HUB6)
> +			snprintf(address, sizeof(address),
> +				 " at I/O 0x%lx offset 0x%x", port->iobase, port->hub6);

Please use scnprintf() so we could perhaps one day get rid of snprintf() 
entirely.

>  	}
>  
> -	pr_info("%s%s%s at %s (irq = %u, base_baud = %u) is a %s\n",
> +	pr_info("%s%s%s%s (irq = %u, base_baud = %u) is a %s\n",
>  		port->dev ? dev_name(port->dev) : "",
>  		port->dev ? ": " : "",
>  		port->name,
> 
> 

-- 
 i.

Re: [PATCH 2/9] serial: core: use uart_iotype_*() to simplify code

[Ilpo Järvinen]

On Thu, 23 Apr 2026, Hugo Villeneuve wrote:

> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> Make use of new functions uart_iotype_mmio() and uart_iotype_legacy_io()
> to simplify and improve code readability.
> 
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  drivers/tty/serial/serial_core.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index b1cf45a8fc854cd97e349ff077d83b42e3ef8b16..fc273f8f9e75de89dca1ac1aca3589567bcf8a18 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -3221,23 +3221,17 @@ bool uart_match_port(const struct uart_port *port1,
>  {
>  	if (port1->iotype != port2->iotype)
>  		return false;
> -
> -	switch (port1->iotype) {
> -	case UPIO_PORT:
> -		return port1->iobase == port2->iobase;
> -	case UPIO_HUB6:
> -		return port1->iobase == port2->iobase &&
> -		       port1->hub6   == port2->hub6;
> -	case UPIO_MEM:
> -	case UPIO_MEM16:
> -	case UPIO_MEM32:
> -	case UPIO_MEM32BE:
> -	case UPIO_AU:
> -	case UPIO_TSI:
> +	else if (uart_iotype_mmio(port1->iotype))
>  		return port1->mapbase == port2->mapbase;
> -	default:
> +	else if (uart_iotype_legacy_io(port1->iotype))
> +		if (port1->iobase != port2->iobase)
> +			return false;
> +		else if (port1->iotype == UPIO_HUB6 && port1->hub6 != port2->hub6)

I suggest placing the entire hub6 related check (including the iobase 
check) into own function in 8250_hub6.c and add a stub for the case when 
hub6 code is not even built into kernel into 8250.h.

> +			return false;
> +		else
> +			return true;
> +	else
>  		return false;
> -	}
>  }
>  EXPORT_SYMBOL(uart_match_port);
>  
> 
> 

-- 
 i.

Re: [PATCH 0/9] serial: add new I/O type for SPI and I2C bus devices

[Ilpo Järvinen]

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

On Thu, 23 Apr 2026, Hugo Villeneuve wrote:

> Hello,
> this patch series add a new I/O type for serial devices on a SPI/I2C bus.
> 
> These changes are based on a suggestion [1] made by Ilpo Järvinen during
> past sc16is7xx driver patches review.

Hehe, I didn't even recall suggesting this (nor why) :-).

I like this series. Some, relatively minor, comments on the individual 
patches. Thanks for looking into solving this properly.

-- 
 i.

> I decided to use UPIO_BUS/SERIAL_IO_BUS names but maybe other names would
> be more appropriate:
>    UPIO_SBUS   / SERIAL_IO_SBUS
>    UPIO_SERBUS / SERIAL_IO_SERBUS
>    UPIO_EXT    / SERIAL_IO_EXT
>    UPIO_CUSTOM / SERIAL_IO_CUSTOM
>    UPIO_REGMAP / SERIAL_IO_REGMAP  /* Maybe not always true in future? */
>    ...
> 
> Tested on a imx6 board with two SC16is752 using SPI mode, and a dummy
> device-tree entry for a MAX3100:
>   dmesg -t | grep "base_baud"
>     2020000.serial: ttymxc0 at MMIO 0x2020000 (irq = 197, base_baud = 5000000) is a IMX
>     21e8000.serial: ttymxc1 at MMIO 0x21e8000 (irq = 198, base_baud = 5000000) is a IMX
>     spi1.0: ttySC0 (irq = 165, base_baud = 1500000) is a SC16IS752
>     spi1.0: ttySC1 (irq = 165, base_baud = 1500000) is a SC16IS752
>     spi3.0: ttySC2 (irq = 37, base_baud = 1500000) is a SC16IS752
>     spi3.0: ttySC3 (irq = 37, base_baud = 1500000) is a SC16IS752
>     spi3.1: ttyMAX0 (irq = 0, base_baud = 0) is a MAX3100
> Note that before these patches, max3100 silently failed in uart_configure_port()
> because membase/iobase/mapbase were zero.
>
> For max310x and SC16is7xx in i2c mode, tested only that driver is properly
> registered by using i2c-stub.
> 
> Also tested on a custom board with a Renesas RZ/G2L cpu (sh-sci driver) to
> confirm there is no regression:
>   dmesg -t | grep "base_baud"
>     1004b800.serial: ttySC0 at MMIO 0x1004b800 (irq = 35, base_baud = 0) is a scif
>     1004bc00.serial: ttySC1 at MMIO 0x1004bc00 (irq = 40, base_baud = 0) is a scif
> 
> Maybe some of the patches could be merged together, but for now I decided
> to keep them separate to help the review process.
> 
> Thank you.
> 
> [1] https://lore.kernel.org/lkml/2936e18f-44ea-faed-9fa0-2ddefe7c3194@linux.intel.com/raw
> 
> ---
> Hugo Villeneuve (9):
>       serial: core: add uart_iotype_mmio/legacy_io helper functions
>       serial: core: use uart_iotype_*() to simplify code
>       serial: 8250: use uart_iotype_*() to simplify code
>       serial: core: fix indentation/alignment
>       serial: core: add new I/O type for SPI and I2C bus devices
>       serial: core: prevent irrelevant I/O infos display for UPIO_BUS
>       serial: sc16is7xx: use new UPIO_BUS as iotype
>       serial: max310x: use new UPIO_BUS as iotype
>       serial: max3100: use new UPIO_BUS as iotype
> 
>  drivers/tty/serial/8250/8250_port.c |  33 ++--------
>  drivers/tty/serial/max3100.c        |   1 +
>  drivers/tty/serial/max310x.c        |   9 +--
>  drivers/tty/serial/sc16is7xx.c      |   9 +--
>  drivers/tty/serial/serial_core.c    | 121 ++++++++++++++++++++----------------
>  include/linux/serial_core.h         |   5 ++
>  include/uapi/linux/serial.h         |   1 +
>  7 files changed, 83 insertions(+), 96 deletions(-)
> ---
> base-commit: 2e68039281932e6dc37718a1ea7cbb8e2cda42e6
> change-id: 20260423-tty-upio-c8b66a0a593e
> 
> Best regards,
> 

Re: [PATCH 6/9] serial: core: prevent irrelevant I/O infos display for UPIO_BUS

[Hugo Villeneuve]

On Fri, 24 Apr 2026 13:51:52 +0300 (EEST)
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:

> On Thu, 23 Apr 2026, Hugo Villeneuve wrote:
> 
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > It doesn't make sense to display irrelevant MMIO or legacy I/O information
> > for serial devices on I2C or SPI busses. Now that we have a separate I/O
> > type for these types of devices, prevent display of I/O information for
> > them. Using uart_iotype_*() functions to do so also addresses the now
> > invalid check for "iotype >= UPIO_MEM".
> > 
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > ---
> >  drivers/tty/serial/serial_core.c | 50 ++++++++++++++++++----------------------
> >  1 file changed, 22 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > index 0bfdb69817e4259681fbc4658c9a68200aa2b65f..42559eda6fc134de77c3a7b850d565ebdc89e216 100644
> > --- a/drivers/tty/serial/serial_core.c
> > +++ b/drivers/tty/serial/serial_core.c
> > @@ -2011,13 +2011,18 @@ static void uart_line_info(struct seq_file *m, struct uart_state *state)
> >  	if (!uport)
> >  		return;
> >  
> > -	mmio = uport->iotype >= UPIO_MEM;
> > -	seq_printf(m, "%u: uart:%s %s%08llX irq:%u",
> > -			uport->line, uart_type(uport),
> > -			mmio ? "mmio:0x" : "port:",
> > -			mmio ? (unsigned long long)uport->mapbase
> > -			     : (unsigned long long)uport->iobase,
> > -			uport->irq);
> > +	seq_printf(m, "%u: uart:%s", uport->line, uart_type(uport));
> > +
> > +	mmio = uart_iotype_mmio(uport->iotype);
> > +
> > +	if (mmio || uart_iotype_legacy_io(uport->iotype)) {
> > +		seq_printf(m, " %s%08llX",
> > +			   mmio ? "mmio:0x" : "port:",
> > +			   mmio ? (unsigned long long)uport->mapbase
> > +			   : (unsigned long long)uport->iobase);
> 
> This should align to ?

Ok, my auto-indent is not working for that.

Will change it to:
    seq_printf(m, " %s%08llX", mmio ? "mmio:0x" : "port:",
               mmio ? (unsigned long long)uport->mapbase
                    : (unsigned long long)uport->iobase);

> 
> > +	}
> > +
> > +	seq_printf(m, "irq:%u", uport->irq);
> >  
> >  	if (uport->type == PORT_UNKNOWN) {
> >  		seq_putc(m, '\n');
> > @@ -2482,31 +2487,20 @@ EXPORT_SYMBOL(uart_resume_port);
> >  static inline void
> >  uart_report_port(struct uart_driver *drv, struct uart_port *port)
> >  {
> > -	char address[64];
> > +	char address[64] = "";
> >  
> > -	switch (port->iotype) {
> > -	case UPIO_PORT:
> > -		snprintf(address, sizeof(address), "I/O 0x%lx", port->iobase);
> > -		break;
> > -	case UPIO_HUB6:
> > +	if (uart_iotype_mmio(port->iotype))
> >  		snprintf(address, sizeof(address),
> > -			 "I/O 0x%lx offset 0x%x", port->iobase, port->hub6);
> > -		break;
> > -	case UPIO_MEM:
> > -	case UPIO_MEM16:
> > -	case UPIO_MEM32:
> > -	case UPIO_MEM32BE:
> > -	case UPIO_AU:
> > -	case UPIO_TSI:
> > -		snprintf(address, sizeof(address),
> > -			 "MMIO 0x%llx", (unsigned long long)port->mapbase);
> > -		break;
> > -	default:
> > -		strscpy(address, "*unknown*", sizeof(address));
> > -		break;
> > +			 " at MMIO 0x%llx", (unsigned long long)port->mapbase);
> > +	else if (uart_iotype_legacy_io(port->iotype)) {
> > +		if (port->iotype == UPIO_PORT)
> > +			snprintf(address, sizeof(address), " at I/O 0x%lx", port->iobase);
> > +		else if (port->iotype == UPIO_HUB6)
> > +			snprintf(address, sizeof(address),
> > +				 " at I/O 0x%lx offset 0x%x", port->iobase, port->hub6);
> 
> Please use scnprintf() so we could perhaps one day get rid of snprintf() 
> entirely.

Ok will do.

> 
> >  	}
> >  
> > -	pr_info("%s%s%s at %s (irq = %u, base_baud = %u) is a %s\n",
> > +	pr_info("%s%s%s%s (irq = %u, base_baud = %u) is a %s\n",
> >  		port->dev ? dev_name(port->dev) : "",
> >  		port->dev ? ": " : "",
> >  		port->name,
> > 
> > 
> 
> -- 
>  i.
> 
> 


-- 
Hugo Villeneuve

Re: [PATCH 2/9] serial: core: use uart_iotype_*() to simplify code

[Hugo Villeneuve]

On Fri, 24 Apr 2026 14:13:02 +0300 (EEST)
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:

> On Thu, 23 Apr 2026, Hugo Villeneuve wrote:
> 
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > Make use of new functions uart_iotype_mmio() and uart_iotype_legacy_io()
> > to simplify and improve code readability.
> > 
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > ---
> >  drivers/tty/serial/serial_core.c | 24 +++++++++---------------
> >  1 file changed, 9 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > index b1cf45a8fc854cd97e349ff077d83b42e3ef8b16..fc273f8f9e75de89dca1ac1aca3589567bcf8a18 100644
> > --- a/drivers/tty/serial/serial_core.c
> > +++ b/drivers/tty/serial/serial_core.c
> > @@ -3221,23 +3221,17 @@ bool uart_match_port(const struct uart_port *port1,
> >  {
> >  	if (port1->iotype != port2->iotype)
> >  		return false;
> > -
> > -	switch (port1->iotype) {
> > -	case UPIO_PORT:
> > -		return port1->iobase == port2->iobase;
> > -	case UPIO_HUB6:
> > -		return port1->iobase == port2->iobase &&
> > -		       port1->hub6   == port2->hub6;
> > -	case UPIO_MEM:
> > -	case UPIO_MEM16:
> > -	case UPIO_MEM32:
> > -	case UPIO_MEM32BE:
> > -	case UPIO_AU:
> > -	case UPIO_TSI:
> > +	else if (uart_iotype_mmio(port1->iotype))
> >  		return port1->mapbase == port2->mapbase;
> > -	default:
> > +	else if (uart_iotype_legacy_io(port1->iotype))
> > +		if (port1->iobase != port2->iobase)
> > +			return false;
> > +		else if (port1->iotype == UPIO_HUB6 && port1->hub6 != port2->hub6)
> 
> I suggest placing the entire hub6 related check (including the iobase 
> check) into own function in 8250_hub6.c and add a stub for the case when 
> hub6 code is not even built into kernel into 8250.h.

Neat. I will make a separate commit for that in V2. This
simplifies uart_match_port() even more...

Thank you,
Hugo.


> 
> > +			return false;
> > +		else
> > +			return true;
> > +	else
> >  		return false;
> > -	}
> >  }
> >  EXPORT_SYMBOL(uart_match_port);
> >  
> > 
> > 
> 
> -- 
>  i.
> 
> 


-- 
Hugo Villeneuve

Re: [PATCH 2/9] serial: core: use uart_iotype_*() to simplify code

[Hugo Villeneuve]

On Fri, 24 Apr 2026 11:30:46 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Fri, 24 Apr 2026 14:13:02 +0300 (EEST)
> Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> 
> > On Thu, 23 Apr 2026, Hugo Villeneuve wrote:
> > 
> > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > 
> > > Make use of new functions uart_iotype_mmio() and uart_iotype_legacy_io()
> > > to simplify and improve code readability.
> > > 
> > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > ---
> > >  drivers/tty/serial/serial_core.c | 24 +++++++++---------------
> > >  1 file changed, 9 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > > index b1cf45a8fc854cd97e349ff077d83b42e3ef8b16..fc273f8f9e75de89dca1ac1aca3589567bcf8a18 100644
> > > --- a/drivers/tty/serial/serial_core.c
> > > +++ b/drivers/tty/serial/serial_core.c
> > > @@ -3221,23 +3221,17 @@ bool uart_match_port(const struct uart_port *port1,
> > >  {
> > >  	if (port1->iotype != port2->iotype)
> > >  		return false;
> > > -
> > > -	switch (port1->iotype) {
> > > -	case UPIO_PORT:
> > > -		return port1->iobase == port2->iobase;
> > > -	case UPIO_HUB6:
> > > -		return port1->iobase == port2->iobase &&
> > > -		       port1->hub6   == port2->hub6;
> > > -	case UPIO_MEM:
> > > -	case UPIO_MEM16:
> > > -	case UPIO_MEM32:
> > > -	case UPIO_MEM32BE:
> > > -	case UPIO_AU:
> > > -	case UPIO_TSI:
> > > +	else if (uart_iotype_mmio(port1->iotype))
> > >  		return port1->mapbase == port2->mapbase;
> > > -	default:
> > > +	else if (uart_iotype_legacy_io(port1->iotype))
> > > +		if (port1->iobase != port2->iobase)
> > > +			return false;
> > > +		else if (port1->iotype == UPIO_HUB6 && port1->hub6 != port2->hub6)
> > 
> > I suggest placing the entire hub6 related check (including the iobase 
> > check) into own function in 8250_hub6.c and add a stub for the case when 
> > hub6 code is not even built into kernel into 8250.h.
> 
> Neat. I will make a separate commit for that in V2. This
> simplifies uart_match_port() even more...

I implemented it, the only thing I am not sure about is the include of
"8250/8250.h" in serial_core.c?

Hugo.


> > 
> > > +			return false;
> > > +		else
> > > +			return true;
> > > +	else
> > >  		return false;
> > > -	}
> > >  }
> > >  EXPORT_SYMBOL(uart_match_port);
> > >  
> > > 
> > > 
> > 
> > -- 
> >  i.
> > 
> > 
> 
> 
> -- 
> Hugo Villeneuve
> 
> 


-- 
Hugo Villeneuve