[PATCH 0/3] clk: meson-g12a: Add EE clock controller driver

[PATCH 0/3] clk: meson-g12a: Add EE clock controller driver

[Jian Hu]

Add a Clock driver for the Everyting-Else part of
the Amlogic Meson-G12A SoC.

Jian Hu (3):
  dt-bindings: clk: g12a: New binding for Meson-G12A SoC
  dt-bindings: clk: meson-g12a: Add G12A EE Clock Bindings
  clk: meson-g12a: Add EE Clock controller driver

 .../bindings/clock/amlogic,gxbb-clkc.txt           |   1 +
 drivers/clk/meson/Kconfig                          |  10 +
 drivers/clk/meson/Makefile                         |   1 +
 drivers/clk/meson/g12a.c                           | 992 +++++++++++++++++++++
 drivers/clk/meson/g12a.h                           | 123 +++
 include/dt-bindings/clock/g12a-clkc.h              |  93 ++
 6 files changed, 1220 insertions(+)
 create mode 100644 drivers/clk/meson/g12a.c
 create mode 100644 drivers/clk/meson/g12a.h
 create mode 100644 include/dt-bindings/clock/g12a-clkc.h

-- 
1.9.1

[PATCH 1/3] dt-bindings: clk: g12a: New binding for Meson-G12A SoC

[Jian Hu]

Add new binding for Meson-G12A SoC Everything-Else part

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
index e950599..0833006 100644
--- a/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
+++ b/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
@@ -9,6 +9,7 @@ Required Properties:
 		"amlogic,gxbb-clkc" for GXBB SoC,
 		"amlogic,gxl-clkc" for GXL and GXM SoC,
 		"amlogic,axg-clkc" for AXG SoC.
+		"amlogic,g12a-clkc" for G12A SoC.
 
 - #clock-cells: should be 1.
 
-- 
1.9.1

Re: [PATCH 1/3] dt-bindings: clk: g12a: New binding for Meson-G12A SoC

[Martin Blumenstingl]

adding Rob Herring so it doesn't get lost on the devicetree mailing list

On Mon, Jul 9, 2018 at 1:13 PM Jian Hu <jian.hu@amlogic.com> wrote:
>
> Add new binding for Meson-G12A SoC Everything-Else part
>
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> ---
>  Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
> index e950599..0833006 100644
> --- a/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
> +++ b/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
> @@ -9,6 +9,7 @@ Required Properties:
>                 "amlogic,gxbb-clkc" for GXBB SoC,
>                 "amlogic,gxl-clkc" for GXL and GXM SoC,
>                 "amlogic,axg-clkc" for AXG SoC.
> +               "amlogic,g12a-clkc" for G12A SoC.
should this be "amlogic,meson-g12a-clkc" instead?
that would make it consistent with virtually all other bindings which
use the schema "amlogic,meson<chip>-<ip-block>" (where chip is 8, 8b,
8m2, -gxbb, -gxl, ...)

see also: $ grep -R "amlogic," Documentation/devicetree/bindings/

>
>  - #clock-cells: should be 1.
>
> --
> 1.9.1
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/3] dt-bindings: clk: g12a: New binding for Meson-G12A SoC

[Jian Hu]

On 2018/7/10 5:57, Martin Blumenstingl wrote:
> adding Rob Herring so it doesn't get lost on the devicetree mailing list
> 
> On Mon, Jul 9, 2018 at 1:13 PM Jian Hu <jian.hu@amlogic.com> wrote:
>>
>> Add new binding for Meson-G12A SoC Everything-Else part
>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>>   Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
>> index e950599..0833006 100644
>> --- a/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
>> +++ b/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
>> @@ -9,6 +9,7 @@ Required Properties:
>>                  "amlogic,gxbb-clkc" for GXBB SoC,
>>                  "amlogic,gxl-clkc" for GXL and GXM SoC,
>>                  "amlogic,axg-clkc" for AXG SoC.
>> +               "amlogic,g12a-clkc" for G12A SoC.
> should this be "amlogic,meson-g12a-clkc" instead?
> that would make it consistent with virtually all other bindings which
> use the schema "amlogic,meson<chip>-<ip-block>" (where chip is 8, 8b,
> 8m2, -gxbb, -gxl, ...)
> 
> see also: $ grep -R "amlogic," Documentation/devicetree/bindings/
> 
Which type should we choose,have a discussion?
.compatible = "amlogic,g12a-clkc or
.compatible = "amlogic,meson-g12a-clkc
>>
>>   - #clock-cells: should be 1.
>>
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> 
> .
> 

Re: [PATCH 1/3] dt-bindings: clk: g12a: New binding for Meson-G12A SoC

[Jerome Brunet]

On Mon, 2018-07-09 at 19:12 +0800, Jian Hu wrote:
> Add new binding for Meson-G12A SoC Everything-Else part

nitpick: I would prefer if the words 'clock' and 'controller' was somewhere in
the description.

Maybe "add new clock controller compatible for the EE domain of the g12a SoC" ?

> 
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> ---
>  Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
> index e950599..0833006 100644
> --- a/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
> +++ b/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
> @@ -9,6 +9,7 @@ Required Properties:
>  		"amlogic,gxbb-clkc" for GXBB SoC,
>  		"amlogic,gxl-clkc" for GXL and GXM SoC,
>  		"amlogic,axg-clkc" for AXG SoC.
> +		"amlogic,g12a-clkc" for G12A SoC.
>  
>  - #clock-cells: should be 1.
>  

Re: [PATCH 1/3] dt-bindings: clk: g12a: New binding for Meson-G12A SoC

[Jian Hu]

On 2018/7/10 17:29, Jerome Brunet wrote:
> On Mon, 2018-07-09 at 19:12 +0800, Jian Hu wrote:
>> Add new binding for Meson-G12A SoC Everything-Else part
> 
> nitpick: I would prefer if the words 'clock' and 'controller' was somewhere in
> the description.
> 
> Maybe "add new clock controller compatible for the EE domain of the g12a SoC" ?

Patch 1 and 2 have squashed, the commit message focuse which one？
> 
>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>>   Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
>> index e950599..0833006 100644
>> --- a/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
>> +++ b/Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt
>> @@ -9,6 +9,7 @@ Required Properties:
>>   		"amlogic,gxbb-clkc" for GXBB SoC,
>>   		"amlogic,gxl-clkc" for GXL and GXM SoC,
>>   		"amlogic,axg-clkc" for AXG SoC.
>> +		"amlogic,g12a-clkc" for G12A SoC.
>>   
>>   - #clock-cells: should be 1.
>>   
> 
> .
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] dt-bindings: clk: g12a: New binding for Meson-G12A SoC

[Martin Blumenstingl]

On Wed, Jul 11, 2018 at 3:06 PM Jian Hu <jian.hu@amlogic.com> wrote:
>
>
>
> On 2018/7/10 17:29, Jerome Brunet wrote:
> > On Mon, 2018-07-09 at 19:12 +0800, Jian Hu wrote:
> >> Add new binding for Meson-G12A SoC Everything-Else part
> >
> > nitpick: I would prefer if the words 'clock' and 'controller' was somewhere in
> > the description.
> >
> > Maybe "add new clock controller compatible for the EE domain of the g12a SoC" ?
>
> Patch 1 and 2 have squashed, the commit message focuse which one？
I would use the information from both commit messages (for example you
could use the what Jerome already suggested and extend it with "and
add a header file which contains preprocessor macros to identify the
clocks provided by this clock controller")


Regards
Martin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] dt-bindings: clk: meson-g12a: Add G12A EE Clock Bindings

[Jian Hu]

Add dt-bindings headers for the Meson-G12A's Everything-Else
part clock controller.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 include/dt-bindings/clock/g12a-clkc.h | 93 +++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 include/dt-bindings/clock/g12a-clkc.h

diff --git a/include/dt-bindings/clock/g12a-clkc.h b/include/dt-bindings/clock/g12a-clkc.h
new file mode 100644
index 0000000..1473225
--- /dev/null
+++ b/include/dt-bindings/clock/g12a-clkc.h
@@ -0,0 +1,93 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Meson-G12A clock tree IDs
+ *
+ * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
+ */
+
+#ifndef __G12A_CLKC_H
+#define __G12A_CLKC_H
+
+#define CLKID_SYS_PLL				0
+#define CLKID_FIXED_PLL				1
+#define CLKID_FCLK_DIV2				2
+#define CLKID_FCLK_DIV3				3
+#define CLKID_FCLK_DIV4				4
+#define CLKID_FCLK_DIV5				5
+#define CLKID_FCLK_DIV7				6
+#define CLKID_GP0_PLL				7
+#define CLKID_CLK81					10
+#define CLKID_MPLL0					11
+#define CLKID_MPLL1					12
+#define CLKID_MPLL2					13
+#define CLKID_MPLL3					14
+#define CLKID_DDR					15
+#define CLKID_DOS					16
+#define CLKID_AUDIO_LOCKER			17
+#define CLKID_MIPI_DSI_HOST			18
+#define CLKID_ETH_PHY				19
+#define CLKID_ISA					20
+#define CLKID_PL301					21
+#define CLKID_PERIPHS				22
+#define CLKID_SPICC0				23
+#define CLKID_I2C					24
+#define CLKID_SANA					25
+#define CLKID_SD					26
+#define CLKID_RNG0					27
+#define CLKID_UART0					28
+#define CLKID_SPICC1				29
+#define CLKID_HIU_IFACE				30
+#define CLKID_MIPI_DSI_PHY			31
+#define CLKID_ASSIST_MISC			32
+#define CLKID_SD_EMMC_A				33
+#define CLKID_SD_EMMC_B				34
+#define CLKID_SD_EMMC_C				35
+#define CLKID_AUDIO_CODEC			36
+#define CLKID_AUDIO					37
+#define CLKID_ETH					38
+#define CLKID_DEMUX					39
+#define CLKID_AUDIO_IFIFO			40
+#define CLKID_ADC					41
+#define CLKID_UART1					42
+#define CLKID_G2D					43
+#define CLKID_RESET					44
+#define CLKID_PCIE_COMB				45
+#define CLKID_PARSER				46
+#define CLKID_USB					47
+#define CLKID_PCIE_PHY				48
+#define CLKID_AHB_ARB0				49
+#define CLKID_AHB_DATA_BUS			50
+#define CLKID_AHB_CTRL_BUS			51
+#define CLKID_HTX_HDCP22			52
+#define CLKID_HTX_PCLK				53
+#define CLKID_BT656					54
+#define CLKID_USB1_DDR_BRIDGE		55
+#define CLKID_MMC_PCLK				56
+#define CLKID_UART2					57
+#define CLKID_VPU_INTR				58
+#define CLKID_GIC					59
+#define CLKID_SD_EMMC_B_CLK0		60
+#define CLKID_SD_EMMC_C_CLK0		61
+#define CLKID_HIFI_PLL				71
+
+#define CLKID_VCLK2_VENCI0			77
+#define CLKID_VCLK2_VENCI1			78
+#define CLKID_VCLK2_VENCP0			79
+#define CLKID_VCLK2_VENCP1			80
+#define CLKID_VCLK2_VENCT0			81
+#define CLKID_VCLK2_VENCT1			82
+#define CLKID_VCLK2_OTHER			83
+#define CLKID_VCLK2_ENCI			84
+#define CLKID_VCLK2_ENCP			85
+#define CLKID_DAC_CLK				86
+#define CLKID_AOCLK					87
+#define CLKID_IEC958				88
+#define CLKID_ENC480P				89
+#define CLKID_RNG1					90
+#define CLKID_VCLK2_ENCT			91
+#define CLKID_VCLK2_ENCL			92
+#define CLKID_VCLK2_VENCLMMC		93
+#define CLKID_VCLK2_VENCL			94
+#define CLKID_VCLK2_OTHER1			95
+
+#endif /* __G12A_CLKC_H */
-- 
1.9.1

Re: [PATCH 2/3] dt-bindings: clk: meson-g12a: Add G12A EE Clock Bindings

[Martin Blumenstingl]

On Mon, Jul 9, 2018 at 1:13 PM Jian Hu <jian.hu@amlogic.com> wrote:
>
> Add dt-bindings headers for the Meson-G12A's Everything-Else
> part clock controller.
I wonder if this should be folded into patch #1 along with an update
to Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt so
it's clear which header has to be used for G12A

>
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> ---
>  include/dt-bindings/clock/g12a-clkc.h | 93 +++++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 include/dt-bindings/clock/g12a-clkc.h
>
> diff --git a/include/dt-bindings/clock/g12a-clkc.h b/include/dt-bindings/clock/g12a-clkc.h
> new file mode 100644
> index 0000000..1473225
> --- /dev/null
> +++ b/include/dt-bindings/clock/g12a-clkc.h
> @@ -0,0 +1,93 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Meson-G12A clock tree IDs
> + *
> + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
> + */
> +
> +#ifndef __G12A_CLKC_H
> +#define __G12A_CLKC_H
> +
> +#define CLKID_SYS_PLL                          0
> +#define CLKID_FIXED_PLL                                1
> +#define CLKID_FCLK_DIV2                                2
> +#define CLKID_FCLK_DIV3                                3
> +#define CLKID_FCLK_DIV4                                4
> +#define CLKID_FCLK_DIV5                                5
> +#define CLKID_FCLK_DIV7                                6
> +#define CLKID_GP0_PLL                          7
> +#define CLKID_CLK81                                    10
> +#define CLKID_MPLL0                                    11
> +#define CLKID_MPLL1                                    12
> +#define CLKID_MPLL2                                    13
> +#define CLKID_MPLL3                                    14
> +#define CLKID_DDR                                      15
> +#define CLKID_DOS                                      16
> +#define CLKID_AUDIO_LOCKER                     17
> +#define CLKID_MIPI_DSI_HOST                    18
> +#define CLKID_ETH_PHY                          19
> +#define CLKID_ISA                                      20
> +#define CLKID_PL301                                    21
> +#define CLKID_PERIPHS                          22
> +#define CLKID_SPICC0                           23
> +#define CLKID_I2C                                      24
> +#define CLKID_SANA                                     25
> +#define CLKID_SD                                       26
> +#define CLKID_RNG0                                     27
> +#define CLKID_UART0                                    28
> +#define CLKID_SPICC1                           29
> +#define CLKID_HIU_IFACE                                30
> +#define CLKID_MIPI_DSI_PHY                     31
> +#define CLKID_ASSIST_MISC                      32
> +#define CLKID_SD_EMMC_A                                33
> +#define CLKID_SD_EMMC_B                                34
> +#define CLKID_SD_EMMC_C                                35
> +#define CLKID_AUDIO_CODEC                      36
> +#define CLKID_AUDIO                                    37
> +#define CLKID_ETH                                      38
> +#define CLKID_DEMUX                                    39
> +#define CLKID_AUDIO_IFIFO                      40
> +#define CLKID_ADC                                      41
> +#define CLKID_UART1                                    42
> +#define CLKID_G2D                                      43
> +#define CLKID_RESET                                    44
> +#define CLKID_PCIE_COMB                                45
> +#define CLKID_PARSER                           46
> +#define CLKID_USB                                      47
> +#define CLKID_PCIE_PHY                         48
> +#define CLKID_AHB_ARB0                         49
> +#define CLKID_AHB_DATA_BUS                     50
> +#define CLKID_AHB_CTRL_BUS                     51
> +#define CLKID_HTX_HDCP22                       52
> +#define CLKID_HTX_PCLK                         53
> +#define CLKID_BT656                                    54
> +#define CLKID_USB1_DDR_BRIDGE          55
> +#define CLKID_MMC_PCLK                         56
> +#define CLKID_UART2                                    57
> +#define CLKID_VPU_INTR                         58
> +#define CLKID_GIC                                      59
> +#define CLKID_SD_EMMC_B_CLK0           60
> +#define CLKID_SD_EMMC_C_CLK0           61
> +#define CLKID_HIFI_PLL                         71
> +
is this empty line here on purpose? a comment would be great if
there's a reason behind it (there's already a gap in the numbering
between CLKID_GP0_PLL and CLKID_CLK81, but there's no empty line there
- either way is fine, please just keep it consistent)


Regards
Martin

Re: [PATCH 2/3] dt-bindings: clk: meson-g12a: Add G12A EE Clock Bindings

[Jerome Brunet]

On Tue, 2018-07-10 at 00:13 +0200, Martin Blumenstingl wrote:
> On Mon, Jul 9, 2018 at 1:13 PM Jian Hu <jian.hu@amlogic.com> wrote:
> > 
> > Add dt-bindings headers for the Meson-G12A's Everything-Else
> > part clock controller.
> 
> I wonder if this should be folded into patch #1 along with an update
> to Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt so
> it's clear which header has to be used for G12A
> 

Yes, please squash patch 1 and 2.

> > 
> > Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> > ---
> >  include/dt-bindings/clock/g12a-clkc.h | 93 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 93 insertions(+)
> >  create mode 100644 include/dt-bindings/clock/g12a-clkc.h
> > 
> > diff --git a/include/dt-bindings/clock/g12a-clkc.h b/include/dt-bindings/clock/g12a-clkc.h
> > new file mode 100644
> > index 0000000..1473225
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/g12a-clkc.h
> > @@ -0,0 +1,93 @@
> > +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> > +/*
> > + * Meson-G12A clock tree IDs
> > + *
> > + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
> > + */
> > +
> > +#ifndef __G12A_CLKC_H
> > +#define __G12A_CLKC_H
> > +
> > +#define CLKID_SYS_PLL                          0
> > +#define CLKID_FIXED_PLL                                1
> > +#define CLKID_FCLK_DIV2                                2
> > +#define CLKID_FCLK_DIV3                                3
> > +#define CLKID_FCLK_DIV4                                4
> > +#define CLKID_FCLK_DIV5                                5
> > +#define CLKID_FCLK_DIV7                                6
> > +#define CLKID_GP0_PLL                          7

Please fix the alignement.

> > +#define CLKID_CLK81                                    10
> > +#define CLKID_MPLL0                                    11
> > +#define CLKID_MPLL1                                    12
> > +#define CLKID_MPLL2                                    13
> > +#define CLKID_MPLL3                                    14
> > +#define CLKID_DDR                                      15
> > +#define CLKID_DOS                                      16
> > +#define CLKID_AUDIO_LOCKER                     17
> > +#define CLKID_MIPI_DSI_HOST                    18
> > +#define CLKID_ETH_PHY                          19
> > +#define CLKID_ISA                                      20
> > +#define CLKID_PL301                                    21
> > +#define CLKID_PERIPHS                          22
> > +#define CLKID_SPICC0                           23
> > +#define CLKID_I2C                                      24
> > +#define CLKID_SANA                                     25
> > +#define CLKID_SD                                       26
> > +#define CLKID_RNG0                                     27
> > +#define CLKID_UART0                                    28
> > +#define CLKID_SPICC1                           29
> > +#define CLKID_HIU_IFACE                                30
> > +#define CLKID_MIPI_DSI_PHY                     31
> > +#define CLKID_ASSIST_MISC                      32
> > +#define CLKID_SD_EMMC_A                                33
> > +#define CLKID_SD_EMMC_B                                34
> > +#define CLKID_SD_EMMC_C                                35
> > +#define CLKID_AUDIO_CODEC                      36
> > +#define CLKID_AUDIO                                    37
> > +#define CLKID_ETH                                      38
> > +#define CLKID_DEMUX                                    39
> > +#define CLKID_AUDIO_IFIFO                      40
> > +#define CLKID_ADC                                      41
> > +#define CLKID_UART1                                    42
> > +#define CLKID_G2D                                      43
> > +#define CLKID_RESET                                    44
> > +#define CLKID_PCIE_COMB                                45
> > +#define CLKID_PARSER                           46
> > +#define CLKID_USB                                      47
> > +#define CLKID_PCIE_PHY                         48
> > +#define CLKID_AHB_ARB0                         49
> > +#define CLKID_AHB_DATA_BUS                     50
> > +#define CLKID_AHB_CTRL_BUS                     51
> > +#define CLKID_HTX_HDCP22                       52
> > +#define CLKID_HTX_PCLK                         53
> > +#define CLKID_BT656                                    54
> > +#define CLKID_USB1_DDR_BRIDGE          55
> > +#define CLKID_MMC_PCLK                         56
> > +#define CLKID_UART2                                    57
> > +#define CLKID_VPU_INTR                         58
> > +#define CLKID_GIC                                      59
> > +#define CLKID_SD_EMMC_B_CLK0           60
> > +#define CLKID_SD_EMMC_C_CLK0           61
> > +#define CLKID_HIFI_PLL                         71
> > +
> 
> is this empty line here on purpose? a comment would be great if
> there's a reason behind it (there's already a gap in the numbering
> between CLKID_GP0_PLL and CLKID_CLK81, but there's no empty line there
> - either way is fine, please just keep it consistent)

Please drop the empty line.
Otherwise, looks good.

> 
> 
> Regards
> Martin

Re: [PATCH 2/3] dt-bindings: clk: meson-g12a: Add G12A EE Clock Bindings

[Jian Hu]

On 2018/7/10 17:26, Jerome Brunet wrote:
> On Tue, 2018-07-10 at 00:13 +0200, Martin Blumenstingl wrote:
>> On Mon, Jul 9, 2018 at 1:13 PM Jian Hu <jian.hu@amlogic.com> wrote:
>>>
>>> Add dt-bindings headers for the Meson-G12A's Everything-Else
>>> part clock controller.
>>
>> I wonder if this should be folded into patch #1 along with an update
>> to Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt so
>> it's clear which header has to be used for G12A
>>
> 
> Yes, please squash patch 1 and 2.
I have squashed patch 1 and 2.
> 
>>>
>>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>>> ---
>>>   include/dt-bindings/clock/g12a-clkc.h | 93 +++++++++++++++++++++++++++++++++++
>>>   1 file changed, 93 insertions(+)
>>>   create mode 100644 include/dt-bindings/clock/g12a-clkc.h
>>>
>>> diff --git a/include/dt-bindings/clock/g12a-clkc.h b/include/dt-bindings/clock/g12a-clkc.h
>>> new file mode 100644
>>> index 0000000..1473225
>>> --- /dev/null
>>> +++ b/include/dt-bindings/clock/g12a-clkc.h
>>> @@ -0,0 +1,93 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>>> +/*
>>> + * Meson-G12A clock tree IDs
>>> + *
>>> + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
>>> + */
>>> +
>>> +#ifndef __G12A_CLKC_H
>>> +#define __G12A_CLKC_H
>>> +
>>> +#define CLKID_SYS_PLL                          0
>>> +#define CLKID_FIXED_PLL                                1
>>> +#define CLKID_FCLK_DIV2                                2
>>> +#define CLKID_FCLK_DIV3                                3
>>> +#define CLKID_FCLK_DIV4                                4
>>> +#define CLKID_FCLK_DIV5                                5
>>> +#define CLKID_FCLK_DIV7                                6
>>> +#define CLKID_GP0_PLL                          7
> 
> Please fix the alignement.
> 
>>> +#define CLKID_CLK81                                    10
>>> +#define CLKID_MPLL0                                    11
>>> +#define CLKID_MPLL1                                    12
>>> +#define CLKID_MPLL2                                    13
>>> +#define CLKID_MPLL3                                    14
>>> +#define CLKID_DDR                                      15
>>> +#define CLKID_DOS                                      16
>>> +#define CLKID_AUDIO_LOCKER                     17
>>> +#define CLKID_MIPI_DSI_HOST                    18
>>> +#define CLKID_ETH_PHY                          19
>>> +#define CLKID_ISA                                      20
>>> +#define CLKID_PL301                                    21
>>> +#define CLKID_PERIPHS                          22
>>> +#define CLKID_SPICC0                           23
>>> +#define CLKID_I2C                                      24
>>> +#define CLKID_SANA                                     25
>>> +#define CLKID_SD                                       26
>>> +#define CLKID_RNG0                                     27
>>> +#define CLKID_UART0                                    28
>>> +#define CLKID_SPICC1                           29
>>> +#define CLKID_HIU_IFACE                                30
>>> +#define CLKID_MIPI_DSI_PHY                     31
>>> +#define CLKID_ASSIST_MISC                      32
>>> +#define CLKID_SD_EMMC_A                                33
>>> +#define CLKID_SD_EMMC_B                                34
>>> +#define CLKID_SD_EMMC_C                                35
>>> +#define CLKID_AUDIO_CODEC                      36
>>> +#define CLKID_AUDIO                                    37
>>> +#define CLKID_ETH                                      38
>>> +#define CLKID_DEMUX                                    39
>>> +#define CLKID_AUDIO_IFIFO                      40
>>> +#define CLKID_ADC                                      41
>>> +#define CLKID_UART1                                    42
>>> +#define CLKID_G2D                                      43
>>> +#define CLKID_RESET                                    44
>>> +#define CLKID_PCIE_COMB                                45
>>> +#define CLKID_PARSER                           46
>>> +#define CLKID_USB                                      47
>>> +#define CLKID_PCIE_PHY                         48
>>> +#define CLKID_AHB_ARB0                         49
>>> +#define CLKID_AHB_DATA_BUS                     50
>>> +#define CLKID_AHB_CTRL_BUS                     51
>>> +#define CLKID_HTX_HDCP22                       52
>>> +#define CLKID_HTX_PCLK                         53
>>> +#define CLKID_BT656                                    54
>>> +#define CLKID_USB1_DDR_BRIDGE          55
>>> +#define CLKID_MMC_PCLK                         56
>>> +#define CLKID_UART2                                    57
>>> +#define CLKID_VPU_INTR                         58
>>> +#define CLKID_GIC                                      59
>>> +#define CLKID_SD_EMMC_B_CLK0           60
>>> +#define CLKID_SD_EMMC_C_CLK0           61
>>> +#define CLKID_HIFI_PLL                         71
>>> +
>>
>> is this empty line here on purpose? a comment would be great if
>> there's a reason behind it (there's already a gap in the numbering
>> between CLKID_GP0_PLL and CLKID_CLK81, but there's no empty line there
>> - either way is fine, please just keep it consistent)
> 
> Please drop the empty line.
> Otherwise, looks good.
> I have dropped it ,and I will push another email later.Thanks.
>>
>>
>> Regards
>> Martin
> 
> .
> 

Re: [PATCH 2/3] dt-bindings: clk: meson-g12a: Add G12A EE Clock Bindings

[Jian Hu]

On 2018/7/10 6:13, Martin Blumenstingl wrote:
> On Mon, Jul 9, 2018 at 1:13 PM Jian Hu <jian.hu@amlogic.com> wrote:
>>
>> Add dt-bindings headers for the Meson-G12A's Everything-Else
>> part clock controller.
> I wonder if this should be folded into patch #1 along with an update
> to Documentation/devicetree/bindings/clock/amlogic,gxbb-clkc.txt so
> it's clear which header has to be used for G12A
> 
As your suggestions, I will squash patch 1 and 2.

>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>>   include/dt-bindings/clock/g12a-clkc.h | 93 +++++++++++++++++++++++++++++++++++
>>   1 file changed, 93 insertions(+)
>>   create mode 100644 include/dt-bindings/clock/g12a-clkc.h
>>
>> diff --git a/include/dt-bindings/clock/g12a-clkc.h b/include/dt-bindings/clock/g12a-clkc.h
>> new file mode 100644
>> index 0000000..1473225
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/g12a-clkc.h
>> @@ -0,0 +1,93 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Meson-G12A clock tree IDs
>> + *
>> + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
>> + */
>> +
>> +#ifndef __G12A_CLKC_H
>> +#define __G12A_CLKC_H
>> +
>> +#define CLKID_SYS_PLL                          0
>> +#define CLKID_FIXED_PLL                                1
>> +#define CLKID_FCLK_DIV2                                2
>> +#define CLKID_FCLK_DIV3                                3
>> +#define CLKID_FCLK_DIV4                                4
>> +#define CLKID_FCLK_DIV5                                5
>> +#define CLKID_FCLK_DIV7                                6
>> +#define CLKID_GP0_PLL                          7
>> +#define CLKID_CLK81                                    10
>> +#define CLKID_MPLL0                                    11
>> +#define CLKID_MPLL1                                    12
>> +#define CLKID_MPLL2                                    13
>> +#define CLKID_MPLL3                                    14
>> +#define CLKID_DDR                                      15
>> +#define CLKID_DOS                                      16
>> +#define CLKID_AUDIO_LOCKER                     17
>> +#define CLKID_MIPI_DSI_HOST                    18
>> +#define CLKID_ETH_PHY                          19
>> +#define CLKID_ISA                                      20
>> +#define CLKID_PL301                                    21
>> +#define CLKID_PERIPHS                          22
>> +#define CLKID_SPICC0                           23
>> +#define CLKID_I2C                                      24
>> +#define CLKID_SANA                                     25
>> +#define CLKID_SD                                       26
>> +#define CLKID_RNG0                                     27
>> +#define CLKID_UART0                                    28
>> +#define CLKID_SPICC1                           29
>> +#define CLKID_HIU_IFACE                                30
>> +#define CLKID_MIPI_DSI_PHY                     31
>> +#define CLKID_ASSIST_MISC                      32
>> +#define CLKID_SD_EMMC_A                                33
>> +#define CLKID_SD_EMMC_B                                34
>> +#define CLKID_SD_EMMC_C                                35
>> +#define CLKID_AUDIO_CODEC                      36
>> +#define CLKID_AUDIO                                    37
>> +#define CLKID_ETH                                      38
>> +#define CLKID_DEMUX                                    39
>> +#define CLKID_AUDIO_IFIFO                      40
>> +#define CLKID_ADC                                      41
>> +#define CLKID_UART1                                    42
>> +#define CLKID_G2D                                      43
>> +#define CLKID_RESET                                    44
>> +#define CLKID_PCIE_COMB                                45
>> +#define CLKID_PARSER                           46
>> +#define CLKID_USB                                      47
>> +#define CLKID_PCIE_PHY                         48
>> +#define CLKID_AHB_ARB0                         49
>> +#define CLKID_AHB_DATA_BUS                     50
>> +#define CLKID_AHB_CTRL_BUS                     51
>> +#define CLKID_HTX_HDCP22                       52
>> +#define CLKID_HTX_PCLK                         53
>> +#define CLKID_BT656                                    54
>> +#define CLKID_USB1_DDR_BRIDGE          55
>> +#define CLKID_MMC_PCLK                         56
>> +#define CLKID_UART2                                    57
>> +#define CLKID_VPU_INTR                         58
>> +#define CLKID_GIC                                      59
>> +#define CLKID_SD_EMMC_B_CLK0           60
>> +#define CLKID_SD_EMMC_C_CLK0           61
>> +#define CLKID_HIFI_PLL                         71
>> +
> is this empty line here on purpose? a comment would be great if
> there's a reason behind it (there's already a gap in the numbering
> between CLKID_GP0_PLL and CLKID_CLK81, but there's no empty line there
> - either way is fine, please just keep it consistent)
> 
I will drop the empty line, The gaps have define in 
drivers/clk/meson/g12a.h file.
> 
> Regards
> Martin
> 
> .
>