Devicetree
 help / color / mirror / Atom feed
* [PATCH v5 0/3] serial: 8250: Add AN7581 UART support
@ 2026-08-10 15:26 Christian Marangi
  2026-08-10 15:26 ` [PATCH v5 1/3] dt-bindings: serial: 8250: Reorganize compatible to single enum Christian Marangi
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Christian Marangi @ 2026-08-10 15:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Andy Shevchenko, Ilpo Järvinen,
	Benjamin Larsson, Christian Marangi, John Ogness, Jacques Nilo,
	Randy Dunlap, Rong Zhang, Jiaxun Yang, Gerhard Engleder,
	Binbin Zhou, Huacai Chen, Haowei Zheng, Lubomir Rintel,
	devicetree, linux-kernel, linux-serial

This is a new version of [1] to support UART driver for
Airoha SoC.

One additional function was needed serial8250_get_baud_rate()
for the driver to work correctly for baud rate calculation.

While at it also try to clarify a long standing issue with
new UART driver when adding new ids for uart_config[].

[1] https://lore.kernel.org/all/20250209210241.2622309-1-benjamin.larsson@genexis.eu/

Changes v5:
- Improve 8250 YAML schema with enum
- Keep alphabetical order for YAML schema

Changes v4:
- Add Review tag
- Add errno and types header
- Drop redundant () from comment
- Use define for get_divisor

Changes v3:
- Address review from ai bot
- Add all missing kernel headers
- Improve divisor calculation
- Fix wrong uart config table
- Simplify divisor table handling

Changes v2:
- Drop intermediate patch
- Use .set_divisor OPs
- Use div_u64 instead of / 40 for 32 bit overflow
- Drop dedicated match table and use compatible for type

Changes compared to [1]:
- Fix all formal error
- Use better compatible names
- Drop unneeded header
- Drop usage of irq (it's filled by the generic function)
- General code cleanup and reorg
- Split to patch and add the UAPI map patch

Benjamin Larsson (1):
  dt-bindings: serial: 8250: Add Airoha compatibles

Christian Marangi (2):
  dt-bindings: serial: 8250: Reorganize compatible to single enum
  serial: 8250: Add Airoha SoC UART and HSUART support

 .../devicetree/bindings/serial/8250.yaml      |  32 +--
 drivers/tty/serial/8250/8250.h                |   6 +
 drivers/tty/serial/8250/8250_airoha.c         | 187 ++++++++++++++++++
 drivers/tty/serial/8250/8250_port.c           |  16 ++
 drivers/tty/serial/8250/Kconfig               |  11 ++
 drivers/tty/serial/8250/Makefile              |   1 +
 6 files changed, 240 insertions(+), 13 deletions(-)
 create mode 100644 drivers/tty/serial/8250/8250_airoha.c

-- 
2.53.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v5 1/3] dt-bindings: serial: 8250: Reorganize compatible to single enum
  2026-08-10 15:26 [PATCH v5 0/3] serial: 8250: Add AN7581 UART support Christian Marangi
@ 2026-08-10 15:26 ` Christian Marangi
  2026-08-11  7:21   ` Krzysztof Kozlowski
  2026-08-10 15:26 ` [PATCH v5 2/3] dt-bindings: serial: 8250: Add Airoha compatibles Christian Marangi
  2026-08-10 15:26 ` [PATCH v5 3/3] serial: 8250: Add Airoha SoC UART and HSUART support Christian Marangi
  2 siblings, 1 reply; 7+ messages in thread
From: Christian Marangi @ 2026-08-10 15:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Andy Shevchenko, Ilpo Järvinen,
	Benjamin Larsson, Christian Marangi, John Ogness, Jacques Nilo,
	Randy Dunlap, Rong Zhang, Jiaxun Yang, Gerhard Engleder,
	Binbin Zhou, Huacai Chen, Haowei Zheng, Lubomir Rintel,
	devicetree, linux-kernel, linux-serial

Instead of declaring each single compatible string with a const, move them
to a single enum and reorder them.

First put the generic ns one and then follow alphabetical order.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../devicetree/bindings/serial/8250.yaml      | 27 ++++++++++---------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index bb7b9c87a807..039bd392f876 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -99,19 +99,20 @@ allOf:
 properties:
   compatible:
     oneOf:
-      - const: ns8250
-      - const: ns16450
-      - const: ns16550
-      - const: ns16550a
-      - const: ns16850
-      - const: aspeed,ast2400-vuart
-      - const: aspeed,ast2500-vuart
-      - const: intel,xscale-uart
-      - const: mrvl,pxa-uart
-      - const: nuvoton,wpcm450-uart
-      - const: nuvoton,npcm750-uart
-      - const: nvidia,tegra20-uart
-      - const: nxp,lpc3220-uart
+      - enum:
+          - ns8250
+          - ns16450
+          - ns16550
+          - ns16550a
+          - ns16850
+          - aspeed,ast2400-vuart
+          - aspeed,ast2500-vuart
+          - intel,xscale-uart
+          - mrvl,pxa-uart
+          - nuvoton,npcm750-uart
+          - nuvoton,wpcm450-uart
+          - nvidia,tegra20-uart
+          - nxp,lpc3220-uart
       - items:
           - enum:
               - exar,xr16l2552
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 2/3] dt-bindings: serial: 8250: Add Airoha compatibles
  2026-08-10 15:26 [PATCH v5 0/3] serial: 8250: Add AN7581 UART support Christian Marangi
  2026-08-10 15:26 ` [PATCH v5 1/3] dt-bindings: serial: 8250: Reorganize compatible to single enum Christian Marangi
@ 2026-08-10 15:26 ` Christian Marangi
  2026-08-11  7:22   ` Krzysztof Kozlowski
  2026-08-10 15:26 ` [PATCH v5 3/3] serial: 8250: Add Airoha SoC UART and HSUART support Christian Marangi
  2 siblings, 1 reply; 7+ messages in thread
From: Christian Marangi @ 2026-08-10 15:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Andy Shevchenko, Ilpo Järvinen,
	Benjamin Larsson, Christian Marangi, John Ogness, Jacques Nilo,
	Randy Dunlap, Rong Zhang, Jiaxun Yang, Gerhard Engleder,
	Binbin Zhou, Huacai Chen, Haowei Zheng, Lubomir Rintel,
	devicetree, linux-kernel, linux-serial

From: Benjamin Larsson <benjamin.larsson@genexis.eu>

The Airoha SoC family have a mostly 16550-compatible UART
and High-Speed UART hardware with the exception of custom
baud rate settings register.

Signed-off-by: Benjamin Larsson <benjamin.larsson@genexis.eu>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 Documentation/devicetree/bindings/serial/8250.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index 039bd392f876..9a3f596f8a0e 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -105,6 +105,8 @@ properties:
           - ns16550
           - ns16550a
           - ns16850
+          - airoha,an7581-hsuart
+          - airoha,en7523-uart
           - aspeed,ast2400-vuart
           - aspeed,ast2500-vuart
           - intel,xscale-uart
@@ -113,6 +115,9 @@ properties:
           - nuvoton,wpcm450-uart
           - nvidia,tegra20-uart
           - nxp,lpc3220-uart
+      - items:
+          - const: airoha,an7581-uart
+          - const: airoha,en7523-uart
       - items:
           - enum:
               - exar,xr16l2552
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 3/3] serial: 8250: Add Airoha SoC UART and HSUART support
  2026-08-10 15:26 [PATCH v5 0/3] serial: 8250: Add AN7581 UART support Christian Marangi
  2026-08-10 15:26 ` [PATCH v5 1/3] dt-bindings: serial: 8250: Reorganize compatible to single enum Christian Marangi
  2026-08-10 15:26 ` [PATCH v5 2/3] dt-bindings: serial: 8250: Add Airoha compatibles Christian Marangi
@ 2026-08-10 15:26 ` Christian Marangi
  2026-08-10 15:52   ` sashiko-bot
  2 siblings, 1 reply; 7+ messages in thread
From: Christian Marangi @ 2026-08-10 15:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Andy Shevchenko, Ilpo Järvinen,
	Benjamin Larsson, Christian Marangi, John Ogness, Jacques Nilo,
	Randy Dunlap, Rong Zhang, Jiaxun Yang, Gerhard Engleder,
	Binbin Zhou, Huacai Chen, Haowei Zheng, Lubomir Rintel,
	devicetree, linux-kernel, linux-serial

Add support for Airoha AN7523 UART and AN7581 HSUART.

These implement a standard 16550 UART with only some custom logic
for baud rate handling.

Co-developed-by: Benjamin Larsson <benjamin.larsson@genexis.eu>
Signed-off-by: Benjamin Larsson <benjamin.larsson@genexis.eu>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/tty/serial/8250/8250.h        |   6 +
 drivers/tty/serial/8250/8250_airoha.c | 187 ++++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_port.c   |  16 +++
 drivers/tty/serial/8250/Kconfig       |  11 ++
 drivers/tty/serial/8250/Makefile      |   1 +
 5 files changed, 221 insertions(+)
 create mode 100644 drivers/tty/serial/8250/8250_airoha.c

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 77fe0588fd6b..e6b2b9431229 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -73,6 +73,12 @@ struct serial8250_config {
 	unsigned int	flags;
 };
 
+enum uart_port_type {
+	/* Internal 8250 only */
+	UART_PORT_AIROHA		= 124,
+	UART_PORT_AIROHA_HS		= 125,
+};
+
 #define UART_CAP_FIFO	BIT(8)	/* UART has FIFO */
 #define UART_CAP_EFR	BIT(9)	/* UART has EFR */
 #define UART_CAP_SLEEP	BIT(10)	/* UART has IER sleep */
diff --git a/drivers/tty/serial/8250/8250_airoha.c b/drivers/tty/serial/8250/8250_airoha.c
new file mode 100644
index 000000000000..fe72a39561c2
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_airoha.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Airoha UART driver
+ *
+ * Copyright (c) 2025 Genexis Sweden AB
+ * Author: Benjamin Larsson <benjamin.larsson@genexis.eu>
+ *	   Christian Marangi <ansuelsmth@gmail.com>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/errno.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/platform_device.h>
+#include <linux/serial_8250.h>
+#include <linux/serial_core.h>
+#include <linux/serial_reg.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include "8250.h"
+
+#define UART_AIROHA_XINCLKDR	10
+#define UART_AIROHA_XYD		11
+#define   UART_AIROHA_XYD_X	GENMASK(31, 16)
+#define   UART_AIROHA_XYD_Y	GENMASK(15, 0)
+
+struct airoha_8250_priv {
+	int line;
+};
+
+#define UART_BRD_20M		0x0001
+
+#define XINDIV_CLOCK		(20 * HZ_PER_MHZ)
+#define XYD_Y			65000
+
+static const unsigned int airoha_clk_divs[] = { 2, 4, 10 };
+
+static unsigned int airoha_get_divisor(struct uart_port *port,
+				       unsigned int baud,
+				       unsigned int *frac)
+{
+	/* Hardware always uses BRDIV = 1. */
+	*frac = 0;
+
+	return UART_BRD_20M;
+}
+
+/*
+ * Airoha UART baud rate calculation logic
+ *
+ * crystal_clock = 20 MHz (fixed frequency)
+ * xindiv_clock = crystal_clock / clock_div
+ * (x/y) = XYD, 32 bit register with 16 bits of x and then 16 bits of y
+ * clock_div = XINCLK_DIVCNT (default set to 10 (0x4)),
+ *           - 3 bit register [ 1, 2, 4, 8, 10, 12, 16, 20 ]
+ *
+ * baud_rate = ((xindiv_clock) * (x/y)) / ([BRDH,BRDL] * 16)
+ *
+ * Selecting divider needs to fulfill
+ * 1.8432 MHz <= xindiv_clk <= APB clock / 2
+ * The clocks are unknown but a divider of value 1 did not result in a valid
+ * waveform.
+ *
+ * XYD_y seems to need to be larger then XYD_x for proper waveform generation.
+ * Setting [BRDH,BRDL] to [0,1] and XYD_y to 65000 gives even values
+ * for usual baud rates.
+ */
+static void airoha_set_divisor(struct uart_port *port, unsigned int baud,
+			       unsigned int quot, unsigned int quot_frac)
+{
+	struct uart_8250_port *up = up_to_u8250p(port);
+	u32 xindiv_clk;
+	u64 xyd_x, nom;
+	int div_bit;
+
+	/* Set baud rate calculation defaults (BRDIV [BRDH,BRDL] to 1) */
+	serial8250_do_set_divisor(port, baud, UART_BRD_20M);
+
+	/*
+	 * Calculate XYD_x and XINCLKDR register by searching
+	 * through a table of crystal_clock divisors.
+	 */
+	nom = (u64)baud * XYD_Y;
+	for (div_bit = ARRAY_SIZE(airoha_clk_divs) - 1; div_bit >= 0; div_bit--) {
+		unsigned int div = airoha_clk_divs[div_bit];
+
+		xindiv_clk = XINDIV_CLOCK / div;
+		xyd_x = div_u64(nom * 16, xindiv_clk);
+
+		/* For the HSUART xyd_x needs to be scaled by a factor of 2 */
+		if (port->type == UART_PORT_AIROHA_HS)
+			xyd_x /= 2;
+
+		if (xyd_x < XYD_Y)
+			break;
+	}
+
+	/* Couldn't find a valid xyd_x */
+	if (div_bit < 0) {
+		dev_err(port->dev, "failed to find suitable clock divisor for baud %u\n",
+			baud);
+		return;
+	}
+
+	serial_port_out(port, UART_AIROHA_XINCLKDR, BIT(div_bit));
+	serial_port_out(port, UART_AIROHA_XYD,
+			FIELD_PREP(UART_AIROHA_XYD_X, xyd_x) |
+			FIELD_PREP(UART_AIROHA_XYD_Y, XYD_Y));
+
+	/* Restore normal register access. */
+	serial_port_out(port, UART_LCR, up->lcr);
+}
+
+static int airoha_8250_probe(struct platform_device *pdev)
+{
+	struct uart_8250_port uart = { };
+	struct device *dev = &pdev->dev;
+	struct airoha_8250_priv *priv;
+	struct resource *res;
+	int ret;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return dev_err_probe(dev, -EINVAL, "invalid address\n");
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	uart.port.dev = dev;
+	if (device_is_compatible(dev, "airoha,an7581-hsuart"))
+		uart.port.type = UART_PORT_AIROHA_HS;
+	else
+		uart.port.type = UART_PORT_AIROHA;
+	uart.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT |
+			  UPF_FIXED_TYPE | UPF_IOREMAP;
+	uart.port.set_divisor = airoha_set_divisor;
+	uart.port.get_divisor = airoha_get_divisor;
+	uart.port.mapbase = res->start;
+	uart.port.mapsize = resource_size(res);
+
+	ret = uart_read_and_validate_port_properties(&uart.port);
+	if (ret)
+		return ret;
+
+	ret = serial8250_register_8250_port(&uart);
+	if (ret < 0)
+		return ret;
+
+	priv->line = ret;
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+}
+
+static void airoha_8250_remove(struct platform_device *ofdev)
+{
+	struct airoha_8250_priv *priv = platform_get_drvdata(ofdev);
+
+	serial8250_unregister_port(priv->line);
+}
+
+static const struct of_device_id airoha_8250_dt_ids[] = {
+	{ .compatible = "airoha,en7523-uart" },
+	{ .compatible = "airoha,an7581-hsuart" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, airoha_8250_dt_ids);
+
+static struct platform_driver airoha_8250_driver = {
+	.driver = {
+		.name = "8250_airoha",
+		.of_match_table = airoha_8250_dt_ids,
+	},
+	.probe = airoha_8250_probe,
+	.remove = airoha_8250_remove,
+};
+module_platform_driver(airoha_8250_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Airoha UART driver");
+MODULE_AUTHOR("Benjamin Larsson <benjamin.larsson@genexis.eu>");
+MODULE_AUTHOR("Christian Marangi <ansuelsmth@gmail.com>");
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 630deb7dd344..c888cae7d98b 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -311,6 +311,22 @@ static const struct serial8250_config uart_config[] = {
 		.rxtrig_bytes	= {1, 8, 16, 30},
 		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
 	},
+	[UART_PORT_AIROHA] = {
+		.name		= "Airoha UART",
+		.fifo_size	= 8,
+		.tx_loadsz	= 1,
+		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01 | UART_FCR_CLEAR_RCVR,
+		.rxtrig_bytes	= {1, 4, 4, 4},
+		.flags		= UART_CAP_FIFO,
+	},
+	[UART_PORT_AIROHA_HS] = {
+		.name		= "Airoha HSUART",
+		.fifo_size	= 128,
+		.tx_loadsz	= 128,
+		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01 | UART_FCR_CLEAR_RCVR,
+		.rxtrig_bytes	= {1, 4},
+		.flags		= UART_CAP_FIFO,
+	},
 };
 
 /* Uart divisor latch read */
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index fc3e58d62233..310da7af7a49 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -337,6 +337,17 @@ config SERIAL_8250_ACORN
 	  system, say Y to this option.  The driver can handle 1, 2, or 3 port
 	  cards.  If unsure, say N.
 
+config SERIAL_8250_AIROHA
+	tristate "Airoha UART support"
+	depends on ARCH_AIROHA || COMPILE_TEST
+	depends on SERIAL_8250
+	help
+	  Selecting this option enables an Airoha SoC specific baud rate
+	  calculation routine on an otherwise 16550 compatible UART hardware.
+
+	  If you have an Airoha based board and want to use the serial port,
+	  say Y to this option. If unsure, say N.
+
 config SERIAL_8250_BCM2835AUX
 	tristate "BCM2835 auxiliar mini UART support"
 	depends on ARCH_BCM2835 || COMPILE_TEST
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 6d21402b4435..83257696ea8a 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_SERIAL_8250_CONSOLE)	+= 8250_early.o
 
 obj-$(CONFIG_SERIAL_8250_ACCENT)	+= 8250_accent.o
 obj-$(CONFIG_SERIAL_8250_ACORN)		+= 8250_acorn.o
+obj-$(CONFIG_SERIAL_8250_AIROHA)	+= 8250_airoha.o
 obj-$(CONFIG_SERIAL_8250_ASPEED_VUART)	+= 8250_aspeed_vuart.o
 obj-$(CONFIG_SERIAL_8250_BCM2835AUX)	+= 8250_bcm2835aux.o
 obj-$(CONFIG_SERIAL_8250_BCM7271)	+= 8250_bcm7271.o
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 3/3] serial: 8250: Add Airoha SoC UART and HSUART support
  2026-08-10 15:26 ` [PATCH v5 3/3] serial: 8250: Add Airoha SoC UART and HSUART support Christian Marangi
@ 2026-08-10 15:52   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-10 15:52 UTC (permalink / raw)
  To: Christian Marangi; +Cc: robh, conor+dt, devicetree

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Deadlock (if console) and Log Flooding (if non-console) via unratelimited dev_err() under port lock
- [High] Silent probe failure or mismatched baud rate validation due to uninitialized port->uartclk
- [Medium] Driver squats on global UAPI port type namespace
- [Medium] Incomplete rxtrig_bytes initialization sets undefined hardware state and breaks sysfs reads
--

--- Patch [3]: [PATCH v5 3/3] serial: 8250: Add Airoha SoC UART and HSUART support ---
commit 5a2afaf1ec03e067e37973b1f2ee37f044a204f9
Author: Christian Marangi <ansuelsmth@gmail.com>

serial: 8250: Add Airoha SoC UART and HSUART support

This commit adds support for Airoha AN7523 UART and AN7581 HSUART. It provides
a standard 16550 UART driver with custom logic for baud rate handling.

> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> index 77fe0588fd6bd..e6b2b94312292 100644
> --- a/drivers/tty/serial/8250/8250.h
> +++ b/drivers/tty/serial/8250/8250.h
> @@ -73,6 +73,12 @@ struct serial8250_config {
>  	unsigned int	flags;
>  };
>  
> +enum uart_port_type {
> +	/* Internal 8250 only */
> +	UART_PORT_AIROHA		= 124,
> +	UART_PORT_AIROHA_HS		= 125,
> +};

[Severity: Medium]
Can this driver safely use these values without them being allocated in the
global UAPI header?

The port->type values are part of the global userspace UAPI, defined in
include/uapi/linux/serial_core.h, and are exposed to userspace via the
TIOCGSERIAL ioctl. Since the highest officially allocated port type is 123,
assigning 124 and 125 internally rather than in the UAPI header risks a future
collision. 

If a new generic serial driver is legitimately assigned 124 in the UAPI
header, it will silently collide with UART_PORT_AIROHA, causing uart_config
array index collisions and userspace misidentification of the port type.

[ ... ]
> diff --git a/drivers/tty/serial/8250/8250_airoha.c b/drivers/tty/serial/8250/8250_airoha.c
> new file mode 100644
> index 0000000000000..fe72a39561c28
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_airoha.c
[ ... ]
> +static void airoha_set_divisor(struct uart_port *port, unsigned int baud,
> +			       unsigned int quot, unsigned int quot_frac)
> +{
> +	struct uart_8250_port *up = up_to_u8250p(port);
> +	u32 xindiv_clk;
> +	u64 xyd_x, nom;
> +	int div_bit;
[ ... ]
> +	/* Couldn't find a valid xyd_x */
> +	if (div_bit < 0) {
> +		dev_err(port->dev, "failed to find suitable clock divisor for baud %u\n",
> +			baud);
> +		return;
> +	}

[Severity: High]
Is it safe to call dev_err() here?

This function is invoked via serial8250_do_set_termios() with the raw
spinlock port->lock held. If this port is the active serial console,
dev_err() invokes printk(), which synchronously calls
serial8250_console_write(). The console write function will attempt to
acquire the exact same port->lock, resulting in an instant self-deadlock.

If the port is not the console, the error is unratelimited. This allows a
local user with write access to the TTY device to spam the kernel log by
requesting unsupported high baud rates via the TCSETS2 ioctl in a tight loop.

[ ... ]
> +static int airoha_8250_probe(struct platform_device *pdev)
> +{
> +	struct uart_8250_port uart = { };
> +	struct device *dev = &pdev->dev;
> +	struct airoha_8250_priv *priv;
> +	struct resource *res;
> +	int ret;
[ ... ]
> +	uart.port.mapbase = res->start;
> +	uart.port.mapsize = resource_size(res);
> +
> +	ret = uart_read_and_validate_port_properties(&uart.port);

[Severity: High]
Does this code need to initialize uart.port.uartclk before parsing properties?

The hardcoded divisor logic in airoha_set_divisor() relies on a 20 MHz
crystal (XINDIV_CLOCK = 20 * HZ_PER_MHZ). However, uartclk isn't explicitly
set before or after calling uart_read_and_validate_port_properties().

If the device tree omits the legacy clock-frequency property (e.g., using the
modern clocks property instead), uartclk remains 0. This causes
serial8250_register_8250_port() to reject the port with -EINVAL, resulting in
a silent probe failure.

If the device tree specifies a different frequency (like a 100MHz APB bus
clock), the serial core will bound baud rates based on that incorrect
frequency. This would allow unsupported high baud rates to reach the hardcoded
20MHz divisor logic, repeatedly triggering the deadlock or log-flooding issue
above.

[ ... ]
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index e94a0802cbddc..11728a789922d 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -311,6 +311,22 @@ static const struct serial8250_config uart_config[] = {
>  		.rxtrig_bytes	= {1, 8, 16, 30},
>  		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
>  	},
> +	[UART_PORT_AIROHA] = {
> +		.name		= "Airoha UART",
> +		.fifo_size	= 8,
> +		.tx_loadsz	= 1,
> +		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01 | UART_FCR_CLEAR_RCVR,
> +		.rxtrig_bytes	= {1, 4, 4, 4},
> +		.flags		= UART_CAP_FIFO,
> +	},
> +	[UART_PORT_AIROHA_HS] = {
> +		.name		= "Airoha HSUART",
> +		.fifo_size	= 128,
> +		.tx_loadsz	= 128,
> +		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01 | UART_FCR_CLEAR_RCVR,
> +		.rxtrig_bytes	= {1, 4},

[Severity: Medium]
Will this partially initialized rxtrig_bytes array break sysfs reads?

The rxtrig_bytes array is 4 elements long, but only two are initialized here,
leaving the remaining elements as implicitly 0. 

When a root user configures a trigger level > 4 via
/sys/class/tty/ttyS*/rx_trig_bytes, bytes_to_fcr_rxtrig() iterates over this
array checking bytes < conf_type->rxtrig_bytes[i]. Because the uninitialized
elements are 0, the condition evaluates to false, and the function returns an
undefined trigger state (UART_FCR_R_TRIG_11).

When sysfs later tries to read the value via fcr_get_rxtrig_bytes(), it will
fetch 0 from the array and return -EOPNOTSUPP. Padding the array with
identical maximum values (e.g., {1, 4, 4, 4}) like the standard Airoha UART
would avoid this.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810152636.658955-1-ansuelsmth@gmail.com?part=3

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 1/3] dt-bindings: serial: 8250: Reorganize compatible to single enum
  2026-08-10 15:26 ` [PATCH v5 1/3] dt-bindings: serial: 8250: Reorganize compatible to single enum Christian Marangi
@ 2026-08-11  7:21   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 7+ messages in thread
From: Krzysztof Kozlowski @ 2026-08-11  7:21 UTC (permalink / raw)
  To: Christian Marangi
  Cc: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Andy Shevchenko, Ilpo Järvinen,
	Benjamin Larsson, John Ogness, Jacques Nilo, Randy Dunlap,
	Rong Zhang, Jiaxun Yang, Gerhard Engleder, Binbin Zhou,
	Huacai Chen, Haowei Zheng, Lubomir Rintel, devicetree,
	linux-kernel, linux-serial

On Mon, Aug 10, 2026 at 05:26:33PM +0200, Christian Marangi wrote:
> Instead of declaring each single compatible string with a const, move them
> to a single enum and reorder them.

Reordering is tricky to spot. You mean reordered Nuvoton?


> 
> First put the generic ns one and then follow alphabetical order.

s/one/first/

> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../devicetree/bindings/serial/8250.yaml      | 27 ++++++++++---------
>  1 file changed, 14 insertions(+), 13 deletions(-)

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 2/3] dt-bindings: serial: 8250: Add Airoha compatibles
  2026-08-10 15:26 ` [PATCH v5 2/3] dt-bindings: serial: 8250: Add Airoha compatibles Christian Marangi
@ 2026-08-11  7:22   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 7+ messages in thread
From: Krzysztof Kozlowski @ 2026-08-11  7:22 UTC (permalink / raw)
  To: Christian Marangi
  Cc: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Andy Shevchenko, Ilpo Järvinen,
	Benjamin Larsson, John Ogness, Jacques Nilo, Randy Dunlap,
	Rong Zhang, Jiaxun Yang, Gerhard Engleder, Binbin Zhou,
	Huacai Chen, Haowei Zheng, Lubomir Rintel, devicetree,
	linux-kernel, linux-serial

On Mon, Aug 10, 2026 at 05:26:34PM +0200, Christian Marangi wrote:
> From: Benjamin Larsson <benjamin.larsson@genexis.eu>
> 
> The Airoha SoC family have a mostly 16550-compatible UART
> and High-Speed UART hardware with the exception of custom
> baud rate settings register.

Looks wrongly wrapped.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-08-11  7:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-10 15:26 [PATCH v5 0/3] serial: 8250: Add AN7581 UART support Christian Marangi
2026-08-10 15:26 ` [PATCH v5 1/3] dt-bindings: serial: 8250: Reorganize compatible to single enum Christian Marangi
2026-08-11  7:21   ` Krzysztof Kozlowski
2026-08-10 15:26 ` [PATCH v5 2/3] dt-bindings: serial: 8250: Add Airoha compatibles Christian Marangi
2026-08-11  7:22   ` Krzysztof Kozlowski
2026-08-10 15:26 ` [PATCH v5 3/3] serial: 8250: Add Airoha SoC UART and HSUART support Christian Marangi
2026-08-10 15:52   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox