RE: [PATCH v2 24/25] arm64: dts: renesas: rzg3s-smarc: Enable SSI3

[Biju Das <biju.das.jz@xxxxxxxxxxxxxx>]

Hi Claudiu,

> Subject: Re: [PATCH v2 24/25] arm64: dts: renesas: rzg3s-smarc: Enable SSI3
> 
> 
> 
> On 12.11.2024 11:03, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: Claudiu Beznea <claudiu.beznea@xxxxxxxxx>
> >> Sent: 12 November 2024 08:31
> >> Subject: Re: [PATCH v2 24/25] arm64: dts: renesas: rzg3s-smarc:
> >> Enable SSI3
> >>
> >> Hi, Biju,
> >>
> >> On 11.11.2024 13:30, Biju Das wrote:
> >>> Hi Claudiu,
> >>>
> >>>> -----Original Message-----
> >>>> From: Claudiu Beznea <claudiu.beznea@xxxxxxxxx>
> >>>> Sent: 11 November 2024 11:20
> >>>> Subject: Re: [PATCH v2 24/25] arm64: dts: renesas: rzg3s-smarc:
> >>>> Enable SSI3
> >>>>
> >>>> Hi, Biju,
> >>>>
> >>>> On 10.11.2024 10:54, Biju Das wrote:
> >>>>> Hi Claudiu,
> >>>>>
> >>>>> Thanks for the patch.
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Claudiu <claudiu.beznea@xxxxxxxxx>
> >>>>>> Sent: 08 November 2024 10:50
> >>>>>> Subject: [PATCH v2 24/25] arm64: dts: renesas: rzg3s-smarc:
> >>>>>> Enable
> >>>>>> SSI3
> >>>>>>
> >>>>>> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >>>>>>
> >>>>>> Enable SSI3.
> >>>>>>
> >>>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >>>>>> ---
> >>>>>>
> >>>>>> Changes in v2:
> >>>>>> - none
> >>>>>>
> >>>>>>  arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi | 26
> >>>>>> ++++++++++++++++++++
> >>>>>>  1 file changed, 26 insertions(+)
> >>>>>>
> >>>>>> diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
> >>>>>> b/arch/arm64/boot/dts/renesas/rzg3s-
> >>>>>> smarc.dtsi
> >>>>>> index 4aa99814b808..6dd439e68bd4 100644
> >>>>>> --- a/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
> >>>>>> +++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
> >>>>>> @@ -64,6 +64,11 @@ vccq_sdhi1: regulator-vccq-sdhi1 {
> >>>>>>  	};
> >>>>>>  };
> >>>>>>
> >>>>>
> >>>>> &audio_clk1 {
> >>>>>        assigned-clocks = <&versa3 xx>;
> >>>>>        clock-frequency = <11289600>; };
> >>>>
> >>>> audio_clk1 node is in the RZ/G3S dtsi to keep the compilation happy.
> >>>>
> >>>> For this board the audio clock1 for the SSI 3 is from <&versa3 2>.
> >>>>
> >>>> If we fill in the audio_clk1 here it will be useless, there will be
> >>>> no consumers for it and it is not available on board.
> >>>
> >>> As per SSI IP needs external clks AUDIO_CLK1 and AUDIO_CLK2.
> >>>
> >>> AUDIO_CLK1 is provided by versa3 generator and
> >>> AUDIO_CLK2 is provided by Crystal.
> >>>
> >>> Currently AUDIO_CLK2 it reports a frequency of 12288000 which is a
> >>> multiple of 48kHz whereas for AUDIO_CLK1, it reports a frequency of 0.
> >>
> >> Why? You mentioned above that "AUDIO_CLK1 is provided by versa3 generator".
> >
> > Output from versa3 generator is connector to AUDIO_CLK1
> 
> According to schematics this is true.
> 
> 
> > that you described in
> > SoC dtsi node with the entries
> >
> > +	audio_clk1: audio-clk1 {
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		/* This value must be overridden by boards that provide it. */
> > +		clock-frequency = <0>;
> > +		status = "disabled";
> > +	};
> 
> That is a clock node, placeholder in the DTSI, to make compilation happy.
> 
> >
> > This needs to be overridden by board dts,
> 
> Only if used, otherwise is an useless node.

I agree it is an useless node, if there is no relation between versa 3 output and
audio_clk1 node.


> 
> > where versa3 is providing this clk.
> > Currently there is no relation between this SoC device node and versa3 clk output for audio clk1.
> 
> I may be wrong or I many not understand what you are trying to say, but isn't what this patch does?
> See this diff from this patch:

The ssi3 definitions are OK. audio_clk2 node, we described crystal frequency of 12288000.
Other audio_clk1 node is useless node, even though versa3 is providing required clock
with 11289600.

I am not sure is this definition is required or not as I am not a DT expert??

Cheers,
Biju

> 
> +&ssi3 {
> +	clocks = <&cpg CPG_MOD R9A08G045_SSI3_PCLK2>,
> +		 <&cpg CPG_MOD R9A08G045_SSI3_PCLK_SFR>,
> +		 <&versa3 2>, <&audio_clk2>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&ssi3_pins>, <&audio_clock_pins>;
> +	status = "okay";
> +};