[PATCH 1/2] dt-bindings: serial: ingenic: Add the X1600

[PATCH 1/2] dt-bindings: serial: ingenic: Add the X1600

[Olivier Louvignes]

The X1600's UARTs have the X1000's registers as far as the 8250 core
uses them, but the driver has to tell them apart to set their bit clock
count. Add ingenic,x1600-uart, with the X1000's as fallback.

Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>
---
 Documentation/devicetree/bindings/serial/ingenic,uart.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/ingenic,uart.yaml b/Documentation/devicetree/bindings/serial/ingenic,uart.yaml
index d5f153bdeb0d..635342d43ddb 100644
--- a/Documentation/devicetree/bindings/serial/ingenic,uart.yaml
+++ b/Documentation/devicetree/bindings/serial/ingenic,uart.yaml
@@ -32,6 +32,9 @@ properties:
       - items:
           - const: ingenic,jz4725b-uart
           - const: ingenic,jz4740-uart
+      - items:
+          - const: ingenic,x1600-uart
+          - const: ingenic,x1000-uart
       - items:
           - const: ingenic,jz4755-uart
           - const: ingenic,jz4750-uart

base-commit: 1184f87e2790e8af9be3116f608d2e86b4792d7d
-- 
2.39.5

[PATCH 2/2] serial: 8250_ingenic: Start the X1600 UART from 16 clocks a bit

[Olivier Louvignes]

The X1600 UART samples each bit with UMR clocks, adjusted per bit by
UACR, while the divisor the 8250 core computes assumes 16. The driver
never sets them, so the UART keeps what ran before it: on a HiBy R1 the
vendor kernel leaves UART0's UMR at 8, which by the register's meaning
doubles every rate set here (the line rate itself was not measured). The
Bluetooth chip on that UART then never answers: "command 0xfc18 tx
timeout" from hci_bcm.

Add an X1600 entry that writes UMR = 16 and UACR = 0 at probe. The
offsets come from Rockbox's X1600 register headers and match the UMR
read on the R1. The X1000, whose compatible the X1600's falls back to,
may have the same registers, but I could not check one, so it is left
alone.

Tested on a HiBy R1: UMR reads 16 and UACR 0 after probe, and the
BCM43430A1 on UART0 comes up at 1.5 Mbaud and streams A2DP audio.

Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>
---
The test on the R1 ran with the same reset placed in the X1000 entry; the
R1's device tree lists "ingenic,x1600-uart" first, so it takes the same
path with this version. Build-tested with W=1 on tty-next.

 drivers/tty/serial/8250/8250_ingenic.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c
index 0cdadacd7019..40307189021b 100644
--- a/drivers/tty/serial/8250/8250_ingenic.c
+++ b/drivers/tty/serial/8250/8250_ingenic.c
@@ -25,6 +25,7 @@
 struct ingenic_uart_config {
 	int tx_loadsz;
 	int fifosize;
+	bool has_umr;
 };
 
 struct ingenic_uart_data {
@@ -40,6 +41,10 @@ static const struct of_device_id of_match[];
 #define UART_MCR_MDCE	BIT(7)
 #define UART_MCR_FCM	BIT(6)
 
+/* Clocks a bit, 16 for the divisor the 8250 core computes, and their per-bit adjustment. */
+#define UART_UMR	9
+#define UART_UACR	10
+
 static struct earlycon_device *early_device;
 
 static uint8_t early_in(struct uart_port *port, int offset)
@@ -298,6 +303,12 @@ static int ingenic_uart_probe(struct platform_device *pdev)
 	}
 	uart.port.uartclk = clk_get_rate(data->clk_baud);
 
+	/* Whatever ran before may have left another count: 8 on a HiBy R1. */
+	if (cdata->has_umr) {
+		writel(16, uart.port.membase + (UART_UMR << uart.port.regshift));
+		writel(0, uart.port.membase + (UART_UACR << uart.port.regshift));
+	}
+
 	data->uport = serial8250_register_8250_port(&uart);
 	if (IS_ERR(data->uport)) {
 		err = PTR_ERR(data->uport);
@@ -344,6 +355,12 @@ static const struct ingenic_uart_config x1000_uart_config = {
 	.fifosize = 64,
 };
 
+static const struct ingenic_uart_config x1600_uart_config = {
+	.tx_loadsz = 32,
+	.fifosize = 64,
+	.has_umr = true,
+};
+
 static const struct of_device_id of_match[] = {
 	{ .compatible = "ingenic,jz4740-uart", .data = &jz4740_uart_config },
 	{ .compatible = "ingenic,jz4750-uart", .data = &jz4760_uart_config },
@@ -352,6 +369,7 @@ static const struct of_device_id of_match[] = {
 	{ .compatible = "ingenic,jz4775-uart", .data = &jz4760_uart_config },
 	{ .compatible = "ingenic,jz4780-uart", .data = &jz4780_uart_config },
 	{ .compatible = "ingenic,x1000-uart", .data = &x1000_uart_config },
+	{ .compatible = "ingenic,x1600-uart", .data = &x1600_uart_config },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, of_match);
-- 
2.39.5

Re: [PATCH 2/2] serial: 8250_ingenic: Start the X1600 UART from 16 clocks a bit

[sashiko-bot]

> The X1600 UART samples each bit with UMR clocks, adjusted per bit by
> UACR, while the divisor the 8250 core computes assumes 16. The driver
> never sets them, so the UART keeps what ran before it: on a HiBy R1 the
> vendor kernel leaves UART0's UMR at 8, which by the register's meaning
> doubles every rate set here (the line rate itself was not measured). The
> Bluetooth chip on that UART then never answers: "command 0xfc18 tx
> timeout" from hci_bcm.
> 
> Add an X1600 entry that writes UMR = 16 and UACR = 0 at probe. The
> offsets come from Rockbox's X1600 register headers and match the UMR
> read on the R1. The X1000, whose compatible the X1600's falls back to,
> may have the same registers, but I could not check one, so it is left
> alone.
> 
> Tested on a HiBy R1: UMR reads 16 and UACR 0 after probe, and the
> BCM43430A1 on UART0 comes up at 1.5 Mbaud and streams A2DP audio.
> 
> Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024051515.55639-1-olivier@mgcrea.io?part=2

Re: [PATCH 2/2] serial: 8250_ingenic: Start the X1600 UART from 16 clocks a bit

[H. Nikolaus Schaller]

Hi,

> Am 24.10.2026 um 07:15 schrieb Olivier Louvignes <olivier@mgcrea.io>:
> 
> The X1600 UART samples each bit with UMR clocks, adjusted per bit by
> UACR, while the divisor the 8250 core computes assumes 16. The driver
> never sets them, so the UART keeps what ran before it: on a HiBy R1 the
> vendor kernel leaves UART0's UMR at 8, which by the register's meaning

So this patch adjusts a driver to a specific board and not the x1600 chip?

Then, this asks for a device tree property if it is really needed.

Or it should be fixed in U-Boot. Otherwise, assuming now that UMR = 16 is
correct for all x1600 boards would introduce a big issue for other boards
which suddenly stop working at the previously correct rate.

> doubles every rate set here (the line rate itself was not measured). The
> Bluetooth chip on that UART then never answers: "command 0xfc18 tx
> timeout" from hci_bcm.
> 
> Add an X1600 entry that writes UMR = 16 and UACR = 0 at probe. The
> offsets come from Rockbox's X1600 register headers and match the UMR
> read on the R1. The X1000, whose compatible the X1600's falls back to,
> may have the same registers, but I could not check one, so it is left
> alone.

Well, it should be checked...

... the x1000 has the same register.

Of course it does not seem to be a standard 8250 register. Anyways,
it should be checked is how other 8250 drivers handle special/private
register setup.

Or this should even become part of the clock setup?

> 
> Tested on a HiBy R1: UMR reads 16 and UACR 0 after probe, and the
> BCM43430A1 on UART0 comes up at 1.5 Mbaud and streams A2DP audio.
> 
> Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>
> ---
> The test on the R1 ran with the same reset placed in the X1000 entry; the
> R1's device tree lists "ingenic,x1600-uart" first, so it takes the same
> path with this version. Build-tested with W=1 on tty-next.
> 
> drivers/tty/serial/8250/8250_ingenic.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c
> index 0cdadacd7019..40307189021b 100644
> --- a/drivers/tty/serial/8250/8250_ingenic.c
> +++ b/drivers/tty/serial/8250/8250_ingenic.c
> @@ -25,6 +25,7 @@
> struct ingenic_uart_config {
> int tx_loadsz;
> int fifosize;
> + bool has_umr;
> };
> 
> struct ingenic_uart_data {
> @@ -40,6 +41,10 @@ static const struct of_device_id of_match[];
> #define UART_MCR_MDCE BIT(7)
> #define UART_MCR_FCM BIT(6)
> 
> +/* Clocks a bit, 16 for the divisor the 8250 core computes, and their per-bit adjustment. */
> +#define UART_UMR 9
> +#define UART_UACR 10
> +
> static struct earlycon_device *early_device;
> 
> static uint8_t early_in(struct uart_port *port, int offset)
> @@ -298,6 +303,12 @@ static int ingenic_uart_probe(struct platform_device *pdev)
> }
> uart.port.uartclk = clk_get_rate(data->clk_baud);
> 
> + /* Whatever ran before may have left another count: 8 on a HiBy R1. */
> + if (cdata->has_umr) {
> + writel(16, uart.port.membase + (UART_UMR << uart.port.regshift));
> + writel(0, uart.port.membase + (UART_UACR << uart.port.regshift));
> + }
> +
> data->uport = serial8250_register_8250_port(&uart);
> if (IS_ERR(data->uport)) {
> err = PTR_ERR(data->uport);
> @@ -344,6 +355,12 @@ static const struct ingenic_uart_config x1000_uart_config = {
> .fifosize = 64,
> };
> 
> +static const struct ingenic_uart_config x1600_uart_config = {
> + .tx_loadsz = 32,
> + .fifosize = 64,
> + .has_umr = true,

the x1000 has it as well. And even the jz4780. So correctly setting this
flag (as it is named) for more SoC variants will break more boards...

> +};
> +
> static const struct of_device_id of_match[] = {
> { .compatible = "ingenic,jz4740-uart", .data = &jz4740_uart_config },
> { .compatible = "ingenic,jz4750-uart", .data = &jz4760_uart_config },
> @@ -352,6 +369,7 @@ static const struct of_device_id of_match[] = {
> { .compatible = "ingenic,jz4775-uart", .data = &jz4760_uart_config },
> { .compatible = "ingenic,jz4780-uart", .data = &jz4780_uart_config },
> { .compatible = "ingenic,x1000-uart", .data = &x1000_uart_config },
> + { .compatible = "ingenic,x1600-uart", .data = &x1600_uart_config },

No, this is not special to the x1600, but a workaround of some peculiar
setup of your bootloader.

> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, of_match);
> -- 
> 2.39.5

BR, H. Nikolaus Schaller <hns@goldelico.com>

Re: [PATCH 1/2] dt-bindings: serial: ingenic: Add the X1600

[sashiko-bot]

> The X1600's UARTs have the X1000's registers as far as the 8250 core
> uses them, but the driver has to tell them apart to set their bit clock
> count. Add ingenic,x1600-uart, with the X1000's as fallback.
> 
> Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024051515.55639-1-olivier@mgcrea.io?part=1

Re: [PATCH 1/2] dt-bindings: serial: ingenic: Add the X1600

[Krzysztof Kozlowski]

On Sat, Oct 24, 2026 at 07:15:14AM +0200, Olivier Louvignes wrote:
> The X1600's UARTs have the X1000's registers as far as the 8250 core
> uses them, but the driver has to tell them apart to set their bit clock
> count. Add ingenic,x1600-uart, with the X1000's as fallback.
> 
> Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>
> ---
>  Documentation/devicetree/bindings/serial/ingenic,uart.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/ingenic,uart.yaml b/Documentation/devicetree/bindings/serial/ingenic,uart.yaml
> index d5f153bdeb0d..635342d43ddb 100644
> --- a/Documentation/devicetree/bindings/serial/ingenic,uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/ingenic,uart.yaml
> @@ -32,6 +32,9 @@ properties:
>        - items:
>            - const: ingenic,jz4725b-uart
>            - const: ingenic,jz4740-uart
> +      - items:
> +          - const: ingenic,x1600-uart
> +          - const: ingenic,x1000-uart

These should be sorted by fallback, so list using x1000 should be the
last (x > j).

With this fixed:

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

Best regards,
Krzysztof