[PATCH v2 0/5] ARM64: dts: meson-axg: UART DT updates

[PATCH v2 0/5] ARM64: dts: meson-axg: UART DT updates

[Yixun Lan]

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 at amlogic.com

[2]
 http://lkml.kernel.org/r/20180105095621.196472-1-yixun.lan at 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

[PATCH v2 1/5] ARM64: dts: meson: uart: fix address space range

[Yixun Lan]

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 at 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 at 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 at 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 at 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 at 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 at 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 at 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

[PATCH v2 1/5] ARM64: dts: meson: uart: fix address space range

[Jerome Brunet]

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(-)

[PATCH v2 2/5] ARM64: dts: meson-axg: uart: drop legacy compatible name from EE UART

[Yixun Lan]

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 at 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 at 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

[PATCH v2 2/5] ARM64: dts: meson-axg: uart: drop legacy compatible name from EE UART

[Jerome Brunet]

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>

[PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description

[Yixun Lan]

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 at 7000 {
-- 
2.15.1

[PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description

[Martin Blumenstingl]

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 at 7000 {
> --
> 2.15.1
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

Regards
Martin

[PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description

[Yixun Lan]

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 at 7000 {
>> --
>> 2.15.1
>>
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> 
> Regards
> Martin
> 
> .
> 

[PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description

[Yixun Lan]

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 at 7000 {
>>> --
>>> 2.15.1
>>>
>>>
>>> _______________________________________________
>>> linux-amlogic mailing list
>>> linux-amlogic at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>>
>> Regards
>> Martin
>>
>> .
>>
> .
> 

[PATCH v2 3/5] ARM64: dts: meson-axg: uart: Add the pinctrl info description

[Jerome Brunet]

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

[PATCH v2 4/5] ARM64: dts: meson-axg: complete the pinctrl info for UART_AO_A

[Yixun Lan]

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

[PATCH v2 5/5] ARM64: dts: meson-axg: enable the UART_A controller

[Yixun Lan]

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