2023-02-22 21:05:38

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v1 RFC 0/3] meson-uart: Use "divide XTAL by 2" bit on G12A

This series improves support for UART attached Bluetooth modules on
Amlogic Meson G12A and newer SoCs. These SoCs also support the "divide
XTAL by 2" bit which (greatly) reduces jitter when generating baud
rates such as 1500000 (which is used by the Bluetooth part of the
RTL8822CS SDIO WiFi and UART Bluetooth combo chip).

Without this the baud rate calculation is based on the XTAL clock
(running at 24MHz) divided by 3 (meaning: 8MHz). 8MHz cannot be divided
with integer division to a 1500000 baud rate. Using the "divide XTAL
by 2" bit however means that we can achieve 1500000 cleanly, without any
jitter.

In future we should allow dynamic switching of these UART controller
internal dividers to pick the best divider automatically for the
requested baud rate. This however still requires the new compatible
string - which is added by this series - to enable the "divide XTAL
by 2" logic on SoCs that support it (G12A and newer).

Why am I sending this as RFC? The last change in this series means
that the resulting .dtbs are not compatible with old kernels anymore.
My understanding is that this is fine and only the opposite case (using
old .dtbs on new kernels) has to be supported (which is still the case
with this series). I'd like to get some confirmation for this.


[0] https://lore.kernel.org/linux-bluetooth/[email protected]/


Martin Blumenstingl (3):
dt-bindings: serial: amlogic,meson-uart: Add compatible string for
G12A
tty: serial: meson: Add a new compatible string for the G12A SoC
arm64: dts: meson-g12-common: Use the G12A UART compatible string

.../devicetree/bindings/serial/amlogic,meson-uart.yaml | 2 ++
arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 10 +++++-----
drivers/tty/serial/meson_uart.c | 8 ++++++--
3 files changed, 13 insertions(+), 7 deletions(-)

--
2.39.2



2023-02-22 21:05:41

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v1 RFC 1/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for G12A

Amlogic G12A SoCs gained a new "divide XTAL by 2" bit. Everything else
(we know about) is identical to the UART IP on GX (GXBB/GXL/GXM) SoCs.
Add a new compatible string for this SoC so this new bit can be managed
accordingly.

Signed-off-by: Martin Blumenstingl <[email protected]>
---
Why am I sending this as RFC? The last change in this series means
that the resulting .dtbs are not compatible with old kernels anymore.
My understanding is that this is fine and only the opposite case (using
old .dtbs on new kernels) has to be supported (which is still the case
with this series). I'd like to get some confirmation for this.


.../devicetree/bindings/serial/amlogic,meson-uart.yaml | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
index 7822705ad16c..3d9d51389171 100644
--- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
@@ -29,6 +29,7 @@ properties:
- amlogic,meson8-uart
- amlogic,meson8b-uart
- amlogic,meson-gx-uart
+ - amlogic,meson-g12a-uart
- amlogic,meson-s4-uart
- const: amlogic,meson-ao-uart
- description: Everything-Else power domain UART controller
@@ -37,6 +38,7 @@ properties:
- amlogic,meson8-uart
- amlogic,meson8b-uart
- amlogic,meson-gx-uart
+ - amlogic,meson-g12a-uart
- amlogic,meson-s4-uart

reg:
--
2.39.2


2023-02-22 21:05:45

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v1 RFC 2/3] tty: serial: meson: Add a new compatible string for the G12A SoC

Amlogic Meson G12A (and later) SoCs also have the "divide XTAL by 2" bit
as the S4 UART controllers. Add a new compatible string for these SoCs
and enable the has_xtal_div2 flag for them.

Tested-by: Christian Hewitt <[email protected]>
Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/tty/serial/meson_uart.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 74110017988a..2501db5a7aaf 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -779,7 +779,7 @@ static int meson_uart_remove(struct platform_device *pdev)
return 0;
}

-static struct meson_uart_data s4_uart_data = {
+static struct meson_uart_data meson_g12a_uart_data = {
.has_xtal_div2 = true,
};

@@ -788,9 +788,13 @@ static const struct of_device_id meson_uart_dt_match[] = {
{ .compatible = "amlogic,meson8-uart" },
{ .compatible = "amlogic,meson8b-uart" },
{ .compatible = "amlogic,meson-gx-uart" },
+ {
+ .compatible = "amlogic,meson-g12a-uart",
+ .data = (void *)&meson_g12a_uart_data,
+ },
{
.compatible = "amlogic,meson-s4-uart",
- .data = (void *)&s4_uart_data,
+ .data = (void *)&meson_g12a_uart_data,
},
{ /* sentinel */ },
};
--
2.39.2


2023-02-22 21:05:48

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v1 RFC 3/3] arm64: dts: meson-g12-common: Use the G12A UART compatible string

Switch meson-12-common.dtsi to use the Meson G12A specific UART
compatible string. This enables the "divide XTAL by 2" divider which
improves support for UART attached Bluetooth modules (for example
RTL8822CS) running at a baud rate of 1500000. Without dividing XTAL
(24MHz) by 2 a baud rate of 1500000 cannot be generated cleanly and the
resulting jitter breaks communication with the module.

Tested-by: Christian Hewitt <[email protected]>
Signed-off-by: Martin Blumenstingl <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index 123a56f7f818..3dc06848f3c4 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -2046,7 +2046,7 @@ pwm_AO_cd: pwm@2000 {
};

uart_AO: serial@3000 {
- compatible = "amlogic,meson-gx-uart",
+ compatible = "amlogic,meson-g12a-uart",
"amlogic,meson-ao-uart";
reg = <0x0 0x3000 0x0 0x18>;
interrupts = <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>;
@@ -2056,7 +2056,7 @@ uart_AO: serial@3000 {
};

uart_AO_B: serial@4000 {
- compatible = "amlogic,meson-gx-uart",
+ compatible = "amlogic,meson-g12a-uart",
"amlogic,meson-ao-uart";
reg = <0x0 0x4000 0x0 0x18>;
interrupts = <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>;
@@ -2293,7 +2293,7 @@ clk_msr: clock-measure@18000 {
};

uart_C: serial@22000 {
- compatible = "amlogic,meson-gx-uart";
+ compatible = "amlogic,meson-g12a-uart";
reg = <0x0 0x22000 0x0 0x18>;
interrupts = <GIC_SPI 93 IRQ_TYPE_EDGE_RISING>;
clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
@@ -2302,7 +2302,7 @@ uart_C: serial@22000 {
};

uart_B: serial@23000 {
- compatible = "amlogic,meson-gx-uart";
+ compatible = "amlogic,meson-g12a-uart";
reg = <0x0 0x23000 0x0 0x18>;
interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>;
clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
@@ -2311,7 +2311,7 @@ uart_B: serial@23000 {
};

uart_A: serial@24000 {
- compatible = "amlogic,meson-gx-uart";
+ compatible = "amlogic,meson-g12a-uart";
reg = <0x0 0x24000 0x0 0x18>;
interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
--
2.39.2


2023-02-23 09:12:41

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1 RFC 1/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for G12A

On 22/02/2023 22:04, Martin Blumenstingl wrote:
> Amlogic G12A SoCs gained a new "divide XTAL by 2" bit. Everything else
> (we know about) is identical to the UART IP on GX (GXBB/GXL/GXM) SoCs.
> Add a new compatible string for this SoC so this new bit can be managed
> accordingly.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
> ---
> Why am I sending this as RFC? The last change in this series means
> that the resulting .dtbs are not compatible with old kernels anymore.
> My understanding is that this is fine and only the opposite case (using
> old .dtbs on new kernels) has to be supported (which is still the case
> with this series). I'd like to get some confirmation for this.

The other way around is also nice to have, because DTS is used in other
projects. You fixed here Linux kernel, but what about all other
out-of-tree kernels, BSDs, firmwares and bootloaders? Did you fix them
as well?

The question is whether the devices can be made compatible thus keeping
DTS working on older kernel. This commit suggests they are. Your DTS
commit is written in different tone - something was broken and is being
fixed.

Best regards,
Krzysztof


2023-02-23 20:14:31

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH v1 RFC 1/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for G12A

Hello Krzysztof,

On Thu, Feb 23, 2023 at 10:12 AM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 22/02/2023 22:04, Martin Blumenstingl wrote:
> > Amlogic G12A SoCs gained a new "divide XTAL by 2" bit. Everything else
> > (we know about) is identical to the UART IP on GX (GXBB/GXL/GXM) SoCs.
> > Add a new compatible string for this SoC so this new bit can be managed
> > accordingly.
> >
> > Signed-off-by: Martin Blumenstingl <[email protected]>
> > ---
> > Why am I sending this as RFC? The last change in this series means
> > that the resulting .dtbs are not compatible with old kernels anymore.
> > My understanding is that this is fine and only the opposite case (using
> > old .dtbs on new kernels) has to be supported (which is still the case
> > with this series). I'd like to get some confirmation for this.
>
> The other way around is also nice to have, because DTS is used in other
> projects. You fixed here Linux kernel, but what about all other
> out-of-tree kernels, BSDs, firmwares and bootloaders? Did you fix them
> as well?
Indeed, u-boot is of concern here (as mainline u-boot does have Meson
G12A SoC support).

> The question is whether the devices can be made compatible thus keeping
> DTS working on older kernel. This commit suggests they are. Your DTS
> commit is written in different tone - something was broken and is being
> fixed.
If we keep "amlogic,meson-gx-uart" as fallback compatible string then
old kernels (or other .dtb consumers - like u-boot) would still work.
Without the new "amlogic,meson-g12a-uart" compatible string we're
unable to make use of a newly added clock divider within the UART IP
block which allows baud rates such as 1500000 to work without (a lot
of) jitter. Old kernels - with the new .dtb - would still be able to
use serial (thanks to the "amlogic,meson-gx-uart" fallback compatible
string) albeit with limited divider support (so not all baud rates can
be used).
Is this a valid plan?


Best regards,
Martin

2023-02-24 08:26:56

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1 RFC 1/3] dt-bindings: serial: amlogic,meson-uart: Add compatible string for G12A

On 23/02/2023 21:14, Martin Blumenstingl wrote:
>
>> The question is whether the devices can be made compatible thus keeping
>> DTS working on older kernel. This commit suggests they are. Your DTS
>> commit is written in different tone - something was broken and is being
>> fixed.
> If we keep "amlogic,meson-gx-uart" as fallback compatible string then
> old kernels (or other .dtb consumers - like u-boot) would still work.
> Without the new "amlogic,meson-g12a-uart" compatible string we're
> unable to make use of a newly added clock divider within the UART IP
> block which allows baud rates such as 1500000 to work without (a lot
> of) jitter. Old kernels - with the new .dtb - would still be able to
> use serial (thanks to the "amlogic,meson-gx-uart" fallback compatible
> string) albeit with limited divider support (so not all baud rates can
> be used).
> Is this a valid plan?

Yes.

Best regards,
Krzysztof