* [PATCH v4 0/2] serial: 8250: Add AN7581 UART support @ 2026-08-09 12:14 Christian Marangi 2026-08-09 12:14 ` [PATCH v4 1/2] dt-bindings: serial: 8250: Add Airoha compatibles Christian Marangi 2026-08-09 12:14 ` [PATCH v4 2/2] serial: 8250: Add Airoha SoC UART and HSUART support Christian Marangi 0 siblings, 2 replies; 7+ messages in thread From: Christian Marangi @ 2026-08-09 12:14 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andy Shevchenko, Ilpo Järvinen, Christian Marangi, Benjamin Larsson, John Ogness, Manuel Lauss, Jacques Nilo, Rong Zhang, Jiaxun Yang, Randy Dunlap, Gerhard Engleder, Binbin Zhou, 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 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 (1): serial: 8250: Add Airoha SoC UART and HSUART support .../devicetree/bindings/serial/8250.yaml | 5 + 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, 226 insertions(+) create mode 100644 drivers/tty/serial/8250/8250_airoha.c -- 2.53.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 1/2] dt-bindings: serial: 8250: Add Airoha compatibles 2026-08-09 12:14 [PATCH v4 0/2] serial: 8250: Add AN7581 UART support Christian Marangi @ 2026-08-09 12:14 ` Christian Marangi 2026-08-09 12:26 ` sashiko-bot 2026-08-10 15:02 ` Rob Herring 2026-08-09 12:14 ` [PATCH v4 2/2] serial: 8250: Add Airoha SoC UART and HSUART support Christian Marangi 1 sibling, 2 replies; 7+ messages in thread From: Christian Marangi @ 2026-08-09 12:14 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andy Shevchenko, Ilpo Järvinen, Christian Marangi, Benjamin Larsson, John Ogness, Manuel Lauss, Jacques Nilo, Rong Zhang, Jiaxun Yang, Randy Dunlap, Gerhard Engleder, Binbin Zhou, Lubomir Rintel, devicetree, linux-kernel, linux-serial Cc: Conor Dooley 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> Acked-by: Conor Dooley <conor.dooley@microchip.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 bb7b9c87a807..b6f7e093d8d7 100644 --- a/Documentation/devicetree/bindings/serial/8250.yaml +++ b/Documentation/devicetree/bindings/serial/8250.yaml @@ -110,6 +110,11 @@ properties: - const: mrvl,pxa-uart - const: nuvoton,wpcm450-uart - const: nuvoton,npcm750-uart + - const: airoha,en7523-uart + - items: + - const: airoha,an7581-uart + - const: airoha,en7523-uart + - const: airoha,an7581-hsuart - const: nvidia,tegra20-uart - const: nxp,lpc3220-uart - items: -- 2.53.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: serial: 8250: Add Airoha compatibles 2026-08-09 12:14 ` [PATCH v4 1/2] dt-bindings: serial: 8250: Add Airoha compatibles Christian Marangi @ 2026-08-09 12:26 ` sashiko-bot 2026-08-10 15:02 ` Rob Herring 1 sibling, 0 replies; 7+ messages in thread From: sashiko-bot @ 2026-08-09 12:26 UTC (permalink / raw) To: Christian Marangi; +Cc: devicetree, robh, conor+dt Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Alphabetical sorting and schema structure violation in the compatible list. -- --- Patch [1]: [PATCH v4 1/2] dt-bindings: serial: 8250: Add Airoha compatibles --- commit 27d01f216d527b904216459304a02e2d8c5f87e2 Author: Benjamin Larsson <benjamin.larsson@genexis.eu> dt-bindings: serial: 8250: Add Airoha compatibles This commit adds compatibles for the Airoha SoC family's 16550-compatible UART and High-Speed UART hardware. The hardware features a custom baud rate settings register. > diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml > index bb7b9c87a8072..b6f7e093d8d7a 100644 > --- a/Documentation/devicetree/bindings/serial/8250.yaml > +++ b/Documentation/devicetree/bindings/serial/8250.yaml > @@ -110,6 +110,11 @@ properties: > - const: mrvl,pxa-uart > - const: nuvoton,wpcm450-uart > - const: nuvoton,npcm750-uart > + - const: airoha,en7523-uart > + - items: > + - const: airoha,an7581-uart > + - const: airoha,en7523-uart > + - const: airoha,an7581-hsuart [Severity: Low] This isn't a bug, but are the airoha entries inserted out of alphabetical order here? The existing const compatibles appear to be grouped together and sorted alphabetically by vendor prefix, meaning airoha should likely appear earlier in the list. Additionally, should the items block for the multi-string compatible be moved? It seems to break the structural convention of the file by inserting an items block into the middle of the standalone const block, whereas other items blocks are typically grouped together after the const entries. > - const: nvidia,tegra20-uart > - const: nxp,lpc3220-uart > - items: -- Sashiko AI review · https://sashiko.dev/#/patchset/20260809121449.574277-1-ansuelsmth@gmail.com?part=1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: serial: 8250: Add Airoha compatibles 2026-08-09 12:14 ` [PATCH v4 1/2] dt-bindings: serial: 8250: Add Airoha compatibles Christian Marangi 2026-08-09 12:26 ` sashiko-bot @ 2026-08-10 15:02 ` Rob Herring 2026-08-10 15:08 ` Christian Marangi 1 sibling, 1 reply; 7+ messages in thread From: Rob Herring @ 2026-08-10 15:02 UTC (permalink / raw) To: Christian Marangi Cc: Greg Kroah-Hartman, Jiri Slaby, Krzysztof Kozlowski, Conor Dooley, Andy Shevchenko, Ilpo Järvinen, Benjamin Larsson, John Ogness, Manuel Lauss, Jacques Nilo, Rong Zhang, Jiaxun Yang, Randy Dunlap, Gerhard Engleder, Binbin Zhou, Lubomir Rintel, devicetree, linux-kernel, linux-serial, Conor Dooley On Sun, Aug 09, 2026 at 02:14:46PM +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. > > Signed-off-by: Benjamin Larsson <benjamin.larsson@genexis.eu> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > Acked-by: Conor Dooley <conor.dooley@microchip.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 bb7b9c87a807..b6f7e093d8d7 100644 > --- a/Documentation/devicetree/bindings/serial/8250.yaml > +++ b/Documentation/devicetree/bindings/serial/8250.yaml > @@ -110,6 +110,11 @@ properties: > - const: mrvl,pxa-uart > - const: nuvoton,wpcm450-uart > - const: nuvoton,npcm750-uart > + - const: airoha,en7523-uart > + - items: > + - const: airoha,an7581-uart > + - const: airoha,en7523-uart Don't stick this in the middle of a bunch single entry items (which should really be an 'enum' if you would like to clean that up). > + - const: airoha,an7581-hsuart > - const: nvidia,tegra20-uart > - const: nxp,lpc3220-uart > - items: > -- > 2.53.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: serial: 8250: Add Airoha compatibles 2026-08-10 15:02 ` Rob Herring @ 2026-08-10 15:08 ` Christian Marangi 0 siblings, 0 replies; 7+ messages in thread From: Christian Marangi @ 2026-08-10 15:08 UTC (permalink / raw) To: Rob Herring Cc: Greg Kroah-Hartman, Jiri Slaby, Krzysztof Kozlowski, Conor Dooley, Andy Shevchenko, Ilpo Järvinen, Benjamin Larsson, John Ogness, Manuel Lauss, Jacques Nilo, Rong Zhang, Jiaxun Yang, Randy Dunlap, Gerhard Engleder, Binbin Zhou, Lubomir Rintel, devicetree, linux-kernel, linux-serial, Conor Dooley On Mon, Aug 10, 2026 at 10:02:21AM -0500, Rob Herring wrote: > On Sun, Aug 09, 2026 at 02:14:46PM +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. > > > > Signed-off-by: Benjamin Larsson <benjamin.larsson@genexis.eu> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > Acked-by: Conor Dooley <conor.dooley@microchip.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 bb7b9c87a807..b6f7e093d8d7 100644 > > --- a/Documentation/devicetree/bindings/serial/8250.yaml > > +++ b/Documentation/devicetree/bindings/serial/8250.yaml > > @@ -110,6 +110,11 @@ properties: > > - const: mrvl,pxa-uart > > - const: nuvoton,wpcm450-uart > > - const: nuvoton,npcm750-uart > > + - const: airoha,en7523-uart > > + - items: > > + - const: airoha,an7581-uart > > + - const: airoha,en7523-uart > > Don't stick this in the middle of a bunch single entry items (which > should really be an 'enum' if you would like to clean that up). > Let me see what I can do... Seems this schema really needs some love. > > + - const: airoha,an7581-hsuart > > - const: nvidia,tegra20-uart > > - const: nxp,lpc3220-uart > > > - items: > > -- > > 2.53.0 > > -- Ansuel ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 2/2] serial: 8250: Add Airoha SoC UART and HSUART support 2026-08-09 12:14 [PATCH v4 0/2] serial: 8250: Add AN7581 UART support Christian Marangi 2026-08-09 12:14 ` [PATCH v4 1/2] dt-bindings: serial: 8250: Add Airoha compatibles Christian Marangi @ 2026-08-09 12:14 ` Christian Marangi 2026-08-09 12:36 ` sashiko-bot 1 sibling, 1 reply; 7+ messages in thread From: Christian Marangi @ 2026-08-09 12:14 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andy Shevchenko, Ilpo Järvinen, Christian Marangi, Benjamin Larsson, John Ogness, Manuel Lauss, Jacques Nilo, Rong Zhang, Jiaxun Yang, Randy Dunlap, Gerhard Engleder, Binbin Zhou, 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 v4 2/2] serial: 8250: Add Airoha SoC UART and HSUART support 2026-08-09 12:14 ` [PATCH v4 2/2] serial: 8250: Add Airoha SoC UART and HSUART support Christian Marangi @ 2026-08-09 12:36 ` sashiko-bot 0 siblings, 0 replies; 7+ messages in thread From: sashiko-bot @ 2026-08-09 12:36 UTC (permalink / raw) To: Christian Marangi; +Cc: robh, devicetree, conor+dt Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [Critical] Calling dev_err() in airoha_set_divisor() causes a spinlock deadlock if the port is the kernel console. - [High] Writing to the 32-bit `UART_AIROHA_XYD` register using `serial_port_out()` will silently truncate data to 8 bits if the device tree does not explicitly specify a 32-bit I/O width. - [High] The custom `rxtrig_bytes` configurations for Airoha UARTs are incompletely populated, breaking the generic 8250 trigger level logic and causing incorrect FIFO trigger values to be programmed. - [Medium] Defining `UART_PORT_AIROHA` and `UART_PORT_AIROHA_HS` in a private header improperly extends the global userspace-visible `PORT_*` numbering sequence. -- --- Patch [2]: [PATCH v4 2/2] serial: 8250: Add Airoha SoC UART and HSUART support --- commit 6a010d96ca9cacd9ae30a034a1c0bf970a220d58 Author: Christian Marangi <ansuelsmth@gmail.com> serial: 8250: Add Airoha SoC UART and HSUART support This patch adds support for Airoha AN7523 UART and AN7581 HSUART. It implements a standard 16550 UART 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] Is it safe to define these port type numbers in a private header instead of include/uapi/linux/serial_core.h? Since userspace can read this value via the TIOCGSERIAL ioctl, could this lead to a collision if another port type is assigned the same ID in the global UAPI header? > 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) > +{ [ ... ] > + /* 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: Critical] Can this cause a spinlock deadlock if the port is the kernel console? The caller serial8250_do_set_termios() holds the port->lock spinlock when invoking the set_divisor callback. If dev_err() is called and this UART is the active console, printk will synchronously try to acquire the same port->lock in serial8250_console_write(), leading to a deadlock. > + > + 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)); [Severity: High] Is there a risk that writing to the 32-bit UART_AIROHA_XYD register using serial_port_out() will silently truncate the data to 8 bits? If the device tree is missing the reg-io-width property, the 8250 core will default to 8-bit I/O. Since the hardware requires 32-bit writes here, should the driver explicitly set uart.port.iotype to UPIO_MEM32 in the probe function to ensure the generic accessor doesn't invoke writeb() and discard the upper 24 bits? > 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 [ ... ] > + [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, > + }, [Severity: High] Does the incomplete initialization of rxtrig_bytes for UART_PORT_AIROHA_HS break the generic trigger level logic? The array defines only the first two elements, leaving indices 2 and 3 as 0. When bytes_to_fcr_rxtrig() iterates over all states: for (i = 1; i < UART_FCR_R_TRIG_MAX_STATE; i++) { if (bytes < conf_type->rxtrig_bytes[i]) Since bytes is unsigned, bytes < 0 is false, so it will fall through and return UART_FCR_R_TRIG_11 for any requested trigger size of 4 or more, which would program 0 bytes into the hardware's FCR. Should the rest of the elements be populated? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260809121449.574277-1-ansuelsmth@gmail.com?part=2 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-08-10 15:08 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-09 12:14 [PATCH v4 0/2] serial: 8250: Add AN7581 UART support Christian Marangi 2026-08-09 12:14 ` [PATCH v4 1/2] dt-bindings: serial: 8250: Add Airoha compatibles Christian Marangi 2026-08-09 12:26 ` sashiko-bot 2026-08-10 15:02 ` Rob Herring 2026-08-10 15:08 ` Christian Marangi 2026-08-09 12:14 ` [PATCH v4 2/2] serial: 8250: Add Airoha SoC UART and HSUART support Christian Marangi 2026-08-09 12:36 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox