diff mbox series

[v5,2/3] clk: meson: add DT documentation for emmc clock controller

Message ID 1539839245-13793-3-git-send-email-jianxin.pan@amlogic.com (mailing list archive)
State Changes Requested, archived
Headers show
Series clk: meson: add a sub EMMC clock controller support | expand

Commit Message

Jianxin Pan Oct. 18, 2018, 5:07 a.m. UTC 
From: Yixun Lan <yixun.lan@amlogic.com>

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 <robh@kernel.org>
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
---
 .../devicetree/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

Comments

Stephen Boyd Oct. 18, 2018, 5:08 p.m. UTC | #1 
Quoting Jianxin Pan (2018-10-17 22:07:24)
> 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 0000000..9e6d343
> --- /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 :

The example only has one node. Can you add two nodes?

> +- 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>;
> +};
Jianxin Pan Oct. 19, 2018, 3:50 p.m. UTC | #2 
On 2018/10/19 1:08, Stephen Boyd wrote:
> Quoting Jianxin Pan (2018-10-17 22:07:24)
>> 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 0000000..9e6d343
>> --- /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 :
> 
> The example only has one node. Can you add two nodes?
OK. This clock is used by nand and emmc. I will add a new example for emmc too.
Thank you for your review.
> 
>> +- 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>;
>> +};
> 
> .
>
Stephen Boyd Oct. 19, 2018, 6:04 p.m. UTC | #3 
Quoting Jianxin Pan (2018-10-19 08:50:08)
> On 2018/10/19 1:08, Stephen Boyd wrote:
> > Quoting Jianxin Pan (2018-10-17 22:07:24)
> >> 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 0000000..9e6d343
> >> --- /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 :
> > 
> > The example only has one node. Can you add two nodes?
> OK. This clock is used by nand and emmc. I will add a new example for emmc too.
> Thank you for your review.

Maybe I misunderstand. I thought the clk controller was two nodes, but
it isn't? This wording is trying to explain what a consumer should look
like?
Jianxin Pan Oct. 22, 2018, 6:05 a.m. UTC | #4 
Hi Stephen,

Thanks for the fully review, we really appreciate your time.

Please see my comments below. 

On 2018/10/20 2:04, Stephen Boyd wrote:
> Quoting Jianxin Pan (2018-10-19 08:50:08)
>> On 2018/10/19 1:08, Stephen Boyd wrote:
>>> Quoting Jianxin Pan (2018-10-17 22:07:24)
>>>> 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 0000000..9e6d343
>>>> --- /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 :
>>>
>>> The example only has one node. Can you add two nodes?
>> OK. This clock is used by nand and emmc. I will add a new example for emmc too.
>> Thank you for your review.
> 
> Maybe I misunderstand. I thought the clk controller was two nodes, but
> it isn't? This wording is trying to explain what a consumer should look
> like?
> 
Yes.There is another clk controller.  I will add it in the next version.
sd_emmc_b_clkc: clock-controller@5000 {                  
         compatible = "amlogic,axg-mmc-clkc", "syscon";   
         reg = <0x0 0x5000 0x0 0x4>;                      
         #clock-cells = <1>;                              
                                                          
         clock-names = "clkin0", "clkin1";                
         clocks = <&clkc CLKID_SD_EMMC_B_CLK0>,           
                 <&clkc CLKID_FCLK_DIV2>;                 
};
sd_emmc_c_clkc is for nadn and mmc portC.
sd_emmc_b_clkc is for mmc portB.
> .
>
Jerome Brunet Oct. 24, 2018, 8:58 a.m. UTC | #5 
On Thu, 2018-10-18 at 13:07 +0800, Jianxin Pan wrote:
> From: Yixun Lan <yixun.lan@amlogic.com>
> 
> 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 <robh@kernel.org>
> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
> ---
>  .../devicetree/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 0000000..9e6d343
> --- /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.

I get why Stephen is confused by your description, I am too. The example
contradict the documentation.

The  documentation above says that the parent node should be a syscon with the
mmc register space.

But your example shows this in the node itself.

> +
> +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 0000000..162b949
> --- /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 <yixun.lan@amlogic.com>
> + */
> +
> +#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
Yixun Lan Oct. 25, 2018, 7:29 a.m. UTC | #6 
Hi Jerome, Jianxin:

see my comments

On 10:58 Wed 24 Oct     , Jerome Brunet wrote:
> On Thu, 2018-10-18 at 13:07 +0800, Jianxin Pan wrote:
> > From: Yixun Lan <yixun.lan@amlogic.com>
> > 
> > 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 <robh@kernel.org>
> > Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
> > Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
> > ---
> >  .../devicetree/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 0000000..9e6d343
> > --- /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.
> 
> I get why Stephen is confused by your description, I am too. The example
> contradict the documentation.
> 
> The  documentation above says that the parent node should be a syscon with the
> mmc register space.
> 
> But your example shows this in the node itself.
> 

yes, I think the documentation need to be fixed

for the final solution, we decide to make 'mmc-clkc' an independent node
instead of being a sub-node of 'mmc', so both of them may exist in parallel..

the DT part may like this:

			sd_emmc_c_clkc: clock-controller@7000 {
				compatible = "amlogic,axg-mmc-clkc", "syscon";
				reg = <0x0 0x7000 0x0 0x4>;
				...
			};

			sd_emmc_c: mmc@7000 {
				compatible = "amlogic,axg-mmc";
				reg = <0x0 0x7000 0x0 0x800>;
				...
			};


> > +
> > +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 0000000..162b949
> > --- /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 <yixun.lan@amlogic.com>
> > + */
> > +
> > +#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
> 
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
Jianxin Pan Oct. 25, 2018, 11:50 a.m. UTC | #7 
On 2018/10/25 15:29, Yixun Lan wrote:
> Hi Jerome, Jianxin:
> 
> see my comments
> 
> On 10:58 Wed 24 Oct     , Jerome Brunet wrote:
>> On Thu, 2018-10-18 at 13:07 +0800, Jianxin Pan wrote:
>>> From: Yixun Lan <yixun.lan@amlogic.com>
>>>
>>> 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 <robh@kernel.org>
>>> Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>
>>> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
>>> ---
>>>  .../devicetree/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 0000000..9e6d343
>>> --- /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.
>>
>> I get why Stephen is confused by your description, I am too. The example
>> contradict the documentation.
>>
>> The  documentation above says that the parent node should be a syscon with the
>> mmc register space.
>>
>> But your example shows this in the node itself.
>>
> 
> yes, I think the documentation need to be fixed
ok, Thankyou. I will fix it in the next version.
> 
> for the final solution, we decide to make 'mmc-clkc' an independent node
> instead of being a sub-node of 'mmc', so both of them may exist in parallel..
> 
> the DT part may like this:
> 
> 			sd_emmc_c_clkc: clock-controller@7000 {
> 				compatible = "amlogic,axg-mmc-clkc", "syscon";
> 				reg = <0x0 0x7000 0x0 0x4>;
> 				...
> 			};
> 
> 			sd_emmc_c: mmc@7000 {
> 				compatible = "amlogic,axg-mmc";
> 				reg = <0x0 0x7000 0x0 0x800>;
> 				...
> 			};
> 
> 
>>> +
>>> +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 0000000..162b949
>>> --- /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 <yixun.lan@amlogic.com>
>>> + */
>>> +
>>> +#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
>>
>>
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>
Stephen Boyd Nov. 4, 2018, 3:04 a.m. UTC | #8 
Quoting Yixun Lan (2018-10-25 00:29:15)
> yes, I think the documentation need to be fixed
> 
> for the final solution, we decide to make 'mmc-clkc' an independent node
> instead of being a sub-node of 'mmc', so both of them may exist in parallel..
> 
> the DT part may like this:
> 
>                         sd_emmc_c_clkc: clock-controller@7000 {
>                                 compatible = "amlogic,axg-mmc-clkc", "syscon";
>                                 reg = <0x0 0x7000 0x0 0x4>;
>                                 ...
>                         };
> 
>                         sd_emmc_c: mmc@7000 {
>                                 compatible = "amlogic,axg-mmc";
>                                 reg = <0x0 0x7000 0x0 0x800>;
>                                 ...
>                         };

That's improper usage of DT. We don't want two devices at the same
register offset.
Jianxin Pan Nov. 4, 2018, 3:39 p.m. UTC | #9 
Hi Stephen,

Thank you for the review.

On 2018/11/4 11:04, Stephen Boyd wrote:
> Quoting Yixun Lan (2018-10-25 00:29:15)
>> yes, I think the documentation need to be fixed
>>
>> for the final solution, we decide to make 'mmc-clkc' an independent node
>> instead of being a sub-node of 'mmc', so both of them may exist in parallel..
>>
>> the DT part may like this:
>>
>>                         sd_emmc_c_clkc: clock-controller@7000 {
>>                                 compatible = "amlogic,axg-mmc-clkc", "syscon";
>>                                 reg = <0x0 0x7000 0x0 0x4>;
>>                                 ...
>>                         };
>>
>>                         sd_emmc_c: mmc@7000 {
>>                                 compatible = "amlogic,axg-mmc";
>>                                 reg = <0x0 0x7000 0x0 0x800>;
>>                                 ...
>>                         };
> 
> That's improper usage of DT. We don't want two devices at the same
> register offset.
sd_emmc_c_clkc is shared by nand and sd_emmc_c controller, and this clock is part of the MMC controller's register space.
 
The idea of adding the clock-controller@7000is introduced during the discussion in the NAND driver mainline effort:
https://lkml.kernel.org/r/20180628090034.0637a062@xps13
> 
> .
>
diff mbox series

Patch

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 0000000..9e6d343
--- /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 0000000..162b949
--- /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 <yixun.lan@amlogic.com>
+ */
+
+#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