Re: [PATCH v4 4/4] ARM64: dts: meson: drop "sana" clock from SAR ADC

[Yixun Lan]

Hi Kevin & others

  I'd like to just re-send the patch [4/4] (while leave others[1-3/4]
unchanged), to have separated DT patch the for 32bit / 64bit platform.
  is this ok for you?


On 11/12/17 09:33, Martin Blumenstingl wrote:
> Hi Yixun,
> 
> On Tue, Nov 7, 2017 at 3:10 PM, Yixun Lan  wrote:
>> From: Xingyu Chen 
>>
>> The SAR ADC modules doesn't require The "sana" clock.
>>
>> Singed-off-by: Xingyu Chen 
>> Signed-off-by: Yixun Lan 
>> ---
>>  arch/arm/boot/dts/meson8.dtsi   | 5 ++---
>>  arch/arm/boot/dts/meson8b.dtsi  | 5 ++---
> these two should go into a separate patch (with "ARM: dts: ..."
> prefix) - the ARM maintainers want separate pull requests for the
> 32-bit and 64-bit .dts changes, so patches should also follow that
> schema
> 
> with that fixed, you can add my ACK on both (32-bit and 64-bit) .dts patches:
> Acked-by: Martin Blumenstingl
> 

thanks, I will send separate patch for this, and I will add your 'Acked-by'

>>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 3 +--
>>  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 3 +--
>>  4 files changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
>> index b98d44fde6b6..f93d6cf6e094 100644
>> --- a/arch/arm/boot/dts/meson8.dtsi
>> +++ b/arch/arm/boot/dts/meson8.dtsi
>> @@ -289,9 +289,8 @@
>>  &saradc {
>> compatible = "amlogic,meson8-saradc", "amlogic,meson-saradc";
>> clocks = <&clkc CLKID_XTAL>,
>> -   <&clkc CLKID_SAR_ADC>,
>> -   <&clkc CLKID_SANA>;
>> -   clock-names = "clkin", "core", "sana";
>> +   <&clkc CLKID_SAR_ADC>;
>> +   clock-names = "clkin", "core";
>>  };
>>
>>  &spifc {
>> diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
>> index bc278da7df0d..4aa444284f0c 100644
>> --- a/arch/arm/boot/dts/meson8b.dtsi
>> +++ b/arch/arm/boot/dts/meson8b.dtsi
>> @@ -185,9 +185,8 @@
>>  &saradc {
>> compatible = "amlogic,meson8b-saradc", "amlogic,meson-saradc";
>> clocks = <&clkc CLKID_XTAL>,
>> -   <&clkc CLKID_SAR_ADC>,
>> -   <&clkc CLKID_SANA>;
>> -   clock-names = "clkin", "core", "sana";
>> +   <&clkc CLKID_SAR_ADC>;
>> +   clock-names = "clkin", "core";
>>  };
>>
>>  &uart_AO {
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi 
>> b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> index af834cdbba79..b77f2593cdc3 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> @@ -686,10 +686,9 @@
>> compatible = "amlogic,meson-gxbb-saradc", "amlogic,meson-saradc";
>> clocks = <&xtal>,
>>  <&clkc CLKID_SAR_ADC>,
>> -<&clkc CLKID_SANA>,
>>  <&clkc CLKID_SAR_ADC_CLK>,
>>  <&clkc CLKID_SAR_ADC_SEL>;
>> -   clock-names = "clkin", "core", "sana", "adc_clk", "adc_sel";
>> +   clock-names = "clkin", "core", "adc_clk", "adc_sel";
>>  };
>>
>>  &sd_emmc_a {
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi 
>> b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>> index d8dd3298b15c..07805a3b4db0 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>> @@ -628,10 +628,9 @@
>> compatible = "amlogic,meson-gxl-saradc", "amlogic,meson-saradc";
>> clocks = <&xtal>,
>>  <&clkc CLKID_SAR_ADC>,
>> -<&clkc CLKID_SANA>,
>>  <&clkc CLKID_SAR_ADC_CLK>,
>>  <&clkc CLKID_SAR_ADC_SEL>;
>> -   clock-names = "clkin", "core", "sana", "adc_clk", "adc_sel";
>> +   clock-names = "clkin", "core", "adc_clk", "adc_sel";
>>  };
>>
>>  &sd_emmc_a {
>> --
>> 2.14.1
>>
> 
> .
> 

[PATCH] ARM64: dts: meson-axg: add ethernet mac controller

[Yixun Lan]

Add DT info for the stmmac ethernet MAC which found in
the Amlogic's Meson-AXG SoC, also describe the ethernet
pinctrl & clock information here.

This is tested in the S400 dev board which use a RTL8211F PHY,
and the pins connect to the 'eth_rgmii_y_pins' group.

Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg-s400.dts |  7 
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 53 ++
 2 files changed, 60 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts 
b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
index 9eb6aaee155d..7b39a9fe2b0f 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
@@ -21,3 +21,10 @@
status = "okay";
 };
 
+ðmac {
+   status = "okay";
+   phy-mode = "rgmii";
+
+   pinctrl-0 = <ð_rgmii_y_pins>;
+   pinctrl-names = "default";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index 65945c6c8b65..57faaa9d8013 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -157,6 +157,19 @@
#address-cells = <0>;
};
 
+   ethmac: ethernet@ff3f {
+   compatible = "amlogic,meson-axg-dwmac", 
"amlogic,meson-gxbb-dwmac", "snps,dwmac";
+   reg = <0x0 0xff3f 0x0 0x1
+   0x0 0xff634540 0x0 0x8>;
+   interrupts = ;
+   interrupt-names = "macirq";
+   status = "disabled";
+   clocks = <&clkc CLKID_ETH>,
+<&clkc CLKID_FCLK_DIV2>,
+<&clkc CLKID_MPLL2>;
+   clock-names = "stmmaceth", "clkin0", "clkin1";
+   };
+
hiubus: hiubus@ff63c000 {
compatible = "simple-bus";
reg = <0x0 0xff63c000 0x0 0x1c00>;
@@ -203,6 +216,46 @@
#gpio-cells = <2>;
gpio-ranges = <&pinctrl_periphs 0 0 86>;
};
+
+   eth_rgmii_x_pins: eth-x-rgmii {
+   mux {
+   groups = "eth_mdio_x",
+  "eth_mdc_x",
+  "eth_rgmii_rx_clk_x",
+  "eth_rx_dv_x",
+  "eth_rxd0_x",
+  "eth_rxd1_x",
+  "eth_rxd2_rgmii",
+  "eth_rxd3_rgmii",
+  "eth_rgmii_tx_clk",
+  "eth_txen_x",
+  "eth_txd0_x",
+  "eth_txd1_x",
+  "eth_txd2_rgmii",
+  "eth_txd3_rgmii";
+   function = "eth";
+   };
+   };
+
+   eth_rgmii_y_pins: eth-y-rgmii {
+   mux {
+   groups = "eth_mdio_y",
+  "eth_mdc_y",
+  "eth_rgmii_rx_clk_y",
+  "eth_rx_dv_y",
+  "eth_rxd0_y",
+  "eth_rxd1_y",
+  "eth_rxd2_rgmii",
+  "eth_rxd3_rgmii",
+  "eth_rgmii_tx_clk",
+  "eth_txen_y",
+  "eth_txd0_y",
+  "eth_txd1_y",
+  "eth_txd2_rgmii",
+  "eth_txd3_rgmii";
+   function = "eth";
+   };
+   };
};
};
 
-- 
2.14.1

[PATCH] clk: meson: gxbb: fix wrong clock for SARADC

[Yixun Lan]

According to the datasheet, the clock gate bit for
SARADC is bit[22] in Meson-GXBB/GXL series.

Change-Id: Ic4fa58276d2a9ea273eef0a08541fc213ac5ac89
Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/gxbb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
index b2d1e8ed7152..4f5b535fcd12 100644
--- a/drivers/clk/meson/gxbb.c
+++ b/drivers/clk/meson/gxbb.c
@@ -1139,7 +1139,6 @@ static MESON_GATE(gxbb_pl301, HHI_GCLK_MPEG0, 6);
 static MESON_GATE(gxbb_periphs, HHI_GCLK_MPEG0, 7);
 static MESON_GATE(gxbb_spicc, HHI_GCLK_MPEG0, 8);
 static MESON_GATE(gxbb_i2c, HHI_GCLK_MPEG0, 9);
-static MESON_GATE(gxbb_sar_adc, HHI_GCLK_MPEG0, 10);
 static MESON_GATE(gxbb_smart_card, HHI_GCLK_MPEG0, 11);
 static MESON_GATE(gxbb_rng0, HHI_GCLK_MPEG0, 12);
 static MESON_GATE(gxbb_uart0, HHI_GCLK_MPEG0, 13);
@@ -1149,6 +1148,7 @@ static MESON_GATE(gxbb_async_fifo, HHI_GCLK_MPEG0, 16);
 static MESON_GATE(gxbb_sdio, HHI_GCLK_MPEG0, 17);
 static MESON_GATE(gxbb_abuf, HHI_GCLK_MPEG0, 18);
 static MESON_GATE(gxbb_hiu_iface, HHI_GCLK_MPEG0, 19);
+static MESON_GATE(gxbb_sar_adc, HHI_GCLK_MPEG0, 22);
 static MESON_GATE(gxbb_assist_misc, HHI_GCLK_MPEG0, 23);
 static MESON_GATE(gxbb_emmc_a, HHI_GCLK_MPEG0, 24);
 static MESON_GATE(gxbb_emmc_b, HHI_GCLK_MPEG0, 25);
-- 
2.14.1

Re: [PATCH] clk: meson: gxbb: fix wrong clock for SARADC

[Yixun Lan]

On 11/04/17 02:17, Yixun Lan wrote:
> According to the datasheet, the clock gate bit for
> SARADC is bit[22] in Meson-GXBB/GXL series.
> 
> Change-Id: Ic4fa58276d2a9ea273eef0a08541fc213ac5ac89
> Signed-off-by: Yixun Lan 
> ---
>  drivers/clk/meson/gxbb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
> index b2d1e8ed7152..4f5b535fcd12 100644
> --- a/drivers/clk/meson/gxbb.c
> +++ b/drivers/clk/meson/gxbb.c
> @@ -1139,7 +1139,6 @@ static MESON_GATE(gxbb_pl301, HHI_GCLK_MPEG0, 6);
>  static MESON_GATE(gxbb_periphs, HHI_GCLK_MPEG0, 7);
>  static MESON_GATE(gxbb_spicc, HHI_GCLK_MPEG0, 8);
>  static MESON_GATE(gxbb_i2c, HHI_GCLK_MPEG0, 9);
> -static MESON_GATE(gxbb_sar_adc, HHI_GCLK_MPEG0, 10);
>  static MESON_GATE(gxbb_smart_card, HHI_GCLK_MPEG0, 11);
>  static MESON_GATE(gxbb_rng0, HHI_GCLK_MPEG0, 12);
>  static MESON_GATE(gxbb_uart0, HHI_GCLK_MPEG0, 13);
> @@ -1149,6 +1148,7 @@ static MESON_GATE(gxbb_async_fifo, HHI_GCLK_MPEG0, 16);
>  static MESON_GATE(gxbb_sdio, HHI_GCLK_MPEG0, 17);
>  static MESON_GATE(gxbb_abuf, HHI_GCLK_MPEG0, 18);
>  static MESON_GATE(gxbb_hiu_iface, HHI_GCLK_MPEG0, 19);
> +static MESON_GATE(gxbb_sar_adc, HHI_GCLK_MPEG0, 22);
>  static MESON_GATE(gxbb_assist_misc, HHI_GCLK_MPEG0, 23);
>  static MESON_GATE(gxbb_emmc_a, HHI_GCLK_MPEG0, 24);
>  static MESON_GATE(gxbb_emmc_b, HHI_GCLK_MPEG0, 25);
> 
Hi Neil
 I assume you will take this via the clk-meson tree..
could you amend the commit msg and drop the 'Change-Id'?
(for other parts, please feel free to adjust if you see fit)
 thanks

Yixun

Re: [PATCH] clk: meson: gxbb: fix wrong clock for SARADC

[Yixun Lan]

Hi Neil

On 11/05/17 00:40, Neil Armstrong wrote:
> Hi Yixun,
> 
> Le 04/11/2017 09:41, Yixun Lan a écrit :
>>
>>
>> On 11/04/17 02:17, Yixun Lan wrote:
>>> According to the datasheet, the clock gate bit for
>>> SARADC is bit[22] in Meson-GXBB/GXL series.
>>>
>>> Change-Id: Ic4fa58276d2a9ea273eef0a08541fc213ac5ac89
>>> Signed-off-by: Yixun Lan 
>>> ---
>>>  drivers/clk/meson/gxbb.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
>>> index b2d1e8ed7152..4f5b535fcd12 100644
>>> --- a/drivers/clk/meson/gxbb.c
>>> +++ b/drivers/clk/meson/gxbb.c
>>> @@ -1139,7 +1139,6 @@ static MESON_GATE(gxbb_pl301, HHI_GCLK_MPEG0, 6);
>>>  static MESON_GATE(gxbb_periphs, HHI_GCLK_MPEG0, 7);
>>>  static MESON_GATE(gxbb_spicc, HHI_GCLK_MPEG0, 8);
>>>  static MESON_GATE(gxbb_i2c, HHI_GCLK_MPEG0, 9);
>>> -static MESON_GATE(gxbb_sar_adc, HHI_GCLK_MPEG0, 10);
>>>  static MESON_GATE(gxbb_smart_card, HHI_GCLK_MPEG0, 11);
>>>  static MESON_GATE(gxbb_rng0, HHI_GCLK_MPEG0, 12);
>>>  static MESON_GATE(gxbb_uart0, HHI_GCLK_MPEG0, 13);
>>> @@ -1149,6 +1148,7 @@ static MESON_GATE(gxbb_async_fifo, HHI_GCLK_MPEG0, 
>>> 16);
>>>  static MESON_GATE(gxbb_sdio, HHI_GCLK_MPEG0, 17);
>>>  static MESON_GATE(gxbb_abuf, HHI_GCLK_MPEG0, 18);
>>>  static MESON_GATE(gxbb_hiu_iface, HHI_GCLK_MPEG0, 19);
>>> +static MESON_GATE(gxbb_sar_adc, HHI_GCLK_MPEG0, 22);
>>>  static MESON_GATE(gxbb_assist_misc, HHI_GCLK_MPEG0, 23);
>>>  static MESON_GATE(gxbb_emmc_a, HHI_GCLK_MPEG0, 24);
>>>  static MESON_GATE(gxbb_emmc_b, HHI_GCLK_MPEG0, 25);
>>>
>> Hi Neil
>>  I assume you will take this via the clk-meson tree..
>> could you amend the commit msg and drop the 'Change-Id'?
>> (for other parts, please feel free to adjust if you see fit)
>>  thanks
> 
> Yes, I will push it as a fix.
> 
> Can you check if the saradc driver is still functional when applied ?
>
sorry, this isn't a correct fix.
please drop this one, and I will push another patch

> Neil
> 
>>
>> Yixun
>>
> 
> .
> 

[PATCH] clk: meson: gxbb: fix wrong clock for SARADC/SANA

[Yixun Lan]

According to the datasheet, in Meson-GXBB/GXL series,
The clock gate bit for SARADC is HHI_GCLK_MPEG2 bit[22],
while clock gate bit for SANA is HHI_GCLK_MPEG0 bit[10].

Test passed at gxl_skt dev board.

Tested-by: Xingyu Chen 
Signed-off-by: Yixun Lan 

---
I think this error was introduced by a copy & paste from meson8 code?
and we didn't notice them due to the SANA clock is also enabled by
DTS (so SAR_ADC works fine)?
---
 drivers/clk/meson/gxbb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
index b2d1e8ed7152..92168348ffa6 100644
--- a/drivers/clk/meson/gxbb.c
+++ b/drivers/clk/meson/gxbb.c
@@ -1139,7 +1139,7 @@ static MESON_GATE(gxbb_pl301, HHI_GCLK_MPEG0, 6);
 static MESON_GATE(gxbb_periphs, HHI_GCLK_MPEG0, 7);
 static MESON_GATE(gxbb_spicc, HHI_GCLK_MPEG0, 8);
 static MESON_GATE(gxbb_i2c, HHI_GCLK_MPEG0, 9);
-static MESON_GATE(gxbb_sar_adc, HHI_GCLK_MPEG0, 10);
+static MESON_GATE(gxbb_sana, HHI_GCLK_MPEG0, 10);
 static MESON_GATE(gxbb_smart_card, HHI_GCLK_MPEG0, 11);
 static MESON_GATE(gxbb_rng0, HHI_GCLK_MPEG0, 12);
 static MESON_GATE(gxbb_uart0, HHI_GCLK_MPEG0, 13);
@@ -1190,7 +1190,7 @@ static MESON_GATE(gxbb_usb0_ddr_bridge, HHI_GCLK_MPEG2, 
9);
 static MESON_GATE(gxbb_mmc_pclk, HHI_GCLK_MPEG2, 11);
 static MESON_GATE(gxbb_dvin, HHI_GCLK_MPEG2, 12);
 static MESON_GATE(gxbb_uart2, HHI_GCLK_MPEG2, 15);
-static MESON_GATE(gxbb_sana, HHI_GCLK_MPEG2, 22);
+static MESON_GATE(gxbb_sar_adc, HHI_GCLK_MPEG2, 22);
 static MESON_GATE(gxbb_vpu_intr, HHI_GCLK_MPEG2, 25);
 static MESON_GATE(gxbb_sec_ahb_ahb3_bridge, HHI_GCLK_MPEG2, 26);
 static MESON_GATE(gxbb_clk81_a53, HHI_GCLK_MPEG2, 29);
-- 
2.14.1

Re: [PATCH] clk: meson: gxbb: fix wrong clock for SARADC/SANA

[Yixun Lan]

Hi Neil:


On 11/06/17 16:57, Neil Armstrong wrote:
> On 06/11/2017 08:52, Yixun Lan wrote:
>> According to the datasheet, in Meson-GXBB/GXL series,
>> The clock gate bit for SARADC is HHI_GCLK_MPEG2 bit[22],
>> while clock gate bit for SANA is HHI_GCLK_MPEG0 bit[10].
>>
>> Test passed at gxl_skt dev board.
>>
>> Tested-by: Xingyu Chen 
>> Signed-off-by: Yixun Lan 
>>
>> ---
>> I think this error was introduced by a copy & paste from meson8 code?
>> and we didn't notice them due to the SANA clock is also enabled by
>> DTS (so SAR_ADC works fine)?
>> ---
>>  drivers/clk/meson/gxbb.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
>> index b2d1e8ed7152..92168348ffa6 100644
>> --- a/drivers/clk/meson/gxbb.c
>> +++ b/drivers/clk/meson/gxbb.c
>> @@ -1139,7 +1139,7 @@ static MESON_GATE(gxbb_pl301, HHI_GCLK_MPEG0, 6);
>>  static MESON_GATE(gxbb_periphs, HHI_GCLK_MPEG0, 7);
>>  static MESON_GATE(gxbb_spicc, HHI_GCLK_MPEG0, 8);
>>  static MESON_GATE(gxbb_i2c, HHI_GCLK_MPEG0, 9);
>> -static MESON_GATE(gxbb_sar_adc, HHI_GCLK_MPEG0, 10);
>> +static MESON_GATE(gxbb_sana, HHI_GCLK_MPEG0, 10);
>>  static MESON_GATE(gxbb_smart_card, HHI_GCLK_MPEG0, 11);
>>  static MESON_GATE(gxbb_rng0, HHI_GCLK_MPEG0, 12);
>>  static MESON_GATE(gxbb_uart0, HHI_GCLK_MPEG0, 13);
>> @@ -1190,7 +1190,7 @@ static MESON_GATE(gxbb_usb0_ddr_bridge, 
>> HHI_GCLK_MPEG2, 9);
>>  static MESON_GATE(gxbb_mmc_pclk, HHI_GCLK_MPEG2, 11);
>>  static MESON_GATE(gxbb_dvin, HHI_GCLK_MPEG2, 12);
>>  static MESON_GATE(gxbb_uart2, HHI_GCLK_MPEG2, 15);
>> -static MESON_GATE(gxbb_sana, HHI_GCLK_MPEG2, 22);
>> +static MESON_GATE(gxbb_sar_adc, HHI_GCLK_MPEG2, 22);
>>  static MESON_GATE(gxbb_vpu_intr, HHI_GCLK_MPEG2, 25);
>>  static MESON_GATE(gxbb_sec_ahb_ahb3_bridge, HHI_GCLK_MPEG2, 26);
>>  static MESON_GATE(gxbb_clk81_a53, HHI_GCLK_MPEG2, 29);
>>
> 
> Hi Yixun,
> 
> Can you precise how it affects the saradc driver ? from the DT and clk PoV ?
>
the saradc module doesn't require sana clock (it's irrelvevant), we will
send a patchset v3 to address this.

> Also, can you add a Fixes: tag on the patch ?
> 
sure, I can do this.

> Thanks,
> Neil
> 
> .
> 

Re: [PATCH] clk: meson: gxbb: fix wrong clock for SARADC/SANA

[Yixun Lan]

Hi Jerome:

On 11/06/17 17:10, Jerome Brunet wrote:
> On Mon, 2017-11-06 at 15:52 +0800, Yixun Lan wrote:
>> According to the datasheet, in Meson-GXBB/GXL series,
>> The clock gate bit for SARADC is HHI_GCLK_MPEG2 bit[22],
>> while clock gate bit for SANA is HHI_GCLK_MPEG0 bit[10].
>>
>> Test passed at gxl_skt dev board.
> I think this refer to a board naming used in amlogic vendor kernel ?
> Would you mind telling what it is ?
> 
>>
>> Tested-by: Xingyu Chen 
>> Signed-off-by: Yixun Lan 
> 
> Subject is missing "v2" tag and a reference to the previous message:
> 20171103181703.30434-1-yixun@amlogic.com
> 
Ok..
I was considering this patch as a new one, so didn't add a v2 tag

>>
>> ---
>> I think this error was introduced by a copy & paste from meson8 code?
>> and we didn't notice them due to the SANA clock is also enabled by
>> DTS (so SAR_ADC works fine)?
>> ---
>>  drivers/clk/meson/gxbb.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
>> index b2d1e8ed7152..92168348ffa6 100644
>> --- a/drivers/clk/meson/gxbb.c
>> +++ b/drivers/clk/meson/gxbb.c
>> @@ -1139,7 +1139,7 @@ static MESON_GATE(gxbb_pl301, HHI_GCLK_MPEG0, 6);
>>  static MESON_GATE(gxbb_periphs, HHI_GCLK_MPEG0, 7);
>>  static MESON_GATE(gxbb_spicc, HHI_GCLK_MPEG0, 8);
>>  static MESON_GATE(gxbb_i2c, HHI_GCLK_MPEG0, 9);
>> -static MESON_GATE(gxbb_sar_adc, HHI_GCLK_MPEG0, 10);
>> +static MESON_GATE(gxbb_sana, HHI_GCLK_MPEG0, 10);
>>  static MESON_GATE(gxbb_smart_card, HHI_GCLK_MPEG0, 11);
>>  static MESON_GATE(gxbb_rng0, HHI_GCLK_MPEG0, 12);
>>  static MESON_GATE(gxbb_uart0, HHI_GCLK_MPEG0, 13);
>> @@ -1190,7 +1190,7 @@ static MESON_GATE(gxbb_usb0_ddr_bridge, HHI_GCLK_MPEG2,
>> 9);
>>  static MESON_GATE(gxbb_mmc_pclk, HHI_GCLK_MPEG2, 11);
>>  static MESON_GATE(gxbb_dvin, HHI_GCLK_MPEG2, 12);
>>  static MESON_GATE(gxbb_uart2, HHI_GCLK_MPEG2, 15);
>> -static MESON_GATE(gxbb_sana, HHI_GCLK_MPEG2, 22);
>> +static MESON_GATE(gxbb_sar_adc, HHI_GCLK_MPEG2, 22);
> 
> The value currently used in the driver are with respect to the
> Datasheet of GXBB (v1.1.4) and GXL (S905X - V0.3-20170314), which are 
> available
> to the public at http://http://linux-meson.com
> 
> the adc driver is claiming both clock, so this patch should not change 
> anything
> to the adc operation.
> 
> * Is this patch fixing any issue ?
this will remove the un-used 'sana' clock from saradc,
to reflect how the hardware actually work..

there is no function changed

> * Is it an error in the published datasheets ?
then, I think the published datasheet need to be updated.

> 
>>  static MESON_GATE(gxbb_vpu_intr, HHI_GCLK_MPEG2, 25);
>>  static MESON_GATE(gxbb_sec_ahb_ahb3_bridge, HHI_GCLK_MPEG2, 26);
>>  static MESON_GATE(gxbb_clk81_a53, HHI_GCLK_MPEG2, 29);
> 
> .
> 

Re: [PATCH] clk: meson: gxbb: fix wrong clock for SARADC/SANA

[Yixun Lan]

Hi Jerome:

On 11/06/17 17:10, Jerome Brunet wrote:
> On Mon, 2017-11-06 at 15:52 +0800, Yixun Lan wrote:
>> According to the datasheet, in Meson-GXBB/GXL series,
>> The clock gate bit for SARADC is HHI_GCLK_MPEG2 bit[22],
>> while clock gate bit for SANA is HHI_GCLK_MPEG0 bit[10].
>>
>> Test passed at gxl_skt dev board.
> I think this refer to a board naming used in amlogic vendor kernel ?
> Would you mind telling what it is ?
> 

sorry, it's actually tested at gxl-s905x-p212 board.

Re: [RFC PATCH v2 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller

[Yixun Lan]

Hi Boris


On 08/02/2018 05:50 AM, Boris Brezillon wrote:
> Hi Yixun,
> 
> On Thu, 19 Jul 2018 17:46:12 +0800
> Yixun Lan  wrote:
> 
> I haven't finished reviewing the driver yet (I'll try to do that later
> this week), but I already pointed a few things to fix/improve.
> 

thanks for the fully review, we really appreciate your time ;-)

I will comment on a few general items first, then clarify others after
talking to the NAND/ASIC team

>> +
>> +static int meson_nfc_exec_op(struct nand_chip *chip,
>> + const struct nand_operation *op, bool check_only)
>> +{
>> +
>> +static int meson_nfc_buffer_init(struct mtd_info *mtd)
>> +{
>> +struct nand_chip *nand = mtd_to_nand(mtd);
>> +struct meson_nfc *nfc = nand_get_controller_data(nand);
>> +int info_bytes, page_bytes;
>> +int nsectors;
>> +
>> +nsectors = mtd->writesize / nand->ecc.size;
>> +info_bytes = nsectors * PER_INFO_BYTE;
>> +page_bytes = mtd->writesize + mtd->oobsize;
>> +
>> +if (nfc->data_buf && nfc->info_buf)
>> +return 0;
>> +
>> +nfc->data_buf = kmalloc(page_bytes, GFP_KERNEL);
> 
> I'm pretty sure that does not work if you have several chips. Either
> you have one buffer tied to the NFC, and it has to be large enough to
> handle the NAND with the largest page, or you have one buffer per chip.
> 
em, we will fix this in next version,

>> +if (!nfc->data_buf)
>> +return -ENOMEM;
>> +
>> +nfc->info_buf = kmalloc(info_bytes, GFP_KERNEL);
>> +if (!nfc->info_buf) {
>> +kfree(nfc->data_buf);
>> +return -ENOMEM;
>> +}
>> +
> 
> Those buffers are not removed in the cleanup/error path.
> 
indeed, thanks for pointing out.
we actually realized this error after sent out this patch ..
>> +return 0;
>> +}
>> +
>> +static int meson_nfc_calc_set_timing(struct meson_nfc *nfc,
>> + int rc_min, int rea_max, int rhoh_min)
..

>> +
>> +static int
>> +meson_nfc_nand_chip_init(struct device *dev,
>> + struct meson_nfc *nfc, struct device_node *np)
>> +{
>> +struct meson_nfc_nand_chip *chip;
>> +struct nand_chip *nand;
>> +struct mtd_info *mtd;
>> +int ret, nsels, i, len = 0;
>> +char cs_id[16];
>> +u32 tmp;
>> +
>> +if (!of_get_property(np, "reg", &nsels))
>> +return -EINVAL;
>> +
>> +nsels /= sizeof(u32);
>> +if (!nsels || nsels > MAX_CE_NUM) {
>> +dev_err(dev, "invalid reg property size\n");
>> +return -EINVAL;
>> +}
>> +
>> +chip = devm_kzalloc(dev, sizeof(*chip) + (nsels * sizeof(u8)),
>> +GFP_KERNEL);
>> +if (!chip)
>> +return -ENOMEM;
>> +
>> +chip->nsels = nsels;
>> +
>> +for (i = 0; i < nsels; i++) {
>> +ret = of_property_read_u32_index(np, "reg", i, &tmp);
>> +if (ret) {
>> +dev_err(dev, "could not retrieve reg property: %d\n",
>> +ret);
>> +return ret;
>> +}
>> +chip->sels[i] = tmp;
> 
> You should probably keep track of all the already assigned CS lines, to
> prevent situations where the same controller-CS is used twice
> (copy&paste error when writing the DT).
> 

will do in next version, we would consider to use a bitmap for tracking
this ..
>> +len += snprintf(cs_id + len, 16, "%s%d", i ? "-" : ".", tmp);
> 
> Hm, do we really need to be that accurate? I'd suggest using the first
> CS only.
> 
ok, this would much simple..
thanks for the suggestion and the detail sample code in the following
section ;-)
>> +}
>> +
>> +chip->is_scramble =
>> +of_property_read_bool(np, "amlogic,nand-enable-scrambler");
> 
> I think I already complained about that :P. If you think this is still
> needed (meaning that the autodetection + NAND_NEED_SCRAMBLING flag are
> not enough), I'll need a detailed explanation ;-).
> 

yes, we saw this kind comment in DT patch already, we will try to fix this..
>> +
>> +nand = &chip->nand;
>> +nand_set_flash_node(nand, np);
>> +nand_set_controller_data(nand, nfc);
>> +
>> +nand->options |= NAND_USE_BOUNCE_BUFFER;
>> +nand-

Re: [PATCH 3/4] ARM64: dts: meson-gx: add dmcbus and canvas nodes.

[Yixun Lan]

Hi Maxime

great job! thanks for contributing the patches..

On Thu, Aug 2, 2018 at 2:51 AM, Maxime Jourdan  wrote:
> Wrap the canvas node in a syscon node.
>
> Signed-off-by: Maxime Jourdan 
> ---
>  arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 17 +
>  1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi 
> b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> index b8dc4dbb391b..d104b9e111fb 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> @@ -423,6 +423,23 @@
> };
> };
>
> +   dmcbus: bus@c8838000 {
> +   compatible = "simple-bus";
> +   reg = <0x0 0xc8838000 0x0 0x1000>;
> +   #address-cells = <2>;
> +   #size-cells = <2>;
> +   ranges = <0x0 0x0 0x0 0xc8838000 0x0 0x1000>;
> +
> +   sysctrl_DMC: system-controller@0 {
> +   compatible = "amlogic,meson-gx-dmc-sysctrl", 
> "syscon", "simple-mfd";

we'd like to drop 'meson-' prefix, so better using "amlogic,gx-dmc-sysctrl",
please take a look at the discussion here [1]

[1] https://lkml.kernel.org/r/7hk1prmg4w@baylibre.com

> +   reg = <0x0 0x0 0x0 0x1000>;
> +
> +   canvas: canvas-provider@0 {
> +   compatible = "amlogic,meson-canvas";
ditto

> +   };
> +   };
> +   };
> +
> hiubus: bus@c883c000 {
> compatible = "simple-bus";
> reg = <0x0 0xc883c000 0x0 0x2000>;
> --
> 2.17.1
>
>
> ___
> linux-amlogic mailing list
> linux-amlo...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/4] soc: amlogic: add meson-canvas driver

[Yixun Lan]

HI Maxime

thanks for contributing the patches ;-)

On Thu, Aug 2, 2018 at 2:51 AM, Maxime Jourdan  wrote:
> Amlogic SoCs have a repository of 256 canvas which they use to
> describe pixel buffers.
>
> They contain metadata like width, height, block mode, endianness [..]
>
> Many IPs within those SoCs like vdec/vpu rely on those canvas to read/write
> pixels.
>
> Signed-off-by: Maxime Jourdan 
> ---
>  drivers/soc/amlogic/Kconfig  |   7 +
>  drivers/soc/amlogic/Makefile |   1 +
>  drivers/soc/amlogic/meson-canvas.c   | 182 +++
>  include/linux/soc/amlogic/meson-canvas.h |  37 +
>  4 files changed, 227 insertions(+)
>  create mode 100644 drivers/soc/amlogic/meson-canvas.c
>  create mode 100644 include/linux/soc/amlogic/meson-canvas.h
>
> diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig
> index b04f6e4aedbc..5bd049899d88 100644
> --- a/drivers/soc/amlogic/Kconfig
> +++ b/drivers/soc/amlogic/Kconfig
> @@ -1,5 +1,12 @@
>  menu "Amlogic SoC drivers"
>
> +config MESON_CANVAS
> +   bool "Amlogic Meson Canvas driver"
shouldn't this a 'tristate'? since you'd make the driver a kernel module..

> +   depends on ARCH_MESON || COMPILE_TEST
> +   default ARCH_MESON
> +   help
> + Say yes to support the canvas IP within Amlogic Meson Soc family.
> +
>  config MESON_GX_SOCINFO
> bool "Amlogic Meson GX SoC Information driver"
> depends on ARCH_MESON || COMPILE_TEST
> diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile
> index 8fa321893928..0ab16d35ac36 100644
> --- a/drivers/soc/amlogic/Makefile
> +++ b/drivers/soc/amlogic/Makefile
> @@ -1,3 +1,4 @@
> +obj-$(CONFIG_MESON_CANVAS) += meson-canvas.o
>  obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o
>  obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o
>  obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o
> diff --git a/drivers/soc/amlogic/meson-canvas.c 
> b/drivers/soc/amlogic/meson-canvas.c
> new file mode 100644
> index ..671eb89c8904
> --- /dev/null
> +++ b/drivers/soc/amlogic/meson-canvas.c
> @@ -0,0 +1,182 @@
> +/*
> + * Copyright (C) 2018 Maxime Jourdan
> + * Copyright (C) 2016 BayLibre, SAS
> + * Author: Neil Armstrong 
> + * Copyright (C) 2015 Amlogic, Inc. All rights reserved.
> + * Copyright (C) 2014 Endless Mobile
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see .
> + */
use SPDX license header as Neil already mentioned
check doc: Documentation/process/license-rules.rst

> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define NUM_CANVAS 256
> +
> +/* DMC Registers */
> +#define DMC_CAV_LUT_DATAL  0x48 /* 0x12 offset in data sheet */
> +   #define CANVAS_WIDTH_LBIT   29
> +   #define CANVAS_WIDTH_LWID   3
> +#define DMC_CAV_LUT_DATAH  0x4c /* 0x13 offset in data sheet */
> +   #define CANVAS_WIDTH_HBIT   0
> +   #define CANVAS_HEIGHT_BIT   9
> +   #define CANVAS_BLKMODE_BIT  24
> +#define DMC_CAV_LUT_ADDR   0x50 /* 0x14 offset in data sheet */
> +   #define CANVAS_LUT_WR_EN(0x2 << 8)
> +   #define CANVAS_LUT_RD_EN(0x1 << 8)
> +
> +struct meson_canvas {
> +   struct device *dev;
> +   struct regmap *regmap_dmc;
> +   struct mutex lock;
> +   u8 used[NUM_CANVAS];
> +};
> +
> +static struct meson_canvas canvas = { 0 };
> +
> +static int meson_canvas_setup(uint8_t canvas_index, uint32_t addr,
> +   uint32_t stride, uint32_t height,
> +   unsigned int wrap,
> +   unsigned int blkmode,
> +   unsigned int endian)
use "./scripts/checkpatch.pl --strict" to check
you will get a few complaints..

> +{
> +   struct regmap *regmap = canvas.regmap_dmc;
> +   u32 val;
> +
> +   mutex_lock(&canvas.lock);
> +
> +   if (!canvas.used[canvas_index]) {
> +   dev_err(canvas.dev,
> +   "Trying to setup non allocated canvas %u\n",
> +   canvas_index);
> +   mutex_unlock(&canvas.lock);
> +   return -EINVAL;
> +   }
> +
> +   regmap_write(regmap, DMC_CAV_LUT_DATAL,
> +   ((addr + 7) >> 3) |
> +   (((stride + 7) >> 3) << CANVAS_WIDTH_LBIT));
> +
> + 

Re: [PATCH 3/4] ARM64: dts: meson-gx: add dmcbus and canvas nodes.

[Yixun Lan]

hi Maxime:

On Sun, Aug 5, 2018 at 4:02 AM, Maxime Jourdan  wrote:
>>> +   sysctrl_DMC: system-controller@0 {
>>> +   compatible = 
>>> "amlogic,meson-gx-dmc-sysctrl", "syscon", "simple-mfd";
>>
>> we'd like to drop 'meson-' prefix, so better using "amlogic,gx-dmc-sysctrl",
>> please take a look at the discussion here [1]
>>
>> [1] https://lkml.kernel.org/r/7hk1prmg4w@baylibre.com
>>
>
> On that subject, should I remove the meson keyword from dts only, or
> from everything ?
>
remove the 'meson-' from dts is enough..

> e.g use amlogic_canvas_* symbols instead of meson_canvas_*, name the
> source file "amlogic-canvas.c", etc. ?
>
Actually, I'd suggest to keep using meson_canvas_* in the code for the
consistency,
unless Kevin or Jerome/Neil has something to say?

Yixun

[PATCH v3 1/2] documentation: pinctrl: Add compatibles for Amlogic Meson G12A pin controllers

[Yixun Lan]

Add new compatible name for Amlogic's Meson-G12A pin controllers,
add a dt-binding header file which document the detail pin names.

Acked-by: Martin Blumenstingl 
Reviewed-by: Rob Herring 
Signed-off-by: Xingyu Chen 
Signed-off-by: Yixun Lan 
---
 .../bindings/pinctrl/meson,pinctrl.txt|   2 +
 include/dt-bindings/gpio/meson-g12a-gpio.h| 114 ++
 2 files changed, 116 insertions(+)
 create mode 100644 include/dt-bindings/gpio/meson-g12a-gpio.h

diff --git a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt 
b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
index 54ecb8ab7788..82ead40311f6 100644
--- a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
@@ -13,6 +13,8 @@ Required properties for the root node:
  "amlogic,meson-gxl-aobus-pinctrl"
  "amlogic,meson-axg-periphs-pinctrl"
  "amlogic,meson-axg-aobus-pinctrl"
+ "amlogic,meson-g12a-periphs-pinctrl"
+ "amlogic,meson-g12a-aobus-pinctrl"
  - reg: address and size of registers controlling irq functionality
 
 === GPIO sub-nodes ===
diff --git a/include/dt-bindings/gpio/meson-g12a-gpio.h 
b/include/dt-bindings/gpio/meson-g12a-gpio.h
new file mode 100644
index ..f7bd69350d18
--- /dev/null
+++ b/include/dt-bindings/gpio/meson-g12a-gpio.h
@@ -0,0 +1,114 @@
+/* SPDX-License-Identifier: (GPL-2.0+ or MIT) */
+/*
+ * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
+ * Author: Xingyu Chen 
+ */
+
+#ifndef _DT_BINDINGS_MESON_G12A_GPIO_H
+#define _DT_BINDINGS_MESON_G12A_GPIO_H
+
+/* First GPIO chip */
+#define GPIOAO_0   0
+#define GPIOAO_1   1
+#define GPIOAO_2   2
+#define GPIOAO_3   3
+#define GPIOAO_4   4
+#define GPIOAO_5   5
+#define GPIOAO_6   6
+#define GPIOAO_7   7
+#define GPIOAO_8   8
+#define GPIOAO_9   9
+#define GPIOAO_10  10
+#define GPIOAO_11  11
+#define GPIOE_012
+#define GPIOE_113
+#define GPIOE_214
+
+/* Second GPIO chip */
+#define GPIOZ_00
+#define GPIOZ_11
+#define GPIOZ_22
+#define GPIOZ_33
+#define GPIOZ_44
+#define GPIOZ_55
+#define GPIOZ_66
+#define GPIOZ_77
+#define GPIOZ_88
+#define GPIOZ_99
+#define GPIOZ_10   10
+#define GPIOZ_11   11
+#define GPIOZ_12   12
+#define GPIOZ_13   13
+#define GPIOZ_14   14
+#define GPIOZ_15   15
+#define GPIOH_016
+#define GPIOH_117
+#define GPIOH_218
+#define GPIOH_319
+#define GPIOH_420
+#define GPIOH_521
+#define GPIOH_622
+#define GPIOH_723
+#define GPIOH_824
+#define BOOT_0 25
+#define BOOT_1 26
+#define BOOT_2 27
+#define BOOT_3 28
+#define BOOT_4 29
+#define BOOT_5 30
+#define BOOT_6 31
+#define BOOT_7 32
+#define BOOT_8 33
+#define BOOT_9 34
+#define BOOT_1035
+#define BOOT_1136
+#define BOOT_1237
+#define BOOT_1338
+#define BOOT_1439
+#define BOOT_1540
+#define GPIOC_041
+#define GPIOC_142
+#define GPIOC_243
+#define GPIOC_344
+#define GPIOC_445
+#define GPIOC_546
+#define GPIOC_647
+#define GPIOC_748
+#define GPIOA_049
+#define GPIOA_150
+#define GPIOA_251
+#define GPIOA_352
+#define GPIOA_453
+#define GPIOA_554
+#define GPIOA_655
+#define GPIOA_756
+#define GPIOA_857
+#define GPIOA_958
+#define GPIOA_10   59
+#define GPIOA_11   60
+#define GPIOA_12   61
+#define GPIOA_13   62
+#define GPIOA_14   63
+#define GPIOA_15   64
+#define GPIOX_065
+#define GPIOX_166
+#define GPIOX_267
+#define GPIOX_368
+#define GPIOX_469
+#define GPIOX_570
+#define GPIOX_671
+#define GPIOX_772
+#define GPIOX_873
+#define GPIOX_974
+#define GPIOX_10   75
+#define GPIOX_11   76
+#define GPIOX_12   77
+#define GPIOX_13   78
+#define GPIOX_14   79
+#define GPIOX_15   80
+#define GPIOX_16   81
+#define GPIOX_17   82
+#define GPIOX_18   83
+#define GPIOX_19   84
+
+#endif /* _DT_BINDINGS_MESON_G12A_GPIO_H */
-- 
2.18.0

[PATCH v3 0/2] pinctrl: meson-g12a: add pinctrl driver support

[Yixun Lan]

  This patch series try to add pinctrl driver support for
the Meson-G12A SoC.

  The pinctrl driver for Meson-G12A SoC share the similar IP as
the previous Meson-AXG SoC, both use same pinmux ops (register layout).
But, a new driver is needed here due to the differences in the pins.

Changes since v2 at [2]
 - use ${FUNCTION}_${DOMAIN}_${PORT}_${PINFUNC}_${BANK}${PINNUM}
   to rename some pins
 - fix tdm groups
 - explain the similarity of pinmux between AXG and G12A - thanks Martin 
 - collect Rob's Reviewed-by

Changes since v1 at [1]
 - add Martin's Ack, Xingyu's Signed-off
 - squash patch 1,2 (documentation & header file)
 - explain pinctrl IP
 - notice GPIOE located in AO bank

[1] https://lkml.kernel.org/r/20180704224511.29350-1-yixun@amlogic.com
[2] https://lkml.kernel.org/r/20180714232754.5402-1-yixun@amlogic.com

Yixun Lan (2):
  documentation: pinctrl: Add compatibles for Amlogic Meson G12A pin
controllers
  pinctrl: meson-g12a: add pinctrl driver support

 .../bindings/pinctrl/meson,pinctrl.txt|2 +
 drivers/pinctrl/meson/Kconfig |6 +
 drivers/pinctrl/meson/Makefile|1 +
 drivers/pinctrl/meson/pinctrl-meson-g12a.c| 1404 +
 include/dt-bindings/gpio/meson-g12a-gpio.h|  114 ++
 5 files changed, 1527 insertions(+)
 create mode 100644 drivers/pinctrl/meson/pinctrl-meson-g12a.c
 create mode 100644 include/dt-bindings/gpio/meson-g12a-gpio.h

-- 
2.18.0

[PATCH v3 2/2] pinctrl: meson-g12a: add pinctrl driver support

[Yixun Lan]

Add the pinctrl driver for Meson-G12A SoC which share the similar IP as
the previous Meson-AXG SoC, both use same pinmux ops (register layout).
A new driver is needed here due to the differences in the pins.

Starting from Meson-AXG SoC, the pinctrl controller block use 4
continues register bits to specific the pin mux function, while comparing
to old generation SoC which using variable length register bits for
the pin mux definition. The new design greatly simplify the software model.

For the detail example, one 32bit register can be divided into 8 parts,
each has 4 bits whose value start from 0 - 7, each can describe one pin,
the value 0 is always devoted to GPIO function, while 1 - 7 devoted to
the mux pin function.

Please note, the GPIOE is actually located at AO (always on) bank.

Acked-by: Martin Blumenstingl 
Signed-off-by: Xingyu Chen 
Signed-off-by: Yixun Lan 
---
 drivers/pinctrl/meson/Kconfig  |6 +
 drivers/pinctrl/meson/Makefile |1 +
 drivers/pinctrl/meson/pinctrl-meson-g12a.c | 1404 
 3 files changed, 1411 insertions(+)
 create mode 100644 drivers/pinctrl/meson/pinctrl-meson-g12a.c

diff --git a/drivers/pinctrl/meson/Kconfig b/drivers/pinctrl/meson/Kconfig
index c80951d6caff..9ab537eb78a3 100644
--- a/drivers/pinctrl/meson/Kconfig
+++ b/drivers/pinctrl/meson/Kconfig
@@ -47,4 +47,10 @@ config PINCTRL_MESON_AXG
 config PINCTRL_MESON_AXG_PMX
bool
 
+config PINCTRL_MESON_G12A
+   bool "Meson g12a Soc pinctrl driver"
+   depends on ARM64
+   select PINCTRL_MESON_AXG_PMX
+   default y
+
 endif
diff --git a/drivers/pinctrl/meson/Makefile b/drivers/pinctrl/meson/Makefile
index 3c6580c2d9d7..cf283f48f9d8 100644
--- a/drivers/pinctrl/meson/Makefile
+++ b/drivers/pinctrl/meson/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_PINCTRL_MESON_GXBB) += pinctrl-meson-gxbb.o
 obj-$(CONFIG_PINCTRL_MESON_GXL) += pinctrl-meson-gxl.o
 obj-$(CONFIG_PINCTRL_MESON_AXG_PMX) += pinctrl-meson-axg-pmx.o
 obj-$(CONFIG_PINCTRL_MESON_AXG) += pinctrl-meson-axg.o
+obj-$(CONFIG_PINCTRL_MESON_G12A) += pinctrl-meson-g12a.o
diff --git a/drivers/pinctrl/meson/pinctrl-meson-g12a.c 
b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
new file mode 100644
index ..d494492e98e9
--- /dev/null
+++ b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
@@ -0,0 +1,1404 @@
+// SPDX-License-Identifier: (GPL-2.0+ or MIT)
+/*
+ * Pin controller and GPIO driver for Amlogic Meson G12A SoC.
+ *
+ * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
+ * Author: Xingyu Chen 
+ * Author: Yixun Lan 
+ */
+
+#include 
+#include "pinctrl-meson.h"
+#include "pinctrl-meson-axg-pmx.h"
+
+static const struct pinctrl_pin_desc meson_g12a_periphs_pins[] = {
+   MESON_PIN(GPIOZ_0),
+   MESON_PIN(GPIOZ_1),
+   MESON_PIN(GPIOZ_2),
+   MESON_PIN(GPIOZ_3),
+   MESON_PIN(GPIOZ_4),
+   MESON_PIN(GPIOZ_5),
+   MESON_PIN(GPIOZ_6),
+   MESON_PIN(GPIOZ_7),
+   MESON_PIN(GPIOZ_8),
+   MESON_PIN(GPIOZ_9),
+   MESON_PIN(GPIOZ_10),
+   MESON_PIN(GPIOZ_11),
+   MESON_PIN(GPIOZ_12),
+   MESON_PIN(GPIOZ_13),
+   MESON_PIN(GPIOZ_14),
+   MESON_PIN(GPIOZ_15),
+   MESON_PIN(GPIOH_0),
+   MESON_PIN(GPIOH_1),
+   MESON_PIN(GPIOH_2),
+   MESON_PIN(GPIOH_3),
+   MESON_PIN(GPIOH_4),
+   MESON_PIN(GPIOH_5),
+   MESON_PIN(GPIOH_6),
+   MESON_PIN(GPIOH_7),
+   MESON_PIN(GPIOH_8),
+   MESON_PIN(BOOT_0),
+   MESON_PIN(BOOT_1),
+   MESON_PIN(BOOT_2),
+   MESON_PIN(BOOT_3),
+   MESON_PIN(BOOT_4),
+   MESON_PIN(BOOT_5),
+   MESON_PIN(BOOT_6),
+   MESON_PIN(BOOT_7),
+   MESON_PIN(BOOT_8),
+   MESON_PIN(BOOT_9),
+   MESON_PIN(BOOT_10),
+   MESON_PIN(BOOT_11),
+   MESON_PIN(BOOT_12),
+   MESON_PIN(BOOT_13),
+   MESON_PIN(BOOT_14),
+   MESON_PIN(BOOT_15),
+   MESON_PIN(GPIOC_0),
+   MESON_PIN(GPIOC_1),
+   MESON_PIN(GPIOC_2),
+   MESON_PIN(GPIOC_3),
+   MESON_PIN(GPIOC_4),
+   MESON_PIN(GPIOC_5),
+   MESON_PIN(GPIOC_6),
+   MESON_PIN(GPIOC_7),
+   MESON_PIN(GPIOA_0),
+   MESON_PIN(GPIOA_1),
+   MESON_PIN(GPIOA_2),
+   MESON_PIN(GPIOA_3),
+   MESON_PIN(GPIOA_4),
+   MESON_PIN(GPIOA_5),
+   MESON_PIN(GPIOA_6),
+   MESON_PIN(GPIOA_7),
+   MESON_PIN(GPIOA_8),
+   MESON_PIN(GPIOA_9),
+   MESON_PIN(GPIOA_10),
+   MESON_PIN(GPIOA_11),
+   MESON_PIN(GPIOA_12),
+   MESON_PIN(GPIOA_13),
+   MESON_PIN(GPIOA_14),
+   MESON_PIN(GPIOA_15),
+   MESON_PIN(GPIOX_0),
+   MESON_PIN(GPIOX_1),
+   MESON_PIN(GPIOX_2),
+   MESON_PIN(GPIOX_3),
+   MESON_PIN(GPIOX_4),
+   MESON_PIN(GPIOX_5),
+   MESON_PIN(GPIOX_6),
+   MESON_PIN(GPIOX_7),
+   MESON_PIN(GPIOX_8),
+   MESON_PIN(GPIOX_9),
+   MESON_PIN(GPIOX_10),
+   MESON_PIN(GPIOX_11),
+   MESON_PIN(GPIOX_12),
+   MESON_PIN(GPIOX_13),
+   MESON_PIN(GPIOX_1

[PATCH] clk: meson-axg: add clocks required by pcie driver

[Yixun Lan]

Adding clocks for pcie driver, due to the ASIC design,
the pcie controller re-use part of the mipi clock logic,
so the mipi clock is also required.

Tested-by: Jianxin Qin 
Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/axg.c  | 145 +++
 drivers/clk/meson/axg.h  |   6 +-
 include/dt-bindings/clock/axg-clkc.h |   3 +
 3 files changed, 153 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c
index bd4dbc696b88..f1dc5433b69d 100644
--- a/drivers/clk/meson/axg.c
+++ b/drivers/clk/meson/axg.c
@@ -626,6 +626,137 @@ static struct clk_regmap axg_mpll3 = {
},
 };
 
+static const struct pll_rate_table axg_pcie_pll_rate_table[] = {
+   {
+   .rate   = 1,
+   .m  = 200,
+   .n  = 3,
+   .od = 1,
+   .od2= 3,
+   },
+   { /* sentinel */ },
+};
+
+static const struct reg_sequence axg_pcie_init_regs[] = {
+   { .reg = HHI_PCIE_PLL_CNTL, .def = 0x400106c8 },
+   { .reg = HHI_PCIE_PLL_CNTL1,.def = 0x0084a2aa },
+   { .reg = HHI_PCIE_PLL_CNTL2,.def = 0xb75020be },
+   { .reg = HHI_PCIE_PLL_CNTL3,.def = 0x0a47488e },
+   { .reg = HHI_PCIE_PLL_CNTL4,.def = 0xc04d },
+   { .reg = HHI_PCIE_PLL_CNTL5,.def = 0x00078000 },
+   { .reg = HHI_PCIE_PLL_CNTL6,.def = 0x002323c6 },
+};
+
+static struct clk_regmap axg_pcie_pll = {
+   .data = &(struct meson_clk_pll_data){
+   .m = {
+   .reg_off = HHI_PCIE_PLL_CNTL,
+   .shift   = 0,
+   .width   = 9,
+   },
+   .n = {
+   .reg_off = HHI_PCIE_PLL_CNTL,
+   .shift   = 9,
+   .width   = 5,
+   },
+   .od = {
+   .reg_off = HHI_PCIE_PLL_CNTL,
+   .shift   = 16,
+   .width   = 2,
+   },
+   .od2 = {
+   .reg_off = HHI_PCIE_PLL_CNTL6,
+   .shift   = 6,
+   .width   = 2,
+   },
+   .frac = {
+   .reg_off = HHI_PCIE_PLL_CNTL1,
+   .shift   = 0,
+   .width   = 12,
+   },
+   .l = {
+   .reg_off = HHI_PCIE_PLL_CNTL,
+   .shift   = 31,
+   .width   = 1,
+   },
+   .rst = {
+   .reg_off = HHI_PCIE_PLL_CNTL,
+   .shift   = 29,
+   .width   = 1,
+   },
+   .table = axg_pcie_pll_rate_table,
+   .init_regs = axg_pcie_init_regs,
+   .init_count = ARRAY_SIZE(axg_pcie_init_regs),
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "pcie_pll",
+   .ops = &meson_clk_pll_ops,
+   .parent_names = (const char *[]){ "xtal" },
+   .num_parents = 1,
+   },
+};
+
+static struct clk_regmap axg_pcie_mux = {
+   .data = &(struct clk_regmap_mux_data){
+   .offset = HHI_PCIE_PLL_CNTL6,
+   .mask = 0x1,
+   .shift = 2,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "pcie_mux",
+   .ops = &clk_regmap_mux_ops,
+   .parent_names = (const char *[]){ "mpll3", "pcie_pll" },
+   .num_parents = 2,
+   .flags = CLK_SET_RATE_PARENT,
+   },
+};
+
+static struct clk_regmap axg_pcie_ref = {
+   .data = &(struct clk_regmap_mux_data){
+   .offset = HHI_PCIE_PLL_CNTL6,
+   .mask = 0x1,
+   .shift = 1,
+   },
+   .hw.init = &(struct clk_init_data){
+   .name = "pcie_ref",
+   .ops = &clk_regmap_mux_ops,
+   /* do not select the first partent, debug only */
+   .parent_names = (const char *[]){ "",
+   "pcie_mux" },
+   .num_parents = 2,
+   .flags = CLK_SET_RATE_PARENT,
+   },
+};
+
+static struct clk_regmap axg_pcie_cml_en0 = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = HHI_PCIE_PLL_CNTL6,
+   .bit_idx = 4,
+   },
+   .hw.init = &(struct clk_init_data) {
+   .name = "pcie_cml_en0",
+   .ops = &clk_regmap_gate_ops,
+   .parent_names = (const char *[]){ "pcie_ref" },
+   .num_parents = 1,
+   .flags = CLK_SET_RATE_PARENT,
+
+   },
+};
+
+static struct clk_regmap axg_pcie_cml_en1 = {
+   .data = &(struct clk_regmap_gate_data){
+   .offset = HHI_PCIE_PLL_C

Re: [PATCH] clk: meson-axg: add clocks required by pcie driver

[Yixun Lan]

Hi Jerome

see my comments below

On 06/25/2018 05:32 PM, Jerome Brunet wrote:
> On Thu, 2018-06-21 at 12:26 +0000, Yixun Lan wrote:
>> Adding clocks for pcie driver, due to the ASIC design,
> Adding clocks for the pcie driver. Due to the ASIC design,
> 
Ok, I will adjust these in v2

>> the pcie controller re-use part of the mipi clock logic,
>> so the mipi clock is also required.
>>
>> Tested-by: Jianxin Qin 
>> Signed-off-by: Yixun Lan 
>> ---
>>  drivers/clk/meson/axg.c  | 145 +++
>>  drivers/clk/meson/axg.h  |   6 +-
>>  include/dt-bindings/clock/axg-clkc.h |   3 +
>>  3 files changed, 153 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c
>> index bd4dbc696b88..f1dc5433b69d 100644
>> --- a/drivers/clk/meson/axg.c
>> +++ b/drivers/clk/meson/axg.c
>> @@ -626,6 +626,137 @@ static struct clk_regmap axg_mpll3 = {
>>  },
>>  };
>>  
>> +static const struct pll_rate_table axg_pcie_pll_rate_table[] = {
>> +{
>> +.rate   = 1,
>> +.m  = 200,
>> +.n  = 3,
>> +.od = 1,
>> +.od2= 3,
>> +},
>> +{ /* sentinel */ },
>> +};
>> +
>> +static const struct reg_sequence axg_pcie_init_regs[] = {
>> +{ .reg = HHI_PCIE_PLL_CNTL, .def = 0x400106c8 },
>> +{ .reg = HHI_PCIE_PLL_CNTL1,.def = 0x0084a2aa },
>> +{ .reg = HHI_PCIE_PLL_CNTL2,.def = 0xb75020be },
>> +{ .reg = HHI_PCIE_PLL_CNTL3,.def = 0x0a47488e },
>> +{ .reg = HHI_PCIE_PLL_CNTL4,.def = 0xc04d },
>> +{ .reg = HHI_PCIE_PLL_CNTL5,.def = 0x00078000 },
>> +{ .reg = HHI_PCIE_PLL_CNTL6,.def = 0x002323c6 },
>> +};
>> +
>> +static struct clk_regmap axg_pcie_pll = {
>> +.data = &(struct meson_clk_pll_data){
>> +.m = {
>> +.reg_off = HHI_PCIE_PLL_CNTL,
>> +.shift   = 0,
>> +.width   = 9,
>> +},
>> +.n = {
>> +.reg_off = HHI_PCIE_PLL_CNTL,
>> +.shift   = 9,
>> +.width   = 5,
>> +},
>> +.od = {
>> +.reg_off = HHI_PCIE_PLL_CNTL,
>> +.shift   = 16,
>> +.width   = 2,
>> +},
>> +.od2 = {
>> +.reg_off = HHI_PCIE_PLL_CNTL6,
>> +.shift   = 6,
>> +.width   = 2,
>> +},
>> +.frac = {
>> +.reg_off = HHI_PCIE_PLL_CNTL1,
>> +.shift   = 0,
>> +.width   = 12,
>> +},
>> +.l = {
>> +.reg_off = HHI_PCIE_PLL_CNTL,
>> +.shift   = 31,
>> +.width   = 1,
>> +},
>> +.rst = {
>> +.reg_off = HHI_PCIE_PLL_CNTL,
>> +.shift   = 29,
>> +.width   = 1,
>> +},
>> +.table = axg_pcie_pll_rate_table,
>> +.init_regs = axg_pcie_init_regs,
>> +.init_count = ARRAY_SIZE(axg_pcie_init_regs),
>> +},
>> +.hw.init = &(struct clk_init_data){
>> +.name = "pcie_pll",
>> +.ops = &meson_clk_pll_ops,
>> +.parent_names = (const char *[]){ "xtal" },
>> +.num_parents = 1,
>> +},
>> +};
>> +
>> +static struct clk_regmap axg_pcie_mux = {
>> +.data = &(struct clk_regmap_mux_data){
>> +.offset = HHI_PCIE_PLL_CNTL6,
>> +.mask = 0x1,
>> +.shift = 2,
>> +},
>> +.hw.init = &(struct clk_init_data){
>> +.name = "pcie_mux",
>> +.ops = &clk_regmap_mux_ops,
>> +.parent_names = (const char *[]){ "mpll3", "pcie_pll" },
>> +.num_parents = 2,
>> +.flags = CLK_SET_RATE_PARENT,
>> +},
>> +};
>> +
>> +static struct clk_regmap axg_pcie_ref = {
>> +.data = &(struct clk_regmap_mux_data){
>> +.offset = HHI_PCIE_PLL_CNTL6,
>> +.mask = 0x1,
>> +.shift = 1,
>> +},
>> +.hw.init = &(struct clk_init_data){
>> +.na

Re: [PATCH v3 2/2] clk: meson: add sub MMC clock controller driver

[Yixun Lan]

Hi Jerome

On 07/30/18 16:57, Jerome Brunet wrote:
> On Fri, 2018-07-27 at 09:45 -0700, Stephen Boyd wrote:
>> Quoting Stephen Boyd (2018-07-27 09:41:40)
>>> Quoting Yixun Lan (2018-07-27 07:52:23)
>>>> HI Stephen:
>>>>
>>>> On 07/26/2018 11:20 PM, Stephen Boyd wrote:
>>>>> Quoting Yixun Lan (2018-07-12 14:12:44)
>>>>>> diff --git a/drivers/clk/meson/mmc-clkc.c b/drivers/clk/meson/mmc-clkc.c
>>>>>> new file mode 100644
>>>>>> index ..36c4c7cd69a6
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/clk/meson/mmc-clkc.c
>>>>>> @@ -0,0 +1,367 @@
>>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>>> +/*
>>>>>> + * Amlogic Meson MMC Sub Clock Controller Driver
>>>>>> + *
>>>>>> + * Copyright (c) 2017 Baylibre SAS.
>>>>>> + * Author: Jerome Brunet 
>>>>>> + *
>>>>>> + * Copyright (c) 2018 Amlogic, inc.
>>>>>> + * Author: Yixun Lan 
>>>>>> + */
>>>>>> +
>>>>>> +#include 
>>>>>
>>>>> Is this include used?
>>>>>
>>>>
>>>> this is needed by clk_get_rate()
>>>> see drivers/clk/meson/mmc-clkc.c:204
>>>
>>> Hmm ok. That's unfortunate.
>>
>> You should be able to read the hardware to figure out the clk frequency?
>> This may be a sign that the phase clk_ops are bad and should be passing
>> in the frequency of the parent clk to the op so that phase can be
>> calculated. Jerome?
>>
> 
> It could be a away to do it but:
> a) if we modify the API, we would need to update every clock driver using it.
>There is not that many users of the phase API but still, it is annoying
> b) This particular driver need the parent rate, other might need something 
> else
> I guess. (parent phase ??, duty cycle ??) 
> 
> I think the real problem here it that you are using the consumer API. You 
> should
> be using the provider API like clk_hw_get_rate. Look at the clk-divider.c 
> which
> use clk_hw_round_rate() on the parent clock. 
I will replace it with clk_hw_get_rate()

> 
> Clock drivers should deal with 'struct clk_hw', not 'struct clk'. I think it 
> was
> mentioned in the past that the 'clk' within 'struct clk_hw' might be removed
> someday.
> 
> Yixun, please don't put your clock driver within the controller driver. Please
> implement your 'phase-delay' clock in its own file and export the ops, like
> every other clock in the amlogic directory. Also, please review your list of
> '#define', some of them are unnecessary copy/paste from the MMC driver.
> 
will implement a clk-phase-delay.c

I can move the extra CC list

Yixun

[PATCH v4 0/3] clk: meson: add a sub EMMC clock controller support

[Yixun Lan]

This driver will add a MMC clock controller driver support.
The original idea about adding a clock controller is during the
discussion in the NAND driver mainline effort[1].

This driver is tested in the S400 board (AXG platform) with NAND driver.

Changes since v3 [4]:
 - separate clk-phase-delay driver
 - replace clk_get_rate() with clk_hw_get_rate()
 - collect Rob's R-Y
 - drop 'meson-' prefix from compatible string

Changes since v2 [3]:
 - squash dt-binding clock-id patch
 - update license
 - fix alignment
 - construct a clk register helper() function

Changes since v1 [2]:
 - implement phase clock
 - update compatible name
 - adjust file name
 - divider probe() into small functions, and re-use them

[1] https://lkml.kernel.org/r/20180628090034.0637a062@xps13
[2] https://lkml.kernel.org/r/20180703145716.31860-1-yixun@amlogic.com
[3] https://lkml.kernel.org/r/20180710163658.6175-1-yixun@amlogic.com
[4] https://lkml.kernel.org/r/20180712211244.11428-1-yixun....@amlogic.com

Yixun Lan (3):
  clk: meson: add emmc sub clock phase delay driver
  clk: meson: add DT documentation for emmc clock controller
  clk: meson: add sub MMC clock controller driver

 .../bindings/clock/amlogic,mmc-clkc.txt   |  31 ++
 drivers/clk/meson/Kconfig |  10 +
 drivers/clk/meson/Makefile|   3 +-
 drivers/clk/meson/clk-phase-delay.c   |  96 ++
 drivers/clk/meson/clkc.h  |  13 +
 drivers/clk/meson/mmc-clkc.c  | 275 ++
 include/dt-bindings/clock/amlogic,mmc-clkc.h  |  17 ++
 7 files changed, 444 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
 create mode 100644 drivers/clk/meson/clk-phase-delay.c
 create mode 100644 drivers/clk/meson/mmc-clkc.c
 create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h

-- 
2.17.1

[PATCH v4 3/3] clk: meson: add sub MMC clock controller driver

[Yixun Lan]

The patch will add a MMC clock controller driver which used by MMC or NAND,
It provide a mux and divider clock, and three phase clocks - core, tx, tx.

Two clocks are provided as the parent of MMC clock controller from
upper layer clock controller - eg "amlogic,axg-clkc" in AXG platform.

To specify which clock the MMC or NAND driver may consume,
the preprocessor macros in the dt-bindings/clock/amlogic,mmc-clkc.h header
can be used in the device tree sources.

Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/Kconfig|  10 ++
 drivers/clk/meson/Makefile   |   1 +
 drivers/clk/meson/mmc-clkc.c | 275 +++
 3 files changed, 286 insertions(+)
 create mode 100644 drivers/clk/meson/mmc-clkc.c

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index efaa70f682b4..8b8ccbcfed1d 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -15,6 +15,16 @@ config COMMON_CLK_MESON_AO
select COMMON_CLK_REGMAP_MESON
select RESET_CONTROLLER
 
+config COMMON_CLK_MMC_MESON
+   tristate "Meson MMC Sub Clock Controller Driver"
+   depends on COMMON_CLK_AMLOGIC
+   select MFD_SYSCON
+   select REGMAP
+   help
+ Support for the MMC sub clock controller on Amlogic Meson Platform,
+ which include S905 (GXBB, GXL), A113D/X (AXG) devices.
+ Say Y if you want this clock enabled.
+
 config COMMON_CLK_REGMAP_MESON
bool
select REGMAP
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 39ce5661b654..31c16d524a4b 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -9,4 +9,5 @@ obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
 obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
 obj-$(CONFIG_COMMON_CLK_AXG)+= axg.o axg-aoclk.o
 obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
+obj-$(CONFIG_COMMON_CLK_MMC_MESON) += mmc-clkc.o
 obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)  += clk-regmap.o
diff --git a/drivers/clk/meson/mmc-clkc.c b/drivers/clk/meson/mmc-clkc.c
new file mode 100644
index ..6aa055f7e62c
--- /dev/null
+++ b/drivers/clk/meson/mmc-clkc.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Amlogic Meson MMC Sub Clock Controller Driver
+ *
+ * Copyright (c) 2017 Baylibre SAS.
+ * Author: Jerome Brunet 
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Yixun Lan 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "clkc.h"
+
+/* clock ID used by internal driver */
+#define CLKID_MMC_MUX  0
+
+#define SD_EMMC_CLOCK  0
+#define   CLK_DIV_MASK GENMASK(5, 0)
+#define   CLK_SRC_MASK GENMASK(7, 6)
+#define   CLK_CORE_PHASE_MASK  GENMASK(9, 8)
+#define   CLK_TX_PHASE_MASKGENMASK(11, 10)
+#define   CLK_RX_PHASE_MASKGENMASK(13, 12)
+#define   CLK_V2_TX_DELAY_MASK GENMASK(19, 16)
+#define   CLK_V2_RX_DELAY_MASK GENMASK(23, 20)
+#define   CLK_V2_ALWAYS_ON BIT(24)
+
+#define   CLK_V3_TX_DELAY_MASK GENMASK(21, 16)
+#define   CLK_V3_RX_DELAY_MASK GENMASK(27, 22)
+#define   CLK_V3_ALWAYS_ON BIT(28)
+
+#define   CLK_DELAY_STEP_PS200
+#define   CLK_PHASE_STEP   30
+#define   CLK_PHASE_POINT_NUM  (360 / CLK_PHASE_STEP)
+
+#define MUX_CLK_NUM_PARENTS2
+#define MMC_MAX_CLKS   5
+
+struct mmc_clkc_data {
+   struct meson_clk_phase_delay_data   tx;
+   struct meson_clk_phase_delay_data   rx;
+};
+
+static struct clk_regmap_mux_data mmc_clkc_mux_data = {
+   .offset = SD_EMMC_CLOCK,
+   .mask   = 0x3,
+   .shift  = 6,
+   .flags  = CLK_DIVIDER_ROUND_CLOSEST,
+};
+
+static struct clk_regmap_div_data mmc_clkc_div_data = {
+   .offset = SD_EMMC_CLOCK,
+   .shift  = 0,
+   .width  = 6,
+   .flags  = CLK_DIVIDER_ROUND_CLOSEST | CLK_DIVIDER_ONE_BASED,
+};
+
+static struct meson_clk_phase_delay_data mmc_clkc_core_phase_delay = {
+   .phase_mask = CLK_CORE_PHASE_MASK,
+};
+
+static const struct mmc_clkc_data mmc_clkc_gx_data = {
+   {
+   .phase_mask = CLK_TX_PHASE_MASK,
+   .delay_mask = CLK_V2_TX_DELAY_MASK,
+   .delay_step_ps  = CLK_DELAY_STEP_PS,
+   },
+   {
+   .phase_mask = CLK_RX_PHASE_MASK,
+   .delay_mask = CLK_V2_RX_DELAY_MASK,
+   .delay_step_ps  = CLK_DELAY_STEP_PS,
+   },
+};
+
+static const struct mmc_clkc_data mmc_clkc_axg_data = {
+   {
+   .phase_mask = CLK_TX_PHASE_MASK,
+   .delay_mask = CLK_V3_TX_DELAY_MASK,
+   .delay_step_ps  = CLK_DELAY_STEP_PS,
+   },
+   {
+   .phase_mask = CLK_RX_PHASE_MAS

[PATCH v4 2/3] clk: meson: add DT documentation for emmc clock controller

[Yixun Lan]

Document the MMC sub clock controller driver, the potential consumer
of this driver is MMC or NAND. Also add four clock bindings IDs which
provided by this driver.

Reviewed-by: Rob Herring 
Signed-off-by: Yixun Lan 
---
 .../bindings/clock/amlogic,mmc-clkc.txt   | 31 +++
 include/dt-bindings/clock/amlogic,mmc-clkc.h  | 17 ++
 2 files changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
 create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h

diff --git a/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt 
b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
new file mode 100644
index ..9e6d34389be8
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
@@ -0,0 +1,31 @@
+* Amlogic MMC Sub Clock Controller Driver
+
+The Amlogic MMC clock controller generates and supplies clock to support
+MMC and NAND controller
+
+Required Properties:
+
+- compatible: should be:
+   "amlogic,gx-mmc-clkc"
+   "amlogic,axg-mmc-clkc"
+
+- #clock-cells: should be 1.
+- clocks: phandles to clocks corresponding to the clock-names property
+- clock-names: list of parent clock names
+   - "clkin0", "clkin1"
+
+Parent node should have the following properties :
+- compatible: "amlogic,axg-mmc-clkc", "syscon".
+- reg: base address and size of the MMC control register space.
+
+Example: Clock controller node:
+
+sd_mmc_c_clkc: clock-controller@7000 {
+   compatible = "amlogic,axg-mmc-clkc", "syscon";
+   reg = <0x0 0x7000 0x0 0x4>;
+   #clock-cells = <1>;
+
+   clock-names = "clkin0", "clkin1";
+   clocks = <&clkc CLKID_SD_MMC_C_CLK0>,
+<&clkc CLKID_FCLK_DIV2>;
+};
diff --git a/include/dt-bindings/clock/amlogic,mmc-clkc.h 
b/include/dt-bindings/clock/amlogic,mmc-clkc.h
new file mode 100644
index ..162b94949119
--- /dev/null
+++ b/include/dt-bindings/clock/amlogic,mmc-clkc.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Meson MMC sub clock tree IDs
+ *
+ * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
+ * Author: Yixun Lan 
+ */
+
+#ifndef __MMC_CLKC_H
+#define __MMC_CLKC_H
+
+#define CLKID_MMC_DIV  1
+#define CLKID_MMC_PHASE_CORE   2
+#define CLKID_MMC_PHASE_TX 3
+#define CLKID_MMC_PHASE_RX 4
+
+#endif
-- 
2.17.1

[PATCH v4 1/3] clk: meson: add emmc sub clock phase delay driver

[Yixun Lan]

Export the emmc sub clock phase delay ops which will be used
by the emmc sub clock driver itself.

Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/Makefile  |  2 +-
 drivers/clk/meson/clk-phase-delay.c | 96 +
 drivers/clk/meson/clkc.h| 13 
 3 files changed, 110 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/meson/clk-phase-delay.c

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 72ec8c40d848..39ce5661b654 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -2,7 +2,7 @@
 # Makefile for Meson specific clk
 #
 
-obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-phase.o
+obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-phase.o 
clk-phase-delay.o
 obj-$(CONFIG_COMMON_CLK_AMLOGIC_AUDIO) += clk-triphase.o sclk-div.o
 obj-$(CONFIG_COMMON_CLK_MESON_AO) += meson-aoclk.o
 obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
diff --git a/drivers/clk/meson/clk-phase-delay.c 
b/drivers/clk/meson/clk-phase-delay.c
new file mode 100644
index ..6f226814cfec
--- /dev/null
+++ b/drivers/clk/meson/clk-phase-delay.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Amlogic Meson MMC Sub Clock Controller Driver
+ *
+ * Copyright (c) 2017 Baylibre SAS.
+ * Author: Jerome Brunet 
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Yixun Lan 
+ */
+
+#include 
+#include "clkc.h"
+
+#define SD_EMMC_CLOCK  0
+
+static int meson_clk_phase_delay_get_phase(struct clk_hw *hw)
+{
+   struct clk_regmap *clk = to_clk_regmap(hw);
+   struct meson_clk_phase_delay_data *ph =
+   meson_clk_get_phase_delay_data(clk);
+   unsigned int phase_num = 1 <<  hweight_long(ph->phase_mask);
+   unsigned long period_ps, p, d;
+   int degrees;
+   u32 val;
+
+   regmap_read(clk->map, SD_EMMC_CLOCK, &val);
+   p = (val & ph->phase_mask) >> __ffs(ph->phase_mask);
+   degrees = p * 360 / phase_num;
+
+   if (ph->delay_mask) {
+   period_ps = DIV_ROUND_UP((unsigned long)NSEC_PER_SEC * 1000,
+clk_hw_get_rate(hw));
+   d = (val & ph->delay_mask) >> __ffs(ph->delay_mask);
+   degrees += d * ph->delay_step_ps * 360 / period_ps;
+   degrees %= 360;
+   }
+
+   return degrees;
+}
+
+static void meson_clk_apply_phase_delay(struct clk_regmap *clk,
+   unsigned int phase,
+   unsigned int delay)
+{
+   struct meson_clk_phase_delay_data *ph = clk->data;
+   u32 val;
+
+   regmap_read(clk->map, SD_EMMC_CLOCK, &val);
+
+   val &= ~ph->phase_mask;
+   val |= phase << __ffs(ph->phase_mask);
+
+   if (ph->delay_mask) {
+   val &= ~ph->delay_mask;
+   val |= delay << __ffs(ph->delay_mask);
+   }
+
+   regmap_write(clk->map, SD_EMMC_CLOCK, val);
+}
+
+static int meson_clk_phase_delay_set_phase(struct clk_hw *hw, int degrees)
+{
+   struct clk_regmap *clk = to_clk_regmap(hw);
+   struct meson_clk_phase_delay_data *ph =
+   meson_clk_get_phase_delay_data(clk);
+   unsigned int phase_num = 1 <<  hweight_long(ph->phase_mask);
+   unsigned long period_ps, d = 0, r;
+   u64 p;
+
+   p = degrees % 360;
+
+   if (!ph->delay_mask) {
+   p = DIV_ROUND_CLOSEST_ULL(p, 360 / phase_num);
+   } else {
+   period_ps = DIV_ROUND_UP((unsigned long)NSEC_PER_SEC * 1000,
+clk_hw_get_rate(hw));
+
+   /* First compute the phase index (p), the remainder (r) is the
+* part we'll try to acheive using the delays (d).
+*/
+   r = do_div(p, 360 / phase_num);
+   d = DIV_ROUND_CLOSEST(r * period_ps,
+ 360 * ph->delay_step_ps);
+   d = min(d, ph->delay_mask >> __ffs(ph->delay_mask));
+   }
+
+   meson_clk_apply_phase_delay(clk, p, d);
+   return 0;
+}
+
+const struct clk_ops meson_clk_phase_delay_ops = {
+   .get_phase = meson_clk_phase_delay_get_phase,
+   .set_phase = meson_clk_phase_delay_set_phase,
+};
+EXPORT_SYMBOL_GPL(meson_clk_phase_delay_ops);
diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h
index 24cec16b6038..499834dd34f2 100644
--- a/drivers/clk/meson/clkc.h
+++ b/drivers/clk/meson/clkc.h
@@ -113,6 +113,18 @@ struct clk_regmap _name = {
\
},  \
 };
 
+struct meson_clk_phase_delay_data {
+   unsigned long   phase_mask;
+   unsigned long   delay_mask;
+   unsigned intd

Re: [PATCH] arm64: dts: meson: Switch simple-mfd and syscon order

[Yixun Lan]

HI Neil

On 07/26/2018 10:13 PM, Neil Armstrong wrote:
> The order between "syscon" and "simple-mfd" is important because in these
> particular cases, the node needs to be first a "simple-mfd" to expose
> it's sub-nodes, and later on a "syscon" to permit other nodes to access
> this register space through the "syscon" mechanism.
> 

could you explain a bit which specific case that it need compatible
"simple-mfd" come first? is this a problem in kernel side? because I see
you have a comment below about problem in U-Boot..


also while Rob review the mmc-clkc patch, he raise a suggestion to  drop
"syscon" in drivers/mfd/syscon.c, see [1], will that solve or affect
your problem?

[1]
https://lkml.kernel.org/r/CAL_Jsq+r+7BaJAPBn3L26sdxATVyDiuC=v3xe+yz0-27q0r...@mail.gmail.com

> Signed-off-by: Neil Armstrong 
> ---
> This also has a consequence for U-Boot since the U-Boot DM is much simpler
> and can only associate a single device to a node, thus here we want these
> nodes to be an mfd bus to be able to probe the sub-nodes.
> 
>  arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 4 ++--
>  arch/arm64/boot/dts/amlogic/meson-gx.dtsi  | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
> b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> index 6adc8d3..3f0de9d 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> @@ -342,7 +342,7 @@
>   ranges = <0x0 0x0 0x0 0xff63c000 0x0 0x1c00>;
>  
>   sysctrl: system-controller@0 {
> - compatible = "amlogic,meson-axg-hhi-sysctrl", 
> "syscon", "simple-mfd";
> + compatible = "amlogic,meson-axg-hhi-sysctrl", 
> "simple-mfd", "syscon";
>   reg = <0 0 0 0x400>;
>  
>   clkc: clock-controller {
> @@ -1180,7 +1180,7 @@
>   ranges = <0x0 0x0 0x0 0xff80 0x0 0x10>;
>  
>   sysctrl_AO: sys-ctrl@0 {
> - compatible = "amlogic,meson-axg-ao-sysctrl", 
> "syscon", "simple-mfd";
> + compatible = "amlogic,meson-axg-ao-sysctrl", 
> "simple-mfd", "syscon";
>   reg =  <0x0 0x0 0x0 0x100>;
>  
>   clkc_AO: clock-controller {
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi 
> b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> index b8dc4db..d88699f 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> @@ -344,7 +344,7 @@
>   ranges = <0x0 0x0 0x0 0xc810 0x0 0x10>;
>  
>   sysctrl_AO: sys-ctrl@0 {
> - compatible = "amlogic,meson-gx-ao-sysctrl", 
> "syscon", "simple-mfd";
> + compatible = "amlogic,meson-gx-ao-sysctrl", 
> "simple-mfd", "syscon";
>   reg =  <0x0 0x0 0x0 0x100>;
>  
>   pwrc_vpu: power-controller-vpu {
> @@ -431,7 +431,7 @@
>   ranges = <0x0 0x0 0x0 0xc883c000 0x0 0x2000>;
>  
>   sysctrl: system-controller@0 {
> - compatible = "amlogic,meson-gx-hhi-sysctrl", 
> "syscon", "simple-mfd";
> + compatible = "amlogic,meson-gx-hhi-sysctrl", 
> "simple-mfd", "syscon";
>   reg = <0 0 0 0x400>;
>   };
>  
> 

Re: [PATCH v3 1/2] clk: meson: add DT documentation for emmc clock controller

[Yixun Lan]

HI Rob

On 07/25/2018 07:29 AM, Rob Herring wrote:
> On Thu, Jul 12, 2018 at 09:12:43PM +0000, Yixun Lan wrote:
>> Document the MMC sub clock controller driver, the potential consumer
>> of this driver is MMC or NAND. Also add three clock bindings IDs which
>> provided by this driver.
>>
>> Signed-off-by: Yixun Lan 
>> ---
>>  .../bindings/clock/amlogic,mmc-clkc.txt   | 31 +++
>>  .../clock/amlogic,meson-mmc-clkc.h| 16 ++
>>  2 files changed, 47 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
>>  create mode 100644 include/dt-bindings/clock/amlogic,meson-mmc-clkc.h
> 
> Reviewed-by: Rob Herring 
> 

Thanks for the review!

There is a discussion about dropping "meson-" prefix in the compatible
string[1]

So, I will send another version with the new compatible name adjusted
from "amlogic,meson-axg-mmc-clkc" to "amlogic,axg-mmc-clkc"..
probably also rename the dt-bindings header file to amlogc,mmc-clkc.h

[1]  https://lkml.kernel.org/r/7hk1prmg4w@baylibre.com


Yixun

Re: [PATCH v3 2/2] clk: meson: add sub MMC clock controller driver

[Yixun Lan]

HI Stephen:

On 07/26/2018 11:20 PM, Stephen Boyd wrote:
> Quoting Yixun Lan (2018-07-12 14:12:44)
>> diff --git a/drivers/clk/meson/mmc-clkc.c b/drivers/clk/meson/mmc-clkc.c
>> new file mode 100644
>> index ..36c4c7cd69a6
>> --- /dev/null
>> +++ b/drivers/clk/meson/mmc-clkc.c
>> @@ -0,0 +1,367 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Amlogic Meson MMC Sub Clock Controller Driver
>> + *
>> + * Copyright (c) 2017 Baylibre SAS.
>> + * Author: Jerome Brunet 
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Yixun Lan 
>> + */
>> +
>> +#include 
> 
> Is this include used?
> 
this is needed by clk_get_rate()
see drivers/clk/meson/mmc-clkc.c:204

>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "clkc.h"
>> +
>> +
>> +static struct clk_regmap *
>> +mmc_clkc_register_other_clk(struct device *dev, struct regmap *map,
>> +   char *suffix, char *parent_suffix,
>> +   unsigned long flags,
>> +   const struct clk_ops *ops, void *data)
>> +{
>> +   struct clk_init_data init;
>> +   struct clk_regmap *clk;
>> +   char *parent;
>> +
>> +   parent = kasprintf(GFP_KERNEL, "%s#%s", dev_name(dev), 
>> parent_suffix);
>> +   if (!parent)
>> +   return ERR_PTR(-ENOMEM);
>> +
>> +   init.ops = ops;
>> +   init.flags = flags;
>> +   init.parent_names = (const char* const []){ parent, };
> 
> Can't this just be &parent?
sure, I can fix this

> 
>> +   init.num_parents = 1;
>> +
>> +   clk = mmc_clkc_register_clk(dev, map, &init, suffix, data);
>> +   if (IS_ERR(clk))
>> +   dev_err(dev, "Core %s clock registration failed\n", suffix);
>> +
>> +   kfree(parent);
>> +   return clk;
>> +}
>> +
>> +static int mmc_clkc_probe(struct platform_device *pdev)
>> +{
>> +   struct clk_hw_onecell_data *onecell_data;
>> +   struct device *dev = &pdev->dev;
>> +   struct mmc_clkc_data *data;
>> +   struct regmap *map;
>> +   struct clk_regmap *mux, *div, *core, *rx, *tx;
>> +
>> +   data = (struct mmc_clkc_data *)of_device_get_match_data(dev);
> 
> This cast is unnecessary. Pleas remove.
> 
Ok, I will try to fix in next version


this was trying to silence the ’const‘ cast warning [1]
I could make a 'const struct mmc_clkc_data *data' declare, but need to
fix further cast warning issue..


[1] drivers/clk/meson/mmc-clkc.c: In function ‘mmc_clkc_probe’:
drivers/clk/meson/mmc-clkc.c:302:7: warning: assignment discards ‘const’
qualifier from pointer target type [-Wdiscarded-qualifiers]
  data = of_device_get_match_data(dev);
   ^

Re: [PATCH 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller

[Yixun Lan]

HI Boris:

thanks for the quick response.

On 07/19/18 03:08, Boris Brezillon wrote:
> Hi Yixun,
> 
> On Wed, 18 Jul 2018 17:38:56 +0800
> Yixun Lan  wrote:
> 
>>>> +
>>>> +#define NFC_REG_CMD   0x00
>>>> +#define NFC_REG_CFG   0x04
>>>> +#define NFC_REG_DADR  0x08
>>>> +#define NFC_REG_IADR  0x0c
>>>> +#define NFC_REG_BUF   0x10
>>>> +#define NFC_REG_INFO  0x14
>>>> +#define NFC_REG_DC0x18
>>>> +#define NFC_REG_ADR   0x1c
>>>> +#define NFC_REG_DL0x20
>>>> +#define NFC_REG_DH0x24
>>>> +#define NFC_REG_CADR  0x28
>>>> +#define NFC_REG_SADR  0x2c
>>>> +#define NFC_REG_PINS  0x30
>>>> +#define NFC_REG_VER   0x38
>>>> +  
>>>
>>> Can you put the reg offsets next to their field definitions?
>>>   
>> actually, we would prefer to put all the CMD definition below the reg
>> offset, so it will better reflect what's it belong to.
> 
> Just to be clear, I meant something like:
> 
> #define NFC_CMD   0x00
> #define NFC_CMD_DRD   (0x8 << 14)
> #define NFC_CMD_IDLE  (0xc << 14)
> ...
> 
> #define NFC_CFG   0x04
> #define NFC_CFG_XXX   xxx
> ...
> 
> I find it easier to guess which register the fields are attached to when
> it's defined like that, but I won't block the driver for such a tiny
> detail. 
> 
yes, this is exactly what I mean

>>>> +static void meson_nfc_cmd_ctrl(struct mtd_info *mtd,
>>>> +  int cmd, unsigned int ctrl)  
>>>   
>>> ->cmd_ctrl() has recently been deprecated in favor of ->exec_op(). You  
>>> can have a look at the marvell, v610 or fsmc drivers if you want to
>>> have an idea of how ->exec_op() should be implemented. Miquel and I are
>>> also here to help if you have any questions.
>>>   
>>
>> follow your suggestion, we have implemented the exec_op() interface,
>> we'd really appreciate if you can help to review this ..
> 
> Sure, just send a v2 and we'll review it.
> 
> 
>>>> +
>>>> +static void meson_nfc_cmd_m2n(struct meson_nfc *nfc, int raw)  
>>>
>>> n2m -> nand2mem ?
>>>   
>> yes, it is
> 
> Then please use nand2mem, it's clearer.
we end at dropping the n2m function. by converting them into

static void
meson_nfc_cmd_access(
struct meson_nfc *nfc,
struct mtd_info *mtd, int raw, bool dir)


> 
>>>> +static int meson_nfc_wait_dma_finish(struct meson_nfc *nfc)
>>>> +{
>>>> +  meson_nfc_cmd_idle(nfc, 0);
>>>> +  meson_nfc_cmd_idle(nfc, 0);  
>>>
>>> Two calls to cmd_idle(), is this expected or a copy&paste error? If
>>> that's expected it definitely deserves a comment explaining why?
>>>   
>>
>> yes, it is intentional
>>
>> we will put these comments into the function.
>>  /*
>>  * The Nand flash controller is designed as two stages pipleline -
>>  *  a) fetch and b) excute.
>>  * So, there might be cases when the driver see command queue is
>> empty,
>>  * but the Nand flash controller still has two commands buffered,
>>  * one is fetched into NFC request queue (ready to run), and another
>>  * is actively executing.
>>  */
>>
> 
> So pushing 2 "IDLE" commands guarantees that the pipeline is emptied,
> right? The comment looks incomplete, you should explain what those
> meson_nfc_cmd_idle() are for.
> 
thanks

the meson_nfc_cmd_idle() function itself is quite straightforward, and
we feel explain that inserting 2 "IDLE" commands to drain out the
pipeline is enough.

>>>> +static int meson_nfc_ecc_init(struct device *dev, struct mtd_info *mtd)
>>>> +{
>>>> +  struct nand_chip *nand = mtd_to_nand(mtd);
>>>> +  struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>>>> +  struct meson_nfc *nfc = nand_get_controller_data(nand);
>>>> +  struct meson_nand_ecc *meson_ecc = nfc->data->ecc;
>>>> +  int num = nfc->data->ecc_num;
>>>> +  int nsectors, i, bytes;
>>>> +
>>>> +  /* support only ecc hw mode */
>>>> +  if (nand->ecc.mode != NAND_ECC_HW) {  
&

[RFC PATCH v2 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller

[Yixun Lan]

Add initial support for the Amlogic NAND flash controller which found
in the Meson-GXBB/GXL/AXG SoCs.

Signed-off-by: Liang Yang 
Signed-off-by: Yixun Lan 
---
 drivers/mtd/nand/raw/Kconfig  |   10 +
 drivers/mtd/nand/raw/Makefile |1 +
 drivers/mtd/nand/raw/meson_nand.c | 1333 +
 3 files changed, 1344 insertions(+)
 create mode 100644 drivers/mtd/nand/raw/meson_nand.c

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 6871ff0fd300..d4a72b258b44 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -530,4 +530,14 @@ config MTD_NAND_MTK
  Enables support for NAND controller on MTK SoCs.
  This controller is found on mt27xx, mt81xx, mt65xx SoCs.
 
+config MTD_NAND_MESON
+   tristate "Support for NAND controller on Amlogic's Meson SoCs"
+   depends on ARCH_MESON || COMPILE_TEST
+   depends on COMMON_CLK_AMLOGIC
+   select COMMON_CLK_REGMAP_MESON
+   select MFD_SYSCON
+   help
+ Enables support for NAND controller on Amlogic's Meson SoCs.
+ This controller is found on Meson GXBB, GXL, AXG SoCs.
+
 endif # MTD_NAND
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index 165b7ef9e9a1..6e9101f7b855 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_HISI504)+= 
hisi504_nand.o
 obj-$(CONFIG_MTD_NAND_BRCMNAND)+= brcmnand/
 obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o
 obj-$(CONFIG_MTD_NAND_MTK) += mtk_ecc.o mtk_nand.o
+obj-$(CONFIG_MTD_NAND_MESON)   += meson_nand.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o
 nand-objs += nand_amd.o
diff --git a/drivers/mtd/nand/raw/meson_nand.c 
b/drivers/mtd/nand/raw/meson_nand.c
new file mode 100644
index ..2458312f22fa
--- /dev/null
+++ b/drivers/mtd/nand/raw/meson_nand.c
@@ -0,0 +1,1333 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Amlogic Meson Nand Flash Controller Driver
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Liang Yang 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define NFC_REG_CMD0x00
+#define   NFC_CMD_DRD  (0x8 << 14)
+#define   NFC_CMD_IDLE (0xc << 14)
+#define   NFC_CMD_DWR  (0x4 << 14)
+#define   NFC_CMD_CLE  (0x5 << 14)
+#define   NFC_CMD_ALE  (0x6 << 14)
+#define   NFC_CMD_ADL  ((0 << 16) | (3 << 20))
+#define   NFC_CMD_ADH  ((1 << 16) | (3 << 20))
+#define   NFC_CMD_AIL  ((2 << 16) | (3 << 20))
+#define   NFC_CMD_AIH  ((3 << 16) | (3 << 20))
+#define   NFC_CMD_SEED ((8 << 16) | (3 << 20))
+#define   NFC_CMD_M2N  ((0 << 17) | (2 << 20))
+#define   NFC_CMD_N2M  ((1 << 17) | (2 << 20))
+#define   NFC_CMD_RB   BIT(20)
+#define   NFC_CMD_IO6  ((0xb << 10) | (1 << 18))
+
+#define NFC_REG_CFG0x04
+#define NFC_REG_DADR   0x08
+#define NFC_REG_IADR   0x0c
+#define NFC_REG_BUF0x10
+#define NFC_REG_INFO   0x14
+#define NFC_REG_DC 0x18
+#define NFC_REG_ADR0x1c
+#define NFC_REG_DL 0x20
+#define NFC_REG_DH 0x24
+#define NFC_REG_CADR   0x28
+#define NFC_REG_SADR   0x2c
+#define NFC_REG_PINS   0x30
+#define NFC_REG_VER0x38
+
+#define CMDRWGEN(cmd_dir, ran, bch, short_mode, page_size, pages)  \
+   (   \
+   (cmd_dir)   |   \
+   ((ran) << 19)   |   \
+   ((bch) << 14)   |   \
+   ((short_mode) << 13)|   \
+   (((page_size) & 0x7f) << 6) |   \
+   ((pages) & 0x3f)\
+   )
+
+#define GENCMDDADDRL(adl, addr)((adl) | ((addr) & 0x))
+#define GENCMDDADDRH(adh, addr)((adh) | (((addr) >> 16) & 
0x))
+#define GENCMDIADDRL(ail, addr)((ail) | ((addr) & 0x))
+#define GENCMDIADDRH(aih, addr)((aih) | (((addr) >> 16) & 
0x))
+
+#define RB_STA(x)  (1 << (26 + (x)))
+#defineDMA_DIR(dir)((dir) ? NFC_CMD_N2M : 
NFC_CMD_M2N)
+
+#define NAND_TWB_TIME_CYCLE10
+
+#define ECC_CHECK_RETURN_FF(-1)
+
+#define NAND_CE0   (0xe << 10)
+#define NAND_CE1 

[RFC PATCH v2 0/2] mtd: rawnand: meson: add Amlogic NAND driver support

[Yixun Lan]

  These two patches try to add initial NAND driver support for Amlogic Meson
SoCs, current it has been tested on GXL(p212) and AXG(s400) platform.

  Note this patch series actually depend on the eMMC clkc patch[2] which
still not merged.

Changes since v1 at [1]:
   - adopt property amlogic,nand-enable-scrambler - thanks Martin
   - drop nand pins in DT
   - convert clk access to emmc clkc model 
   - fix regiser field definition alignment
   - convert devm_kmalloc to kmalloc API
   - drop nand-user-mode 
   - parse cs id from DT
   - rework n2m, m2n function
   - explain why insert two "IDLE" command
   - implement exec_op()
   - drop meson_nfc_get_nand_chip_dts()
   - release resource once error occur in meson_nfc_nand_chips_init(), 
   - call nand_cleanup(nand) once mtd_device_register fail

Items not addressed in this version:
   - convert to ECC conf helper() 
   - how to construct mtd->name

[1] https://lkml.kernel.org/r/20180613161314.14894-1-yixun@amlogic.com
[2] https://lkml.kernel.org/r/20180712211244.11428-1-yixun@amlogic.com

Liang Yang (1):
  dt-bindings: nand: meson: add Amlogic NAND controller driver

Yixun Lan (1):
  mtd: rawnand: meson: add support for Amlogic NAND flash controller

 .../bindings/mtd/amlogic,meson-nand.txt   |   95 ++
 drivers/mtd/nand/raw/Kconfig  |   10 +
 drivers/mtd/nand/raw/Makefile |1 +
 drivers/mtd/nand/raw/meson_nand.c | 1333 +
 4 files changed, 1439 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
 create mode 100644 drivers/mtd/nand/raw/meson_nand.c

-- 
2.18.0

[RFC PATCH v2 1/2] dt-bindings: nand: meson: add Amlogic NAND controller driver

[Yixun Lan]

From: Liang Yang 

Add Amlogic NAND controller dt-bindings for Meson SoC,
Current this driver support GXBB/GXL/AXG platform.

Signed-off-by: Liang Yang 
Signed-off-by: Yixun Lan 
---
 .../bindings/mtd/amlogic,meson-nand.txt   | 95 +++
 1 file changed, 95 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt

diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt 
b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
new file mode 100644
index ..31f910dcd27a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
@@ -0,0 +1,95 @@
+Amlogic NAND Flash Controller (NFC) for GXBB/GXL/AXG family SoCs
+
+This file documents the properties in addition to those available in
+the MTD NAND bindings.
+
+Required properties:
+- compatible : contains one of:
+  - "amlogic,meson-gxl-nfc"
+  - "amlogic,meson-axg-nfc"
+- clocks :
+   A list of phandle + clock-specifier pairs for the clocks listed
+   in clock-names.
+
+- clock-names: Should contain the following:
+   "core" - NFC module gate clock
+   "device" - device clock from eMMC sub clock controller
+
+- pins : Select pins which NFC need.
+- nand_pins: Detail NAND pins information.
+- amlogic,mmc-syscon   : Required for NAND clocks, it's shared with SD/eMMC
+   controller port C
+
+Optional children nodes:
+Children nodes represent the available nand chips.
+
+Optional properties:
+- amlogic,nand-enable-scrambler: enable the NAND scrambler feature.
+   - (absent) = scrambler is disabled
+   - (present) = scrambler is enabled
+
+
+Other properties:
+see Documentation/devicetree/bindings/mtd/nand.txt for generic bindings.
+
+Example demonstrate on AXG SoC:
+
+   sd_emmc_c_clkc: mmc@7000 {
+   compatible = "amlogic,meson-axg-mmc-clkc", "syscon";
+   reg = <0x0 0x7000 0x0 0x800>;
+   status = "okay";
+   };
+
+   nand: nfc@7800 {
+   compatible = "amlogic,meson-axg-nfc";
+   reg = <0x0 0x7800 0x0 0x100>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   interrupts = ;
+   status = "disabled";
+
+   clocks = <&clkc CLKID_SD_EMMC_C>,
+   <&sd_emmc_c_clkc CLKID_MMC_DIV>;
+   clock-names = "core", "device";
+   amlogic,mmc-syscon = <&sd_emmc_c_clkc>;
+
+   status = "okay";
+
+   pinctrl-names = "default";
+   pinctrl-0 = <&nand_pins>;
+
+   nand@0 {
+   reg = <0>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   nand-on-flash-bbt;
+   nand-ecc-mode = "hw";
+   nand-ecc-strength = <8>;
+   nand-ecc-step-size = <1024>;
+
+   amlogic,nand-enable-scrambler;
+
+   partition@0 {
+   label = "boot";
+   reg = <0x 0x0020>;
+   read-only;
+   };
+   partition@20 {
+   label = "env";
+   reg = <0x0020 0x0040>;
+   };
+   partition@60 {
+   label = "system";
+   reg = <0x0060 0x00a0>;
+   };
+   partition@100 {
+   label = "rootfs";
+   reg = <0x0100 0x0300>;
+   };
+   partition@400 {
+   label = "media";
+   reg = <0x0400 0x800>;
+   };
+   };
+   };
-- 
2.18.0

Re: [PATCH 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller

[Yixun Lan]

HI Boris


On 07/19/18 16:39, Boris Brezillon wrote:
> Hi Yixun,
> 
> On Thu, 19 Jul 2018 16:13:47 +0800
> Yixun Lan  wrote:
> 
>>>>> You're doing DMA on those buffers, and devm_kzalloc() is not
>>>>> DMA-friendly (returned buffers are not aligned on a cache line). Also,
>>>>> you don't have to allocate your own buffers because the core already
>>>>> allocate them (chip->data_buf, chip->oob_poi). All you need to do is
>>>>> set the NAND_USE_BOUNCE_BUFFER flag in chip->options to make sure
>>>>> you're always passed a DMA-able buffer.
>>>>> 
>>>>
>>>> thanks for the suggestion, we've migrated to use the
>>>> dmam_alloc_coherent() API  
>>>
>>> kzalloc() should be just fine, no need to alloc a DMA coherent region. 
>>>   
>>
>> we're a little bit confused here, isn't devm_kzalloc (previously we are
>> using) a variant of kzalloc? and since the NAND controller is doing DMA
>> here, using DMA coherent API is more proper way?
> 
> Well, making buffers DMA coherent might be expensive, especially if you
> access them a lot (unless you have a coherency unit and the cache is
> kept enabled).
> 
> Regarding the "why is devm_kzalloc() is not DMA-safe?" question, I'd
> recommend that you read this discussion [1].
> 
great, thanks for the info.

we fixed this in patch v2

>>>>>> +mtd->name = devm_kasprintf(nfc->dev, GFP_KERNEL,
>>>>>> +   "%s:nand", dev_name(dev));
>>>>>> +if (!mtd->name) {
>>>>>> +dev_err(nfc->dev, "Failed to allocate mtd->name\n");
>>>>>> +return -ENOMEM;
>>>>>> +}
>>>>>
>>>>> You set the name after nand_scan_ident() and make it conditional (only
>>>>> if ->name == NULL) so that the label property defined in the DT takes
>>>>> precedence over the default name.
>>>>  
>> for setting mtd->name conditional, do you mean doing something like this?
>>
>> if (!mtd->name)
>>  mtd->name = devm_kasprintf(..)
> 
> Yes, that's what I meant.
> 
>>
>> but we found mtd->name = "ffe07800.nfc" after function
>> nand_scan_ident(), which is same value as dev_name(dev)..
>> and there is no cs information encoded there.
> 
> Hm, that shouldn't be the case. Maybe you can add traces to find out
> who is setting mtd->name to this value.
> 
will trace this, then get back to you
>>
>>>>  
>>>>> Also, I recommend suffixing this name
>>>>> with the CS id, just in case you ever need to support connecting several
>>>>> chips to the same controller. 
>>>>> 
>>>>
>>>> we actually didn't get the point here, cs is about chip selection with
>>>> multiple nand chip? and how to get this information?  
>>>
>>> Well, you currently seem to only support one chip per controller, but I
>>> guess the IP can handle several CS lines. So my recommendation is about
>>> choosing a name so that you can later easily add support for multiple
>>> chips without breaking setups where mtdparts is used.
>>>
>>> To sum-up, assuming your NAND chip is always connected to CS0 (on the
>>> controller side), I'd suggest doing:
>>>   
>> yes, this is exactly how the hardware connected.
>>> mtd->name = devm_kasprintf(nfc->dev, GFP_KERNEL,
>>>"%s:nand.%d", dev_name(dev), cs_id);
>>>
>>> where cs_id is the value you extracted from the reg property of the
>>> NAND node.
>>>   
>> Ok, you right.
>> current, the NAND chip is only use one CS (which CE0) for now, what's in
>> the DT is
>>
>> nand@0 {
>>  reg = < 0 >;
>>  ..
>> };
>>
>> so for the multiple chips it would something like this in DT?
>>
>> nand@0 {
>>   reg = < 0 >;
>> };
>>
>> nand@1 {
>>   reg = < 1 >;
>> };
> 
> Yep, that's for 2 single-die chips.
> 
>>
>> or even
>> nand@0 {
>>   reg = < 0 2 >;
>> };
>>
>> nand@1 {
> 
> nand@3 {
> 
>>   reg = < 3 4 >;
>> };
> 
> And this is describing 2 dual-die chips.
> 
>>
>> do we need to encode all the cs information here? not sure if we
>> understand this correctly, but could send out the patch for review..
> 
> Yes, reg should contain an array of controller-side CS lines used to
> select the chip (or a specific die in a chip, the index in the reg
> table being the id of the die).
>
much clear about this, thanks

Yixun

Re: [RFC PATCH v2 1/2] dt-bindings: nand: meson: add Amlogic NAND controller driver

[Yixun Lan]

Hi Boris:
 see my comments, thanks for the quick response

On 07/19/18 17:57, Boris Brezillon wrote:
> On Thu, 19 Jul 2018 17:46:11 +0800
> Yixun Lan  wrote:
> 
>> From: Liang Yang 
>>
>> Add Amlogic NAND controller dt-bindings for Meson SoC,
>> Current this driver support GXBB/GXL/AXG platform.
>>
>> Signed-off-by: Liang Yang 
>> Signed-off-by: Yixun Lan 
>> ---
>>  .../bindings/mtd/amlogic,meson-nand.txt   | 95 +++
>>  1 file changed, 95 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt 
>> b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
>> new file mode 100644
>> index ..31f910dcd27a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
>> @@ -0,0 +1,95 @@
>> +Amlogic NAND Flash Controller (NFC) for GXBB/GXL/AXG family SoCs
>> +
>> +This file documents the properties in addition to those available in
>> +the MTD NAND bindings.
>> +
>> +Required properties:
>> +- compatible : contains one of:
>> +  - "amlogic,meson-gxl-nfc"
>> +  - "amlogic,meson-axg-nfc"
>> +- clocks :
>> +A list of phandle + clock-specifier pairs for the clocks listed
>> +in clock-names.
>> +
>> +- clock-names: Should contain the following:
>> +"core" - NFC module gate clock
>> +"device" - device clock from eMMC sub clock controller
>> +
>> +- pins : Select pins which NFC need.
>> +- nand_pins: Detail NAND pins information.
> 
> You should document pinctrl-0 and pinctrl-names, not pins and nand_pins.
> 
Ok, will fix this

>> +- amlogic,mmc-syscon: Required for NAND clocks, it's shared with 
>> SD/eMMC
>> +controller port C
> 
> Are you sure this is still needed, even after exposing MMC/NAND clks
> through the CCF?
> 
yes, the SD_EMMC_CLOCK register from eMMC space explore a few other bit
that not fit well into clock model, thus we still need to access them
from NAND driver, we know it's kind of ugly..

#define SD_EMMC_CLOCK   0x00
#define   CLK_ALWAYS_ON BIT(28)
#define   CLK_SELECT_NAND BIT(31)
#define   CLK_DIV_MASK  GENMASK(5, 0)

we probably could get rid of CLK_DIV_MASK, but need to keep other two


> You forgot
> - #address-cells
> - #size-cells
> - reg
> - interrupts
> - 
> 
will fix these
>> +
>> +Optional children nodes:
>> +Children nodes represent the available nand chips.
>> +
>> +Optional properties:
>> +- amlogic,nand-enable-scrambler: enable the NAND scrambler feature.
>> +- (absent) = scrambler is disabled
>> +- (present) = scrambler is enabled
> 
> I keep thinking this is not needed if you have the NAND chip properly
> defined (NAND_NEED_SCRAMBLING flag set in chip->options).
> 
Ok, we will try this flag
>> +
>> +
>> +Other properties:
>> +see Documentation/devicetree/bindings/mtd/nand.txt for generic bindings.
>> +
>> +Example demonstrate on AXG SoC:
>> +
>> +sd_emmc_c_clkc: mmc@7000 {
>> +compatible = "amlogic,meson-axg-mmc-clkc", "syscon";
>> +reg = <0x0 0x7000 0x0 0x800>;
>> +status = "okay";
>> +};
>> +
>> +nand: nfc@7800 {
>> +compatible = "amlogic,meson-axg-nfc";
>> +reg = <0x0 0x7800 0x0 0x100>;
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +interrupts = ;
>> +status = "disabled";
>> +
>> +clocks = <&clkc CLKID_SD_EMMC_C>,
>> +<&sd_emmc_c_clkc CLKID_MMC_DIV>;
>> +clock-names = "core", "device";
>> +amlogic,mmc-syscon = <&sd_emmc_c_clkc>;
>> +
>> +status = "okay";
>> +
>> +pinctrl-names = "default";
>> +pinctrl-0 = <&nand_pins>;
>> +
>> +nand@0 {
>> +reg = <0>;
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +
>> +nand-on-flash-bbt;
>> +nand-ecc-mode = "hw";
>> +nand-ecc-strength = <8>;
>> + 

Re: [PATCH 3/3] clk: meson-g12a: Add EE Clock controller driver

[Yixun Lan]

On 07/19/2018 10:06 PM, Kevin Hilman wrote:
> Yixun Lan  writes:
> 
> [...]
> 
>>>
>>> As Jerome said, I think consistency is an important goal, so leaving off
>>> the "meson-" for drivers/clk is fine with me.
>>>
>> ok
>>
>>> Also for consistency, for the rest of the drivers, keeping "meson-" is
>>> probably the right thing to do.
>>>
>> ok, for the above, you are talking about the already exist compatible
>> names? or the patches we will push in the future?
> 
> I'm talking about future patches.
> 
ok

>>> However, patches would be welcome to add compatibles without the "meson"
>>> for the rest of the drivers & documentation as well, but please do not
>>> remove any compatibles.
>>>
>> I assume you are talking about the patches in the future we will push?
>> to drop the "meson".
>>
>> I'm confused here, as the above two paragraphs conflict with each other
> 
> Yeah, what I wrote is confusing, sorry about that.
> 
> What I mean is that new patches should stay consistent with compatibles
> in their existing drivers/subsystems.  That means keeping the "meson-"
> prefix for everything except drivers/clk.
> 
sounds good to me.

> In addition to that, patches would be welcome for *adding* compatibles
> without "meson" to all the current drivers.
> 
not sure if I parse above correctly, so let me rephrase..

for "all the current drivers", do you mean all the drivers which already
accepted at mainline kernel, which are mmc, uart, spi, i2c ..

all of them follow "amlogic,meson-${SOC}-${FUNCTION}", so do you propose
to add compatbiles like "amlogic,${SOC}-${FUNCTION}" ?

take i2c for example, there is "amlogic,meson-axg-i2c" already, do you
want to add another "amlogic,axg-i2c" ? I feel this will adding more
confusion..

How about only following the "amlogic,${SOC}-${FUNCTION}" syntax in the
new added drivers in the future, for example NAND, PCIE driver which we
want to push (those drivers doesn't exist in current mainline kernel).


Yixun

[PATCH] clk: meson-axg: pcie: drop the mpll3 clock parent

[Yixun Lan]

We found the PCIe driver doesn't really work with
the mpll3 clock which is actually reserved for debug,
So drop it from the mux list.

Fixes: 33b89db68236 ("clk: meson-axg: add clocks required by pcie driver")
Tested-by: Jianxin Qin 
Signed-off-by: Yixun Lan 

---
hi Jerome:
  I'm sorry we found this during latest PCIe driver test.

  I'm fine with either pull this as a fixup for 4.18 or
queued for next 4.19, since the PCIe driver is not merged yet,
just do as you feel what's fit best, thanks.

Yixun
---
 drivers/clk/meson/axg.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c
index 2d458092884a..246c23df64a8 100644
--- a/drivers/clk/meson/axg.c
+++ b/drivers/clk/meson/axg.c
@@ -700,12 +700,14 @@ static struct clk_regmap axg_pcie_mux = {
.offset = HHI_PCIE_PLL_CNTL6,
.mask = 0x1,
.shift = 2,
+   /* skip the parent mpll3, reserved for debug */
+   .table = (u32[]){ 1 },
},
.hw.init = &(struct clk_init_data){
.name = "pcie_mux",
.ops = &clk_regmap_mux_ops,
-   .parent_names = (const char *[]){ "mpll3", "pcie_pll" },
-   .num_parents = 2,
+   .parent_names = (const char *[]){ "pcie_pll" },
+   .num_parents = 1,
.flags = CLK_SET_RATE_PARENT,
},
 };
-- 
2.18.0

Re: [PATCH v2 1/3] clk: meson: add DT documentation for emmc clock controller

[Yixun Lan]

HI Kevin

On 07/23/2018 10:12 PM, Kevin Hilman wrote:
> Yixun Lan  writes:
> 
> [...]
> 
>>>
>>>> Second, we might like to convert eMMC driver to also use mmc-clkc model.
>>>
>>> IMO, this should be done as part of merging this series. Otherwise, we
>>> have duplicated code for the same thing.
>>
>> IMO, I'd leave this out of this series, since this patch series is quite
>> complete as itself. Although, the downside is code duplication.
>>
>> Still, I need to hear Jerome, or Kevin's option, to see if or how we
>> should proceed the eMMC's clock conversion.
>>
>> I could think of three option myself
>> 1) don't do the conversion, downside is code duplication, upside is NO
>> DT change, no compatibility issue
>> 2) add a syscon node into eMMC DT node, then only convert clock part
>> into this mmc-clkc model, while still leave other eMMC register access
>> as the usual iomap way (still no race condition)
>> 3) convert all eMMC register access by using regmap interface.
>>
>> both 2) and 3) need to update the DT.
>>
>> and probably 2) is a compromise way, and 1) is also OK, 3) is probably
>> the worst way due to dramatically change (I think this was already
>> rejected in the previous discussion)
> 
> Because the devices (NAND and eMMC_C) are mutually exclusive, taking the
> step-by-step approach is fine (and preferred) by me.
> 
> Phase 1:
> - add new mmc-clk provider
> - add NAND driver using new mmc-clk provider
> - boards using NAND should ensure emmc_c is disabled in DT
> 
> This allows us to not touch the MMC driver or existing upstream
> bindings.  Yes, this means there is duplicate code in the MMC driver and
> the new mmc-clk provider, but that can be removed in the next phase.
> 
Great, the approach to address this issue is reasonable.
We'd like to focus on phase 1 first, thanks

> Phase 2:
> - convert MMC driver to use new mmc-clk provider
> - update MMC users in DT and bindings
> 
Ok.


Yixun

Re: [PATCH v2 1/3] clk: meson: add DT documentation for emmc clock controller

[Yixun Lan]

Hi Rob

see my comments

On 07/12/18 03:43, Rob Herring wrote:
> On Tue, Jul 10, 2018 at 04:36:56PM +0000, Yixun Lan wrote:
>> Document the MMC sub clock controller driver, the potential consumer
>> of this driver is MMC or NAND.
> 
> So you all have decided to properly model this now?
> 
Yes, ;-)

>>
>> Signed-off-by: Yixun Lan 
>> ---
>>  .../bindings/clock/amlogic,mmc-clkc.txt   | 31 +++
>>  1 file changed, 31 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt 
>> b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
>> new file mode 100644
>> index ..ff6b4bf3ecf9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
>> @@ -0,0 +1,31 @@
>> +* Amlogic MMC Sub Clock Controller Driver
>> +
>> +The Amlogic MMC clock controller generates and supplies clock to support
>> +MMC and NAND controller
>> +
>> +Required Properties:
>> +
>> +- compatible: should be:
>> +"amlogic,meson-gx-mmc-clkc"
>> +"amlogic,meson-axg-mmc-clkc"
>> +
>> +- #clock-cells: should be 1.
>> +- clocks: phandles to clocks corresponding to the clock-names property
>> +- clock-names: list of parent clock names
>> +- "clkin0", "clkin1"
>> +
>> +Parent node should have the following properties :
>> +- compatible: "syscon", "simple-mfd, and "amlogic,meson-axg-mmc-clkc"
> 
> You don't need "simple-mfd" and probably not syscon either. The order is 
> wrong too. Most specific first.
> 
Ok, I will drop "simple-mfd"..

but the syscon is a must, since this mmc clock model access registers
via the regmap interface

I will fix the order, thanks for pointing this out

>> +- reg: base address and size of the MMC control register space.
>> +
>> +Example: Clock controller node:
>> +
>> +sd_mmc_c_clkc: clock-controller@7000 {
>> +compatible = "amlogic,mmc-clkc", "syscon", "simple-mfd";
> 
> Doesn't match the binding...
> 
oops, I will update this

>> +reg = <0x0 0x7000 0x0 0x4>;
>> +#clock-cells = <1>;
>> +
>> +clock-names = "clkin0", "clkin1";
>> +clocks = <&clkc CLKID_SD_MMC_C_CLK0>,
>> +<&clkc CLKID_FCLK_DIV2>;
>> +};
>> -- 
>> 2.18.0
>>
> 
> .
> 

Re: [PATCH v2 2/3] clk: meson: add sub MMC clock dt-bindings IDs

[Yixun Lan]

Hi Rob

On 07/12/18 03:45, Rob Herring wrote:
> On Tue, Jul 10, 2018 at 04:36:57PM +0000, Yixun Lan wrote:
>> Add two clock bindings IDs which provided by the MMC clock controller,
>> These two clocks will be used by MMC or NAND driver.
> 
> I count 3 ids.
I will update this

> 
>>
>> Signed-off-by: Yixun Lan 
>> ---
>>  .../dt-bindings/clock/amlogic,meson-mmc-clkc.h   | 16 
>>  1 file changed, 16 insertions(+)
>>  create mode 100644 include/dt-bindings/clock/amlogic,meson-mmc-clkc.h
> 
> This can go with the binding patch.
> 
sure

.

Re: [PATCH v2 3/3] clk: meson: add sub MMC clock controller driver

[Yixun Lan]

Hi Jerome

thanks for the review

On 07/12/18 17:09, Jerome Brunet wrote:
> On Tue, 2018-07-10 at 16:36 +0000, Yixun Lan wrote:
>> The patch will add a MMC clock controller driver which used by MMC or NAND,
>> It provide a mux and divider clock, and three phase clocks - core, tx, tx.
>>
>> Two clocks are provided as the parent of MMC clock controller from
>> upper layer clock controller - eg "amlogic,axg-clkc" in AXG platform.
>>
>> To specify which clock the MMC or NAND driver may consume,
>> the preprocessor macros in the dt-bindings/clock/emmc-clkc.h header
>> can be used in the device tree sources.
>>
>> Signed-off-by: Yixun Lan 
>> ---
>>  drivers/clk/meson/Kconfig|   9 +
>>  drivers/clk/meson/Makefile   |   1 +
>>  drivers/clk/meson/mmc-clkc.c | 419 +++
>>  3 files changed, 429 insertions(+)
>>  create mode 100644 drivers/clk/meson/mmc-clkc.c
>>
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index efaa70f682b4..edc18e65c89b 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -15,6 +15,15 @@ config COMMON_CLK_MESON_AO
>>  select COMMON_CLK_REGMAP_MESON
>>  select RESET_CONTROLLER
>>  
>> +config COMMON_CLK_MMC_MESON
>> +tristate "Meson MMC Sub Clock Controller Driver"
>> +depends on COMMON_CLK_AMLOGIC
>> +select MFD_SYSCON
>> +select REGMAP
>> +help
>> +  Support for the MMC sub clock controller on Amlogic Meson Platform,
>> +  Say Y if you want this clock enabled.
>> +
>>  config COMMON_CLK_REGMAP_MESON
>>  bool
>>  select REGMAP
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index 72ec8c40d848..4b3817f80ba1 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -9,4 +9,5 @@ obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
>>  obj-$(CONFIG_COMMON_CLK_GXBB)+= gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
>>  obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>>  obj-$(CONFIG_COMMON_CLK_AXG_AUDIO)  += axg-audio.o
>> +obj-$(CONFIG_COMMON_CLK_MMC_MESON)  += mmc-clkc.o
>>  obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)   += clk-regmap.o
>> diff --git a/drivers/clk/meson/mmc-clkc.c b/drivers/clk/meson/mmc-clkc.c
>> new file mode 100644
>> index ..43b7a376746d
>> --- /dev/null
>> +++ b/drivers/clk/meson/mmc-clkc.c
>> @@ -0,0 +1,419 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Amlogic Meson MMC Sub Clock Controller Driver
> 
>  * Copyright (c) 2017 Baylibre SAS.
>  * Author: Jerome Brunet 
> 
> Considering that a fair share of the code below has been copied from the clock
> portion of the eMMC driver, which I wrote last year.
> 
Ok, fair enough, will fix in next version

>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Yixun Lan 
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "clkc.h"
>> +
>> +/* clock ID used by internal driver */
>> +#define CLKID_MMC_MUX   0
>> +#define CLKID_MMC_PHASE_CORE2
>> +
>> +#define SD_EMMC_CLOCK   0
>> +#define   CLK_DIV_MASK GENMASK(5, 0)
>> +#define   CLK_SRC_MASK GENMASK(7, 6)
>> +#define   CLK_CORE_PHASE_MASK GENMASK(9, 8)
>> +#define   CLK_TX_PHASE_MASK GENMASK(11, 10)
>> +#define   CLK_RX_PHASE_MASK GENMASK(13, 12)
>> +#define   CLK_V2_TX_DELAY_MASK GENMASK(19, 16)
>> +#define   CLK_V2_RX_DELAY_MASK GENMASK(23, 20)
>> +#define   CLK_V2_ALWAYS_ON BIT(24)
>> +
>> +#define   CLK_V3_TX_DELAY_MASK GENMASK(21, 16)
>> +#define   CLK_V3_RX_DELAY_MASK GENMASK(27, 22)
>> +#define   CLK_V3_ALWAYS_ON BIT(28)
>> +
>> +#define   CLK_DELAY_STEP_PS 200
>> +#define   CLK_PHASE_STEP 30
>> +#define   CLK_PHASE_POINT_NUM (360 / CLK_PHASE_STEP)
>> +
>> +#define MUX_CLK_NUM_PARENTS 2
>> +#define MMC_MAX_CLKS5
> 
> Some defines are aligned, some aren't. please be consistent about it.
> I personally prefer when things are aligned but it is just a preference.  
> 
sounds good to me, I can fix this in next version

>> +
>> +struct clk_regmap_phase_data {
> 
> Considering the recent addition of clk_phase in clk/meson, it is not the best
> name to choose.
> 
> clk_phase_delay_data ?
> 
ok

>> +

[PATCH v3 0/2] clk: meson: add a sub EMMC clock controller support

[Yixun Lan]

This driver will add a MMC clock controller driver support.
The original idea about adding a clock controller is during the
discussion in the NAND driver mainline effort[1].

I've tested this in the S400 board (AXG platform) by using NAND driver.

Changes since v2 [3]:
 - squash dt-binding clock-id patch
 - update license
 - fix alignment
 - construct a clk register helper() function

Changes since v1 [2]:
 - implement phase clock
 - update compatible name
 - adjust file name
 - divider probe() into small functions, and re-use them

[1] https://lkml.kernel.org/r/20180628090034.0637a062@xps13
[2] https://lkml.kernel.org/r/20180703145716.31860-1-yixun@amlogic.com
[3] https://lkml.kernel.org/r/20180710163658.6175-1-yixun@amlogic.com


Yixun Lan (2):
  clk: meson: add DT documentation for emmc clock controller
  clk: meson: add sub MMC clock controller driver

 .../bindings/clock/amlogic,mmc-clkc.txt   |  31 ++
 drivers/clk/meson/Kconfig |   9 +
 drivers/clk/meson/Makefile|   1 +
 drivers/clk/meson/mmc-clkc.c  | 367 ++
 .../clock/amlogic,meson-mmc-clkc.h|  16 +
 5 files changed, 424 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
 create mode 100644 drivers/clk/meson/mmc-clkc.c
 create mode 100644 include/dt-bindings/clock/amlogic,meson-mmc-clkc.h

-- 
2.18.0

[PATCH v3 1/2] clk: meson: add DT documentation for emmc clock controller

[Yixun Lan]

Document the MMC sub clock controller driver, the potential consumer
of this driver is MMC or NAND. Also add three clock bindings IDs which
provided by this driver.

Signed-off-by: Yixun Lan 
---
 .../bindings/clock/amlogic,mmc-clkc.txt   | 31 +++
 .../clock/amlogic,meson-mmc-clkc.h| 16 ++
 2 files changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
 create mode 100644 include/dt-bindings/clock/amlogic,meson-mmc-clkc.h

diff --git a/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt 
b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
new file mode 100644
index ..91018221df1a
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
@@ -0,0 +1,31 @@
+* Amlogic MMC Sub Clock Controller Driver
+
+The Amlogic MMC clock controller generates and supplies clock to support
+MMC and NAND controller
+
+Required Properties:
+
+- compatible: should be:
+   "amlogic,meson-gx-mmc-clkc"
+   "amlogic,meson-axg-mmc-clkc"
+
+- #clock-cells: should be 1.
+- clocks: phandles to clocks corresponding to the clock-names property
+- clock-names: list of parent clock names
+   - "clkin0", "clkin1"
+
+Parent node should have the following properties :
+- compatible: "amlogic,meson-axg-mmc-clkc", "syscon".
+- reg: base address and size of the MMC control register space.
+
+Example: Clock controller node:
+
+sd_mmc_c_clkc: clock-controller@7000 {
+   compatible = "amlogic,meson-axg-mmc-clkc", "syscon";
+   reg = <0x0 0x7000 0x0 0x4>;
+   #clock-cells = <1>;
+
+   clock-names = "clkin0", "clkin1";
+   clocks = <&clkc CLKID_SD_MMC_C_CLK0>,
+<&clkc CLKID_FCLK_DIV2>;
+};
diff --git a/include/dt-bindings/clock/amlogic,meson-mmc-clkc.h 
b/include/dt-bindings/clock/amlogic,meson-mmc-clkc.h
new file mode 100644
index ..2ae988ebc3ae
--- /dev/null
+++ b/include/dt-bindings/clock/amlogic,meson-mmc-clkc.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Meson MMC sub clock tree IDs
+ *
+ * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
+ * Author: Yixun Lan 
+ */
+
+#ifndef __MMC_CLKC_H
+#define __MMC_CLKC_H
+
+#define CLKID_MMC_DIV  1
+#define CLKID_MMC_PHASE_TX 3
+#define CLKID_MMC_PHASE_RX 4
+
+#endif
-- 
2.18.0

[PATCH v3 2/2] clk: meson: add sub MMC clock controller driver

[Yixun Lan]

The patch will add a MMC clock controller driver which used by MMC or NAND,
It provide a mux and divider clock, and three phase clocks - core, tx, tx.

Two clocks are provided as the parent of MMC clock controller from
upper layer clock controller - eg "amlogic,axg-clkc" in AXG platform.

To specify which clock the MMC or NAND driver may consume,
the preprocessor macros in the dt-bindings/clock/emmc-clkc.h header
can be used in the device tree sources.

Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/Kconfig|   9 +
 drivers/clk/meson/Makefile   |   1 +
 drivers/clk/meson/mmc-clkc.c | 367 +++
 3 files changed, 377 insertions(+)
 create mode 100644 drivers/clk/meson/mmc-clkc.c

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index efaa70f682b4..edc18e65c89b 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -15,6 +15,15 @@ config COMMON_CLK_MESON_AO
select COMMON_CLK_REGMAP_MESON
select RESET_CONTROLLER
 
+config COMMON_CLK_MMC_MESON
+   tristate "Meson MMC Sub Clock Controller Driver"
+   depends on COMMON_CLK_AMLOGIC
+   select MFD_SYSCON
+   select REGMAP
+   help
+ Support for the MMC sub clock controller on Amlogic Meson Platform,
+ Say Y if you want this clock enabled.
+
 config COMMON_CLK_REGMAP_MESON
bool
select REGMAP
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 72ec8c40d848..4b3817f80ba1 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -9,4 +9,5 @@ obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
 obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
 obj-$(CONFIG_COMMON_CLK_AXG)+= axg.o axg-aoclk.o
 obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
+obj-$(CONFIG_COMMON_CLK_MMC_MESON) += mmc-clkc.o
 obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)  += clk-regmap.o
diff --git a/drivers/clk/meson/mmc-clkc.c b/drivers/clk/meson/mmc-clkc.c
new file mode 100644
index ..36c4c7cd69a6
--- /dev/null
+++ b/drivers/clk/meson/mmc-clkc.c
@@ -0,0 +1,367 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Amlogic Meson MMC Sub Clock Controller Driver
+ *
+ * Copyright (c) 2017 Baylibre SAS.
+ * Author: Jerome Brunet 
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Yixun Lan 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "clkc.h"
+
+/* clock ID used by internal driver */
+#define CLKID_MMC_MUX  0
+#define CLKID_MMC_PHASE_CORE   2
+
+#define SD_EMMC_CLOCK  0
+#define   CLK_DIV_MASK GENMASK(5, 0)
+#define   CLK_SRC_MASK GENMASK(7, 6)
+#define   CLK_CORE_PHASE_MASK  GENMASK(9, 8)
+#define   CLK_TX_PHASE_MASKGENMASK(11, 10)
+#define   CLK_RX_PHASE_MASKGENMASK(13, 12)
+#define   CLK_V2_TX_DELAY_MASK GENMASK(19, 16)
+#define   CLK_V2_RX_DELAY_MASK GENMASK(23, 20)
+#define   CLK_V2_ALWAYS_ON BIT(24)
+
+#define   CLK_V3_TX_DELAY_MASK GENMASK(21, 16)
+#define   CLK_V3_RX_DELAY_MASK GENMASK(27, 22)
+#define   CLK_V3_ALWAYS_ON BIT(28)
+
+#define   CLK_DELAY_STEP_PS200
+#define   CLK_PHASE_STEP   30
+#define   CLK_PHASE_POINT_NUM  (360 / CLK_PHASE_STEP)
+
+#define MUX_CLK_NUM_PARENTS2
+#define MMC_MAX_CLKS   5
+
+struct clk_phase_delay_data {
+   unsigned long   phase_mask;
+   unsigned long   delay_mask;
+   unsigned intdelay_step_ps;
+};
+
+struct mmc_clkc_data {
+   struct clk_phase_delay_data tx;
+   struct clk_phase_delay_data rx;
+};
+
+static inline struct clk_phase_delay_data *
+clk_get_regmap_phase_data(struct clk_regmap *clk)
+{
+   return (struct clk_phase_delay_data *)clk->data;
+}
+
+static struct clk_regmap_mux_data mmc_clkc_mux_data = {
+   .offset = SD_EMMC_CLOCK,
+   .mask   = 0x3,
+   .shift  = 6,
+   .flags  = CLK_DIVIDER_ROUND_CLOSEST,
+};
+
+static struct clk_regmap_div_data mmc_clkc_div_data = {
+   .offset = SD_EMMC_CLOCK,
+   .shift  = 0,
+   .width  = 6,
+   .flags  = CLK_DIVIDER_ROUND_CLOSEST | CLK_DIVIDER_ONE_BASED,
+};
+
+static struct clk_phase_delay_data mmc_clkc_core_delay_phase = {
+   .phase_mask = CLK_CORE_PHASE_MASK,
+};
+
+static const struct mmc_clkc_data mmc_clkc_gx_data = {
+   {
+   .phase_mask = CLK_TX_PHASE_MASK,
+   .delay_mask = CLK_V2_TX_DELAY_MASK,
+   .delay_step_ps  = CLK_DELAY_STEP_PS,
+   },
+   {
+   .phase_mask = CLK_RX_PHASE_MASK,
+   .delay_mask = CLK_V2_RX_DELAY_MASK,
+   .delay_step_ps  = CLK_DELAY_STEP_PS,
+   },
+};
+
+

Re: [PATCH v2 1/3] clk: meson: add DT documentation for emmc clock controller

[Yixun Lan]

HI Rob

see my comments

On 07/12/2018 10:17 PM, Rob Herring wrote:
> On Wed, Jul 11, 2018 at 8:47 PM Yixun Lan  wrote:
>>
>> Hi Rob
>>
>> see my comments
>>
>> On 07/12/18 03:43, Rob Herring wrote:
>>> On Tue, Jul 10, 2018 at 04:36:56PM +, Yixun Lan wrote:
>>>> Document the MMC sub clock controller driver, the potential consumer
>>>> of this driver is MMC or NAND.
>>>
>>> So you all have decided to properly model this now?
>>>
>> Yes, ;-)
>>
>>>>
>>>> Signed-off-by: Yixun Lan 
>>>> ---
>>>>  .../bindings/clock/amlogic,mmc-clkc.txt   | 31 +++
>>>>  1 file changed, 31 insertions(+)
>>>>  create mode 100644 
>>>> Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt 
>>>> b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
>>>> new file mode 100644
>>>> index ..ff6b4bf3ecf9
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
>>>> @@ -0,0 +1,31 @@
>>>> +* Amlogic MMC Sub Clock Controller Driver
>>>> +
>>>> +The Amlogic MMC clock controller generates and supplies clock to support
>>>> +MMC and NAND controller
>>>> +
>>>> +Required Properties:
>>>> +
>>>> +- compatible: should be:
>>>> +"amlogic,meson-gx-mmc-clkc"
>>>> +"amlogic,meson-axg-mmc-clkc"
>>>> +
>>>> +- #clock-cells: should be 1.
>>>> +- clocks: phandles to clocks corresponding to the clock-names property
>>>> +- clock-names: list of parent clock names
>>>> +- "clkin0", "clkin1"
>>>> +
>>>> +Parent node should have the following properties :
>>>> +- compatible: "syscon", "simple-mfd, and "amlogic,meson-axg-mmc-clkc"
>>>
>>> You don't need "simple-mfd" and probably not syscon either. The order is
>>> wrong too. Most specific first.
>>>
>> Ok, I will drop "simple-mfd"..
>>
>> but the syscon is a must, since this mmc clock model access registers
>> via the regmap interface
> 
> A syscon compatible should not be the only way to get a regmap.
do you have any suggestion about other function that I can use? is
devm_regmap_init_mmio() feasible

> Removing lines 56/57 of drivers/mfd/syscon.c should be sufficient.
> 
I'm not sure what's the valid point of removing compatible 'syscon' in
driver/mfd/syscon.c, sounds this will break a lot DT/or need to fix?
will you propose a patch for this? then I can certainly adjust here

> Why do you need a regmap in the first place? What else needs to access
> this register directly?
Yes, the SD_EMMC_CLOCK register contain several bits which not fit well
into common clock model, and they need to be access in the NAND or eMMC
driver itself, Martin had explained this in early thread[1]

In this register
Bit[31] select NAND or eMMC function
Bit[25] enable SDIO IRQ
Bit[24] Clock always on
Bit[15:14] SRAM Power down

[1]
https://lkml.kernel.org/r/CAFBinCBeyXf6LNaZzAw6WnsxzDAv8E=yp2eem0xcpwmeui6...@mail.gmail.com

> Don't you need a patch removing the clock code
> from within the emmc driver? It's not even using regmap, so using
> regmap here doesn't help.
>
No, and current eMMC driver still use iomap to access the register

I think we probably would like to take two steps approach.
first, from the hardware perspective, the NAND and eMMC(port C) driver
can't exist at same time, since they share the pins, clock, internal
ram, So we have to only enable one of NAND or eMMC in DT, not enable
both of them.
Second, we might like to convert eMMC driver to also use mmc-clkc model.


Yixun

Re: [PATCH v2 1/3] clk: meson: add DT documentation for emmc clock controller

[Yixun Lan]

Hi Rob, Jerome, Kevin

see my comments

On 07/13/18 08:15, Rob Herring wrote:
> On Thu, Jul 12, 2018 at 5:29 PM Yixun Lan  wrote:
>>
>> HI Rob
>>
>> see my comments
>>
>> On 07/12/2018 10:17 PM, Rob Herring wrote:
>>> On Wed, Jul 11, 2018 at 8:47 PM Yixun Lan  wrote:
>>>>
>>>> Hi Rob
>>>>
>>>> see my comments
>>>>
>>>> On 07/12/18 03:43, Rob Herring wrote:
>>>>> On Tue, Jul 10, 2018 at 04:36:56PM +, Yixun Lan wrote:
>>>>>> Document the MMC sub clock controller driver, the potential consumer
>>>>>> of this driver is MMC or NAND.
>>>>>
>>>>> So you all have decided to properly model this now?
>>>>>
>>>> Yes, ;-)
>>>>
>>>>>>
>>>>>> Signed-off-by: Yixun Lan 
>>>>>> ---
>>>>>>  .../bindings/clock/amlogic,mmc-clkc.txt   | 31 +++
>>>>>>  1 file changed, 31 insertions(+)
>>>>>>  create mode 100644 
>>>>>> Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
>>>>>>
>>>>>> diff --git 
>>>>>> a/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt 
>>>>>> b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
>>>>>> new file mode 100644
>>>>>> index ..ff6b4bf3ecf9
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.txt
>>>>>> @@ -0,0 +1,31 @@
>>>>>> +* Amlogic MMC Sub Clock Controller Driver
>>>>>> +
>>>>>> +The Amlogic MMC clock controller generates and supplies clock to support
>>>>>> +MMC and NAND controller
>>>>>> +
>>>>>> +Required Properties:
>>>>>> +
>>>>>> +- compatible: should be:
>>>>>> +"amlogic,meson-gx-mmc-clkc"
>>>>>> +"amlogic,meson-axg-mmc-clkc"
>>>>>> +
>>>>>> +- #clock-cells: should be 1.
>>>>>> +- clocks: phandles to clocks corresponding to the clock-names property
>>>>>> +- clock-names: list of parent clock names
>>>>>> +- "clkin0", "clkin1"
>>>>>> +
>>>>>> +Parent node should have the following properties :
>>>>>> +- compatible: "syscon", "simple-mfd, and "amlogic,meson-axg-mmc-clkc"
>>>>>
>>>>> You don't need "simple-mfd" and probably not syscon either. The order is
>>>>> wrong too. Most specific first.
>>>>>
>>>> Ok, I will drop "simple-mfd"..
>>>>
>>>> but the syscon is a must, since this mmc clock model access registers
>>>> via the regmap interface
>>>
>>> A syscon compatible should not be the only way to get a regmap.
>> do you have any suggestion about other function that I can use? is
>> devm_regmap_init_mmio() feasible
>>
>>> Removing lines 56/57 of drivers/mfd/syscon.c should be sufficient.
>>>
>> I'm not sure what's the valid point of removing compatible 'syscon' in
>> driver/mfd/syscon.c, sounds this will break a lot DT/or need to fix?
>> will you propose a patch for this? then I can certainly adjust here
> 
> Removing the 2 lines will simply allow any node to be a syscon. If
> there's a specific driver for a node, then that makes sense to allow
> that.
> 
>>
>>> Why do you need a regmap in the first place? What else needs to access
>>> this register directly?
>> Yes, the SD_EMMC_CLOCK register contain several bits which not fit well
>> into common clock model, and they need to be access in the NAND or eMMC
>> driver itself, Martin had explained this in early thread[1]
>>
>> In this register
>> Bit[31] select NAND or eMMC function
>> Bit[25] enable SDIO IRQ
>> Bit[24] Clock always on
>> Bit[15:14] SRAM Power down
>>
>> [1]
>> https://lkml.kernel.org/r/CAFBinCBeyXf6LNaZzAw6WnsxzDAv8E=yp2eem0xcpwmeui6...@mail.gmail.com
>>
>>> Don't you need a patch removing the clock code
>>> from within the emmc driver? It's not even using regmap, so using
>>> regmap here doesn't help.
>>>
>> No, and current eMMC driver still use iomap to access the register
> 
> Which mean

Re: [PATCH 0/3] pinctrl: meson-g12a: add pinctrl driver support

[Yixun Lan]

Hi Martin

see my comments

On 07/14/2018 10:36 PM, Martin Blumenstingl wrote:
> On Tue, Jul 10, 2018 at 12:07 AM Martin Blumenstingl
>  wrote:
>>
>> Hi Linus,
>>
>> On Mon, Jul 9, 2018 at 3:35 PM Linus Walleij  
>> wrote:
>>>
>>> On Wed, Jul 4, 2018 at 4:48 PM Yixun Lan  wrote:
>>>>
>>>> This patch series try to add pinctrl driver support for
>>>> the Meson-G12A SoC.
>>>>
>>>>
>>>> Yixun Lan (3):
>>>>   documentation: Add compatibles for Amlogic Meson G12A pin controllers
>>>>   dt-bindings: pinctrl: meson-g12a: document pin name
>>>>   pinctrl: meson-g12a: add pinctrl driver support
>>>
>>> Overall this looks good to me, could we get some review or ACKs
>>> from Beniamino, Martin and/or Carlo as an indication that everything
>>> seems all right before I apply the series?
>> I do not have any documentation for this hardware (I'm not even sure
>> if this is the successor to AXG/"the Audio SoCs" or rather a successor
>> of GXL or GXM/"the OTT/STB SoCs"
> for those who are interested: according to [0] the marketing name of
> this SoC will be "A311D" - specs from the linked PDF:
A311D is the G12B SoC series
This pinctrl patch is target for G12A for now, but can be very easily
extend to support G12B..
> - CPU: Quad Cortex-A73 + Dual Cortex-A53 (max frequency: TBD)
> - GPU: ARM G52 MP4 (4ppc)
> - Memory: DDR3/4 LPDDR3/4
> - Video decoding: 4K H265&VP9&AVS2
> - Video Encoding: 1080P H264 H265
> - HDMI-Tx: 4K2K
> - Ethernet: 10/100M/1000M
> - AV output: CVBS
> - IP License: Dolby, DTS
> 
> W400 is the reference board for this SoC, it comes with:
> - EMMC
> - WIFI AP6398S
> - DDR 2GB
> 
yes, W400 for G12B, and U200 for G12A

> if I had to guess then I would say it's the successor of the GXL (or
> GXM?) family
> 
Yes, successor of GXL

> 
> Regards
> Martin
> 
> 
> [0] 
> http://openlinux.amlogic.com:8000/download/doc/Amlogic_A311D_Buildroot_Preview_Release_Notes_V20180706.pdf
> 
> .
> 

Re: [PATCH 2/3] dt-bindings: pinctrl: meson-g12a: document pin name

[Yixun Lan]

HI Martin

thanks for the comments

On 07/14/2018 10:47 PM, Martin Blumenstingl wrote:
> Hi Yixun,
> 
> On Wed, Jul 4, 2018 at 4:49 PM Yixun Lan  wrote:
>>
>> Document the pins for Amlogic's Meson-G12A SoC.
> I suggest to combine patch 1 (adding the compatible string) and 2 (this one)
> 
> as discussed in the cover letter: it would be great if you could add a
> comment (at least to the commit message) stating that GPIOE is
> actually located (checked with the ASIC / hardware team) in the AO
> bank
> 
sure, I will state this in commit message, and will also put a note in code.

>> Signed-off-by: Yixun Lan 
> with that:
> Acked-by: Martin Blumenstingl 
> 
thanks, I assume I can add your Ack to this whole patch series
(dt-binding patch and pinctrl driver patch)?

>> ---
>>  include/dt-bindings/gpio/meson-g12a-gpio.h | 114 +
>>  1 file changed, 114 insertions(+)
>>  create mode 100644 include/dt-bindings/gpio/meson-g12a-gpio.h
>>
>> diff --git a/include/dt-bindings/gpio/meson-g12a-gpio.h 
>> b/include/dt-bindings/gpio/meson-g12a-gpio.h
>> new file mode 100644
>> index ..f7bd69350d18
>> --- /dev/null
>> +++ b/include/dt-bindings/gpio/meson-g12a-gpio.h
>> @@ -0,0 +1,114 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ or MIT) */
>> +/*
>> + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
>> + * Author: Xingyu Chen 
> (since I'm not sure about all the licensing bits:) do we also need a
> "Signed-off-by" from Xingyu Chen?
> 
sure, this pinctrl driver is mostly Xingyu's work, I will add this.
> 
> Regards
> Martin
> 
> .
> 

[PATCH v2 0/2] pinctrl: meson-g12a: add pinctrl driver support

[Yixun Lan]

This patch series try to add pinctrl driver support for
the Meson-G12A SoC.

Changes since v1 at [1]
 - add Martin's Ack, Xingyu's Signed-off
 - squash patch 1,2 (documentation & header file)
 - explain pinctrl IP
 - notice GPIOE located in AO bank

[1] https://lkml.kernel.org/r/20180704224511.29350-1-yixun@amlogic.com

Yixun Lan (2):
  documentation: pinctrl: Add compatibles for Amlogic Meson G12A pin
controllers
  pinctrl: meson-g12a: add pinctrl driver support

 .../bindings/pinctrl/meson,pinctrl.txt|2 +
 drivers/pinctrl/meson/Kconfig |6 +
 drivers/pinctrl/meson/Makefile|1 +
 drivers/pinctrl/meson/pinctrl-meson-g12a.c| 1433 +
 include/dt-bindings/gpio/meson-g12a-gpio.h|  114 ++
 5 files changed, 1556 insertions(+)
 create mode 100644 drivers/pinctrl/meson/pinctrl-meson-g12a.c
 create mode 100644 include/dt-bindings/gpio/meson-g12a-gpio.h

-- 
2.18.0

[PATCH v2 1/2] documentation: pinctrl: Add compatibles for Amlogic Meson G12A pin controllers

[Yixun Lan]

Add new compatible name for Amlogic's Meson-G12A pin controllers,
add a dt-binding header file which document the detail pin names.

Acked-by: Martin Blumenstingl 
Signed-off-by: Xingyu Chen 
Signed-off-by: Yixun Lan 
---
 .../bindings/pinctrl/meson,pinctrl.txt|   2 +
 include/dt-bindings/gpio/meson-g12a-gpio.h| 114 ++
 2 files changed, 116 insertions(+)
 create mode 100644 include/dt-bindings/gpio/meson-g12a-gpio.h

diff --git a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt 
b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
index 54ecb8ab7788..82ead40311f6 100644
--- a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
@@ -13,6 +13,8 @@ Required properties for the root node:
  "amlogic,meson-gxl-aobus-pinctrl"
  "amlogic,meson-axg-periphs-pinctrl"
  "amlogic,meson-axg-aobus-pinctrl"
+ "amlogic,meson-g12a-periphs-pinctrl"
+ "amlogic,meson-g12a-aobus-pinctrl"
  - reg: address and size of registers controlling irq functionality
 
 === GPIO sub-nodes ===
diff --git a/include/dt-bindings/gpio/meson-g12a-gpio.h 
b/include/dt-bindings/gpio/meson-g12a-gpio.h
new file mode 100644
index ..f7bd69350d18
--- /dev/null
+++ b/include/dt-bindings/gpio/meson-g12a-gpio.h
@@ -0,0 +1,114 @@
+/* SPDX-License-Identifier: (GPL-2.0+ or MIT) */
+/*
+ * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
+ * Author: Xingyu Chen 
+ */
+
+#ifndef _DT_BINDINGS_MESON_G12A_GPIO_H
+#define _DT_BINDINGS_MESON_G12A_GPIO_H
+
+/* First GPIO chip */
+#define GPIOAO_0   0
+#define GPIOAO_1   1
+#define GPIOAO_2   2
+#define GPIOAO_3   3
+#define GPIOAO_4   4
+#define GPIOAO_5   5
+#define GPIOAO_6   6
+#define GPIOAO_7   7
+#define GPIOAO_8   8
+#define GPIOAO_9   9
+#define GPIOAO_10  10
+#define GPIOAO_11  11
+#define GPIOE_012
+#define GPIOE_113
+#define GPIOE_214
+
+/* Second GPIO chip */
+#define GPIOZ_00
+#define GPIOZ_11
+#define GPIOZ_22
+#define GPIOZ_33
+#define GPIOZ_44
+#define GPIOZ_55
+#define GPIOZ_66
+#define GPIOZ_77
+#define GPIOZ_88
+#define GPIOZ_99
+#define GPIOZ_10   10
+#define GPIOZ_11   11
+#define GPIOZ_12   12
+#define GPIOZ_13   13
+#define GPIOZ_14   14
+#define GPIOZ_15   15
+#define GPIOH_016
+#define GPIOH_117
+#define GPIOH_218
+#define GPIOH_319
+#define GPIOH_420
+#define GPIOH_521
+#define GPIOH_622
+#define GPIOH_723
+#define GPIOH_824
+#define BOOT_0 25
+#define BOOT_1 26
+#define BOOT_2 27
+#define BOOT_3 28
+#define BOOT_4 29
+#define BOOT_5 30
+#define BOOT_6 31
+#define BOOT_7 32
+#define BOOT_8 33
+#define BOOT_9 34
+#define BOOT_1035
+#define BOOT_1136
+#define BOOT_1237
+#define BOOT_1338
+#define BOOT_1439
+#define BOOT_1540
+#define GPIOC_041
+#define GPIOC_142
+#define GPIOC_243
+#define GPIOC_344
+#define GPIOC_445
+#define GPIOC_546
+#define GPIOC_647
+#define GPIOC_748
+#define GPIOA_049
+#define GPIOA_150
+#define GPIOA_251
+#define GPIOA_352
+#define GPIOA_453
+#define GPIOA_554
+#define GPIOA_655
+#define GPIOA_756
+#define GPIOA_857
+#define GPIOA_958
+#define GPIOA_10   59
+#define GPIOA_11   60
+#define GPIOA_12   61
+#define GPIOA_13   62
+#define GPIOA_14   63
+#define GPIOA_15   64
+#define GPIOX_065
+#define GPIOX_166
+#define GPIOX_267
+#define GPIOX_368
+#define GPIOX_469
+#define GPIOX_570
+#define GPIOX_671
+#define GPIOX_772
+#define GPIOX_873
+#define GPIOX_974
+#define GPIOX_10   75
+#define GPIOX_11   76
+#define GPIOX_12   77
+#define GPIOX_13   78
+#define GPIOX_14   79
+#define GPIOX_15   80
+#define GPIOX_16   81
+#define GPIOX_17   82
+#define GPIOX_18   83
+#define GPIOX_19   84
+
+#endif /* _DT_BINDINGS_MESON_G12A_GPIO_H */
-- 
2.18.0

[PATCH v2 2/2] pinctrl: meson-g12a: add pinctrl driver support

[Yixun Lan]

Add the pinctrl driver for Meson-G12A SoC which share the similar IP as
the previous Meson-AXG SoC.

Starting from Meson-AXG SoC, the pinctrl controller block use 4
continues register bits to specific the pin mux function, while comparing
to old generation SoC which using variable length register bits for
the pin mux definition. The new design greatly simplify the software model.

For the detail example, one 32bit register can be divided into 8 parts,
each has 4 bits whose value start from 0 - 7, each can describe one pin,
the value 0 is always devoted to GPIO function, while 1 - 7 devoted to
the mux pin function.

Please note, the GPIOE is actually located at AO (always on) bank.

Acked-by: Martin Blumenstingl 
Signed-off-by: Xingyu Chen 
Signed-off-by: Yixun Lan 
---
 drivers/pinctrl/meson/Kconfig  |6 +
 drivers/pinctrl/meson/Makefile |1 +
 drivers/pinctrl/meson/pinctrl-meson-g12a.c | 1433 
 3 files changed, 1440 insertions(+)
 create mode 100644 drivers/pinctrl/meson/pinctrl-meson-g12a.c

diff --git a/drivers/pinctrl/meson/Kconfig b/drivers/pinctrl/meson/Kconfig
index c80951d6caff..9ab537eb78a3 100644
--- a/drivers/pinctrl/meson/Kconfig
+++ b/drivers/pinctrl/meson/Kconfig
@@ -47,4 +47,10 @@ config PINCTRL_MESON_AXG
 config PINCTRL_MESON_AXG_PMX
bool
 
+config PINCTRL_MESON_G12A
+   bool "Meson g12a Soc pinctrl driver"
+   depends on ARM64
+   select PINCTRL_MESON_AXG_PMX
+   default y
+
 endif
diff --git a/drivers/pinctrl/meson/Makefile b/drivers/pinctrl/meson/Makefile
index 3c6580c2d9d7..cf283f48f9d8 100644
--- a/drivers/pinctrl/meson/Makefile
+++ b/drivers/pinctrl/meson/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_PINCTRL_MESON_GXBB) += pinctrl-meson-gxbb.o
 obj-$(CONFIG_PINCTRL_MESON_GXL) += pinctrl-meson-gxl.o
 obj-$(CONFIG_PINCTRL_MESON_AXG_PMX) += pinctrl-meson-axg-pmx.o
 obj-$(CONFIG_PINCTRL_MESON_AXG) += pinctrl-meson-axg.o
+obj-$(CONFIG_PINCTRL_MESON_G12A) += pinctrl-meson-g12a.o
diff --git a/drivers/pinctrl/meson/pinctrl-meson-g12a.c 
b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
new file mode 100644
index ..ca5cec40ac34
--- /dev/null
+++ b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
@@ -0,0 +1,1433 @@
+// SPDX-License-Identifier: (GPL-2.0+ or MIT)
+/*
+ * Pin controller and GPIO driver for Amlogic Meson G12A SoC.
+ *
+ * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
+ * Author: Xingyu Chen 
+ * Author: Yixun Lan 
+ */
+
+#include 
+#include "pinctrl-meson.h"
+#include "pinctrl-meson-axg-pmx.h"
+
+static const struct pinctrl_pin_desc meson_g12a_periphs_pins[] = {
+   MESON_PIN(GPIOZ_0),
+   MESON_PIN(GPIOZ_1),
+   MESON_PIN(GPIOZ_2),
+   MESON_PIN(GPIOZ_3),
+   MESON_PIN(GPIOZ_4),
+   MESON_PIN(GPIOZ_5),
+   MESON_PIN(GPIOZ_6),
+   MESON_PIN(GPIOZ_7),
+   MESON_PIN(GPIOZ_8),
+   MESON_PIN(GPIOZ_9),
+   MESON_PIN(GPIOZ_10),
+   MESON_PIN(GPIOZ_11),
+   MESON_PIN(GPIOZ_12),
+   MESON_PIN(GPIOZ_13),
+   MESON_PIN(GPIOZ_14),
+   MESON_PIN(GPIOZ_15),
+   MESON_PIN(GPIOH_0),
+   MESON_PIN(GPIOH_1),
+   MESON_PIN(GPIOH_2),
+   MESON_PIN(GPIOH_3),
+   MESON_PIN(GPIOH_4),
+   MESON_PIN(GPIOH_5),
+   MESON_PIN(GPIOH_6),
+   MESON_PIN(GPIOH_7),
+   MESON_PIN(GPIOH_8),
+   MESON_PIN(BOOT_0),
+   MESON_PIN(BOOT_1),
+   MESON_PIN(BOOT_2),
+   MESON_PIN(BOOT_3),
+   MESON_PIN(BOOT_4),
+   MESON_PIN(BOOT_5),
+   MESON_PIN(BOOT_6),
+   MESON_PIN(BOOT_7),
+   MESON_PIN(BOOT_8),
+   MESON_PIN(BOOT_9),
+   MESON_PIN(BOOT_10),
+   MESON_PIN(BOOT_11),
+   MESON_PIN(BOOT_12),
+   MESON_PIN(BOOT_13),
+   MESON_PIN(BOOT_14),
+   MESON_PIN(BOOT_15),
+   MESON_PIN(GPIOC_0),
+   MESON_PIN(GPIOC_1),
+   MESON_PIN(GPIOC_2),
+   MESON_PIN(GPIOC_3),
+   MESON_PIN(GPIOC_4),
+   MESON_PIN(GPIOC_5),
+   MESON_PIN(GPIOC_6),
+   MESON_PIN(GPIOC_7),
+   MESON_PIN(GPIOA_0),
+   MESON_PIN(GPIOA_1),
+   MESON_PIN(GPIOA_2),
+   MESON_PIN(GPIOA_3),
+   MESON_PIN(GPIOA_4),
+   MESON_PIN(GPIOA_5),
+   MESON_PIN(GPIOA_6),
+   MESON_PIN(GPIOA_7),
+   MESON_PIN(GPIOA_8),
+   MESON_PIN(GPIOA_9),
+   MESON_PIN(GPIOA_10),
+   MESON_PIN(GPIOA_11),
+   MESON_PIN(GPIOA_12),
+   MESON_PIN(GPIOA_13),
+   MESON_PIN(GPIOA_14),
+   MESON_PIN(GPIOA_15),
+   MESON_PIN(GPIOX_0),
+   MESON_PIN(GPIOX_1),
+   MESON_PIN(GPIOX_2),
+   MESON_PIN(GPIOX_3),
+   MESON_PIN(GPIOX_4),
+   MESON_PIN(GPIOX_5),
+   MESON_PIN(GPIOX_6),
+   MESON_PIN(GPIOX_7),
+   MESON_PIN(GPIOX_8),
+   MESON_PIN(GPIOX_9),
+   MESON_PIN(GPIOX_10),
+   MESON_PIN(GPIOX_11),
+   MESON_PIN(GPIOX_12),
+   MESON_PIN(GPIOX_13),
+   MESON_PIN(GPIOX_14),
+   MESON_PIN(GPIOX_15),
+   MESON_PIN(GPIOX_16),
+   MESON_PIN(GPIOX_17),
+   MESON_PIN(GPIOX_1

Re: [v2,2/5] i2c: meson: add configurable divider factors

[Yixun Lan]

Hi Wolfram：



On 01/24/18 14:28, Wolfram Sang wrote:
> On Mon, Nov 20, 2017 at 10:54:12PM +0800, Yixun Lan wrote:
>> From: Jian Hu 
>>
>> This patch try to add support for I2C controller in Meson-AXG SoC,
>> Due to the IP changes between I2C controller, we need to introduce
>> a compatible data to make the divider factor configurable.
>>
>> Reviewed-by: Neil Armstrong 
>> Signed-off-by: Jian Hu 
>> Signed-off-by: Yixun Lan 
> 
> Applied to for-next, thanks!
> 
> But I got two build warnings, please fix them with an incremental patch:
> 
> drivers/i2c/busses/i2c-meson.c:103: warning: No description found for 
> parameter 'data'
> drivers/i2c/busses/i2c-meson.c:103: warning: Excess struct member 'irq' 
> description in 'meson_i2c'
> 
first, many thanks for pushing this..

I understand from above warnings, and can compose a patch to fix this.

but, I didn't get this build warnings while test locally, so is there
any specific compiler option that I need to pass to?

Yixun

[PATCH] i2c: meson: update doc description to fix build warnings

[Yixun Lan]

Add description for 'data' parameter and drop unused 'irq' memeber.

Here is the warnings:
drivers/i2c/busses/i2c-meson.c:103: warning: No description found for
parameter 'data'
drivers/i2c/busses/i2c-meson.c:103: warning: Excess struct member 'irq'
description in 'meson_i2c'

Suggested-by: Wolfram Sang 
Signed-off-by: Yixun Lan 
---
 drivers/i2c/busses/i2c-meson.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 37c4aa76f37a..90f5d0407d73 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -69,7 +69,6 @@ struct meson_i2c_data {
  * @dev:   Pointer to device structure
  * @regs:  Base address of the device memory mapped registers
  * @clk:   Pointer to clock structure
- * @irq:   IRQ number
  * @msg:   Pointer to the current I2C message
  * @state: Current state in the driver state machine
  * @last:  Flag set for the last message in the transfer
@@ -80,6 +79,7 @@ struct meson_i2c_data {
  * @done:  Completion used to wait for transfer termination
  * @tokens:Sequence of tokens to be written to the device
  * @num_tokens:Number of tokens
+ * @data:  Pointer to the controlller's platform data
  */
 struct meson_i2c {
struct i2c_adapter  adap;
-- 
2.15.1

[PATCH v3 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 
Signed-off-by: Yixun Lan 
---
 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 v3 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 v2 at [3]:
  -- add Jerome's Reviewed-by to patch 1
  -- adjust commit message, drop snip of code
  -- drop extra blank line

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

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

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

[1] 
 http://lkml.kernel.org/r/20171215141741.175985-1-yixun....@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 | 108 -
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi  |  10 +--
 3 files changed, 118 insertions(+), 9 deletions(-)

-- 
2.15.1

[PATCH v3 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 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.

Acked-by: Jerome Brunet 
Signed-off-by: Yixun Lan 
---
 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 = ;
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 = ;
status = "disabled";
+   clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
+   clock-names = "xtal", "pclk", "baud";
};
};
 
-- 
2.15.1

[PATCH v3 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 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 96 ++
 1 file changed, 96 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index 644d0f9eaf8c..6cd28045e89c 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";
+   };
+   };
};
};
 
@@ -498,6 +562,38 @@
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_rt

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

[Yixun Lan]

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

Reviewed-by: Jerome Brunet 
Signed-off-by: Yixun Lan 
---
 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 = ;
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 = ;
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 = ;
status = "disabled";
};
 
uart_B: serial@84dc {
compatible = "amlogic,meson-gx-uart";
-   reg = <0x0 0x84dc 0x0 0x14>;
+   reg = <0x0 0x84dc 0x0 0x18>;
interrupts = ;
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 = ;
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 = ;
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 = ;
status = "disabled";
};
-- 
2.15.1

[PATCH v3 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 
Signed-off-by: Yixun Lan 
---
 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

[PATCH] ARM64: dts: meson-axg: add RMII pins for ethernet controller

[Yixun Lan]

Comparing to RGMII interface, the RMII interface require few pins.
So it's worth describing them here.

Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index a80632641b39..ab4a0e8bc446 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -251,6 +251,36 @@
gpio-ranges = <&pinctrl_periphs 0 0 86>;
};
 
+   eth_rmii_x_pins: eth-x-rmii {
+   mux {
+   groups = "eth_mdio_x",
+  "eth_mdc_x",
+  "eth_rgmii_rx_clk_x",
+  "eth_rx_dv_x",
+  "eth_rxd0_x",
+  "eth_rxd1_x",
+  "eth_txen_x",
+  "eth_txd0_x",
+  "eth_txd1_x";
+   function = "eth";
+   };
+   };
+
+   eth_rmii_y_pins: eth-y-rmii {
+   mux {
+   groups = "eth_mdio_y",
+  "eth_mdc_y",
+  "eth_rgmii_rx_clk_y",
+  "eth_rx_dv_y",
+  "eth_rxd0_y",
+  "eth_rxd1_y",
+  "eth_txen_y",
+  "eth_txd0_y",
+  "eth_txd1_y";
+   function = "eth";
+   };
+   };
+
eth_rgmii_x_pins: eth-x-rgmii {
mux {
groups = "eth_mdio_x",
-- 
2.15.1

Re: [PATCH] ARM64: dts: meson-axg: add RMII pins for ethernet controller

[Yixun Lan]

Hi Jerome：

On 01/11/18 16:37, Jerome Brunet wrote:
> On Thu, 2018-01-11 at 11:04 +0800, Yixun Lan wrote:
>> Comparing to RGMII interface, the RMII interface require few pins.
>> So it's worth describing them here.
>>
>> Signed-off-by: Yixun Lan 
> 
> The only axg platform we have upstream is the s400 and is using rgmii.
> May I ask how this was tested ?
> 
It's true that S400 using RGMII interface.

but, we have customer using RTL8201FR-VB/VD which is a RMII PHY,

This is actually tested with the 'eth_rmii_x_pins' group.

Yixun

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

[Yixun Lan]

On 01/31/18 08:22, Kevin Hilman wrote:
> On Tue, Jan 30, 2018 at 4:04 PM, Kevin Hilman  wrote:
>> Yixun Lan  writes:
>>
>>> 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.
>>
>> Applied to v4.16/dt64,
> 
> Oops, I meant v4.17.
> 

Hi Kevin
 I should warn you about the patch 3 [1], it actually depend on the gpio
renaming patch [2]

 I'm still waiting Linus to take patch [2], then update according
(or you could help amend the patch? it's simply a
's/uart_ao_b_gpioz/uart_ao_b_z/g)

Yixun



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

http://lists.infradead.org/pipermail/linux-amlogic/2018-January/006096.html


[2] [PATCH] pinctrl: meson-axg: adjust uart_ao_b pin group naming
http://lists.infradead.org/pipermail/linux-amlogic/2018-January/006201.html

Re: [PATCH 3/9] clk: meson: remove unnecessary rounding in the pll clock

[Yixun Lan]

On 01/19/18 02:45, Jerome Brunet wrote:
> The pll driver perform the rate calculation in Mhz, which adds an
> unnecessary rounding down to the Mhz of the rate. Use 64bits long
> integer to perform this calculation safely on meson8b and perform the
> calculation in Hz instead
> 
> Fixes: 7a29a869434e ("clk: meson: Add support for Meson clock controller")
> Signed-off-by: Jerome Brunet 
> ---
>  drivers/clk/meson/clk-pll.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index 2614341fc4ad..fa4cec13d6e8 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -51,8 +51,7 @@ static unsigned long meson_clk_pll_recalc_rate(struct 
> clk_hw *hw,
>  {
>   struct meson_clk_pll *pll = to_meson_clk_pll(hw);
>   struct parm *p;
> - unsigned long parent_rate_mhz = parent_rate / 100;
> - unsigned long rate_mhz;
> + u64 rate;
>   u16 n, m, frac = 0, od, od2 = 0;
>   u32 reg;
>  
> @@ -74,17 +73,18 @@ static unsigned long meson_clk_pll_recalc_rate(struct 
> clk_hw *hw,
>   od2 = PARM_GET(p->width, p->shift, reg);
>   }
>  
> + rate = (u64)m * parent_rate;
> +
>   p = &pll->frac;
>   if (p->width) {
>   reg = readl(pll->base + p->reg_off);
>   frac = PARM_GET(p->width, p->shift, reg);
> - rate_mhz = (parent_rate_mhz * m + \
> - (parent_rate_mhz * frac >> 12)) * 2 / n;
> - rate_mhz = rate_mhz >> od >> od2;
> - } else
> - rate_mhz = (parent_rate_mhz * m / n) >> od >> od2;
>  
> - return rate_mhz * 100;
> + rate += (u64)parent_rate * frac >> 12;
> + rate *= 2;
> + }
> +
> + return (rate / n) >> od >> od2;
>  }
>  
>  static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate,
> 

Hi Jerome:
 This is exactly what I want to propose, thanks for pushing this!

 With the whole series, the fixed_pll is more accurate, and the ethernet
driver on axg is capable of choosing fclk_div2..

Yixun

[PATCH] clk: meson: axg: fix the od shift of the sys_pll

[Yixun Lan]

According to datasheet, the od shift of sys_pll is 16,
fix the typo which introduced at previous commit.

Fixes: 78b4af312f91 ('clk: meson-axg: add clock controller drivers')
Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/axg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c
index 7988dc8506b0..04a231eaf648 100644
--- a/drivers/clk/meson/axg.c
+++ b/drivers/clk/meson/axg.c
@@ -64,7 +64,7 @@ static struct meson_clk_pll axg_sys_pll = {
},
.od = {
.reg_off = HHI_SYS_PLL_CNTL,
-   .shift   = 10,
+   .shift   = 16,
.width   = 2,
},
.lock = &meson_clk_lock,
-- 
2.15.1

[PATCH 4/6] ARM64: dts: meson-axg: uart: Add the pinctrl info description

[Yixun Lan]

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

Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 96 ++
 1 file changed, 96 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index f6bf01cfff4b..78bb206e2897 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -303,6 +303,70 @@
function = "pwm_d";
};
};
+
+   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_ctx_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_groupz";
+   };
+   };
+
+   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_groupz";
+   };
+   };
};
};
 
@@ -346,6 +410,38 @@
#gpio-cells = <2>;
gpio-ranges = <&pinctrl_aobus 0 0 15>;
};
+
+   uart_ao_a_pins: uart_ao_a {
+   mux {
+   groups = "uart_ao_tx_a",
+   "uart_ao_rx_a";
+   function = "uart_ao_a";
+   };
+   };
+
+   uart_ao_a_

[PATCH 0/6] 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, 2 are two general fixes.
Other patches are about adding clock & pinctrl info, then using them.
Last patch enable UART_A which connect to BT module in 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.

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

Yixun Lan (6):
  ARM64: dts: meson-axg: uart: drop legacy compatible name from EE UART
  ARM64: dts: meson-axg: uart: fix address space range
  ARM64: dts: meson-axg: uart: Add the clock info description
  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 | 108 -
 2 files changed, 113 insertions(+), 4 deletions(-)

-- 
2.15.1

[PATCH 3/6] ARM64: dts: meson-axg: uart: Add the clock info description

[Yixun Lan]

Add the clock info description for the EE UART controller.

Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index 9636a7c5f6ed..f6bf01cfff4b 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -146,6 +146,8 @@
reg = <0x0 0x24000 0x0 0x18>;
interrupts = ;
status = "disabled";
+   clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
+   clock-names = "xtal", "pclk", "baud";
};
 
uart_B: serial@23000 {
@@ -153,6 +155,8 @@
reg = <0x0 0x23000 0x0 0x18>;
interrupts = ;
status = "disabled";
+   clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
+   clock-names = "xtal", "pclk", "baud";
};
};
 
-- 
2.15.1

[PATCH 5/6] 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 my rely on bootloader for the initialization.

Signed-off-by: Yixun Lan 
---
 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 70eca1f8736a..2b79be356996 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
@@ -19,4 +19,6 @@
 
 &uart_AO {
status = "okay";
+   pinctrl-0 = <&uart_ao_a_pins>;
+   pinctrl-names = "default";
 };
-- 
2.15.1

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

[Yixun Lan]

The UART_A is connect to a BT module in the S400 board.

Signed-off-by: Yixun Lan 
---
 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 2b79be356996..7e03b8da4856 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
@@ -14,9 +14,16 @@
 
aliases {
serial0 = &uart_AO;
+   serial1 = &uart_A;
};
 };
 
+&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

[PATCH 1/6] ARM64: dts: meson-axg: uart: drop legacy compatible name from EE UART

[Yixun Lan]

For the UART controller in EE domain, they require 'pclk' to work.
Current logic of the code will force to go for legacy clock probe
if it found current compatible string match to 'amlogic,meson-ao-uart'.

Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index e2b8a9c8bf0b..1c6002b3fe34 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -142,14 +142,14 @@
};
 
uart_A: serial@24000 {
-   compatible = "amlogic,meson-gx-uart", 
"amlogic,meson-uart";
+   compatible = "amlogic,meson-gx-uart";
reg = <0x0 0x24000 0x0 0x14>;
interrupts = ;
status = "disabled";
};
 
uart_B: serial@23000 {
-   compatible = "amlogic,meson-gx-uart", 
"amlogic,meson-uart";
+   compatible = "amlogic,meson-gx-uart";
reg = <0x0 0x23000 0x0 0x14>;
interrupts = ;
status = "disabled";
-- 
2.15.1

[PATCH 2/6] ARM64: dts: meson-axg: uart: fix address space range

[Yixun Lan]

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

Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index 1c6002b3fe34..9636a7c5f6ed 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -143,14 +143,14 @@
 
uart_A: serial@24000 {
compatible = "amlogic,meson-gx-uart";
-   reg = <0x0 0x24000 0x0 0x14>;
+   reg = <0x0 0x24000 0x0 0x18>;
interrupts = ;
status = "disabled";
};
 
uart_B: serial@23000 {
compatible = "amlogic,meson-gx-uart";
-   reg = <0x0 0x23000 0x0 0x14>;
+   reg = <0x0 0x23000 0x0 0x18>;
interrupts = ;
status = "disabled";
};
-- 
2.15.1

Re: [PATCH 1/6] ARM64: dts: meson-axg: uart: drop legacy compatible name from EE UART

[Yixun Lan]

On 01/05/2018 06:30 PM, Jerome Brunet wrote:
> On Fri, 2018-01-05 at 17:56 +0800, Yixun Lan wrote:
>> For the UART controller in EE domain, they require 'pclk' to work.
> 
> they ? "the driver" maybe ?
> 
>> Current logic of the code will force to go for legacy clock probe
>> if it found current compatible string match to 'amlogic,meson-ao-uart'.
> 
> did you mean "amlogic,meson-uart" instead ?
> 
good catch! it's "amlogic,meson-uart"


> Apart from that
> 
> Acked-by: Jerome Brunet 
> 
>>
>> Signed-off-by: Yixun Lan 
>> ---
>>  arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
>> b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>> index e2b8a9c8bf0b..1c6002b3fe34 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>> @@ -142,14 +142,14 @@
>>  };
>>  
>>  uart_A: serial@24000 {
>> -compatible = "amlogic,meson-gx-uart", 
>> "amlogic,meson-uart";
>> +compatible = "amlogic,meson-gx-uart";
>>  reg = <0x0 0x24000 0x0 0x14>;
>>  interrupts = ;
>>  status = "disabled";
>>  };
>>  
>>  uart_B: serial@23000 {
>> -compatible = "amlogic,meson-gx-uart", 
>> "amlogic,meson-uart";
>> +compatible = "amlogic,meson-gx-uart";
>>  reg = <0x0 0x23000 0x0 0x14>;
>>  interrupts = ;
>>  status = "disabled";
> 
> .
> 

Re: [PATCH 2/6] ARM64: dts: meson-axg: uart: fix address space range

[Yixun Lan]

On 01/05/2018 06:30 PM, Jerome Brunet wrote:
> On Fri, 2018-01-05 at 17:56 +0800, Yixun Lan wrote:
>> The address space range is actually 0x18, fixed here.
> 
> Isn't it the same for other meson SoC ? If they are compatible, it should.
> Could you please re-submit this change addressing all the required platforms ?
> 

sure

sorry, I was in hurry to send the patch set without do a full tree check..

you right, the fix should also apply to other SoCs, I will fold them
into this patch together, thanks for raising the idea.



>>
>> Signed-off-by: Yixun Lan 
>> ---
>>  arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
>> b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>> index 1c6002b3fe34..9636a7c5f6ed 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>> @@ -143,14 +143,14 @@
>>  
>>  uart_A: serial@24000 {
>>  compatible = "amlogic,meson-gx-uart";
>> -reg = <0x0 0x24000 0x0 0x14>;
>> +reg = <0x0 0x24000 0x0 0x18>;
>>  interrupts = ;
>>  status = "disabled";
>>  };
>>  
>>  uart_B: serial@23000 {
>>  compatible = "amlogic,meson-gx-uart";
>> -reg = <0x0 0x23000 0x0 0x14>;
>> +reg = <0x0 0x23000 0x0 0x18>;
>>  interrupts = ;
>>  status = "disabled";
>>  };
> 
> .
> 

Re: [PATCH 4/6] ARM64: dts: meson-axg: uart: Add the pinctrl info description

[Yixun Lan]

On 01/05/2018 06:28 PM, Jerome Brunet wrote:
> On Fri, 2018-01-05 at 17:56 +0800, Yixun Lan wrote:
>> Describe the pinctrl info for the UART controller which 
.

>> +uart_a_cts_rts_pins: uart_a_cts_rts {
>> +mux {
>> +groups = "uart_ctx_a",
> 
> uart_ctx_a does not exist in pinctrl
> 
sorry, it's a typo, it's uart_cts_a

em. end of the Friday is really bad time for me to compose the patches..


>> +"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_groupz";
> 
> "uart_ao_b_groupz" function does not exist in pinctrl
typo, uart_ao_b_gpioz

> 
.

[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 
Signed-off-by: Yixun Lan 
---
 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 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 
Signed-off-by: Yixun Lan 
---
 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 = ;
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 = ;
status = "disabled";
+   clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
+   clock-names = "xtal", "pclk", "baud";
};
};
 
-- 
2.15.1

[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@amlogic.com

[2]
 http://lkml.kernel.org/r/20180105095621.196472-1-yixun....@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 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 
Signed-off-by: Yixun Lan 
---
 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

[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 
---
 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 = ;
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 = ;
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 = ;
status = "disabled";
};
 
uart_B: serial@84dc {
compatible = "amlogic,meson-gx-uart";
-   reg = <0x0 0x84dc 0x0 0x14>;
+   reg = <0x0 0x84dc 0x0 0x18>;
interrupts = ;
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 = ;
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 = ;
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 = ;
status = "disabled";
};
-- 
2.15.1

[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 
---
 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",
+

Re: [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  wrote:
>> Describe the pinctrl info for the UART controller which is found
>> in the Meson-AXG SoCs.
>>
>> Signed-off-by: Yixun Lan 
>> ---
>>  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 h

Re: [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  wrote:
>>> Describe the pinctrl info for the UART controller which is found
>>> in the Meson-AXG SoCs.
>>>
>>> Signed-off-by: Yixun Lan 
>>> ---
>>>  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-amlo...@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>>
>> Regards
>> Martin
>>
>> .
>>
> .
> 

[PATCH 2/2] pinctrl: meson-axg: correct the pin expansion of UART_AO_B

[Yixun Lan]

The 'uart_ao_b_groups' for the UART_AO_B pins is already defined which is
living inside the AO domain, for these pins which are routed out from EE domain,
we need to correct them with the 'FUNCTION_EX' macro, otherwise there is
a conflict in the code level.

Also slightly adjust the name to make it short and more consistent.

Signed-off-by: Yixun Lan 
---
 drivers/pinctrl/meson/pinctrl-meson-axg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg.c 
b/drivers/pinctrl/meson/pinctrl-meson-axg.c
index 1fda9d6c7ea3..308e5433bd04 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-axg.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-axg.c
@@ -716,7 +716,7 @@ static const char * const uart_b_groups[] = {
"uart_tx_b_x", "uart_rx_b_x", "uart_cts_b_x", "uart_rts_b_x",
 };
 
-static const char * const uart_ao_b_gpioz_groups[] = {
+static const char * const uart_ao_b_z_groups[] = {
"uart_ao_tx_b_z", "uart_ao_rx_b_z",
"uart_ao_cts_b_z", "uart_ao_rts_b_z",
 };
@@ -855,7 +855,7 @@ static struct meson_pmx_func meson_axg_periphs_functions[] 
= {
FUNCTION(nand),
FUNCTION(uart_a),
FUNCTION(uart_b),
-   FUNCTION(uart_ao_b_gpioz),
+   FUNCTION_EX(uart_ao_b, _z),
FUNCTION(i2c0),
FUNCTION(i2c1),
FUNCTION(i2c2),
-- 
2.15.1

[PATCH 1/2] pinctrl: meson: introduce a macro to have name/groups seperated

[Yixun Lan]

We introduce a macro FUNCTION_EX here, the main motivation is
trying to have the possibility to expand the macro with the same of the
'.name' number but different multiple '.groups/.num_groups' numbers.

With this change, the meson pinctrl drivr is capable of have one uniform
'function' name but with different pin 'groups', as we face the sitiuation
that two pin groups may live inside different hardware domain (EE vs AO domain),
which mean we couldn't put them in one single group.

Signed-off-by: Yixun Lan 
---
 drivers/pinctrl/meson/pinctrl-meson.h | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/meson/pinctrl-meson.h 
b/drivers/pinctrl/meson/pinctrl-meson.h
index 12a391109329..d8f705098810 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.h
+++ b/drivers/pinctrl/meson/pinctrl-meson.h
@@ -124,13 +124,15 @@ struct meson_pinctrl {
struct device_node *of_node;
 };
 
-#define FUNCTION(fn)   \
+#define FUNCTION_EX(fn, ex)\
{   \
.name = #fn,\
-   .groups = fn ## _groups,\
-   .num_groups = ARRAY_SIZE(fn ## _groups),\
+   .groups = fn ## ex ## _groups,  \
+   .num_groups = ARRAY_SIZE(fn ## ex ## _groups),  \
}
 
+#define FUNCTION(fn)   FUNCTION_EX(fn, )
+
 #define BANK(n, f, l, fi, li, per, peb, pr, pb, dr, db, or, ob, ir, ib)
\
{   \
.name   = n,\
-- 
2.15.1

[PATCH 0/2] pinctrl: meson: use one uniform 'function' name

[Yixun Lan]

These two patches are general improvement for meson pinctrl driver.
It make the two pinctrl trees (ee/ao) to share one uniform 'function' name for
one hardware block even its pin groups live inside two differet hardware 
domains,
which for example EE vs AO domain here.

This idea is motivated by Martin's question at [1]

[1]
 
http://lkml.kernel.org/r/CAFBinCCuQ-NK747+GHDkhZty_UMMgzCYOYFcNTrRDJgU8OM=g...@mail.gmail.com


Yixun Lan (2):
  pinctrl: meson: introduce a macro to have name/groups seperated
  pinctrl: meson-axg: correct the pin expansion of UART_AO_B

 drivers/pinctrl/meson/pinctrl-meson-axg.c | 4 ++--
 drivers/pinctrl/meson/pinctrl-meson.h | 8 +---
 2 files changed, 7 insertions(+), 5 deletions(-)

-- 
2.15.1

Re: [PATCH 0/2] pinctrl: meson: use one uniform 'function' name

[Yixun Lan]

On 01/17/2018 08:14 AM, Kevin Hilman wrote:
> Yixun Lan  writes:
> 
>> Hi Jerome:
>>
>> On 01/10/2018 03:28 PM, Jerome Brunet wrote:
>>> On Wed, 2018-01-10 at 10:12 +0800, Yixun Lan wrote:
>>>>
>>>> On 01/08/18 16:52, Jerome Brunet wrote:
>>>>> On Mon, 2018-01-08 at 15:33 +0800, Yixun Lan wrote:
>>>>>> These two patches are general improvement for meson pinctrl driver.
>>>>>> It make the two pinctrl trees (ee/ao) to share one uniform 'function' 
>>>>>> name for
>>>>>> one hardware block even its pin groups live inside two differet hardware 
>>>>>> domains,
>>>>>> which for example EE vs AO domain here.
>>>>>>
>>>>>> This idea is motivated by Martin's question at [1]
>>>>>>
>>>>>> [1]
>>>>>>  
>>>>>> http://lkml.kernel.org/r/CAFBinCCuQ-NK747+GHDkhZty_UMMgzCYOYFcNTrRDJgU8OM=g...@mail.gmail.com
>>>>>>
>>>>>>
>>>>>> Yixun Lan (2):
>>>>>>   pinctrl: meson: introduce a macro to have name/groups seperated
>>>>>>   pinctrl: meson-axg: correct the pin expansion of UART_AO_B
>>>>>>
>>>>>>  drivers/pinctrl/meson/pinctrl-meson-axg.c | 4 ++--
>>>>>>  drivers/pinctrl/meson/pinctrl-meson.h | 8 +---
>>>>>>  2 files changed, 7 insertions(+), 5 deletions(-)
>>>>>
>>>>> Hi Yixun,
>>>>>
>>>>> Honestly, I don't like the idea. I think it adds an unnecessary 
>>>>> complexity.
>>>>> I don't see the point of FUNCTION_EX(uart_ao_b, _z) when you could simply 
>>>>> write 
>>>>> FUNCTION(uart_ao_b_z) ... especially when there is just a couple of 
>>>>> function per
>>>>> SoC available on different domains.
>>>>>
>>>>> A pinctrl driver can already be challenging to understand at first, let's 
>>>>> keep
>>>>> it simple and avoid adding more macros.
>>>>>
>>>>
>>>> Hi Jerome：
>>>>   In my opinion, the idea of keeping one uniform 'function' in DT (thus
>>>> introducing another macro) is worth considering. It would make the DT
>>>> part much clean.
>>>
>>> Ok this is your opinion. I don't share it. Keeping function names tidy is 
>>> good,
>>> I don't think we need another macro to do so.
>>>
>>>>   And yes, it's a trade-off here, either we 1) do more in code to make
>>>> DT clean or 2) do nothing in the code level to make DT live with it.
>>>
>>> I don't see how adding a macro doing just string concatenation is going to 
>>> make
>>> anything more clean. It does not prevent one to write FUNCTION_EX(uart_ao_b,
>>> _gpioz), resulting in uart_ao_b_gpioz, which is what is apparently 
>>> considered
>>> 'not clean'
>>>
>> for the benefits of introducing macro 'FUNCTION_EX', it will end with
>>  .name = "uart_ao_b", -> same for both EE, AO domain, and it will match
>> the DT part (although still different for '.groups')
>>
>>
>>> BTW, there no cleanness issue here, the name is just out of the 'usual 
>>> scheme'
>>> but there is no problem with. If you want to change this, and
>>> s/uart_ao_b_gpioz/uart_ao_b_z/, now is the time to change it. 
>>>
>> I'd rather *NOT* to push a pinctrl patch for just changing
>> 'uart_ao_b_gpioz' to 'uart_ao_b_z' (it's a cosmetic change, and still
>> end with two different name - 'uart_ao_b_gpioz/z' & 'uart_ao_b' in DT)
> 
> FWIW, I agree with Jerome.
> 
> Rather than this patch adding a(nother) hard-to-understand macro, please
> submit a pinctrl rename/cleanup to s/uart_ao_b_gpioz/uart_ao_b_z/.
> Since there are not any users of the _gpioz name, now is the time to do
> it.
> 
> We're already using the _x _y _z suffixes all over the place, and IMO,
> adding this new macro would make that even more confusing that it
> already is.
> 
> Kevin
> 

HI Linus
  In this case, please drop this series, and I will send another patch
which simply adjust the pin group name.
  Thanks

Yixun

[PATCH] pinctrl: meson-axg: adjust uart_ao_b pin group naming

[Yixun Lan]

Simply adjust the pin group to _x _y _z style, as to
keep the consistency in DT with previous naming scheme.

Fixes: 83c566806a68 ("pinctrl: meson-axg: Add new pinctrl driver for Meson AXG 
SoC")
Signed-off-by: Yixun Lan 

---
Hi Linus,

 Please also consider merging this patch into 'for-next', since it
fixes issue added in the same cycle.
 This patch will also obsolete previous one patches [1]

[1] pinctrl: meson: use one uniform 'function' name
http://lkml.kernel.org/r/20180108073328.205769-1-yixun@amlogic.com
---
 drivers/pinctrl/meson/pinctrl-meson-axg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg.c 
b/drivers/pinctrl/meson/pinctrl-meson-axg.c
index 1fda9d6c7ea3..4b91ff74779b 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-axg.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-axg.c
@@ -716,7 +716,7 @@ static const char * const uart_b_groups[] = {
"uart_tx_b_x", "uart_rx_b_x", "uart_cts_b_x", "uart_rts_b_x",
 };
 
-static const char * const uart_ao_b_gpioz_groups[] = {
+static const char * const uart_ao_b_z_groups[] = {
"uart_ao_tx_b_z", "uart_ao_rx_b_z",
"uart_ao_cts_b_z", "uart_ao_rts_b_z",
 };
@@ -855,7 +855,7 @@ static struct meson_pmx_func meson_axg_periphs_functions[] 
= {
FUNCTION(nand),
FUNCTION(uart_a),
FUNCTION(uart_b),
-   FUNCTION(uart_ao_b_gpioz),
+   FUNCTION(uart_ao_b_z),
FUNCTION(i2c0),
FUNCTION(i2c1),
FUNCTION(i2c2),
-- 
2.15.1

Re: [PATCH 0/2] pinctrl: meson: use one uniform 'function' name

[Yixun Lan]

On 01/08/18 16:52, Jerome Brunet wrote:
> On Mon, 2018-01-08 at 15:33 +0800, Yixun Lan wrote:
>> These two patches are general improvement for meson pinctrl driver.
>> It make the two pinctrl trees (ee/ao) to share one uniform 'function' name 
>> for
>> one hardware block even its pin groups live inside two differet hardware 
>> domains,
>> which for example EE vs AO domain here.
>>
>> This idea is motivated by Martin's question at [1]
>>
>> [1]
>>  
>> http://lkml.kernel.org/r/CAFBinCCuQ-NK747+GHDkhZty_UMMgzCYOYFcNTrRDJgU8OM=g...@mail.gmail.com
>>
>>
>> Yixun Lan (2):
>>   pinctrl: meson: introduce a macro to have name/groups seperated
>>   pinctrl: meson-axg: correct the pin expansion of UART_AO_B
>>
>>  drivers/pinctrl/meson/pinctrl-meson-axg.c | 4 ++--
>>  drivers/pinctrl/meson/pinctrl-meson.h | 8 +---
>>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> Hi Yixun,
> 
> Honestly, I don't like the idea. I think it adds an unnecessary complexity.
> I don't see the point of FUNCTION_EX(uart_ao_b, _z) when you could simply 
> write 
> FUNCTION(uart_ao_b_z) ... especially when there is just a couple of function 
> per
> SoC available on different domains.
> 
> A pinctrl driver can already be challenging to understand at first, let's keep
> it simple and avoid adding more macros.
> 

Hi Jerome：
  In my opinion, the idea of keeping one uniform 'function' in DT (thus
introducing another macro) is worth considering. It would make the DT
part much clean.
  And yes, it's a trade-off here, either we 1) do more in code to make
DT clean or 2) do nothing in the code level to make DT live with it.

Yixun

Re: [PATCH 0/2] pinctrl: meson: use one uniform 'function' name

[Yixun Lan]

Hi Jerome:

On 01/10/2018 03:28 PM, Jerome Brunet wrote:
> On Wed, 2018-01-10 at 10:12 +0800, Yixun Lan wrote:
>>
>> On 01/08/18 16:52, Jerome Brunet wrote:
>>> On Mon, 2018-01-08 at 15:33 +0800, Yixun Lan wrote:
>>>> These two patches are general improvement for meson pinctrl driver.
>>>> It make the two pinctrl trees (ee/ao) to share one uniform 'function' name 
>>>> for
>>>> one hardware block even its pin groups live inside two differet hardware 
>>>> domains,
>>>> which for example EE vs AO domain here.
>>>>
>>>> This idea is motivated by Martin's question at [1]
>>>>
>>>> [1]
>>>>  
>>>> http://lkml.kernel.org/r/CAFBinCCuQ-NK747+GHDkhZty_UMMgzCYOYFcNTrRDJgU8OM=g...@mail.gmail.com
>>>>
>>>>
>>>> Yixun Lan (2):
>>>>   pinctrl: meson: introduce a macro to have name/groups seperated
>>>>   pinctrl: meson-axg: correct the pin expansion of UART_AO_B
>>>>
>>>>  drivers/pinctrl/meson/pinctrl-meson-axg.c | 4 ++--
>>>>  drivers/pinctrl/meson/pinctrl-meson.h | 8 +---
>>>>  2 files changed, 7 insertions(+), 5 deletions(-)
>>>
>>> Hi Yixun,
>>>
>>> Honestly, I don't like the idea. I think it adds an unnecessary complexity.
>>> I don't see the point of FUNCTION_EX(uart_ao_b, _z) when you could simply 
>>> write 
>>> FUNCTION(uart_ao_b_z) ... especially when there is just a couple of 
>>> function per
>>> SoC available on different domains.
>>>
>>> A pinctrl driver can already be challenging to understand at first, let's 
>>> keep
>>> it simple and avoid adding more macros.
>>>
>>
>> Hi Jerome：
>>   In my opinion, the idea of keeping one uniform 'function' in DT (thus
>> introducing another macro) is worth considering. It would make the DT
>> part much clean.
> 
> Ok this is your opinion. I don't share it. Keeping function names tidy is 
> good,
> I don't think we need another macro to do so.
> 
>>   And yes, it's a trade-off here, either we 1) do more in code to make
>> DT clean or 2) do nothing in the code level to make DT live with it.
> 
> I don't see how adding a macro doing just string concatenation is going to 
> make
> anything more clean. It does not prevent one to write FUNCTION_EX(uart_ao_b,
> _gpioz), resulting in uart_ao_b_gpioz, which is what is apparently considered
> 'not clean'
> 
for the benefits of introducing macro 'FUNCTION_EX', it will end with
 .name = "uart_ao_b", -> same for both EE, AO domain, and it will match
the DT part (although still different for '.groups')


> BTW, there no cleanness issue here, the name is just out of the 'usual scheme'
> but there is no problem with. If you want to change this, and
> s/uart_ao_b_gpioz/uart_ao_b_z/, now is the time to change it. 
> 
I'd rather *NOT* to push a pinctrl patch for just changing
'uart_ao_b_gpioz' to 'uart_ao_b_z' (it's a cosmetic change, and still
end with two different name - 'uart_ao_b_gpioz/z' & 'uart_ao_b' in DT)

>>
>> Yixun
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> ___
> linux-amlogic mailing list
> linux-amlo...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> 

[PATCH 3/3] ARM64: dts: meson: fix clock source of the pclk for UART_AO

[Yixun Lan]

>From the hardware perspective, the clk81 is the parent of
the UART_AO clocks. So it make more sense to mark
the CLKID_AO_UART1/2 as the pclk.

Previous we had flag=CLK_IGNORE_UNUSED in AO clock driver.
This will make kernel leave clock as it is while bootloader
set the clock on.

Signed-off-by: Yixun Lan 
---
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi  | 4 ++--
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 ++--
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index 48584d5a329b..e538e2448bb5 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -533,7 +533,7 @@
compatible = "amlogic,meson-gx-uart", 
"amlogic,meson-ao-uart";
reg = <0x0 0x3000 0x0 0x18>;
interrupts = ;
-   clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+   clocks = <&xtal>, <&clkc_AO CLKID_AO_UART1>, 
<&xtal>;
clock-names = "xtal", "pclk", "baud";
status = "disabled";
};
@@ -542,7 +542,7 @@
compatible = "amlogic,meson-gx-uart", 
"amlogic,meson-ao-uart";
reg = <0x0 0x4000 0x0 0x18>;
interrupts = ;
-   clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+   clocks = <&xtal>, <&clkc_AO CLKID_AO_UART2>, 
<&xtal>;
clock-names = "xtal", "pclk", "baud";
status = "disabled";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 3290a4dc3522..a458ea50c2ef 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -783,12 +783,12 @@
 };
 
 &uart_AO {
-   clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+   clocks = <&xtal>, <&clkc_AO CLKID_AO_UART1>, <&xtal>;
clock-names = "xtal", "pclk", "baud";
 };
 
 &uart_AO_B {
-   clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+   clocks = <&xtal>, <&clkc_AO CLKID_AO_UART2>, <&xtal>;
clock-names = "xtal", "pclk", "baud";
 };
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index 4f355f17eed6..474f4f9630e4 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -733,12 +733,12 @@
 };
 
 &uart_AO {
-   clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+   clocks = <&xtal>, <&clkc_AO CLKID_AO_UART1>, <&xtal>;
clock-names = "xtal", "pclk", "baud";
 };
 
 &uart_AO_B {
-   clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+   clocks = <&xtal>, <&clkc_AO CLKID_AO_UART2>, <&xtal>;
clock-names = "xtal", "pclk", "baud";
 };
 
-- 
2.15.1

[PATCH 0/3] clk: meson: drop unnecessary AO clock flags

[Yixun Lan]

  This first two patches will drop unnecessary flags for
the AO clock driver.
  The third DTS patch is a follow-up fix for the second patch,
and I've tested them on P200, P212, Q200, S400 boards.

 Note: this patch series was made on top of patch [0]

[0] https://lkml.kernel.org/r/20180323143816.200573-1-yixun@amlogic.com

Yixun Lan (3):
  clk: meson: drop CLK_SET_RATE_PARENT flag
  clk: meson: drop CLK_IGNORE_UNUSED flag
  ARM64: dts: meson: fix clock source of the pclk for UART_AO

 arch/arm64/boot/dts/amlogic/meson-axg.dtsi  | 4 ++--
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 ++--
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 4 ++--
 drivers/clk/meson/axg-aoclk.c   | 1 -
 drivers/clk/meson/gxbb-aoclk.c  | 1 -
 5 files changed, 6 insertions(+), 8 deletions(-)

-- 
2.15.1

[PATCH 2/3] clk: meson: drop CLK_IGNORE_UNUSED flag

[Yixun Lan]

Rely on drivers to request the clock explicitly.

Previous the kernel will leave the clock on while
bootloader adready initilized the clock, this wasn't
optimal way, so fix it here.

Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/axg-aoclk.c  | 1 -
 drivers/clk/meson/gxbb-aoclk.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/clk/meson/axg-aoclk.c b/drivers/clk/meson/axg-aoclk.c
index 121ecb45264f..026b2fe56a29 100644
--- a/drivers/clk/meson/axg-aoclk.c
+++ b/drivers/clk/meson/axg-aoclk.c
@@ -27,7 +27,6 @@ static struct clk_regmap _name##_ao = {   
\
.ops = &clk_regmap_gate_ops,\
.parent_names = (const char *[]){ "clk81" },\
.num_parents = 1,   \
-   .flags = CLK_IGNORE_UNUSED, \
},  \
 }
 
diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index e7bc04d3e24f..005d84ff590d 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -70,7 +70,6 @@ static struct clk_regmap _name##_ao = {   
\
.ops = &clk_regmap_gate_ops,\
.parent_names = (const char *[]){ "clk81" },\
.num_parents = 1,   \
-   .flags = CLK_IGNORE_UNUSED, \
},  \
 }
 
-- 
2.15.1

[PATCH 1/3] clk: meson: drop CLK_SET_RATE_PARENT flag

[Yixun Lan]

The clk81 is not expected to be changed, so drop this flag.

Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/gxbb-aoclk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 0f089cbce594..e7bc04d3e24f 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -70,7 +70,7 @@ static struct clk_regmap _name##_ao = {   
\
.ops = &clk_regmap_gate_ops,\
.parent_names = (const char *[]){ "clk81" },\
.num_parents = 1,   \
-   .flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED), \
+   .flags = CLK_IGNORE_UNUSED, \
},  \
 }
 
-- 
2.15.1

[PATCH 0/3] iio: adc: meson-axg: add saradc driver support

[Yixun Lan]

 The patch 1 is a general small improvement, no function changed!
 The last two patches try to add saradc support for Amlogic'S Meson-AXG
SoC platfrom.
 I will send the DTS part patch in another thread, since it depend on
the clock AO driver[0].


[0] https://lkml.kernel.org/r/20180323143816.200573-1-yixun@amlogic.com

Xingyu Chen (2):
  dt-bindings: iio: adc: document the Meson AXG support
  iio: adc: meson-axg: add saradc driver

Yixun Lan (1):
  iio: adc: meson-saradc: squash and share the common adc platform data

 .../bindings/iio/adc/amlogic,meson-saradc.txt  |  1 +
 drivers/iio/adc/meson_saradc.c | 83 +-
 2 files changed, 51 insertions(+), 33 deletions(-)

-- 
2.15.1

[PATCH 2/3] dt-bindings: iio: adc: document the Meson AXG support

[Yixun Lan]

From: Xingyu Chen 

Update the documentation to expicitly support the Meson-AXG SoC.

Signed-off-by: Xingyu Chen 
---
 Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt 
b/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
index 1e6ee3deb4fa..d1acd5ea2737 100644
--- a/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
@@ -7,6 +7,7 @@ Required properties:
- "amlogic,meson-gxbb-saradc" for GXBB
- "amlogic,meson-gxl-saradc" for GXL
- "amlogic,meson-gxm-saradc" for GXM
+   - "amlogic,meson-axg-saradc" for AXG
along with the generic "amlogic,meson-saradc"
 - reg: the physical base address and length of the registers
 - interrupts:  the interrupt indicating end of sampling
-- 
2.15.1

[PATCH 3/3] iio: adc: meson-axg: add saradc driver

[Yixun Lan]

From: Xingyu Chen 

Add the SAR ADC driver for the Amlogic Meson-AXG SoC.

Signed-off-by: Xingyu Chen 
---
 drivers/iio/adc/meson_saradc.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 799ed929ab99..a5d481a2b4ef 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -935,6 +935,11 @@ static const struct meson_sar_adc_data 
meson_sar_adc_gxm_data = {
.name = "meson-gxm-saradc",
 };
 
+static const struct meson_sar_adc_data meson_sar_adc_axg_data = {
+   .param = &meson_sar_adc_gxl_param,
+   .name = "meson-axg-saradc",
+};
+
 static const struct of_device_id meson_sar_adc_of_match[] = {
{
.compatible = "amlogic,meson8-saradc",
@@ -953,6 +958,9 @@ static const struct of_device_id meson_sar_adc_of_match[] = 
{
}, {
.compatible = "amlogic,meson-gxm-saradc",
.data = &meson_sar_adc_gxm_data,
+   }, {
+   .compatible = "amlogic,meson-axg-saradc",
+   .data = &meson_sar_adc_axg_data,
},
{},
 };
-- 
2.15.1

[PATCH 1/3] iio: adc: meson-saradc: squash and share the common adc platform data

[Yixun Lan]

Extract and promote common adc platform data into a new structure,
to make it better share the info between several SoCs,
this will avoid duplicating the code all over the place,
Save a few memory and make the code more maintainable.

Signed-off-by: Yixun Lan 
---
 drivers/iio/adc/meson_saradc.c | 75 +++---
 1 file changed, 42 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index 29fa7736d80c..799ed929ab99 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -219,15 +219,19 @@ enum meson_sar_adc_chan7_mux_sel {
CHAN7_MUX_CH7_INPUT = 0x7,
 };
 
-struct meson_sar_adc_data {
+struct meson_sar_adc_param {
boolhas_bl30_integration;
unsigned long   clock_rate;
u32 bandgap_reg;
unsigned intresolution;
-   const char  *name;
const struct regmap_config  *regmap_config;
 };
 
+struct meson_sar_adc_data {
+   const struct meson_sar_adc_param*param;
+   const char  *name;
+};
+
 struct meson_sar_adc_priv {
struct regmap   *regmap;
struct regulator*vref;
@@ -276,7 +280,7 @@ static int meson_sar_adc_calib_val(struct iio_dev 
*indio_dev, int val)
/* use val_calib = scale * val_raw + offset calibration function */
tmp = div_s64((s64)val * priv->calibscale, MILLION) + priv->calibbias;
 
-   return clamp(tmp, 0, (1 << priv->data->resolution) - 1);
+   return clamp(tmp, 0, (1 << priv->data->param->resolution) - 1);
 }
 
 static int meson_sar_adc_wait_busy_clear(struct iio_dev *indio_dev)
@@ -328,7 +332,7 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev 
*indio_dev,
}
 
fifo_val = FIELD_GET(MESON_SAR_ADC_FIFO_RD_SAMPLE_VALUE_MASK, regval);
-   fifo_val &= GENMASK(priv->data->resolution - 1, 0);
+   fifo_val &= GENMASK(priv->data->param->resolution - 1, 0);
*val = meson_sar_adc_calib_val(indio_dev, fifo_val);
 
return 0;
@@ -447,7 +451,7 @@ static int meson_sar_adc_lock(struct iio_dev *indio_dev)
 
mutex_lock(&indio_dev->mlock);
 
-   if (priv->data->has_bl30_integration) {
+   if (priv->data->param->has_bl30_integration) {
/* prevent BL30 from using the SAR ADC while we are using it */
regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
MESON_SAR_ADC_DELAY_KERNEL_BUSY,
@@ -473,7 +477,7 @@ static void meson_sar_adc_unlock(struct iio_dev *indio_dev)
 {
struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 
-   if (priv->data->has_bl30_integration)
+   if (priv->data->param->has_bl30_integration)
/* allow BL30 to use the SAR ADC again */
regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
MESON_SAR_ADC_DELAY_KERNEL_BUSY, 0);
@@ -557,7 +561,7 @@ static int meson_sar_adc_iio_info_read_raw(struct iio_dev 
*indio_dev,
}
 
*val = ret / 1000;
-   *val2 = priv->data->resolution;
+   *val2 = priv->data->param->resolution;
return IIO_VAL_FRACTIONAL_LOG2;
 
case IIO_CHAN_INFO_CALIBBIAS:
@@ -630,7 +634,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 */
meson_sar_adc_set_chan7_mux(indio_dev, CHAN7_MUX_CH7_INPUT);
 
-   if (priv->data->has_bl30_integration) {
+   if (priv->data->param->has_bl30_integration) {
/*
 * leave sampling delay and the input clocks as configured by
 * BL30 to make sure BL30 gets the values it expects when
@@ -710,7 +714,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
return ret;
}
 
-   ret = clk_set_rate(priv->adc_clk, priv->data->clock_rate);
+   ret = clk_set_rate(priv->adc_clk, priv->data->param->clock_rate);
if (ret) {
dev_err(indio_dev->dev.parent,
"failed to set adc clock rate\n");
@@ -723,14 +727,15 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
 static void meson_sar_adc_set_bandgap(struct iio_dev *indio_dev, bool on_off)
 {
struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+   const struct meson_sar_adc_param *param = priv->data->param;
u32 enable_mask;
 
-   if (priv->data->bandgap_reg == MESON_SAR_ADC_REG11)
+   if (param->bandgap_reg == MESON_SAR_ADC_REG11)
enable_mask = MESON_SAR_ADC_REG11_BANDGAP_EN;
else
 

[PATCH] ARM64: dts: meson-axg: add saradc support

[Yixun Lan]

From: Xingyu Chen 

Add the DT info for SAR ADC of the Amlogic's Meson-AXG SoC.

Signed-off-by: Xingyu Chen 
Signed-off-by: Yixun Lan 

---
Hi Kevin:

  Please note, the saradc driver in Meson-AXG platfrom actually
depend on the AO clock driver [0] & saradc patch itself [1].
  And I've rebased this patch on top of your v4.17/dt64 branch

[0] https://lkml.kernel.org/r/20180323143816.200573-1-yixun@amlogic.com
[1] https://lkml.kernel.org/r/20180326084629.100070-1-yixun@amlogic.com

Yixun
---
 arch/arm64/boot/dts/amlogic/meson-axg-s400.dts |  5 +
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 21 +
 2 files changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts 
b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
index 57eedced5a51..bbb88debbc04 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
@@ -47,3 +47,8 @@
pinctrl-0 = <&i2c1_z_pins>;
pinctrl-names = "default";
 };
+
+&saradc {
+   status = "okay";
+   vref-supply = <&vddio_ao18>;
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index 40ca49fb94a6..105884efc033 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -818,6 +818,27 @@
interrupts = ;
status = "disabled";
};
+
+   saradc: adc@9000 {
+   compatible = "amlogic,meson-axg-saradc",
+   "amlogic,meson-saradc";
+   reg = <0x0 0x9000 0x0 0x38>;
+   #io-channel-cells = <1>;
+   interrupts = ;
+   clocks = <&xtal>,
+   <&clkc_AO CLKID_AO_SAR_ADC>,
+   <&clkc_AO CLKID_AO_SAR_ADC_CLK>,
+   <&clkc_AO CLKID_AO_SAR_ADC_SEL>;
+   clock-names = "clkin", "core", "adc_clk", 
"adc_sel";
+   status = "disabled";
+   };
};
};
+
+   vddio_ao18: regulator-vddio_ao18 {
+   compatible = "regulator-fixed";
+   regulator-name = "VDDIO_AO18";
+   regulator-min-microvolt = <180>;
+   regulator-max-microvolt = <180>;
+   };
 };
-- 
2.15.1

Re: [PATCH 3/3] ARM64: dts: meson: fix clock source of the pclk for UART_AO

[Yixun Lan]

On 03/27/2018 05:20 PM, Jerome Brunet wrote:
> On Mon, 2018-03-26 at 16:18 +0800, Yixun Lan wrote:
>> From the hardware perspective, the clk81 is the parent of
>> the UART_AO clocks. So it make more sense to mark
>> the CLKID_AO_UART1/2 as the pclk.
>>
>> Previous we had flag=CLK_IGNORE_UNUSED in AO clock driver.
>> This will make kernel leave clock as it is while bootloader
>> set the clock on.
>>
> 
> Hi Yixun,
> 
> I'm not sure I understand your description correctly so I'll phrase. Please 
> tell
> me if I misunderstood something
> 
> The actual pclk of the AO uarts is the corresponding clkc_ao uart gate, not 
> the
> main clock controller clk81. This was not problem so far, because the 
> uart_gate
> had the CLK_IGNORE_UNUSED flag, which kept the gate open.
> 
Yes, exactly

> In another patch, you removed this flag, so you need to give the correct clock
> in DT to make things work again ? right ?
> 
Yes, right.


> If I got it correctly, please rephrase the description a bit.
> Also you need to be careful in which order patches get merged.
> 
> 1. You need the clkc_ao in place, exporting CLKID_AO_UARTx with
> CLK_IGNORE_UNUSED.
> 
> 2. This patch can be merged (still with CLK_IGNORE_UNUSED)
> 
> 3. Once the DT patch have reached mainline, you should a patch to remove to
> CLK_IGNORE_UNUSED flag
> 
> This way, we avoid breaking bisect, with revision where CLKID_AO_UARTx would 
> get
> disabled by the CCF clk_disable_unused() mechanism while the UART driver does
> not claim the appropriate clock.
> 
I wasn't sure about what's the best approach
I just gave a warning about that patch 2 (removing CLK_IGNORE_UNUSED)
and patch 3 (fix the DTS) should be merged together

and your suggestion sounds good, I will adjust

> 
>> Signed-off-by: Yixun Lan 
>> ---
>>  arch/arm64/boot/dts/amlogic/meson-axg.dtsi  | 4 ++--
>>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 ++--
>>  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 4 ++--
>>  3 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi 
>> b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>> index 48584d5a329b..e538e2448bb5 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
>> @@ -533,7 +533,7 @@
>>  compatible = "amlogic,meson-gx-uart", 
>> "amlogic,meson-ao-uart";
>>  reg = <0x0 0x3000 0x0 0x18>;
>>  interrupts = ;
>> -clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
>> +clocks = <&xtal>, <&clkc_AO CLKID_AO_UART1>, 
>> <&xtal>;
>>  clock-names = "xtal", "pclk", "baud";
>>  status = "disabled";
>>  };
>> @@ -542,7 +542,7 @@
>>  compatible = "amlogic,meson-gx-uart", 
>> "amlogic,meson-ao-uart";
>>  reg = <0x0 0x4000 0x0 0x18>;
>>  interrupts = ;
>> -clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
>> +clocks = <&xtal>, <&clkc_AO CLKID_AO_UART2>, 
>> <&xtal>;
>>  clock-names = "xtal", "pclk", "baud";
>>  status = "disabled";
>>  };
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi 
>> b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> index 3290a4dc3522..a458ea50c2ef 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> @@ -783,12 +783,12 @@
>>  };
>>  
>>  &uart_AO {
>> -clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
>> +clocks = <&xtal>, <&clkc_AO CLKID_AO_UART1>, <&xtal>;
>>  clock-names = "xtal", "pclk", "baud";
>>  };
>>  
>>  &uart_AO_B {
>> -clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
>> +clocks = <&xtal>, <&clkc_AO CLKID_AO_UART2>, <&xtal>;
>>  clock-names = "xtal", "pclk", "baud";
>>  };
>>  
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi 
>> b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>> index 4f355f17eed6..474f4f9630e4 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>> @@ -733,12 +733,12 @@
>>  };
>>  
>>  &uart_AO {
>> -clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
>> +clocks = <&xtal>, <&clkc_AO CLKID_AO_UART1>, <&xtal>;
>>  clock-names = "xtal", "pclk", "baud";
>>  };
>>  
>>  &uart_AO_B {
>> -clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
>> +clocks = <&xtal>, <&clkc_AO CLKID_AO_UART2>, <&xtal>;
>>  clock-names = "xtal", "pclk", "baud";
>>  };
>>  
> 
> .
> 

Re: [PATCH 0/3] clk: meson: drop unnecessary AO clock flags

[Yixun Lan]

HI Jerome

On 03/27/2018 05:21 PM, Jerome Brunet wrote:
> On Mon, 2018-03-26 at 16:18 +0800, Yixun Lan wrote:
>>   This first two patches will drop unnecessary flags for
>> the AO clock driver.
>>   The third DTS patch is a follow-up fix for the second patch,
>> and I've tested them on P200, P212, Q200, S400 boards.
>>
>>  Note: this patch series was made on top of patch [0]
>>
>> [0] https://lkml.kernel.org/r/20180323143816.200573-1-yixun@amlogic.com
>>
>> Yixun Lan (3):
>>   clk: meson: drop CLK_SET_RATE_PARENT flag
>>   clk: meson: drop CLK_IGNORE_UNUSED flag
>>   ARM64: dts: meson: fix clock source of the pclk for UART_AO
> 
> Please add patch 1 and 2 to the next version of your clkc_ao series
> 
I was thinking about patch 1 & 2 as independent patch of the clkc_ao
series..

anyway, as your suggestion in another thread,
I could include the patch 1 into next clkc_ao series
send patch 3 (DTS part) independently to Kevin, once patch 3 accepted
into mainline, I will then send patch 2 as a fix
(this will avoid breaking bisect)

> Patch 3 should sent independently to Kevin, with note warning about the
> different dependencies (see comment in patch 3)
> 
>>
>>  arch/arm64/boot/dts/amlogic/meson-axg.dtsi  | 4 ++--
>>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 ++--
>>  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 4 ++--
>>  drivers/clk/meson/axg-aoclk.c   | 1 -
>>  drivers/clk/meson/gxbb-aoclk.c  | 1 -
>>  5 files changed, 6 insertions(+), 8 deletions(-)
>>
> 
> .
> 

Re: [PATCH v3 1/6] clk: meson: aoclk: refactor common code into dedicated file

[Yixun Lan]

HI Stephen
 thanks for the review

On 04/07/18 02:39, Stephen Boyd wrote:
> Quoting Yixun Lan (2018-03-27 19:50:45)
>> diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
>> index 9ec23ae9a219..5a922639a264 100644
>> --- a/drivers/clk/meson/gxbb-aoclk.c
>> +++ b/drivers/clk/meson/gxbb-aoclk.c
>> @@ -165,38 +135,39 @@ static int gxbb_aoclkc_probe(struct platform_device 
>> *pdev)
>> return -ENODEV;
>> }
>>  
>> -   /* Reset Controller */
>> -   rstc->regmap = regmap;
>> -   rstc->data = gxbb_aoclk_reset;
>> -   rstc->reset.ops = &gxbb_aoclk_reset_ops;
>> -   rstc->reset.nr_resets = ARRAY_SIZE(gxbb_aoclk_reset);
>> -   rstc->reset.of_node = dev->of_node;
>> -   ret = devm_reset_controller_register(dev, &rstc->reset);
>> -
>> -   /*
>> -* Populate regmap and register all clks
>> -*/
>> -   for (clkid = 0; clkid < ARRAY_SIZE(gxbb_aoclk_gate); clkid++) {
>> -   gxbb_aoclk_gate[clkid]->map = regmap;
>> -
>> -   ret = devm_clk_hw_register(dev,
>> -  
>> gxbb_aoclk_onecell_data.hws[clkid]);
>> -   if (ret)
>> -   return ret;
>> -   }
>> -
>> /* Specific clocks */
>> cec_32k_ao.regmap = regmap;
>> ret = devm_clk_hw_register(dev, &cec_32k_ao.hw);
>> -   if (ret)
>> +   if (ret) {
>> +   dev_err(&pdev->dev, "clk cec_32k_ao register failed.\n");
>> return ret;
>> +   }
>> +
>> +   return 0;
>> +}
>> +
>> +static struct meson_aoclk_data gxbb_aoclkc_data = {
> 
> Can this be const?
> 
sure, I'll update at v4
>> +   .reset_reg  = AO_RTI_GEN_CNTL_REG0,
>> +   .num_reset  = ARRAY_SIZE(gxbb_aoclk_reset),
>> +   .reset  = gxbb_aoclk_reset,
>> +   .num_clks   = ARRAY_SIZE(gxbb_aoclk_gate),
>> +   .clks   = gxbb_aoclk_gate,
>> +   .hw_data= &gxbb_aoclk_onecell_data,
>> +};
>> +
>> diff --git a/drivers/clk/meson/meson-aoclk.c 
>> b/drivers/clk/meson/meson-aoclk.c
>> new file mode 100644
>> index ..36067c801f7b
>> --- /dev/null
>> +++ b/drivers/clk/meson/meson-aoclk.c
>> @@ -0,0 +1,79 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Amlogic Meson-AXG Clock Controller Driver
>> + *
>> + * Copyright (c) 2016 BayLibre, SAS.
>> + * Author: Neil Armstrong 
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Qiufang Dai 
>> + * Author: Yixun Lan 
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include "clk-regmap.h"
>> +#include "meson-aoclk.h"
>> +
>> +static int meson_aoclk_do_reset(struct reset_controller_dev *rcdev,
>> +  unsigned long id)
>> +{
>> +   struct meson_aoclk_reset_controller *rstc =
>> +   container_of(rcdev, struct meson_aoclk_reset_controller, 
>> reset);
>> +
>> +   return regmap_write(rstc->regmap, rstc->data->reset_reg,
>> +   BIT(rstc->data->reset[id]));
>> +}
>> +
>> +static const struct reset_control_ops meson_aoclk_reset_ops = {
>> +   .reset = meson_aoclk_do_reset,
>> +};
>> +
>> +int meson_aoclkc_probe(struct platform_device *pdev)
>> +{
>> +   struct meson_aoclk_reset_controller *rstc;
>> +   struct meson_aoclk_data *data;
>> +   struct device *dev = &pdev->dev;
>> +   struct regmap *regmap;
>> +   int ret, clkid;
>> +
>> +   data = (struct meson_aoclk_data *)
>> +   of_device_get_match_data(dev);
>> +   if (!data)
>> +   return -ENODEV;
>> +
>> +   rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
>> +   if (!rstc)
>> +   return -ENOMEM;
>> +
>> +   regmap = syscon_node_to_regmap(of_get_parent(dev->of_node));
>> +   if (IS_ERR(regmap)) {
>> +   dev_err(dev, "failed to get regmap\n");
>> +   return -ENODEV;
>> +   }
>> +
>> +   /* Reset Controller */
>> +   rstc->data = data;
>> +   rstc->regmap = regmap;
>> +   rstc->reset.ops = &meson_aoclk_reset_ops;
>> + 

[PATCH v4 1/7] clk: meson: aoclk: refactor common code into dedicated file

[Yixun Lan]

We try to refactor the common code into one dedicated file,
while preparing to add new Meson-AXG aoclk driver, this would
help us to better share the code by all aoclk drivers.

Suggested-by: Jerome Brunet 
Signed-off-by: Yixun Lan 
---
 drivers/clk/meson/Makefile  |  2 +-
 drivers/clk/meson/gxbb-aoclk.c  | 91 ++---
 drivers/clk/meson/gxbb-aoclk.h  |  7 
 drivers/clk/meson/meson-aoclk.c | 83 +
 drivers/clk/meson/meson-aoclk.h | 35 
 5 files changed, 157 insertions(+), 61 deletions(-)
 create mode 100644 drivers/clk/meson/meson-aoclk.c
 create mode 100644 drivers/clk/meson/meson-aoclk.h

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index ffee82e60b7a..555ab9c6ab64 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -4,6 +4,6 @@
 
 obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-mpll.o clk-audio-divider.o
 obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
-obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
+obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o meson-aoclk.o gxbb-aoclk.o 
gxbb-aoclk-32k.o
 obj-$(CONFIG_COMMON_CLK_AXG)+= axg.o
 obj-$(CONFIG_COMMON_CLK_REGMAP_MESON)  += clk-regmap.o
diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 9ec23ae9a219..59db8e92f8cf 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -52,39 +52,13 @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
-#include 
-#include 
 #include 
 #include 
 #include 
-#include 
 #include 
-#include 
-#include 
-#include 
 #include "clk-regmap.h"
 #include "gxbb-aoclk.h"
 
-struct gxbb_aoclk_reset_controller {
-   struct reset_controller_dev reset;
-   unsigned int *data;
-   struct regmap *regmap;
-};
-
-static int gxbb_aoclk_do_reset(struct reset_controller_dev *rcdev,
-  unsigned long id)
-{
-   struct gxbb_aoclk_reset_controller *reset =
-   container_of(rcdev, struct gxbb_aoclk_reset_controller, reset);
-
-   return regmap_write(reset->regmap, AO_RTI_GEN_CNTL_REG0,
-   BIT(reset->data[id]));
-}
-
-static const struct reset_control_ops gxbb_aoclk_reset_ops = {
-   .reset = gxbb_aoclk_do_reset,
-};
-
 #define GXBB_AO_GATE(_name, _bit)  \
 static struct clk_regmap _name##_ao = {
\
.data = &(struct clk_regmap_gate_data) {\
@@ -117,7 +91,7 @@ static struct aoclk_cec_32k cec_32k_ao = {
},
 };
 
-static unsigned int gxbb_aoclk_reset[] = {
+static const unsigned int gxbb_aoclk_reset[] = {
[RESET_AO_REMOTE] = 16,
[RESET_AO_I2C_MASTER] = 18,
[RESET_AO_I2C_SLAVE] = 19,
@@ -135,7 +109,7 @@ static struct clk_regmap *gxbb_aoclk_gate[] = {
[CLKID_AO_IR_BLASTER] = &ir_blaster_ao,
 };
 
-static struct clk_hw_onecell_data gxbb_aoclk_onecell_data = {
+static const struct clk_hw_onecell_data gxbb_aoclk_onecell_data = {
.hws = {
[CLKID_AO_REMOTE] = &remote_ao.hw,
[CLKID_AO_I2C_MASTER] = &i2c_master_ao.hw,
@@ -145,19 +119,15 @@ static struct clk_hw_onecell_data gxbb_aoclk_onecell_data 
= {
[CLKID_AO_IR_BLASTER] = &ir_blaster_ao.hw,
[CLKID_AO_CEC_32K] = &cec_32k_ao.hw,
},
-   .num = 7,
+   .num = NR_CLKS,
 };
 
-static int gxbb_aoclkc_probe(struct platform_device *pdev)
+static int gxbb_aoclkc_register_specific_clk(
+   struct platform_device *pdev)
 {
-   struct gxbb_aoclk_reset_controller *rstc;
struct device *dev = &pdev->dev;
struct regmap *regmap;
-   int ret, clkid;
-
-   rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL);
-   if (!rstc)
-   return -ENOMEM;
+   int ret;
 
regmap = syscon_node_to_regmap(of_get_parent(dev->of_node));
if (IS_ERR(regmap)) {
@@ -165,38 +135,39 @@ static int gxbb_aoclkc_probe(struct platform_device *pdev)
return -ENODEV;
}
 
-   /* Reset Controller */
-   rstc->regmap = regmap;
-   rstc->data = gxbb_aoclk_reset;
-   rstc->reset.ops = &gxbb_aoclk_reset_ops;
-   rstc->reset.nr_resets = ARRAY_SIZE(gxbb_aoclk_reset);
-   rstc->reset.of_node = dev->of_node;
-   ret = devm_reset_controller_register(dev, &rstc->reset);
-
-   /*
-* Populate regmap and register all clks
-*/
-   for (clkid = 0; clkid < ARRAY_SIZE(gxbb_aoclk_gate); clkid++) {
-   gxbb_aoclk_gate[clkid]->map = regmap;
-
-   ret = devm_clk_hw_register(dev,
-  gxbb_aoclk_onecell_data.hws[clkid]);
-   

[PATCH v4 4/7] dt-bindings: clock: reset: Add AXG AO Clock and Reset Bindings

[Yixun Lan]

Add dt-bindings headers for the Meson-AXG's AO clock and
reset controller.

Reviewed-by: Rob Herring 
Signed-off-by: Yixun Lan 
---
 include/dt-bindings/clock/axg-aoclkc.h | 26 ++
 include/dt-bindings/reset/axg-aoclkc.h | 20 
 2 files changed, 46 insertions(+)
 create mode 100644 include/dt-bindings/clock/axg-aoclkc.h
 create mode 100644 include/dt-bindings/reset/axg-aoclkc.h

diff --git a/include/dt-bindings/clock/axg-aoclkc.h 
b/include/dt-bindings/clock/axg-aoclkc.h
new file mode 100644
index ..61955016a55b
--- /dev/null
+++ b/include/dt-bindings/clock/axg-aoclkc.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/*
+ * Copyright (c) 2016 BayLibre, SAS
+ * Author: Neil Armstrong 
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai 
+ */
+
+#ifndef DT_BINDINGS_CLOCK_AMLOGIC_MESON_AXG_AOCLK
+#define DT_BINDINGS_CLOCK_AMLOGIC_MESON_AXG_AOCLK
+
+#define CLKID_AO_REMOTE0
+#define CLKID_AO_I2C_MASTER1
+#define CLKID_AO_I2C_SLAVE 2
+#define CLKID_AO_UART1 3
+#define CLKID_AO_UART2 4
+#define CLKID_AO_IR_BLASTER5
+#define CLKID_AO_SAR_ADC   6
+#define CLKID_AO_CLK81 7
+#define CLKID_AO_SAR_ADC_SEL   8
+#define CLKID_AO_SAR_ADC_DIV   9
+#define CLKID_AO_SAR_ADC_CLK   10
+#define CLKID_AO_ALT_XTAL  11
+
+#endif
diff --git a/include/dt-bindings/reset/axg-aoclkc.h 
b/include/dt-bindings/reset/axg-aoclkc.h
new file mode 100644
index ..d342c0b6b2a7
--- /dev/null
+++ b/include/dt-bindings/reset/axg-aoclkc.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/*
+ * Copyright (c) 2016 BayLibre, SAS
+ * Author: Neil Armstrong 
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Qiufang Dai 
+ */
+
+#ifndef DT_BINDINGS_RESET_AMLOGIC_MESON_AXG_AOCLK
+#define DT_BINDINGS_RESET_AMLOGIC_MESON_AXG_AOCLK
+
+#define RESET_AO_REMOTE0
+#define RESET_AO_I2C_MASTER1
+#define RESET_AO_I2C_SLAVE 2
+#define RESET_AO_UART1 3
+#define RESET_AO_UART2 4
+#define RESET_AO_IR_BLASTER5
+
+#endif
-- 
2.15.1