[PATCH v2 0/2] serial: 8250_ingenic: Set UMR and UACR, add the X1600

[PATCH v2 0/2] serial: 8250_ingenic: Set UMR and UACR, add the X1600

[Olivier Louvignes]

Ingenic UARTs carry a bit clock count (UMR) and a per-bit adjustment
(UACR) that the driver never sets, so a UART runs with whatever was left
there. On the X1600 of a HiBy R1 neither the vendor's 8 nor the reset
value 0 gives the 16 clocks a bit the 8250 divisor assumes, and the
Bluetooth chip on UART0 fails to set up. Patch 2 sets 16 and 0 at probe.

Changes since v1:
- Set UMR and UACR on every Ingenic UART rather than in an X1600 entry,
  as Nikolaus asked: the JZ4725, JZ4740, JZ4755, JZ4760, X1000, X1830,
  X2000 and X1600 programming manuals all have them at 0x24 and 0x28.
- Write 16 rather than the reset value: with UMR at 0 the R1's Bluetooth
  chip fails just as it does with 8.
- The binding keeps the X1600 compatible with the X1000's as fallback;
  the driver needs no entry of its own.
- Rebased on tty-next.

I could only test the X1600. On the other SoCs this writes the value the
divisor already assumes.

v1: https://lore.kernel.org/all/20261024051515.55639-1-olivier@mgcrea.io/

Olivier Louvignes (2):
  dt-bindings: serial: ingenic: Add the X1600
  serial: 8250_ingenic: Set UMR and UACR at probe

 .../devicetree/bindings/serial/ingenic,uart.yaml     |  3 +++
 drivers/tty/serial/8250/8250_ingenic.c               | 12 ++++++++++++
 2 files changed, 15 insertions(+)


base-commit: 1184f87e2790e8af9be3116f608d2e86b4792d7d
--
2.39.5

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

[Olivier Louvignes]

The X1600's UARTs have the X1000's registers. 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
-- 
2.39.5

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

[Krzysztof Kozlowski]

On Sun, Oct 25, 2026 at 12:07:11AM +0200, Olivier Louvignes wrote:
> The X1600's UARTs have the X1000's registers. 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(+)
> 

Please slow down. You sent multiple patchsets, multiple versions, all
during the merge window.

I reviewed some and then found out there is a newer version...

Best regards,
Krzysztof

[PATCH v2 2/2] serial: 8250_ingenic: Set UMR and UACR at probe

[Olivier Louvignes]

Ingenic UARTs have two registers beyond the 16550's: UMR, the number of
clocks a bit takes, and UACR, which adds one clock to chosen bits of a
frame, for fractional baud rates. The JZ4725, JZ4740, JZ4755, JZ4760,
X1000, X1830, X2000 and X1600 programming manuals all have them at 0x24
and 0x28. The divisor the 8250 core computes assumes 16 clocks a bit and
none added.

The driver never writes them, so a UART keeps whatever was there before.
On a HiBy R1 (X1600) the vendor boot loader sets UMR to 8 on its console
UART, and the vendor kernel had left 8 on UART0, which carries the
Bluetooth chip: every rate came out doubled, and the chip never answered
("command 0xfc18 tx timeout" from hci_bcm). Their reset value, 0, which
the X1000 and X1600 manuals call normal mode, does not work on the X1600
either: with UMR at 0 the same command times out.

Set UMR to 16 and UACR to 0 at probe.

Tested on a HiBy R1: with UMR at 0 or 8 the BCM43430A1 on UART0 fails to
set up; with this patch it comes up at 1.5 Mbaud and streams A2DP audio.

Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>
---
 drivers/tty/serial/8250/8250_ingenic.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c
index 0cdadacd7019..ca45e74b999b 100644
--- a/drivers/tty/serial/8250/8250_ingenic.c
+++ b/drivers/tty/serial/8250/8250_ingenic.c
@@ -40,6 +40,10 @@ static const struct of_device_id of_match[];
 #define UART_MCR_MDCE	BIT(7)
 #define UART_MCR_FCM	BIT(6)
 
+/* Clocks a bit, and an extra clock for chosen bits */
+#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 +302,14 @@ static int ingenic_uart_probe(struct platform_device *pdev)
 	}
 	uart.port.uartclk = clk_get_rate(data->clk_baud);
 
+	/*
+	 * The 8250 core's divisor assumes 16 clocks a bit. Whatever ran before
+	 * may have left another count, and on the X1600 even the reset value,
+	 * 0, does not give 16.
+	 */
+	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);
-- 
2.39.5

Re: [PATCH v2 2/2] serial: 8250_ingenic: Set UMR and UACR at probe

[Paul Cercueil]

Hi Olivier,

Le dimanche 25 octobre 2026 à 00:07 +0200, Olivier Louvignes a écrit :
> Ingenic UARTs have two registers beyond the 16550's: UMR, the number
> of
> clocks a bit takes, and UACR, which adds one clock to chosen bits of
> a
> frame, for fractional baud rates. The JZ4725, JZ4740, JZ4755, JZ4760,
> X1000, X1830, X2000 and X1600 programming manuals all have them at
> 0x24
> and 0x28. The divisor the 8250 core computes assumes 16 clocks a bit
> and
> none added.
> 
> The driver never writes them, so a UART keeps whatever was there
> before.
> On a HiBy R1 (X1600) the vendor boot loader sets UMR to 8 on its
> console
> UART, and the vendor kernel had left 8 on UART0, which carries the
> Bluetooth chip: every rate came out doubled, and the chip never
> answered
> ("command 0xfc18 tx timeout" from hci_bcm). Their reset value, 0,
> which
> the X1000 and X1600 manuals call normal mode, does not work on the
> X1600
> either: with UMR at 0 the same command times out.
> 
> Set UMR to 16 and UACR to 0 at probe.
> 
> Tested on a HiBy R1: with UMR at 0 or 8 the BCM43430A1 on UART0 fails
> to
> set up; with this patch it comes up at 1.5 Mbaud and streams A2DP
> audio.
> 
> Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>
> ---
>  drivers/tty/serial/8250/8250_ingenic.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_ingenic.c
> b/drivers/tty/serial/8250/8250_ingenic.c
> index 0cdadacd7019..ca45e74b999b 100644
> --- a/drivers/tty/serial/8250/8250_ingenic.c
> +++ b/drivers/tty/serial/8250/8250_ingenic.c
> @@ -40,6 +40,10 @@ static const struct of_device_id of_match[];
>  #define UART_MCR_MDCE	BIT(7)
>  #define UART_MCR_FCM	BIT(6)
>  
> +/* Clocks a bit, and an extra clock for chosen bits */
> +#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 +302,14 @@ static int ingenic_uart_probe(struct
> platform_device *pdev)
>  	}
>  	uart.port.uartclk = clk_get_rate(data->clk_baud);
>  
> +	/*
> +	 * The 8250 core's divisor assumes 16 clocks a bit. Whatever
> ran before
> +	 * may have left another count, and on the X1600 even the
> reset value,
> +	 * 0, does not give 16.
> +	 */
> +	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);


This assumes that these registers are available on all Ingenic SoCs,
which is not the case. Looking at the PM it looks like they were added
in the JZ4725B.

Cheers,
-Paul

Re: [PATCH v2 0/2] serial: 8250_ingenic: Set UMR and UACR, add the X1600

[H. Nikolaus Schaller]

Hi,

> Am 25.10.2026 um 00:07 schrieb Olivier Louvignes <olivier@mgcrea.io>:
> 
> Ingenic UARTs carry a bit clock count (UMR) and a per-bit adjustment
> (UACR) that the driver never sets, so a UART runs with whatever was left
> there. On the X1600 of a HiBy R1 neither the vendor's 8 nor the reset
> value 0 gives the 16 clocks a bit the 8250 divisor assumes, and the
> Bluetooth chip on UART0 fails to set up. Patch 2 sets 16 and 0 at probe.

I am still not convinced that this is a x1600 kernel issue or something
that should be solved in kernel code.

On our LX16 board all 4 UARTs come up with UMR value 0x10.

> 
> Changes since v1:
> - Set UMR and UACR on every Ingenic UART rather than in an X1600 entry,
>  as Nikolaus asked: the JZ4725, JZ4740, JZ4755, JZ4760, X1000, X1830,
>  X2000 and X1600 programming manuals all have them at 0x24 and 0x28.

That was not what I have suggested. I asked to a mechanism that allows to
tailor this if the boot loader fails to provide the correct value.

> - Write 16 rather than the reset value: with UMR at 0 the R1's Bluetooth
>  chip fails just as it does with 8.

Ok, it would not harm our boards, but there we get the right value anyways.

> - The binding keeps the X1600 compatible with the X1000's as fallback;
>  the driver needs no entry of its own.
> - Rebased on tty-next.
> 
> I could only test the X1600. On the other SoCs this writes the value the
> divisor already assumes.

On our LX16 I find in U-Boot the default value 0x00 for UART0, 1 and 3 
and 0x10 for UART2 (which is the serial console activated by U-Boot):

lx16# 
lx16# md.l 10030024 1
10030024: 00000000                               ....
lx16# md.l 10031024 1
10031024: 00000000                               ....
lx16# md.l 10032024 1
10032024: 00000010                               ....
lx16# md.l 10033024 1
10033024: 00000000                               ....
lx16# 

But after booting into Linux (without your patch):

root@letux-lx16v0-3:~# devmem2 0x10030024
/dev/mem opened.
Memory mapped at address 0x77e61000.
Value at address 0x0x10030024 (0x77e61024): 0x10
root@letux-lx16v0-3:~# devmem2 0x10031024
/dev/mem opened.
Memory mapped at address 0x77e05000.
Value at address 0x0x10031024 (0x77e05024): 0x10
root@letux-lx16v0-3:~# devmem2 0x10032024
/dev/mem opened.
Memory mapped at address 0x77eb0000.
Value at address 0x0x10032024 (0x77eb0024): 0x10
root@letux-lx16v0-3:~# devmem2 0x10033024
/dev/mem opened.
Memory mapped at address 0x77ea1000.
Value at address 0x0x10033024 (0x77ea1024): 0x10
root@letux-lx16v0-3:~# 

So something is already setting the UMR register properly on our x1600
without your patch.

I am not at all a specialist for the 8250/16650 but I got the impression that
what we call UMR in x1600 language could be the UART_XR_EFR register
in 8250 language.

There is a function called default_setup() in 8250_exar.c which says that
some UARTs have an extra divisor register.

This all sounds like some standard code might already be responsible to set
the UMR registers to 0x10 for UARTs initialized on the x1600?

But that needs deeper analysis than I have done for now.

BR,
Nikolaus