diff mbox series

[v4,1/7] dt-bindings: clk: at91: add an I2S mux clock

Message ID 1527251668-31396-2-git-send-email-codrin.ciubotariu@microchip.com
State Changes Requested, archived
Headers show
Series ASoC: add driver for Atmel I2S controller | expand

Commit Message

Codrin Ciubotariu May 25, 2018, 12:34 p.m. UTC
The I2S mux clock can be used to select the I2S input clock. The
available parents are the peripheral and the generated clocks.

Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
---
 .../devicetree/bindings/clock/at91-clock.txt       | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Comments

Rob Herring May 31, 2018, 12:58 a.m. UTC | #1
On Fri, May 25, 2018 at 03:34:22PM +0300, Codrin Ciubotariu wrote:
> The I2S mux clock can be used to select the I2S input clock. The
> available parents are the peripheral and the generated clocks.
> 
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> ---
>  .../devicetree/bindings/clock/at91-clock.txt       | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt b/Documentation/devicetree/bindings/clock/at91-clock.txt
> index 51c259a..1c46b3c 100644
> --- a/Documentation/devicetree/bindings/clock/at91-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/at91-clock.txt
> @@ -90,6 +90,8 @@ Required properties:
>  	"atmel,sama5d2-clk-audio-pll-pmc"
>  		at91 audio pll output on AUDIOPLLCLK that feeds the PMC
>  		and can be used by peripheral clock or generic clock
> +	"atmel,sama5d2-clk-i2s-mux":
> +		at91 I2S clock source selection

Is this boolean or takes some values. If latter, what are valid values?

>  
>  Required properties for SCKC node:
>  - reg : defines the IO memory reserved for the SCKC.
> @@ -507,3 +509,35 @@ For example:
>  			atmel,clk-output-range = <0 83000000>;
>  		};
>  	};
> +
> +Required properties for I2S mux clocks:
> +- #size-cells : shall be 0 (reg is used to encode I2S bus id).
> +- #address-cells : shall be 1 (reg is used to encode I2S bus id).
> +- name: device tree node describing a specific mux clock.
> +	* #clock-cells : from common clock binding; shall be set to 0.
> +	* clocks : shall be the mux clock parent phandles; shall be 2 phandles:
> +	  peripheral and generated clock; the first phandle shall belong to the
> +	  peripheral clock and the second one shall belong to the generated
> +	  clock; "clock-indices" property can be user to specify
> +	  the correct order.
> +	* reg: I2S bus id of the corresponding mux clock.
> +	  e.g. reg = <0>; for i2s0, reg = <1>; for i2s1
> +
> +For example:
> +	i2s_clkmux {

What is this a child of?

> +		compatible = "atmel,sama5d2-clk-i2s-mux";
> +		#address-cells = <1>;
> +		#size-cells = <0>;

How do you address this block? My guess is you don't because it is just 
part of some other block and you are just creating this node to 
instantiate a driver. Just make the node for the actual h/w block a 
clock provider and define the clock ids (0 and 1).

> +
> +		i2s0muxck: i2s0_muxclk {
> +			clocks = <&i2s0_clk>, <&i2s0_gclk>;
> +			#clock-cells = <0>;
> +			reg = <0>;
> +		};
> +
> +		i2s1muxck: i2s1_muxclk {
> +			clocks = <&i2s1_clk>, <&i2s1_gclk>;
> +			#clock-cells = <0>;
> +			reg = <1>;
> +		};
> +	};
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Codrin Ciubotariu May 31, 2018, 10:25 a.m. UTC | #2
On 31.05.2018 03:58, Rob Herring wrote:
> On Fri, May 25, 2018 at 03:34:22PM +0300, Codrin Ciubotariu wrote:
>> The I2S mux clock can be used to select the I2S input clock. The
>> available parents are the peripheral and the generated clocks.
>>
>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
>> ---
>>   .../devicetree/bindings/clock/at91-clock.txt       | 34 ++++++++++++++++++++++
>>   1 file changed, 34 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt b/Documentation/devicetree/bindings/clock/at91-clock.txt
>> index 51c259a..1c46b3c 100644
>> --- a/Documentation/devicetree/bindings/clock/at91-clock.txt
>> +++ b/Documentation/devicetree/bindings/clock/at91-clock.txt
>> @@ -90,6 +90,8 @@ Required properties:
>>   	"atmel,sama5d2-clk-audio-pll-pmc"
>>   		at91 audio pll output on AUDIOPLLCLK that feeds the PMC
>>   		and can be used by peripheral clock or generic clock
>> +	"atmel,sama5d2-clk-i2s-mux":
>> +		at91 I2S clock source selection
> 
> Is this boolean or takes some values. If latter, what are valid values?

This is the compatible string of the clock driver.

> 
>>   
>>   Required properties for SCKC node:
>>   - reg : defines the IO memory reserved for the SCKC.
>> @@ -507,3 +509,35 @@ For example:
>>   			atmel,clk-output-range = <0 83000000>;
>>   		};
>>   	};
>> +
>> +Required properties for I2S mux clocks:
>> +- #size-cells : shall be 0 (reg is used to encode I2S bus id).
>> +- #address-cells : shall be 1 (reg is used to encode I2S bus id).
>> +- name: device tree node describing a specific mux clock.
>> +	* #clock-cells : from common clock binding; shall be set to 0.
>> +	* clocks : shall be the mux clock parent phandles; shall be 2 phandles:
>> +	  peripheral and generated clock; the first phandle shall belong to the
>> +	  peripheral clock and the second one shall belong to the generated
>> +	  clock; "clock-indices" property can be user to specify
>> +	  the correct order.
>> +	* reg: I2S bus id of the corresponding mux clock.
>> +	  e.g. reg = <0>; for i2s0, reg = <1>; for i2s1
>> +
>> +For example:
>> +	i2s_clkmux {
> 
> What is this a child of?

It is a child of PMC node, since both parent clocks are generated by PMC.

> 
>> +		compatible = "atmel,sama5d2-clk-i2s-mux";
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
> 
> How do you address this block? My guess is you don't because it is just
> part of some other block and you are just creating this node to
> instantiate a driver. Just make the node for the actual h/w block a
> clock provider and define the clock ids (0 and 1).

This block is not addressed, but its children are. The register we 
access in this driver is not part of other block. It's a SFR register, 
accessed through syscon and it has nothing to do with the I2S IP (see 
SAMA5D2 DS, page 1256, fig. 44-1: I2SC Block Diagram) that is the 
consumer of this clock. Adding a clock-id property in the I2S node would 
be just like v3 of this series, with the difference that we use clock-id 
instead of alias id to set the clock parent, which is not how you 
suggested back then.

Thank you for your review.

Best regards,
Codrin

[...]

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring May 31, 2018, 2:20 p.m. UTC | #3
On Thu, May 31, 2018 at 5:25 AM, Codrin Ciubotariu
<codrin.ciubotariu@microchip.com> wrote:
> On 31.05.2018 03:58, Rob Herring wrote:
>>
>> On Fri, May 25, 2018 at 03:34:22PM +0300, Codrin Ciubotariu wrote:
>>>
>>> The I2S mux clock can be used to select the I2S input clock. The
>>> available parents are the peripheral and the generated clocks.
>>>
>>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
>>> ---
>>>   .../devicetree/bindings/clock/at91-clock.txt       | 34
>>> ++++++++++++++++++++++
>>>   1 file changed, 34 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt
>>> b/Documentation/devicetree/bindings/clock/at91-clock.txt
>>> index 51c259a..1c46b3c 100644
>>> --- a/Documentation/devicetree/bindings/clock/at91-clock.txt
>>> +++ b/Documentation/devicetree/bindings/clock/at91-clock.txt
>>> @@ -90,6 +90,8 @@ Required properties:
>>>         "atmel,sama5d2-clk-audio-pll-pmc"
>>>                 at91 audio pll output on AUDIOPLLCLK that feeds the PMC
>>>                 and can be used by peripheral clock or generic clock
>>> +       "atmel,sama5d2-clk-i2s-mux":
>>> +               at91 I2S clock source selection
>>
>>
>> Is this boolean or takes some values. If latter, what are valid values?
>
>
> This is the compatible string of the clock driver.

Ah, now I remember. AT91 uses fine grained clock nodes in DT. Is there
still a plan to fix this?

>>>     Required properties for SCKC node:
>>>   - reg : defines the IO memory reserved for the SCKC.
>>> @@ -507,3 +509,35 @@ For example:
>>>                         atmel,clk-output-range = <0 83000000>;
>>>                 };
>>>         };
>>> +
>>> +Required properties for I2S mux clocks:
>>> +- #size-cells : shall be 0 (reg is used to encode I2S bus id).
>>> +- #address-cells : shall be 1 (reg is used to encode I2S bus id).
>>> +- name: device tree node describing a specific mux clock.
>>> +       * #clock-cells : from common clock binding; shall be set to 0.
>>> +       * clocks : shall be the mux clock parent phandles; shall be 2
>>> phandles:
>>> +         peripheral and generated clock; the first phandle shall belong
>>> to the
>>> +         peripheral clock and the second one shall belong to the
>>> generated
>>> +         clock; "clock-indices" property can be user to specify
>>> +         the correct order.
>>> +       * reg: I2S bus id of the corresponding mux clock.
>>> +         e.g. reg = <0>; for i2s0, reg = <1>; for i2s1
>>> +
>>> +For example:
>>> +       i2s_clkmux {
>>
>>
>> What is this a child of?
>
>
> It is a child of PMC node, since both parent clocks are generated by PMC.
>
>>
>>> +               compatible = "atmel,sama5d2-clk-i2s-mux";
>>> +               #address-cells = <1>;
>>> +               #size-cells = <0>;
>>
>>
>> How do you address this block? My guess is you don't because it is just
>> part of some other block and you are just creating this node to
>> instantiate a driver. Just make the node for the actual h/w block a
>> clock provider and define the clock ids (0 and 1).
>
>
> This block is not addressed, but its children are. The register we access in
> this driver is not part of other block. It's a SFR register, accessed
> through syscon and it has nothing to do with the I2S IP (see SAMA5D2 DS,
> page 1256, fig. 44-1: I2SC Block Diagram) that is the consumer of this
> clock. Adding a clock-id property in the I2S node would be just like v3 of
> this series, with the difference that we use clock-id instead of alias id to
> set the clock parent, which is not how you suggested back then.

I wasn't suggesting a clock-id property, but a clock specifier (i.e.
make #clock-cells 1).

But AT91 clocks are all a mess, so I don't know what to tell you.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd May 31, 2018, 3:31 p.m. UTC | #4
Quoting Rob Herring (2018-05-31 07:20:57)
> On Thu, May 31, 2018 at 5:25 AM, Codrin Ciubotariu
> <codrin.ciubotariu@microchip.com> wrote:
> > On 31.05.2018 03:58, Rob Herring wrote:
> >>
> >> On Fri, May 25, 2018 at 03:34:22PM +0300, Codrin Ciubotariu wrote:
> >>>
> >>> The I2S mux clock can be used to select the I2S input clock. The
> >>> available parents are the peripheral and the generated clocks.
> >>>
> >>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> >>> ---
> >>>   .../devicetree/bindings/clock/at91-clock.txt       | 34
> >>> ++++++++++++++++++++++
> >>>   1 file changed, 34 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt
> >>> b/Documentation/devicetree/bindings/clock/at91-clock.txt
> >>> index 51c259a..1c46b3c 100644
> >>> --- a/Documentation/devicetree/bindings/clock/at91-clock.txt
> >>> +++ b/Documentation/devicetree/bindings/clock/at91-clock.txt
> >>> @@ -90,6 +90,8 @@ Required properties:
> >>>         "atmel,sama5d2-clk-audio-pll-pmc"
> >>>                 at91 audio pll output on AUDIOPLLCLK that feeds the PMC
> >>>                 and can be used by peripheral clock or generic clock
> >>> +       "atmel,sama5d2-clk-i2s-mux":
> >>> +               at91 I2S clock source selection
> >>
> >>
> >> Is this boolean or takes some values. If latter, what are valid values?
> >
> >
> > This is the compatible string of the clock driver.
> 
> Ah, now I remember. AT91 uses fine grained clock nodes in DT. Is there
> still a plan to fix this?

I'm also interested in a plan.

> >>
> >>> +               compatible = "atmel,sama5d2-clk-i2s-mux";
> >>> +               #address-cells = <1>;
> >>> +               #size-cells = <0>;
> >>
> >>
> >> How do you address this block? My guess is you don't because it is just
> >> part of some other block and you are just creating this node to
> >> instantiate a driver. Just make the node for the actual h/w block a
> >> clock provider and define the clock ids (0 and 1).
> >
> >
> > This block is not addressed, but its children are. The register we access in
> > this driver is not part of other block. It's a SFR register, accessed
> > through syscon and it has nothing to do with the I2S IP (see SAMA5D2 DS,
> > page 1256, fig. 44-1: I2SC Block Diagram) that is the consumer of this
> > clock. Adding a clock-id property in the I2S node would be just like v3 of
> > this series, with the difference that we use clock-id instead of alias id to
> > set the clock parent, which is not how you suggested back then.
> 
> I wasn't suggesting a clock-id property, but a clock specifier (i.e.
> make #clock-cells 1).
> 
> But AT91 clocks are all a mess, so I don't know what to tell you.
> 

If #clock-cells of 1 works then we should go with that. It's still weird
that we need random nodes to add more clks, but I guess that's how it's
going to be for each at91 clk driver until it changes to be one big
provider node.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring May 31, 2018, 3:56 p.m. UTC | #5
On Thu, May 31, 2018 at 10:31 AM, Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Rob Herring (2018-05-31 07:20:57)
>> On Thu, May 31, 2018 at 5:25 AM, Codrin Ciubotariu
>> <codrin.ciubotariu@microchip.com> wrote:
>> > On 31.05.2018 03:58, Rob Herring wrote:
>> >>
>> >> On Fri, May 25, 2018 at 03:34:22PM +0300, Codrin Ciubotariu wrote:
>> >>>
>> >>> The I2S mux clock can be used to select the I2S input clock. The
>> >>> available parents are the peripheral and the generated clocks.
>> >>>
>> >>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
>> >>> ---
>> >>>   .../devicetree/bindings/clock/at91-clock.txt       | 34
>> >>> ++++++++++++++++++++++
>> >>>   1 file changed, 34 insertions(+)
>> >>>
>> >>> diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt
>> >>> b/Documentation/devicetree/bindings/clock/at91-clock.txt
>> >>> index 51c259a..1c46b3c 100644
>> >>> --- a/Documentation/devicetree/bindings/clock/at91-clock.txt
>> >>> +++ b/Documentation/devicetree/bindings/clock/at91-clock.txt
>> >>> @@ -90,6 +90,8 @@ Required properties:
>> >>>         "atmel,sama5d2-clk-audio-pll-pmc"
>> >>>                 at91 audio pll output on AUDIOPLLCLK that feeds the PMC
>> >>>                 and can be used by peripheral clock or generic clock
>> >>> +       "atmel,sama5d2-clk-i2s-mux":
>> >>> +               at91 I2S clock source selection
>> >>
>> >>
>> >> Is this boolean or takes some values. If latter, what are valid values?
>> >
>> >
>> > This is the compatible string of the clock driver.
>>
>> Ah, now I remember. AT91 uses fine grained clock nodes in DT. Is there
>> still a plan to fix this?
>
> I'm also interested in a plan.
>
>> >>
>> >>> +               compatible = "atmel,sama5d2-clk-i2s-mux";
>> >>> +               #address-cells = <1>;
>> >>> +               #size-cells = <0>;
>> >>
>> >>
>> >> How do you address this block? My guess is you don't because it is just
>> >> part of some other block and you are just creating this node to
>> >> instantiate a driver. Just make the node for the actual h/w block a
>> >> clock provider and define the clock ids (0 and 1).
>> >
>> >
>> > This block is not addressed, but its children are. The register we access in
>> > this driver is not part of other block. It's a SFR register, accessed
>> > through syscon and it has nothing to do with the I2S IP (see SAMA5D2 DS,
>> > page 1256, fig. 44-1: I2SC Block Diagram) that is the consumer of this
>> > clock. Adding a clock-id property in the I2S node would be just like v3 of
>> > this series, with the difference that we use clock-id instead of alias id to
>> > set the clock parent, which is not how you suggested back then.
>>
>> I wasn't suggesting a clock-id property, but a clock specifier (i.e.
>> make #clock-cells 1).
>>
>> But AT91 clocks are all a mess, so I don't know what to tell you.
>>
>
> If #clock-cells of 1 works then we should go with that. It's still weird
> that we need random nodes to add more clks, but I guess that's how it's
> going to be for each at91 clk driver until it changes to be one big
> provider node.

Seems to me that clock additions could use a new binding and we start
with a new driver that handles these few clocks initially. But I
haven't looked whether both can coexist.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Belloni May 31, 2018, 4:01 p.m. UTC | #6
On 31/05/2018 09:20:57-0500, Rob Herring wrote:
> On Thu, May 31, 2018 at 5:25 AM, Codrin Ciubotariu
> <codrin.ciubotariu@microchip.com> wrote:
> > On 31.05.2018 03:58, Rob Herring wrote:
> >>
> >> On Fri, May 25, 2018 at 03:34:22PM +0300, Codrin Ciubotariu wrote:
> >>>
> >>> The I2S mux clock can be used to select the I2S input clock. The
> >>> available parents are the peripheral and the generated clocks.
> >>>
> >>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> >>> ---
> >>>   .../devicetree/bindings/clock/at91-clock.txt       | 34
> >>> ++++++++++++++++++++++
> >>>   1 file changed, 34 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt
> >>> b/Documentation/devicetree/bindings/clock/at91-clock.txt
> >>> index 51c259a..1c46b3c 100644
> >>> --- a/Documentation/devicetree/bindings/clock/at91-clock.txt
> >>> +++ b/Documentation/devicetree/bindings/clock/at91-clock.txt
> >>> @@ -90,6 +90,8 @@ Required properties:
> >>>         "atmel,sama5d2-clk-audio-pll-pmc"
> >>>                 at91 audio pll output on AUDIOPLLCLK that feeds the PMC
> >>>                 and can be used by peripheral clock or generic clock
> >>> +       "atmel,sama5d2-clk-i2s-mux":
> >>> +               at91 I2S clock source selection
> >>
> >>
> >> Is this boolean or takes some values. If latter, what are valid values?
> >
> >
> > This is the compatible string of the clock driver.
> 
> Ah, now I remember. AT91 uses fine grained clock nodes in DT. Is there
> still a plan to fix this?
> 

There is still a plan to do that, hopefully soon (I'd like to aim for
the next release or the one after).

I think this one should go in as-is so it can be fixed with all the
other one at once.
Codrin Ciubotariu June 7, 2018, 10:30 a.m. UTC | #7
On 31.05.2018 18:56, Rob Herring wrote:
> On Thu, May 31, 2018 at 10:31 AM, Stephen Boyd <sboyd@kernel.org> wrote:
>> Quoting Rob Herring (2018-05-31 07:20:57)
>>> On Thu, May 31, 2018 at 5:25 AM, Codrin Ciubotariu
>>> <codrin.ciubotariu@microchip.com> wrote:
>>>> On 31.05.2018 03:58, Rob Herring wrote:
>>>>>
>>>>> On Fri, May 25, 2018 at 03:34:22PM +0300, Codrin Ciubotariu wrote:
>>>>>>
>>>>>> The I2S mux clock can be used to select the I2S input clock. The
>>>>>> available parents are the peripheral and the generated clocks.
>>>>>>
>>>>>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
>>>>>> ---
>>>>>>    .../devicetree/bindings/clock/at91-clock.txt       | 34
>>>>>> ++++++++++++++++++++++
>>>>>>    1 file changed, 34 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt
>>>>>> b/Documentation/devicetree/bindings/clock/at91-clock.txt
>>>>>> index 51c259a..1c46b3c 100644
>>>>>> --- a/Documentation/devicetree/bindings/clock/at91-clock.txt
>>>>>> +++ b/Documentation/devicetree/bindings/clock/at91-clock.txt
>>>>>> @@ -90,6 +90,8 @@ Required properties:
>>>>>>          "atmel,sama5d2-clk-audio-pll-pmc"
>>>>>>                  at91 audio pll output on AUDIOPLLCLK that feeds the PMC
>>>>>>                  and can be used by peripheral clock or generic clock
>>>>>> +       "atmel,sama5d2-clk-i2s-mux":
>>>>>> +               at91 I2S clock source selection
>>>>>
>>>>>
>>>>> Is this boolean or takes some values. If latter, what are valid values?
>>>>
>>>>
>>>> This is the compatible string of the clock driver.
>>>
>>> Ah, now I remember. AT91 uses fine grained clock nodes in DT. Is there
>>> still a plan to fix this?
>>
>> I'm also interested in a plan.
>>
>>>>>
>>>>>> +               compatible = "atmel,sama5d2-clk-i2s-mux";
>>>>>> +               #address-cells = <1>;
>>>>>> +               #size-cells = <0>;
>>>>>
>>>>>
>>>>> How do you address this block? My guess is you don't because it is just
>>>>> part of some other block and you are just creating this node to
>>>>> instantiate a driver. Just make the node for the actual h/w block a
>>>>> clock provider and define the clock ids (0 and 1).
>>>>
>>>>
>>>> This block is not addressed, but its children are. The register we access in
>>>> this driver is not part of other block. It's a SFR register, accessed
>>>> through syscon and it has nothing to do with the I2S IP (see SAMA5D2 DS,
>>>> page 1256, fig. 44-1: I2SC Block Diagram) that is the consumer of this
>>>> clock. Adding a clock-id property in the I2S node would be just like v3 of
>>>> this series, with the difference that we use clock-id instead of alias id to
>>>> set the clock parent, which is not how you suggested back then.
>>>
>>> I wasn't suggesting a clock-id property, but a clock specifier (i.e.
>>> make #clock-cells 1).
>>>
>>> But AT91 clocks are all a mess, so I don't know what to tell you.
>>>
>>
>> If #clock-cells of 1 works then we should go with that. It's still weird
>> that we need random nodes to add more clks, but I guess that's how it's
>> going to be for each at91 clk driver until it changes to be one big
>> provider node.
> 
> Seems to me that clock additions could use a new binding and we start
> with a new driver that handles these few clocks initially. But I
> haven't looked whether both can coexist.

Mark already applied to broonie/sound.git the I2S bindings that have a 
phandle to this clock. If I am to change #clock-cells to 1, I will have 
to change the bindings to include the clock-id.
Which approach should I take now?

Thanks and best regards,
Codrin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd June 12, 2018, 8:06 a.m. UTC | #8
Quoting Codrin Ciubotariu (2018-06-07 03:30:14)
> > 
> > Seems to me that clock additions could use a new binding and we start
> > with a new driver that handles these few clocks initially. But I
> > haven't looked whether both can coexist.
> 
> Mark already applied to broonie/sound.git the I2S bindings that have a 
> phandle to this clock. If I am to change #clock-cells to 1, I will have 
> to change the bindings to include the clock-id.
> Which approach should I take now?
> 

You're talking about changing the example in the binding doc? That
doesn't really matter. Consumer side of the provider has to match the
cell count from the provider so it doesn't really need to be 'fixed' in
the example at all.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt b/Documentation/devicetree/bindings/clock/at91-clock.txt
index 51c259a..1c46b3c 100644
--- a/Documentation/devicetree/bindings/clock/at91-clock.txt
+++ b/Documentation/devicetree/bindings/clock/at91-clock.txt
@@ -90,6 +90,8 @@  Required properties:
 	"atmel,sama5d2-clk-audio-pll-pmc"
 		at91 audio pll output on AUDIOPLLCLK that feeds the PMC
 		and can be used by peripheral clock or generic clock
+	"atmel,sama5d2-clk-i2s-mux":
+		at91 I2S clock source selection
 
 Required properties for SCKC node:
 - reg : defines the IO memory reserved for the SCKC.
@@ -507,3 +509,35 @@  For example:
 			atmel,clk-output-range = <0 83000000>;
 		};
 	};
+
+Required properties for I2S mux clocks:
+- #size-cells : shall be 0 (reg is used to encode I2S bus id).
+- #address-cells : shall be 1 (reg is used to encode I2S bus id).
+- name: device tree node describing a specific mux clock.
+	* #clock-cells : from common clock binding; shall be set to 0.
+	* clocks : shall be the mux clock parent phandles; shall be 2 phandles:
+	  peripheral and generated clock; the first phandle shall belong to the
+	  peripheral clock and the second one shall belong to the generated
+	  clock; "clock-indices" property can be user to specify
+	  the correct order.
+	* reg: I2S bus id of the corresponding mux clock.
+	  e.g. reg = <0>; for i2s0, reg = <1>; for i2s1
+
+For example:
+	i2s_clkmux {
+		compatible = "atmel,sama5d2-clk-i2s-mux";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2s0muxck: i2s0_muxclk {
+			clocks = <&i2s0_clk>, <&i2s0_gclk>;
+			#clock-cells = <0>;
+			reg = <0>;
+		};
+
+		i2s1muxck: i2s1_muxclk {
+			clocks = <&i2s1_clk>, <&i2s1_gclk>;
+			#clock-cells = <0>;
+			reg = <1>;
+		};
+	};