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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
> >>>>>> ---
> >>>>>>  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




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux