linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] ARM64: dts: meson-axg: UART DT updates
@ 2018-01-06  0:10 Yixun Lan
  2018-01-06  0:10 ` [PATCH v2 1/5] ARM64: dts: meson: uart: fix address space range Yixun Lan
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Yixun Lan @ 2018-01-06  0:10 UTC (permalink / raw)
  To: Kevin Hilman, devicetree
  Cc: Neil Armstrong, Jerome Brunet, Rob Herring, Mark Rutland,
	Carlo Caione, Yixun Lan, linux-amlogic, linux-arm-kernel,
	linux-kernel

HI Kevin
 These are the UART DT updates for the Meson-AXG platform.

 The patch 1 is a general fix.
Other patches are about adding clock & pinctrl info, then using them.
Last patch enable UART_A which connect to a BT module on the S400 board.

Note: 
This series depend on previous UART_AO clock switch patch[1]
also, these patch request clocks, so they need the
tag:meson-clk-for-v4.16-2 from clk-meson's tree in order to compile.

Changes since v1 at [2]:
  -- fix address range for all platform
  -- squash patch 1, 3 (drop compatible & add clock)
  -- fix typo in pinctrl info
  -- add Jerome's Ack

[1] 
 http://lkml.kernel.org/r/20171215141741.175985-1-yixun.lan@amlogic.com

[2]
 http://lkml.kernel.org/r/20180105095621.196472-1-yixun.lan@amlogic.com


Yixun Lan (5):
  ARM64: dts: meson: uart: fix address space range
  ARM64: dts: meson-axg: uart: drop legacy compatible name from EE UART
  ARM64: dts: meson-axg: uart: Add the pinctrl info description
  ARM64: dts: meson-axg: complete the pinctrl info for UART_AO_A
  ARM64: dts: meson-axg: enable the UART_A controller

 arch/arm64/boot/dts/amlogic/meson-axg-s400.dts |   9 ++
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi     | 109 ++++++++++++++++++++++++-
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi      |  10 +--
 3 files changed, 119 insertions(+), 9 deletions(-)

-- 
2.15.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/5] ARM64: dts: meson: uart: fix address space range
  2018-01-06  0:10 [PATCH v2 0/5] ARM64: dts: meson-axg: UART DT updates Yixun Lan
@ 2018-01-06  0:10 ` Yixun Lan
  2018-01-08  8:54   ` Jerome Brunet
  2018-01-06  0:10 ` [PATCH v2 2/5] ARM64: dts: meson-axg: uart: drop legacy compatible name from EE UART Yixun Lan
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Yixun Lan @ 2018-01-06  0:10 UTC (permalink / raw)
  To: Kevin Hilman, devicetree
  Cc: Neil Armstrong, Jerome Brunet, Rob Herring, Mark Rutland,
	Carlo Caione, Yixun Lan, linux-amlogic, linux-arm-kernel,
	linux-kernel

The address space range is actually 0x18, fixed here.

Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi |  4 ++--
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi  | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index a80632641b39..70c776ef7aa7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -165,14 +165,14 @@
 
 			uart_A: serial@24000 {
 				compatible = "amlogic,meson-gx-uart", "amlogic,meson-uart";
-				reg = <0x0 0x24000 0x0 0x14>;
+				reg = <0x0 0x24000 0x0 0x18>;
 				interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
 			};
 
 			uart_B: serial@23000 {
 				compatible = "amlogic,meson-gx-uart", "amlogic,meson-uart";
-				reg = <0x0 0x23000 0x0 0x14>;
+				reg = <0x0 0x23000 0x0 0x18>;
 				interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
 			};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 6cb3c2a52baf..4ee2e7951482 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -235,14 +235,14 @@
 
 			uart_A: serial@84c0 {
 				compatible = "amlogic,meson-gx-uart";
-				reg = <0x0 0x84c0 0x0 0x14>;
+				reg = <0x0 0x84c0 0x0 0x18>;
 				interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
 			};
 
 			uart_B: serial@84dc {
 				compatible = "amlogic,meson-gx-uart";
-				reg = <0x0 0x84dc 0x0 0x14>;
+				reg = <0x0 0x84dc 0x0 0x18>;
 				interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
 			};
@@ -287,7 +287,7 @@
 
 			uart_C: serial@8700 {
 				compatible = "amlogic,meson-gx-uart";
-				reg = <0x0 0x8700 0x0 0x14>;
+				reg = <0x0 0x8700 0x0 0x18>;
 				interrupts = <GIC_SPI 93 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
 			};
@@ -404,14 +404,14 @@
 
 			uart_AO: serial@4c0 {
 				compatible = "amlogic,meson-gx-uart", "amlogic,meson-ao-uart";
-				reg = <0x0 0x004c0 0x0 0x14>;
+				reg = <0x0 0x004c0 0x0 0x18>;
 				interrupts = <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
 			};
 
 			uart_AO_B: serial@4e0 {
 				compatible = "amlogic,meson-gx-uart", "amlogic,meson-ao-uart";
-				reg = <0x0 0x004e0 0x0 0x14>;
+				reg = <0x0 0x004e0 0x0 0x18>;
 				interrupts = <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
 			};
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/5] ARM64: dts: meson-axg: uart: drop legacy compatible name from EE UART
  2018-01-06  0:10 [PATCH v2 0/5] ARM64: dts: meson-axg: UART DT updates Yixun Lan
  2018-01-06  0:10 ` [PATCH v2 1/5] ARM64: dts: meson: uart: fix address space range Yixun Lan
@ 2018-01-06  0:10 ` Yixun Lan
  2018-01-08  8:56   ` Jerome Brunet
  2018-01-06  0:10 ` [PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description Yixun Lan
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Yixun Lan @ 2018-01-06  0:10 UTC (permalink / raw)
  To: Kevin Hilman, devicetree
  Cc: Neil Armstrong, Jerome Brunet, Rob Herring, Mark Rutland,
	Carlo Caione, Yixun Lan, linux-amlogic, linux-arm-kernel,
	linux-kernel

When update the clock info for the UART controller in the EE domain,
the driver explicitly require 'pclk' in order to work properly.

With current logic of the code, the driver will go for the legacy clock probe
routine[1] if it find current compatible string match to 'amlogic,meson-uart',
which result in not requesting the 'pclk' clock, thus break the driver in the end.

[1] drivers/tty/serial/meson_uart.c:685

        /* Use legacy way until all platforms switch to new bindings */
	if (of_device_is_compatible(pdev->dev.of_node, "amlogic,meson-uart"))
		ret = meson_uart_probe_clocks_legacy(pdev, port);
	else
		ret = meson_uart_probe_clocks(pdev, port);

Acked-by: Jerome Brunet <jbrunet@baylibre.com>
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index 70c776ef7aa7..644d0f9eaf8c 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -164,17 +164,21 @@
 			};
 
 			uart_A: serial@24000 {
-				compatible = "amlogic,meson-gx-uart", "amlogic,meson-uart";
+				compatible = "amlogic,meson-gx-uart";
 				reg = <0x0 0x24000 0x0 0x18>;
 				interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
+				clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
+				clock-names = "xtal", "pclk", "baud";
 			};
 
 			uart_B: serial@23000 {
-				compatible = "amlogic,meson-gx-uart", "amlogic,meson-uart";
+				compatible = "amlogic,meson-gx-uart";
 				reg = <0x0 0x23000 0x0 0x18>;
 				interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>;
 				status = "disabled";
+				clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
+				clock-names = "xtal", "pclk", "baud";
 			};
 		};
 
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description
  2018-01-06  0:10 [PATCH v2 0/5] ARM64: dts: meson-axg: UART DT updates Yixun Lan
  2018-01-06  0:10 ` [PATCH v2 1/5] ARM64: dts: meson: uart: fix address space range Yixun Lan
  2018-01-06  0:10 ` [PATCH v2 2/5] ARM64: dts: meson-axg: uart: drop legacy compatible name from EE UART Yixun Lan
@ 2018-01-06  0:10 ` Yixun Lan
  2018-01-07 20:19   ` Martin Blumenstingl
  2018-01-06  0:10 ` [PATCH v2 4/5] ARM64: dts: meson-axg: complete the pinctrl info for UART_AO_A Yixun Lan
  2018-01-06  0:10 ` [PATCH v2 5/5] ARM64: dts: meson-axg: enable the UART_A controller Yixun Lan
  4 siblings, 1 reply; 13+ messages in thread
From: Yixun Lan @ 2018-01-06  0:10 UTC (permalink / raw)
  To: Kevin Hilman, devicetree
  Cc: Neil Armstrong, Jerome Brunet, Rob Herring, Mark Rutland,
	Carlo Caione, Yixun Lan, linux-amlogic, linux-arm-kernel,
	linux-kernel

Describe the pinctrl info for the UART controller which is found
in the Meson-AXG SoCs.

Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 97 ++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index 644d0f9eaf8c..1eb45781c850 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -448,6 +448,70 @@
 						function = "spi1";
 					};
 				};
+
+				uart_a_pins: uart_a {
+					mux {
+						groups = "uart_tx_a",
+							"uart_rx_a";
+						function = "uart_a";
+					};
+				};
+
+				uart_a_cts_rts_pins: uart_a_cts_rts {
+					mux {
+						groups = "uart_cts_a",
+							"uart_rts_a";
+						function = "uart_a";
+					};
+				};
+
+				uart_b_x_pins: uart_b_x {
+					mux {
+						groups = "uart_tx_b_x",
+							"uart_rx_b_x";
+						function = "uart_b";
+					};
+				};
+
+				uart_b_x_cts_rts_pins: uart_b_x_cts_rts {
+					mux {
+						groups = "uart_cts_b_x",
+							"uart_rts_b_x";
+						function = "uart_b";
+					};
+				};
+
+				uart_b_z_pins: uart_b_z {
+					mux {
+						groups = "uart_tx_b_z",
+							"uart_rx_b_z";
+						function = "uart_b";
+					};
+				};
+
+				uart_b_z_cts_rts_pins: uart_b_z_cts_rts {
+					mux {
+						groups = "uart_cts_b_z",
+							"uart_rts_b_z";
+						function = "uart_b";
+					};
+				};
+
+				uart_ao_b_z_pins: uart_ao_b_z {
+					mux {
+						groups = "uart_ao_tx_b_z",
+							"uart_ao_rx_b_z";
+						function = "uart_ao_b_gpioz";
+					};
+				};
+
+				uart_ao_b_z_cts_rts_pins: uart_ao_b_z_cts_rts {
+					mux {
+						groups = "uart_ao_cts_b_z",
+							"uart_ao_rts_b_z";
+						function = "uart_ao_b_gpioz";
+					};
+				};
 			};
 		};
 
@@ -492,12 +556,45 @@
 					gpio-ranges = <&pinctrl_aobus 0 0 15>;
 				};
 
+
 				remote_input_ao_pins: remote_input_ao {
 					mux {
 						groups = "remote_input_ao";
 						function = "remote_input_ao";
 					};
 				};
+
+				uart_ao_a_pins: uart_ao_a {
+					mux {
+						groups = "uart_ao_tx_a",
+							"uart_ao_rx_a";
+						function = "uart_ao_a";
+					};
+				};
+
+				uart_ao_a_cts_rts_pins: uart_ao_a_cts_rts {
+					mux {
+						groups = "uart_ao_cts_a",
+							"uart_ao_rts_a";
+						function = "uart_ao_a";
+					};
+				};
+
+				uart_ao_b_pins: uart_ao_b {
+					mux {
+						groups = "uart_ao_tx_b",
+							"uart_ao_rx_b";
+						function = "uart_ao_b";
+					};
+				};
+
+				uart_ao_b_cts_rts_pins: uart_ao_b_cts_rts {
+					mux {
+						groups = "uart_ao_cts_b",
+							"uart_ao_rts_b";
+						function = "uart_ao_b";
+					};
+				};
 			};
 
 			pwm_AO_ab: pwm@7000 {
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 4/5] ARM64: dts: meson-axg: complete the pinctrl info for UART_AO_A
  2018-01-06  0:10 [PATCH v2 0/5] ARM64: dts: meson-axg: UART DT updates Yixun Lan
                   ` (2 preceding siblings ...)
  2018-01-06  0:10 ` [PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description Yixun Lan
@ 2018-01-06  0:10 ` Yixun Lan
  2018-01-06  0:10 ` [PATCH v2 5/5] ARM64: dts: meson-axg: enable the UART_A controller Yixun Lan
  4 siblings, 0 replies; 13+ messages in thread
From: Yixun Lan @ 2018-01-06  0:10 UTC (permalink / raw)
  To: Kevin Hilman, devicetree
  Cc: Neil Armstrong, Jerome Brunet, Rob Herring, Mark Rutland,
	Carlo Caione, Yixun Lan, linux-amlogic, linux-arm-kernel,
	linux-kernel

Explictly request the pinctrl info for the UART_AO_A controller,
otherwise we may need to rely on bootloader for the initialization.

Acked-by: Jerome Brunet <jbrunet@baylibre.com>
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/meson-axg-s400.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
index 447b98d30921..9c1b78028ccb 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
@@ -26,6 +26,8 @@
 
 &uart_AO {
 	status = "okay";
+	pinctrl-0 = <&uart_ao_a_pins>;
+	pinctrl-names = "default";
 };
 
 &ir {
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 5/5] ARM64: dts: meson-axg: enable the UART_A controller
  2018-01-06  0:10 [PATCH v2 0/5] ARM64: dts: meson-axg: UART DT updates Yixun Lan
                   ` (3 preceding siblings ...)
  2018-01-06  0:10 ` [PATCH v2 4/5] ARM64: dts: meson-axg: complete the pinctrl info for UART_AO_A Yixun Lan
@ 2018-01-06  0:10 ` Yixun Lan
  4 siblings, 0 replies; 13+ messages in thread
From: Yixun Lan @ 2018-01-06  0:10 UTC (permalink / raw)
  To: Kevin Hilman, devicetree
  Cc: Neil Armstrong, Jerome Brunet, Rob Herring, Mark Rutland,
	Carlo Caione, Yixun Lan, linux-amlogic, linux-arm-kernel,
	linux-kernel

The UART_A is connected to a BT module on the S400 board.

Acked-by: Jerome Brunet <jbrunet@baylibre.com>
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/meson-axg-s400.dts | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
index 9c1b78028ccb..d56894dbb209 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
@@ -14,6 +14,7 @@
 
 	aliases {
 		serial0 = &uart_AO;
+		serial1 = &uart_A;
 	};
 };
 
@@ -24,6 +25,12 @@
 	pinctrl-names = "default";
 };
 
+&uart_A {
+	status = "okay";
+	pinctrl-0 = <&uart_a_pins>;
+	pinctrl-names = "default";
+};
+
 &uart_AO {
 	status = "okay";
 	pinctrl-0 = <&uart_ao_a_pins>;
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description
  2018-01-06  0:10 ` [PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description Yixun Lan
@ 2018-01-07 20:19   ` Martin Blumenstingl
  2018-01-08  6:07     ` Yixun Lan
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Blumenstingl @ 2018-01-07 20:19 UTC (permalink / raw)
  To: Yixun Lan
  Cc: Kevin Hilman, devicetree, Mark Rutland, Neil Armstrong,
	linux-kernel, Rob Herring, Carlo Caione, linux-amlogic,
	linux-arm-kernel, Jerome Brunet

Hi Yixun,

On Sat, Jan 6, 2018 at 1:10 AM, Yixun Lan <yixun.lan@amlogic.com> wrote:
> Describe the pinctrl info for the UART controller which is found
> in the Meson-AXG SoCs.
>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> ---
>  arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 97 ++++++++++++++++++++++++++++++
>  1 file changed, 97 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> index 644d0f9eaf8c..1eb45781c850 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> @@ -448,6 +448,70 @@
>                                                 function = "spi1";
>                                         };
>                                 };
> +
> +                               uart_a_pins: uart_a {
> +                                       mux {
> +                                               groups = "uart_tx_a",
> +                                                       "uart_rx_a";
> +                                               function = "uart_a";
> +                                       };
> +                               };
> +
> +                               uart_a_cts_rts_pins: uart_a_cts_rts {
> +                                       mux {
> +                                               groups = "uart_cts_a",
> +                                                       "uart_rts_a";
> +                                               function = "uart_a";
> +                                       };
> +                               };
> +
> +                               uart_b_x_pins: uart_b_x {
> +                                       mux {
> +                                               groups = "uart_tx_b_x",
> +                                                       "uart_rx_b_x";
> +                                               function = "uart_b";
> +                                       };
> +                               };
> +
> +                               uart_b_x_cts_rts_pins: uart_b_x_cts_rts {
> +                                       mux {
> +                                               groups = "uart_cts_b_x",
> +                                                       "uart_rts_b_x";
> +                                               function = "uart_b";
> +                                       };
> +                               };
> +
> +                               uart_b_z_pins: uart_b_z {
> +                                       mux {
> +                                               groups = "uart_tx_b_z",
> +                                                       "uart_rx_b_z";
> +                                               function = "uart_b";
> +                                       };
> +                               };
> +
> +                               uart_b_z_cts_rts_pins: uart_b_z_cts_rts {
> +                                       mux {
> +                                               groups = "uart_cts_b_z",
> +                                                       "uart_rts_b_z";
> +                                               function = "uart_b";
> +                                       };
> +                               };
> +
> +                               uart_ao_b_z_pins: uart_ao_b_z {
> +                                       mux {
> +                                               groups = "uart_ao_tx_b_z",
> +                                                       "uart_ao_rx_b_z";
> +                                               function = "uart_ao_b_gpioz";
(the following question just came up while I was looking at this
patch, but I guess it's more a question towards the pinctrl driver)
the name of the function looks a bit "weird" since below you are also
using "uart_ao_b"
did you choose "uart_ao_b_gpioz" here because we cannot have the same
function name for the periphs and AO pinctrl or is there some other
reason?

I am asking because I would have expected it to look like this:
    groups = "uart_ao_tx_b_z", "uart_ao_rx_b_z";
    function = "uart_ao_b";

(same for the cts/rts pins below)

> +                                       };
> +                               };
> +
> +                               uart_ao_b_z_cts_rts_pins: uart_ao_b_z_cts_rts {
> +                                       mux {
> +                                               groups = "uart_ao_cts_b_z",
> +                                                       "uart_ao_rts_b_z";
> +                                               function = "uart_ao_b_gpioz";
> +                                       };
> +                               };
>                         };
>                 };
>
> @@ -492,12 +556,45 @@
>                                         gpio-ranges = <&pinctrl_aobus 0 0 15>;
>                                 };
>
> +
did you add this additional newline on purpose?
>                                 remote_input_ao_pins: remote_input_ao {
>                                         mux {
>                                                 groups = "remote_input_ao";
>                                                 function = "remote_input_ao";
>                                         };
>                                 };
> +
> +                               uart_ao_a_pins: uart_ao_a {
> +                                       mux {
> +                                               groups = "uart_ao_tx_a",
> +                                                       "uart_ao_rx_a";
> +                                               function = "uart_ao_a";
> +                                       };
> +                               };
> +
> +                               uart_ao_a_cts_rts_pins: uart_ao_a_cts_rts {
> +                                       mux {
> +                                               groups = "uart_ao_cts_a",
> +                                                       "uart_ao_rts_a";
> +                                               function = "uart_ao_a";
> +                                       };
> +                               };
> +
> +                               uart_ao_b_pins: uart_ao_b {
> +                                       mux {
> +                                               groups = "uart_ao_tx_b",
> +                                                       "uart_ao_rx_b";
> +                                               function = "uart_ao_b";
> +                                       };
> +                               };
> +
> +                               uart_ao_b_cts_rts_pins: uart_ao_b_cts_rts {
> +                                       mux {
> +                                               groups = "uart_ao_cts_b",
> +                                                       "uart_ao_rts_b";
> +                                               function = "uart_ao_b";
> +                                       };
> +                               };
>                         };
>
>                         pwm_AO_ab: pwm@7000 {
> --
> 2.15.1
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

Regards
Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description
  2018-01-07 20:19   ` Martin Blumenstingl
@ 2018-01-08  6:07     ` Yixun Lan
  2018-01-08  6:44       ` Yixun Lan
  2018-01-08  8:42       ` Jerome Brunet
  0 siblings, 2 replies; 13+ messages in thread
From: Yixun Lan @ 2018-01-08  6:07 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: yixun.lan, Kevin Hilman, devicetree, Mark Rutland,
	Neil Armstrong, linux-kernel, Rob Herring, Carlo Caione,
	linux-amlogic, linux-arm-kernel, Jerome Brunet, Xingyun Chen

Hi Martin

On 01/08/18 04:19, Martin Blumenstingl wrote:
> Hi Yixun,
> 
> On Sat, Jan 6, 2018 at 1:10 AM, Yixun Lan <yixun.lan@amlogic.com> wrote:
>> Describe the pinctrl info for the UART controller which is found
>> in the Meson-AXG SoCs.
>>
>> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
>> ---
>>  arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 97 ++++++++++++++++++++++++++++++
>>  1 file changed, 97 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>> index 644d0f9eaf8c..1eb45781c850 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>> @@ -448,6 +448,70 @@
>>                                                 function = "spi1";
>>                                         };
>>                                 };
>> +
>> +                               uart_a_pins: uart_a {
>> +                                       mux {
>> +                                               groups = "uart_tx_a",
>> +                                                       "uart_rx_a";
>> +                                               function = "uart_a";
>> +                                       };
>> +                               };
>> +
>> +                               uart_a_cts_rts_pins: uart_a_cts_rts {
>> +                                       mux {
>> +                                               groups = "uart_cts_a",
>> +                                                       "uart_rts_a";
>> +                                               function = "uart_a";
>> +                                       };
>> +                               };
>> +
>> +                               uart_b_x_pins: uart_b_x {
>> +                                       mux {
>> +                                               groups = "uart_tx_b_x",
>> +                                                       "uart_rx_b_x";
>> +                                               function = "uart_b";
>> +                                       };
>> +                               };
>> +
>> +                               uart_b_x_cts_rts_pins: uart_b_x_cts_rts {
>> +                                       mux {
>> +                                               groups = "uart_cts_b_x",
>> +                                                       "uart_rts_b_x";
>> +                                               function = "uart_b";
>> +                                       };
>> +                               };
>> +
>> +                               uart_b_z_pins: uart_b_z {
>> +                                       mux {
>> +                                               groups = "uart_tx_b_z",
>> +                                                       "uart_rx_b_z";
>> +                                               function = "uart_b";
>> +                                       };
>> +                               };
>> +
>> +                               uart_b_z_cts_rts_pins: uart_b_z_cts_rts {
>> +                                       mux {
>> +                                               groups = "uart_cts_b_z",
>> +                                                       "uart_rts_b_z";
>> +                                               function = "uart_b";
>> +                                       };
>> +                               };
>> +
>> +                               uart_ao_b_z_pins: uart_ao_b_z {
>> +                                       mux {
>> +                                               groups = "uart_ao_tx_b_z",
>> +                                                       "uart_ao_rx_b_z";
>> +                                               function = "uart_ao_b_gpioz";
> (the following question just came up while I was looking at this
> patch, but I guess it's more a question towards the pinctrl driver)
> the name of the function looks a bit "weird" since below you are also
> using "uart_ao_b"
you right here, it's a question related to pinctrl subsystem.
from my point of view, it's even weird from the hardware perspective:
 that, the UART function of AO domain route the pin of EE domain..

> did you choose "uart_ao_b_gpioz" here because we cannot have the same
> function name for the periphs and AO pinctrl or is there some other
> reason?
> 
Current there is a conflict in the code level which both two pinctrl
tree (EE, AO) are using the same macro[1] to expand the definitions, so
there would be conflict symbol if we name both as 'uart_ao_b'

I think your idea of having an uniform function 'uart_ao_b' for both
pinctrl subsystem is actually possible/positive..

I will think about your suggestion and come up with a patch later,
thanks a lot!


[1] drivers/pinctrl/meson/pinctrl-meson.h

#define FUNCTION(fn)                                                    \
        {                                                               \
                .name = #fn,                                            \
                .groups = fn ## _groups,                                \
                .num_groups = ARRAY_SIZE(fn ## _groups),                \
        }




> I am asking because I would have expected it to look like this:
>     groups = "uart_ao_tx_b_z", "uart_ao_rx_b_z";
>     function = "uart_ao_b";
> 
> (same for the cts/rts pins below)
> 
>> +                                       };
>> +                               };
>> +
>> +                               uart_ao_b_z_cts_rts_pins: uart_ao_b_z_cts_rts {
>> +                                       mux {
>> +                                               groups = "uart_ao_cts_b_z",
>> +                                                       "uart_ao_rts_b_z";
>> +                                               function = "uart_ao_b_gpioz";
>> +                                       };
>> +                               };
>>                         };
>>                 };
>>
>> @@ -492,12 +556,45 @@
>>                                         gpio-ranges = <&pinctrl_aobus 0 0 15>;
>>                                 };
>>
>> +
> did you add this additional newline on purpose?
>>                                 remote_input_ao_pins: remote_input_ao {
>>                                         mux {
>>                                                 groups = "remote_input_ao";
>>                                                 function = "remote_input_ao";
>>                                         };
>>                                 };
>> +
>> +                               uart_ao_a_pins: uart_ao_a {
>> +                                       mux {
>> +                                               groups = "uart_ao_tx_a",
>> +                                                       "uart_ao_rx_a";
>> +                                               function = "uart_ao_a";
>> +                                       };
>> +                               };
>> +
>> +                               uart_ao_a_cts_rts_pins: uart_ao_a_cts_rts {
>> +                                       mux {
>> +                                               groups = "uart_ao_cts_a",
>> +                                                       "uart_ao_rts_a";
>> +                                               function = "uart_ao_a";
>> +                                       };
>> +                               };
>> +
>> +                               uart_ao_b_pins: uart_ao_b {
>> +                                       mux {
>> +                                               groups = "uart_ao_tx_b",
>> +                                                       "uart_ao_rx_b";
>> +                                               function = "uart_ao_b";
>> +                                       };
>> +                               };
>> +
>> +                               uart_ao_b_cts_rts_pins: uart_ao_b_cts_rts {
>> +                                       mux {
>> +                                               groups = "uart_ao_cts_b",
>> +                                                       "uart_ao_rts_b";
>> +                                               function = "uart_ao_b";
>> +                                       };
>> +                               };
>>                         };
>>
>>                         pwm_AO_ab: pwm@7000 {
>> --
>> 2.15.1
>>
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> 
> Regards
> Martin
> 
> .
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description
  2018-01-08  6:07     ` Yixun Lan
@ 2018-01-08  6:44       ` Yixun Lan
  2018-01-08  8:42       ` Jerome Brunet
  1 sibling, 0 replies; 13+ messages in thread
From: Yixun Lan @ 2018-01-08  6:44 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: yixun.lan, Kevin Hilman, devicetree, Mark Rutland,
	Neil Armstrong, linux-kernel, Rob Herring, Carlo Caione,
	linux-amlogic, linux-arm-kernel, Jerome Brunet, Xingyun Chen

HI Martin:

On 01/08/18 14:07, Yixun Lan wrote:
> Hi Martin
> 
> On 01/08/18 04:19, Martin Blumenstingl wrote:
>> Hi Yixun,
>>
>> On Sat, Jan 6, 2018 at 1:10 AM, Yixun Lan <yixun.lan@amlogic.com> wrote:
>>> Describe the pinctrl info for the UART controller which is found
>>> in the Meson-AXG SoCs.
>>>
>>> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
>>> ---
>>>  arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 97 ++++++++++++++++++++++++++++++
>>>  1 file changed, 97 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>>> index 644d0f9eaf8c..1eb45781c850 100644
>>> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>>> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>>> @@ -448,6 +448,70 @@
>>>                                                 function = "spi1";
>>>                                         };

.

>>>
>>> +
>> did you add this additional newline on purpose?
oops, this is added by mistake..
thanks for catching this, I will fix it

>>>                                 remote_input_ao_pins: remote_input_ao {
>>>                                         mux {
>>>                                                 groups = "remote_input_ao";
>>>                                                 function = "remote_input_ao";
>>>                                         };
>>>                                 };
>>> +
>>> +                               uart_ao_a_pins: uart_ao_a {
>>> +                                       mux {
>>> +                                               groups = "uart_ao_tx_a",
>>> +                                                       "uart_ao_rx_a";
>>> +                                               function = "uart_ao_a";
>>> +                                       };
>>> +                               };
>>> +
>>> +                               uart_ao_a_cts_rts_pins: uart_ao_a_cts_rts {
>>> +                                       mux {
>>> +                                               groups = "uart_ao_cts_a",
>>> +                                                       "uart_ao_rts_a";
>>> +                                               function = "uart_ao_a";
>>> +                                       };
>>> +                               };
>>> +
>>> +                               uart_ao_b_pins: uart_ao_b {
>>> +                                       mux {
>>> +                                               groups = "uart_ao_tx_b",
>>> +                                                       "uart_ao_rx_b";
>>> +                                               function = "uart_ao_b";
>>> +                                       };
>>> +                               };
>>> +
>>> +                               uart_ao_b_cts_rts_pins: uart_ao_b_cts_rts {
>>> +                                       mux {
>>> +                                               groups = "uart_ao_cts_b",
>>> +                                                       "uart_ao_rts_b";
>>> +                                               function = "uart_ao_b";
>>> +                                       };
>>> +                               };
>>>                         };
>>>
>>>                         pwm_AO_ab: pwm@7000 {
>>> --
>>> 2.15.1
>>>
>>>
>>> _______________________________________________
>>> linux-amlogic mailing list
>>> linux-amlogic@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>>
>> Regards
>> Martin
>>
>> .
>>
> .
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description
  2018-01-08  6:07     ` Yixun Lan
  2018-01-08  6:44       ` Yixun Lan
@ 2018-01-08  8:42       ` Jerome Brunet
  1 sibling, 0 replies; 13+ messages in thread
From: Jerome Brunet @ 2018-01-08  8:42 UTC (permalink / raw)
  To: Yixun Lan, Martin Blumenstingl
  Cc: Kevin Hilman, devicetree, Mark Rutland, Neil Armstrong,
	linux-kernel, Rob Herring, Carlo Caione, linux-amlogic,
	linux-arm-kernel, Xingyun Chen

On Mon, 2018-01-08 at 14:07 +0800, Yixun Lan wrote:
> > (the following question just came up while I was looking at this
> > patch, but I guess it's more a question towards the pinctrl driver)
> > the name of the function looks a bit "weird" since below you are also
> > using "uart_ao_b"
> 
> you right here, it's a question related to pinctrl subsystem.
> from my point of view, it's even weird from the hardware perspective:
>  that, the UART function of AO domain route the pin of EE domain..
> 
> > did you choose "uart_ao_b_gpioz" here because we cannot have the same
> > function name for the periphs and AO pinctrl or is there some other
> > reason?
> > 
> 
> Current there is a conflict in the code level which both two pinctrl
> tree (EE, AO) are using the same macro[1] to expand the definitions, so
> there would be conflict symbol if we name both as 'uart_ao_b'
> 
> I think your idea of having an uniform function 'uart_ao_b' for both
> pinctrl subsystem is actually possible/positive..
> 
> I will think about your suggestion and come up with a patch later,
> thanks a lot!
> 
> 
> [1] drivers/pinctrl/meson/pinctrl-meson.h
> 
> #define FUNCTION(fn)                                                    \
>         {                                                               \
>                 .name = #fn,                                            \
>                 .groups = fn ## _groups,                                \
>                 .num_groups = ARRAY_SIZE(fn ## _groups),                \
>         }

The name feels weird because it should have been uart_ao_b_z ... We missed it in
the initial review. Except for correcting the function name, I don't think this
justify a change a pinctrl

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/5] ARM64: dts: meson: uart: fix address space range
  2018-01-06  0:10 ` [PATCH v2 1/5] ARM64: dts: meson: uart: fix address space range Yixun Lan
@ 2018-01-08  8:54   ` Jerome Brunet
  2018-01-17  0:01     ` Kevin Hilman
  0 siblings, 1 reply; 13+ messages in thread
From: Jerome Brunet @ 2018-01-08  8:54 UTC (permalink / raw)
  To: Yixun Lan, Kevin Hilman, devicetree
  Cc: Neil Armstrong, Rob Herring, Mark Rutland, Carlo Caione,
	linux-amlogic, linux-arm-kernel, linux-kernel

On Sat, 2018-01-06 at 08:10 +0800, Yixun Lan wrote:
> The address space range is actually 0x18, fixed here.
> 
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>

Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>

> ---
>  arch/arm64/boot/dts/amlogic/meson-axg.dtsi |  4 ++--
>  arch/arm64/boot/dts/amlogic/meson-gx.dtsi  | 10 +++++-----
>  2 files changed, 7 insertions(+), 7 deletions(-)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] ARM64: dts: meson-axg: uart: drop legacy compatible name from EE UART
  2018-01-06  0:10 ` [PATCH v2 2/5] ARM64: dts: meson-axg: uart: drop legacy compatible name from EE UART Yixun Lan
@ 2018-01-08  8:56   ` Jerome Brunet
  0 siblings, 0 replies; 13+ messages in thread
From: Jerome Brunet @ 2018-01-08  8:56 UTC (permalink / raw)
  To: Yixun Lan, Kevin Hilman, devicetree
  Cc: Neil Armstrong, Rob Herring, Mark Rutland, Carlo Caione,
	linux-amlogic, linux-arm-kernel, linux-kernel

On Sat, 2018-01-06 at 08:10 +0800, Yixun Lan wrote:
> When update the clock info for the UART controller in the EE domain,
> the driver explicitly require 'pclk' in order to work properly.
> 
> With current logic of the code, the driver will go for the legacy clock probe
> routine[1] if it find current compatible string match to 'amlogic,meson-uart',
> which result in not requesting the 'pclk' clock, thus break the driver in the end.
> 
> [1] drivers/tty/serial/meson_uart.c:685
> 
>         /* Use legacy way until all platforms switch to new bindings */
>         if (of_device_is_compatible(pdev->dev.of_node, "amlogic,meson-uart"))
>                 ret = meson_uart_probe_clocks_legacy(pdev, port);
>         else
>                 ret = meson_uart_probe_clocks(pdev, port);

I don't think you should add this code snip here. Anybody can look at the driver
code to see that

> 
> Acked-by: Jerome Brunet <jbrunet@baylibre.com>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/5] ARM64: dts: meson: uart: fix address space range
  2018-01-08  8:54   ` Jerome Brunet
@ 2018-01-17  0:01     ` Kevin Hilman
  0 siblings, 0 replies; 13+ messages in thread
From: Kevin Hilman @ 2018-01-17  0:01 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Yixun Lan, devicetree, Neil Armstrong, Rob Herring, Mark Rutland,
	Carlo Caione, linux-amlogic, linux-arm-kernel, linux-kernel

Jerome Brunet <jbrunet@baylibre.com> writes:

> On Sat, 2018-01-06 at 08:10 +0800, Yixun Lan wrote:
>> The address space range is actually 0x18, fixed here.
>> 
>> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
>
> Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>

Applied to v4.16/fixes. Will submit after v4.16-rc1 is out.

Kevin

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-01-17  0:01 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-06  0:10 [PATCH v2 0/5] ARM64: dts: meson-axg: UART DT updates Yixun Lan
2018-01-06  0:10 ` [PATCH v2 1/5] ARM64: dts: meson: uart: fix address space range Yixun Lan
2018-01-08  8:54   ` Jerome Brunet
2018-01-17  0:01     ` Kevin Hilman
2018-01-06  0:10 ` [PATCH v2 2/5] ARM64: dts: meson-axg: uart: drop legacy compatible name from EE UART Yixun Lan
2018-01-08  8:56   ` Jerome Brunet
2018-01-06  0:10 ` [PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description Yixun Lan
2018-01-07 20:19   ` Martin Blumenstingl
2018-01-08  6:07     ` Yixun Lan
2018-01-08  6:44       ` Yixun Lan
2018-01-08  8:42       ` Jerome Brunet
2018-01-06  0:10 ` [PATCH v2 4/5] ARM64: dts: meson-axg: complete the pinctrl info for UART_AO_A Yixun Lan
2018-01-06  0:10 ` [PATCH v2 5/5] ARM64: dts: meson-axg: enable the UART_A controller Yixun Lan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).