[PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[Biju Das]

Fix audio clk node names with "_" -> "-" and add suffix '-clk' for can and
extal clks.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
index 19287cccb1f0..4f9a84d7af08 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
@@ -13,14 +13,14 @@ / {
 	#address-cells = <2>;
 	#size-cells = <2>;
 
-	audio_clk1: audio_clk1 {
+	audio_clk1: audio-clk1 {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		/* This value must be overridden by boards that provide it */
 		clock-frequency = <0>;
 	};
 
-	audio_clk2: audio_clk2 {
+	audio_clk2: audio-clk2 {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		/* This value must be overridden by boards that provide it */
@@ -28,14 +28,14 @@ audio_clk2: audio_clk2 {
 	};
 
 	/* External CAN clock - to be overridden by boards that provide it */
-	can_clk: can {
+	can_clk: can-clk {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		clock-frequency = <0>;
 	};
 
 	/* clock can be either from exclk or crystal oscillator (XIN/XOUT) */
-	extal_clk: extal {
+	extal_clk: extal-clk {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		/* This value must be overridden by the board */
-- 
2.25.1

[PATCH 2/2] arm64: dts: renesas: r9a07g054: Fix external clk node names

[Biju Das]

Fix audio clk node names with "_" -> "-" and add suffix '-clk' for can and
extal clks.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r9a07g054.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r9a07g054.dtsi b/arch/arm64/boot/dts/renesas/r9a07g054.dtsi
index f35aa0311e9c..4313b9e3abed 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g054.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g054.dtsi
@@ -13,14 +13,14 @@ / {
 	#address-cells = <2>;
 	#size-cells = <2>;
 
-	audio_clk1: audio_clk1 {
+	audio_clk1: audio-clk1 {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		/* This value must be overridden by boards that provide it */
 		clock-frequency = <0>;
 	};
 
-	audio_clk2: audio_clk2 {
+	audio_clk2: audio-clk2 {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		/* This value must be overridden by boards that provide it */
@@ -28,14 +28,14 @@ audio_clk2: audio_clk2 {
 	};
 
 	/* External CAN clock - to be overridden by boards that provide it */
-	can_clk: can {
+	can_clk: can-clk {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		clock-frequency = <0>;
 	};
 
 	/* clock can be either from exclk or crystal oscillator (XIN/XOUT) */
-	extal_clk: extal {
+	extal_clk: extal-clk {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		/* This value must be overridden by the board */
-- 
2.25.1

Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[Krzysztof Kozlowski]

On 23/04/2022 16:06, Biju Das wrote:
> Fix audio clk node names with "_" -> "-" and add suffix '-clk' for can and
> extal clks.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> index 19287cccb1f0..4f9a84d7af08 100644
> --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> @@ -13,14 +13,14 @@ / {
>  	#address-cells = <2>;
>  	#size-cells = <2>;
>  
> -	audio_clk1: audio_clk1 {
> +	audio_clk1: audio-clk1 {

How about in such case keeping suffix "clk" everywhere and moving the
index (1/2) to the first part? This way one day maybe schema will match
the clocks.

Best regards,
Krzysztof

RE: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[Biju Das]

Hi Krzysztof Kozlowski,

Thanks for the feedback.

> Subject: Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk
> node names
> 
> On 23/04/2022 16:06, Biju Das wrote:
> > Fix audio clk node names with "_" -> "-" and add suffix '-clk' for can
> > and extal clks.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > index 19287cccb1f0..4f9a84d7af08 100644
> > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > @@ -13,14 +13,14 @@ / {
> >  	#address-cells = <2>;
> >  	#size-cells = <2>;
> >
> > -	audio_clk1: audio_clk1 {
> > +	audio_clk1: audio-clk1 {
> 
> How about in such case keeping suffix "clk" everywhere and moving the index
> (1/2) to the first part? This way one day maybe schema will match the
> clocks.

Just a question,

Your suggestion is "audio_clk1: audio-clk1" -> "audio_clk1: audio-clk"

In that case, If you plan to drop the label in future, how will you differentiate between
audio-clk1 and audio-clk2 with just node names?

Cheers,
Biju

RE: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[Biju Das]

> Subject: RE: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk
> node names
> 
> Hi Krzysztof Kozlowski,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external
> > clk node names
> >
> > On 23/04/2022 16:06, Biju Das wrote:
> > > Fix audio clk node names with "_" -> "-" and add suffix '-clk' for
> > > can and extal clks.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > >  arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > index 19287cccb1f0..4f9a84d7af08 100644
> > > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > @@ -13,14 +13,14 @@ / {
> > >  	#address-cells = <2>;
> > >  	#size-cells = <2>;
> > >
> > > -	audio_clk1: audio_clk1 {
> > > +	audio_clk1: audio-clk1 {
> >
> > How about in such case keeping suffix "clk" everywhere and moving the
> > index
> > (1/2) to the first part? This way one day maybe schema will match the
> > clocks.
> 
> Just a question,
> 
> Your suggestion is "audio_clk1: audio-clk1" -> "audio_clk1: audio-clk"
> 
> In that case, If you plan to drop the label in future, how will you
> differentiate between
> audio-clk1 and audio-clk2 with just node names?

Or

Do you want me to do the change like this?

"audio_clk1: audio-clk1" -> "audio_clk_1: audio-clk-1"

And fix the phandle reference in other dtsi files??

Please let me know.

Cheers,
Biju

Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[Krzysztof Kozlowski]

On 24/04/2022 09:50, Biju Das wrote:
>> Subject: RE: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk
>> node names
>>
>> Hi Krzysztof Kozlowski,
>>
>> Thanks for the feedback.
>>
>>> Subject: Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external
>>> clk node names
>>>
>>> On 23/04/2022 16:06, Biju Das wrote:
>>>> Fix audio clk node names with "_" -> "-" and add suffix '-clk' for
>>>> can and extal clks.
>>>>
>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>> ---
>>>>  arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 8 ++++----
>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
>>>> b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
>>>> index 19287cccb1f0..4f9a84d7af08 100644
>>>> --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
>>>> +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
>>>> @@ -13,14 +13,14 @@ / {
>>>>  	#address-cells = <2>;
>>>>  	#size-cells = <2>;
>>>>
>>>> -	audio_clk1: audio_clk1 {
>>>> +	audio_clk1: audio-clk1 {
>>>
>>> How about in such case keeping suffix "clk" everywhere and moving the
>>> index
>>> (1/2) to the first part? This way one day maybe schema will match the
>>> clocks.
>>
>> Just a question,
>>
>> Your suggestion is "audio_clk1: audio-clk1" -> "audio_clk1: audio-clk"
>>
>> In that case, If you plan to drop the label in future, how will you
>> differentiate between
>> audio-clk1 and audio-clk2 with just node names?
> 
> Or
> 
> Do you want me to do the change like this?
> 
> "audio_clk1: audio-clk1" -> "audio_clk_1: audio-clk-1"
> 
> And fix the phandle reference in other dtsi files??

My suggestion was to move the [12] part into the first part, so the
suffix "clk" stays consistent:
audio1-clk
audio2-clk


Best regards,
Krzysztof

RE: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[Biju Das]

Hi Krzysztof Kozlowski,

Thanks for the feedback.

> Subject: Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk
> node names
> 
> On 24/04/2022 09:50, Biju Das wrote:
> >> Subject: RE: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external
> >> clk node names
> >>
> >> Hi Krzysztof Kozlowski,
> >>
> >> Thanks for the feedback.
> >>
> >>> Subject: Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix
> >>> external clk node names
> >>>
> >>> On 23/04/2022 16:06, Biju Das wrote:
> >>>> Fix audio clk node names with "_" -> "-" and add suffix '-clk' for
> >>>> can and extal clks.
> >>>>
> >>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>>> ---
> >>>>  arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 8 ++++----
> >>>>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> >>>> b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> >>>> index 19287cccb1f0..4f9a84d7af08 100644
> >>>> --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> >>>> +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> >>>> @@ -13,14 +13,14 @@ / {
> >>>>  	#address-cells = <2>;
> >>>>  	#size-cells = <2>;
> >>>>
> >>>> -	audio_clk1: audio_clk1 {
> >>>> +	audio_clk1: audio-clk1 {
> >>>
> >>> How about in such case keeping suffix "clk" everywhere and moving
> >>> the index
> >>> (1/2) to the first part? This way one day maybe schema will match
> >>> the clocks.
> >>
> >> Just a question,
> >>
> >> Your suggestion is "audio_clk1: audio-clk1" -> "audio_clk1: audio-clk"
> >>
> >> In that case, If you plan to drop the label in future, how will you
> >> differentiate between
> >> audio-clk1 and audio-clk2 with just node names?
> >
> > Or
> >
> > Do you want me to do the change like this?
> >
> > "audio_clk1: audio-clk1" -> "audio_clk_1: audio-clk-1"
> >
> > And fix the phandle reference in other dtsi files??
> 
> My suggestion was to move the [12] part into the first part, so the suffix
> "clk" stays consistent:
> audio1-clk
> audio2-clk

From HW perspective,  there are 2 audio clocks, audio clock1(multiple and sub multiple of 44.1 Khz) 
and audio clk 2(Multiple and submultiple of 48Khz) connected to a single audio Codec.

Based on the sampling rate, through clock generator driver we can switch the clock source for 
audio mclock along with audio clock for SSI and we can support both these rates 

Since there is a single audio codec, I am not sure, audio1-clk and audio2-clk is a good choise.

What about like 

audio_clk1: audio-clk-1 ?
audio_clk2: audio-clk-2 ?

Which is consistent with naming used for cpu and opp-tables?

Cheers,
Biju

Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[Krzysztof Kozlowski]

On 24/04/2022 12:22, Biju Das wrote:
> Hi Krzysztof Kozlowski,
> 
> Thanks for the feedback.
> 
>> Subject: Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk
>> node names
>>
>> On 24/04/2022 09:50, Biju Das wrote:
>>>> Subject: RE: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external
>>>> clk node names
>>>>
>>>> Hi Krzysztof Kozlowski,
>>>>
>>>> Thanks for the feedback.
>>>>
>>>>> Subject: Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix
>>>>> external clk node names
>>>>>
>>>>> On 23/04/2022 16:06, Biju Das wrote:
>>>>>> Fix audio clk node names with "_" -> "-" and add suffix '-clk' for
>>>>>> can and extal clks.
>>>>>>
>>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>>>> ---
>>>>>>  arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 8 ++++----
>>>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
>>>>>> b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
>>>>>> index 19287cccb1f0..4f9a84d7af08 100644
>>>>>> --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
>>>>>> @@ -13,14 +13,14 @@ / {
>>>>>>  	#address-cells = <2>;
>>>>>>  	#size-cells = <2>;
>>>>>>
>>>>>> -	audio_clk1: audio_clk1 {
>>>>>> +	audio_clk1: audio-clk1 {
>>>>>
>>>>> How about in such case keeping suffix "clk" everywhere and moving
>>>>> the index
>>>>> (1/2) to the first part? This way one day maybe schema will match
>>>>> the clocks.
>>>>
>>>> Just a question,
>>>>
>>>> Your suggestion is "audio_clk1: audio-clk1" -> "audio_clk1: audio-clk"
>>>>
>>>> In that case, If you plan to drop the label in future, how will you
>>>> differentiate between
>>>> audio-clk1 and audio-clk2 with just node names?
>>>
>>> Or
>>>
>>> Do you want me to do the change like this?
>>>
>>> "audio_clk1: audio-clk1" -> "audio_clk_1: audio-clk-1"
>>>
>>> And fix the phandle reference in other dtsi files??
>>
>> My suggestion was to move the [12] part into the first part, so the suffix
>> "clk" stays consistent:
>> audio1-clk
>> audio2-clk
> 
> From HW perspective,  there are 2 audio clocks, audio clock1(multiple and sub multiple of 44.1 Khz) 
> and audio clk 2(Multiple and submultiple of 48Khz) connected to a single audio Codec.
> 
> Based on the sampling rate, through clock generator driver we can switch the clock source for 
> audio mclock along with audio clock for SSI and we can support both these rates 
> 
> Since there is a single audio codec, I am not sure, audio1-clk and audio2-clk is a good choise.

The name of the clock is not "audio clock" but "audio", because you do
not call a car "Ford Mustang car", but just "Ford Mustang". Therefore
"clock" is not part of the name, but just description of a type.

> 
> What about like 
> 
> audio_clk1: audio-clk-1 ?
> audio_clk2: audio-clk-2 ?
> 
> Which is consistent with naming used for cpu and opp-tables?


It's not consistent with clk naming. Nodes should have generic names, so
the generic part is "clk". You add specific audio/audio-X prefix or
suffix - it's fine, but not both.

This is exactly the trouble when you start using specific names and
Devicetree spec explicitly asks for generic names. So maybe go with the
spec and call of these "clk-[0-9]" and problem is gone.


Best regards,
Krzysztof

RE: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[Biju Das]

Hi Krzysztof Kozlowski,

Thanks for the feedback.

> Subject: Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk
> node names
> 
> On 24/04/2022 12:22, Biju Das wrote:
> > Hi Krzysztof Kozlowski,
> >
> > Thanks for the feedback.
> >
> >> Subject: Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external
> >> clk node names
> >>
> >> On 24/04/2022 09:50, Biju Das wrote:
> >>>> Subject: RE: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix
> >>>> external clk node names
> >>>>
> >>>> Hi Krzysztof Kozlowski,
> >>>>
> >>>> Thanks for the feedback.
> >>>>
> >>>>> Subject: Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix
> >>>>> external clk node names
> >>>>>
> >>>>> On 23/04/2022 16:06, Biju Das wrote:
> >>>>>> Fix audio clk node names with "_" -> "-" and add suffix '-clk'
> >>>>>> for can and extal clks.
> >>>>>>
> >>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>>>>> ---
> >>>>>>  arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 8 ++++----
> >>>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>>>>>
> >>>>>> diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> >>>>>> b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> >>>>>> index 19287cccb1f0..4f9a84d7af08 100644
> >>>>>> --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> >>>>>> +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> >>>>>> @@ -13,14 +13,14 @@ / {
> >>>>>>  	#address-cells = <2>;
> >>>>>>  	#size-cells = <2>;
> >>>>>>
> >>>>>> -	audio_clk1: audio_clk1 {
> >>>>>> +	audio_clk1: audio-clk1 {
> >>>>>
> >>>>> How about in such case keeping suffix "clk" everywhere and moving
> >>>>> the index
> >>>>> (1/2) to the first part? This way one day maybe schema will match
> >>>>> the clocks.
> >>>>
> >>>> Just a question,
> >>>>
> >>>> Your suggestion is "audio_clk1: audio-clk1" -> "audio_clk1: audio-
> clk"
> >>>>
> >>>> In that case, If you plan to drop the label in future, how will you
> >>>> differentiate between
> >>>> audio-clk1 and audio-clk2 with just node names?
> >>>
> >>> Or
> >>>
> >>> Do you want me to do the change like this?
> >>>
> >>> "audio_clk1: audio-clk1" -> "audio_clk_1: audio-clk-1"
> >>>
> >>> And fix the phandle reference in other dtsi files??
> >>
> >> My suggestion was to move the [12] part into the first part, so the
> >> suffix "clk" stays consistent:
> >> audio1-clk
> >> audio2-clk
> >
> > From HW perspective,  there are 2 audio clocks, audio clock1(multiple
> > and sub multiple of 44.1 Khz) and audio clk 2(Multiple and submultiple
> of 48Khz) connected to a single audio Codec.
> >
> > Based on the sampling rate, through clock generator driver we can
> > switch the clock source for audio mclock along with audio clock for
> > SSI and we can support both these rates
> >
> > Since there is a single audio codec, I am not sure, audio1-clk and
> audio2-clk is a good choise.
> 
> The name of the clock is not "audio clock" but "audio", because you do not
> call a car "Ford Mustang car", but just "Ford Mustang". Therefore "clock"
> is not part of the name, but just description of a type.

The hardware mention the name as AUDIO_CLK1 and AUDIO_CLK2.
There are 2 Clock availables for audio interface.
In that case if you term it as audio1-clk and audio-clk2,
But as you said clk-1-audio and clk-2-audio will be correct?

> 
> >
> > What about like
> >
> > audio_clk1: audio-clk-1 ?
> > audio_clk2: audio-clk-2 ?
> >
> > Which is consistent with naming used for cpu and opp-tables?
> 
> 
> It's not consistent with clk naming. Nodes should have generic names, so
> the generic part is "clk". You add specific audio/audio-X prefix or suffix
> - it's fine, but not both.
> 
> This is exactly the trouble when you start using specific names and
> Devicetree spec explicitly asks for generic names. So maybe go with the
> spec and call of these "clk-[0-9]" and problem is gone.

Ok Will change like

"audio_clk1: clk-1-audio"

Label name matches with hardware manual and node names as per Device tree spec.

Regards,
Biju

Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[Krzysztof Kozlowski]

On 25/04/2022 17:28, Biju Das wrote:
>>>> My suggestion was to move the [12] part into the first part, so the
>>>> suffix "clk" stays consistent:
>>>> audio1-clk
>>>> audio2-clk
>>>
>>> From HW perspective,  there are 2 audio clocks, audio clock1(multiple
>>> and sub multiple of 44.1 Khz) and audio clk 2(Multiple and submultiple
>> of 48Khz) connected to a single audio Codec.
>>>
>>> Based on the sampling rate, through clock generator driver we can
>>> switch the clock source for audio mclock along with audio clock for
>>> SSI and we can support both these rates
>>>
>>> Since there is a single audio codec, I am not sure, audio1-clk and
>> audio2-clk is a good choise.
>>
>> The name of the clock is not "audio clock" but "audio", because you do not
>> call a car "Ford Mustang car", but just "Ford Mustang". Therefore "clock"
>> is not part of the name, but just description of a type.
> 
> The hardware mention the name as AUDIO_CLK1 and AUDIO_CLK2.

The hardware document might call it "AUDIO_CLK_REAL_CLK_CLK" and it
won't be an argument to call device node that way in DTS.

> There are 2 Clock availables for audio interface.
> In that case if you term it as audio1-clk and audio-clk2,
> But as you said clk-1-audio and clk-2-audio will be correct?

If you change all other clocks to follow same principle - generic name
followed by specific suffix - then yes. Then you should have
"clk-extal", "clk-can" etc.

> 
>>
>>>
>>> What about like
>>>
>>> audio_clk1: audio-clk-1 ?
>>> audio_clk2: audio-clk-2 ?
>>>
>>> Which is consistent with naming used for cpu and opp-tables?
>>
>>
>> It's not consistent with clk naming. Nodes should have generic names, so
>> the generic part is "clk". You add specific audio/audio-X prefix or suffix
>> - it's fine, but not both.
>>
>> This is exactly the trouble when you start using specific names and
>> Devicetree spec explicitly asks for generic names. So maybe go with the
>> spec and call of these "clk-[0-9]" and problem is gone.
> 
> Ok Will change like
> 
> "audio_clk1: clk-1-audio"
> 

What do you mean "ok"? I said "clk-[0-9]", so "clk-0", "clk-1", "clk-2"
and so on. No specific prefix.

> Label name matches with hardware manual and node names as per Device tree spec.


Best regards,
Krzysztof

RE: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[Biju Das]

Hi Krzysztof Kozlowski,

Thanks for feedback.


> Subject: Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk
> node names
> 
> On 25/04/2022 17:28, Biju Das wrote:
> >>>> My suggestion was to move the [12] part into the first part, so the
> >>>> suffix "clk" stays consistent:
> >>>> audio1-clk
> >>>> audio2-clk
> >>>
> >>> From HW perspective,  there are 2 audio clocks, audio
> >>> clock1(multiple and sub multiple of 44.1 Khz) and audio clk
> >>> 2(Multiple and submultiple
> >> of 48Khz) connected to a single audio Codec.
> >>>
> >>> Based on the sampling rate, through clock generator driver we can
> >>> switch the clock source for audio mclock along with audio clock for
> >>> SSI and we can support both these rates
> >>>
> >>> Since there is a single audio codec, I am not sure, audio1-clk and
> >> audio2-clk is a good choise.
> >>
> >> The name of the clock is not "audio clock" but "audio", because you
> >> do not call a car "Ford Mustang car", but just "Ford Mustang".
> Therefore "clock"
> >> is not part of the name, but just description of a type.
> >
> > The hardware mention the name as AUDIO_CLK1 and AUDIO_CLK2.
> 
> The hardware document might call it "AUDIO_CLK_REAL_CLK_CLK" and it won't
> be an argument to call device node that way in DTS.

Got it.

> > > There are 2 Clock availables for audio interface.
> > In that case if you term it as audio1-clk and audio2-clk it may not be correct, But as you
> > said clk-1-audio and clk-2-audio will it be correct?
> 
> If you change all other clocks to follow same principle - generic name
> followed by specific suffix - then yes. Then you should have "clk-extal",
> "clk-can" etc.

Ok.

> 
> >
> >>
> >>>
> >>> What about like
> >>>
> >>> audio_clk1: audio-clk-1 ?
> >>> audio_clk2: audio-clk-2 ?
> >>>
> >>> Which is consistent with naming used for cpu and opp-tables?
> >>
> >>
> >> It's not consistent with clk naming. Nodes should have generic names,
> >> so the generic part is "clk". You add specific audio/audio-X prefix
> >> or suffix
> >> - it's fine, but not both.
> >>
> >> This is exactly the trouble when you start using specific names and
> >> Devicetree spec explicitly asks for generic names. So maybe go with
> >> the spec and call of these "clk-[0-9]" and problem is gone.
> >
> > Ok Will change like
> >
> > "audio_clk1: clk-1-audio"
> >
> 
> What do you mean "ok"? I said "clk-[0-9]", so "clk-0", "clk-1", "clk-2"
> and so on. No specific prefix.

Ah ok.

As you mentioned above "generic part is "clk". You add specific audio/audio-X prefix
or suffix"

So based on that, I thought "clk" is generic part and "-1-audio" as suffix, "clk-1-audio" 
should be acceptable.

For eg:- If I plan to match the label name with the hardware manual(audio_clk1),

then, as per the discussion we have, the node names should be

either

"audio_clk1: clk-0"

or 

"audio_clk1: clk-1"

Or

"audio_clk1: audio1-clk"

Correct?

Regards,
Biju
> 
> > Label name matches with hardware manual and node names as per Device
> tree spec.
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[Krzysztof Kozlowski]

On 25/04/2022 18:26, Biju Das wrote:
>>
>> What do you mean "ok"? I said "clk-[0-9]", so "clk-0", "clk-1", "clk-2"
>> and so on. No specific prefix.
> 
> Ah ok.
> 
> As you mentioned above "generic part is "clk". You add specific audio/audio-X prefix
> or suffix"
> 
> So based on that, I thought "clk" is generic part and "-1-audio" as suffix, "clk-1-audio" 
> should be acceptable.
> 
> For eg:- If I plan to match the label name with the hardware manual(audio_clk1),

Labels are not restricted (except using [a-z0-9_], no dashes), so it is
perfectly fine.

> 
> then, as per the discussion we have, the node names should be
> 
> either
> 
> "audio_clk1: clk-0"
> 
> or 
> 
> "audio_clk1: clk-1"
> 
> Or
> 
> "audio_clk1: audio1-clk"
> 
> Correct?

Yes, correct.

Best regards,
Krzysztof

RE: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk node names

[Biju Das]

Hi Krzysztof Kozlowski,

> Subject: Re: [PATCH 1/2] arm64: dts: renesas: r9a07g044: Fix external clk
> node names
> 
> On 25/04/2022 18:26, Biju Das wrote:
> >>
> >> What do you mean "ok"? I said "clk-[0-9]", so "clk-0", "clk-1", "clk-2"
> >> and so on. No specific prefix.
> >
> > Ah ok.
> >
> > As you mentioned above "generic part is "clk". You add specific
> > audio/audio-X prefix or suffix"
> >
> > So based on that, I thought "clk" is generic part and "-1-audio" as
> suffix, "clk-1-audio"
> > should be acceptable.
> >
> > For eg:- If I plan to match the label name with the hardware
> > manual(audio_clk1),
> 
> Labels are not restricted (except using [a-z0-9_], no dashes), so it is
> perfectly fine.

OK.

> 
> >
> > then, as per the discussion we have, the node names should be
> >
> > either
> >
> > "audio_clk1: clk-0"
> >
> > or
> >
> > "audio_clk1: clk-1"
> >
> > Or
> >
> > "audio_clk1: audio1-clk"
> >
> > Correct?
> 
> Yes, correct.

Thanks for the clarification.

Cheers,
Biju