[PATCH v5.1 0/2] tty/serial: meson_uart: add support for core clock handling

[PATCH v5.1 0/2] tty/serial: meson_uart: add support for core clock handling

[Neil Armstrong]

This patchset is a re-spin of Helmut Klein's v3 patchset at [0] and the v4/v5 patchset at [1] & [2].

Initially, the original patchset was made to enable usage on the non-AO UARTS
not enabled by the Bootloader (uart_B and uart_C), but the patchset needed
an overall change to have clean and stable DT bindings.

The Amlogic Meson UART Driver did not have stable DT bindings and mismatched
clock handling on non-AO UARTs since these "EE" UARTs needs a clock gate to
be ungated to works correctly.
In the same way, the AO UARTs does not need gating and can be used as
Early Consoles.

In the same time, the UART Interfaces can take clock input for the baudrate
generate from either the external Xtal or the internal Bus Clock (clk81).

So new bindings was necessary to meet these requirements and the DT
maintainers requirements.

The "legacy" binding actually used in the driver is left until all the DT
files are switched to the new bindings.

The GX DT has been tested, but the last 4 Meson6/Meson8/b are only
compile-tested, and testing is welcome.
Thus only the first 3 patches can be merged until the Meson6/Meson8/b are
formally tested.

It must be noted that the meson6 cannot work today except using an early
console since the UART driver could not probe without a clocks property.

Changes since v5 at [2]:
 - Dropped bindings and tty patch since merged by greg kh into tty next branch
 - Keep old bindings in DT files for retro-compatibility

Changes since v4 at [1]:
 - Droped meson8/meson8b DT patches
 - Fixes copy/paste error in patch 2
 - Refactored clock probing in patch 2
 - merged meson6 patches together to avoid breaking bisect

[0] http://lkml.kernel.org/r/20170331165437.26227-1-hgkr.klein@gmail.com
[1] http://lkml.kernel.org/r/1497001756-942-1-git-send-email-narmstrong@baylibre.com
[2] http://lkml.kernel.org/r/1497428957-19942-1-git-send-email-narmstrong@baylibre.com

Helmut Klein (1):
  ARM64: dts: meson-gx: use stable UART bindings with correct gate clock

Neil Armstrong (1):
  ARM: dts: meson6: use stable UART bindings

 arch/arm/boot/dts/meson.dtsi                |  8 ++++----
 arch/arm/boot/dts/meson6.dtsi               | 28 ++++++++++++++++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi   | 12 +++++-------
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 25 +++++++++++++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 25 +++++++++++++++++++++++++
 5 files changed, 87 insertions(+), 11 deletions(-)

-- 
1.9.1

[PATCH v5.1 1/2] ARM64: dts: meson-gx: use stable UART bindings with correct gate clock

[Neil Armstrong]

From: Helmut Klein <hgkr.klein@gmail.com>

This patch switches to the stable UART bindings but also add the correct
gate clock to the non-AO UART nodes for GXBB and GXL SoCs.

Acked-by: Jerome Brunet <jbrunet@baylibre.com>
Signed-off-by: Helmut Klein <hgkr.klein@gmail.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi   | 12 +++++-------
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 25 +++++++++++++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 25 +++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 35b8c88..1c6e4ed 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -225,7 +225,7 @@
 			};
 
 			uart_A: serial@84c0 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson-gx-uart", "amlogic,meson-uart";
 				reg = <0x0 0x84c0 0x0 0x14>;
 				interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
 				clocks = <&xtal>;
@@ -233,7 +233,7 @@
 			};
 
 			uart_B: serial@84dc {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson-gx-uart", "amlogic,meson-uart";
 				reg = <0x0 0x84dc 0x0 0x14>;
 				interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>;
 				clocks = <&xtal>;
@@ -279,7 +279,7 @@
 			};
 
 			uart_C: serial@8700 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson-gx-uart", "amlogic,meson-uart";
 				reg = <0x0 0x8700 0x0 0x14>;
 				interrupts = <GIC_SPI 93 IRQ_TYPE_EDGE_RISING>;
 				clocks = <&xtal>;
@@ -375,18 +375,16 @@
 			};
 
 			uart_AO: serial@4c0 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson-gx-uart", "amlogic,meson-ao-uart", "amlogic,meson-uart";
 				reg = <0x0 0x004c0 0x0 0x14>;
 				interrupts = <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>;
-				clocks = <&xtal>;
 				status = "disabled";
 			};
 
 			uart_AO_B: serial@4e0 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson-gx-uart", "amlogic,meson-ao-uart", "amlogic,meson-uart";
 				reg = <0x0 0x004e0 0x0 0x14>;
 				interrupts = <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>;
-				clocks = <&xtal>;
 				status = "disabled";
 			};
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 17d3efd..ea53cc2 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -682,6 +682,31 @@
 	clocks = <&clkc CLKID_SPI>;
 };
 
+&uart_A {
+	clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_AO {
+	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_AO_B {
+	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_B {
+	clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
+	clock-names = "xtal", "core", "baud";
+};
+
+&uart_C {
+	clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
+	clock-names = "xtal", "core", "baud";
+};
+
 &vpu {
 	compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
 };
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index 8d4f316..9e67444 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -623,6 +623,31 @@
 	clocks = <&clkc CLKID_SPI>;
 };
 
+&uart_A {
+	clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
+	clock-names = "xtal", "core", "baud";
+};
+
+&uart_AO {
+	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_AO_B {
+	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_B {
+	clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
+	clock-names = "xtal", "core", "baud";
+};
+
+&uart_C {
+	clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
+	clock-names = "xtal", "core", "baud";
+};
+
 &vpu {
 	compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
 };
-- 
1.9.1

[PATCH v5.1 2/2] ARM: dts: meson6: use stable UART bindings

[Neil Armstrong]

The UART bindings needs specifying a SoC family, use the meson6 family
for the UART nodes like the other nodes.
Switch to the stable UART bindings for meson6 by adding a XTAL node and
using the proper compatible strings.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm/boot/dts/meson.dtsi  |  8 ++++----
 arch/arm/boot/dts/meson6.dtsi | 28 ++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
index 15204e4..1ce9aa0 100644
--- a/arch/arm/boot/dts/meson.dtsi
+++ b/arch/arm/boot/dts/meson.dtsi
@@ -86,14 +86,14 @@
 			};
 
 			uart_A: serial@84c0 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson6-uart", "amlogic,meson-uart";
 				reg = <0x84c0 0x18>;
 				interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
 			};
 
 			uart_B: serial@84dc {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson6-uart", "amlogic,meson-uart";
 				reg = <0x84dc 0x18>;
 				interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
@@ -117,7 +117,7 @@
 			};
 
 			uart_C: serial@8700 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson6-uart", "amlogic,meson-uart";
 				reg = <0x8700 0x18>;
 				interrupts = <GIC_SPI 93 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
@@ -182,7 +182,7 @@
 			};
 
 			uart_AO: serial@4c0 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson6-uart", "amlogic,meson-ao-uart", "amlogic,meson-uart";
 				reg = <0x4c0 0x18>;
 				interrupts = <GIC_SPI 90 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
diff --git a/arch/arm/boot/dts/meson6.dtsi b/arch/arm/boot/dts/meson6.dtsi
index 8557b61..ef281d2 100644
--- a/arch/arm/boot/dts/meson6.dtsi
+++ b/arch/arm/boot/dts/meson6.dtsi
@@ -70,9 +70,37 @@
 		};
 	};
 
+	xtal: xtal-clk {
+		compatible = "fixed-clock";
+		clock-frequency = <24000000>;
+		clock-output-names = "xtal";
+		#clock-cells = <0>;
+	};
+
 	clk81: clk@0 {
 		#clock-cells = <0>;
 		compatible = "fixed-clock";
 		clock-frequency = <200000000>;
 	};
 }; /* end of / */
+
+
+&uart_AO {
+	clocks = <&xtal>, <&clk81>, <&clk81>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_A {
+	clocks = <&xtal>, <&clk81>, <&clk81>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_B {
+	clocks = <&xtal>, <&clk81>, <&clk81>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_C {
+	clocks = <&xtal>, <&clk81>, <&clk81>;
+	clock-names = "xtal", "pclk", "baud";
+};
-- 
1.9.1

Re: [PATCH v5.1 1/2] ARM64: dts: meson-gx: use stable UART bindings with correct gate clock

[Kevin Hilman]

Neil Armstrong <narmstrong@baylibre.com> writes:

> From: Helmut Klein <hgkr.klein@gmail.com>
>
> This patch switches to the stable UART bindings but also add the correct
> gate clock to the non-AO UART nodes for GXBB and GXL SoCs.
>
> Acked-by: Jerome Brunet <jbrunet@baylibre.com>
> Signed-off-by: Helmut Klein <hgkr.klein@gmail.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Applied to v4.14/dt64,

Thanks,

Kevin

Re: [PATCH v5.1 2/2] ARM: dts: meson6: use stable UART bindings

[Kevin Hilman]

Neil Armstrong <narmstrong@baylibre.com> writes:

> The UART bindings needs specifying a SoC family, use the meson6 family
> for the UART nodes like the other nodes.
> Switch to the stable UART bindings for meson6 by adding a XTAL node and
> using the proper compatible strings.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Applied to v4.14/dt,

Thanks,

Kevin

Re: [PATCH v5.1 1/2] ARM64: dts: meson-gx: use stable UART bindings with correct gate clock

[Andreas Färber]

Hi,

Am 21.06.2017 um 16:42 schrieb Neil Armstrong:
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> index 17d3efd..ea53cc2 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> @@ -682,6 +682,31 @@
>  	clocks = <&clkc CLKID_SPI>;
>  };
>  
> +&uart_A {
> +	clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
> +	clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_AO {
> +	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
> +	clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_AO_B {
> +	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
> +	clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_B {
> +	clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
> +	clock-names = "xtal", "core", "baud";

Looking at the meson_uart driver, it only looks for a "pclk" clock,
never for "core", and the only unnamed clock used should be the first.

There is no bindings documentation for "core", so I assume this was an
oversight and should be "pclk" everywhere?

> +};
> +
> +&uart_C {
> +	clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
> +	clock-names = "xtal", "core", "baud";
> +};

The issue I'm facing is that uart_C on NanoPi K2 is not working in
4.14.1. To my surprise it appears to be probing okay though.

Sadly just overwriting the clock-names property via overlay does not fix
my issue. Any ideas?

> +
>  &vpu {
>  	compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
>  };
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> index 8d4f316..9e67444 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> @@ -623,6 +623,31 @@
>  	clocks = <&clkc CLKID_SPI>;
>  };
>  
> +&uart_A {
> +	clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
> +	clock-names = "xtal", "core", "baud";

Same issue spotted here and below.

> +};
> +
> +&uart_AO {
> +	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
> +	clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_AO_B {
> +	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
> +	clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_B {
> +	clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
> +	clock-names = "xtal", "core", "baud";
> +};
> +
> +&uart_C {
> +	clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
> +	clock-names = "xtal", "core", "baud";
> +};
> +
>  &vpu {
>  	compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
>  };

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v5.1 1/2] ARM64: dts: meson-gx: use stable UART bindings with correct gate clock

[Andreas Färber]

Am 03.12.2017 um 15:15 schrieb Andreas Färber:
> Hi,
> 
> Am 21.06.2017 um 16:42 schrieb Neil Armstrong:
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> index 17d3efd..ea53cc2 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> @@ -682,6 +682,31 @@
>>  	clocks = <&clkc CLKID_SPI>;
>>  };
>>  
>> +&uart_A {
>> +	clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
>> +	clock-names = "xtal", "pclk", "baud";
>> +};
>> +
>> +&uart_AO {
>> +	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
>> +	clock-names = "xtal", "pclk", "baud";
>> +};
>> +
>> +&uart_AO_B {
>> +	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
>> +	clock-names = "xtal", "pclk", "baud";
>> +};
>> +
>> +&uart_B {
>> +	clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
>> +	clock-names = "xtal", "core", "baud";
> 
> Looking at the meson_uart driver, it only looks for a "pclk" clock,
> never for "core", and the only unnamed clock used should be the first.
> 
> There is no bindings documentation for "core", so I assume this was an
> oversight and should be "pclk" everywhere?
> 
>> +};
>> +
>> +&uart_C {
>> +	clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
>> +	clock-names = "xtal", "core", "baud";
>> +};
> 
> The issue I'm facing is that uart_C on NanoPi K2 is not working in
> 4.14.1. To my surprise it appears to be probing okay though.

In 4.14 the serial driver checks for the legacy compatible and if
present (even alongside the new compatible) always takes the legacy code
path, using only the first clock, thus not tripping over "core" and not
enabling the "pclk" if supplied.

This was changed for v4.15-rc1, so we should start seeing errors for
missing "pclk" clocks.

> Sadly just overwriting the clock-names property via overlay does not fix
> my issue. Any ideas?

For reference here's my latest overlay, overriding compatible, too:
https://github.com/afaerber/dt-overlays/blob/master/meson-gxbb-nanopi-k2%2Barpi600%2Bnucleo-lrwan1.dts

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v5.1 1/2] ARM64: dts: meson-gx: use stable UART bindings with correct gate clock

[Neil Armstrong]

On 04/12/2017 01:16, Andreas Färber wrote:
> Am 03.12.2017 um 15:15 schrieb Andreas Färber:
>> Hi,
>>
>> Am 21.06.2017 um 16:42 schrieb Neil Armstrong:
>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>> index 17d3efd..ea53cc2 100644
>>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>> @@ -682,6 +682,31 @@
>>>  	clocks = <&clkc CLKID_SPI>;
>>>  };
>>>  
>>> +&uart_A {
>>> +	clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
>>> +	clock-names = "xtal", "pclk", "baud";
>>> +};
>>> +
>>> +&uart_AO {
>>> +	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
>>> +	clock-names = "xtal", "pclk", "baud";
>>> +};
>>> +
>>> +&uart_AO_B {
>>> +	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
>>> +	clock-names = "xtal", "pclk", "baud";
>>> +};
>>> +
>>> +&uart_B {
>>> +	clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
>>> +	clock-names = "xtal", "core", "baud";
>>
>> Looking at the meson_uart driver, it only looks for a "pclk" clock,
>> never for "core", and the only unnamed clock used should be the first.
>>
>> There is no bindings documentation for "core", so I assume this was an
>> oversight and should be "pclk" everywhere?
>>
>>> +};
>>> +
>>> +&uart_C {
>>> +	clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
>>> +	clock-names = "xtal", "core", "baud";
>>> +};
>>
>> The issue I'm facing is that uart_C on NanoPi K2 is not working in
>> 4.14.1. To my surprise it appears to be probing okay though.
> 
> In 4.14 the serial driver checks for the legacy compatible and if
> present (even alongside the new compatible) always takes the legacy code
> path, using only the first clock, thus not tripping over "core" and not
> enabling the "pclk" if supplied.

I only used UART_A, this explains a lot.

> 
> This was changed for v4.15-rc1, so we should start seeing errors for
> missing "pclk" clocks.
> 
>> Sadly just overwriting the clock-names property via overlay does not fix
>> my issue. Any ideas?

You need to rebind the devices.

> 
> For reference here's my latest overlay, overriding compatible, too:
> https://github.com/afaerber/dt-overlays/blob/master/meson-gxbb-nanopi-k2%2Barpi600%2Bnucleo-lrwan1.dts
> 
> Regards,
> Andreas
> 

Wow I must have been very tired when pushing this....

Thanks for spotting this.

I'll send a fix asap.

Neil