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

[Claudiu Beznea <claudiu.beznea@xxxxxxxxx>]

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".
It will report the frequency provided by the versa3 clock generator, isn't it?

> By defining the node, it will report as the value as
> 11289600 which is a multiple of 44.1kHZ.

Defining the node as you proposed have no meaning as it will be anyway
disabled (see the dtsi) and will appear nowhere as no driver will be probed
for it.

Defining it's frequency and enabling will have no meaning either for the
SSI3, as the SSI3 is connected to <&versa3 2> (as of the binding proposed
in this patch).

> 
> From the schematic we know that versa 3 is providing this clock and the audio_clk1 has
> a frequency of "11289600".

<&versa3 2> connected to AUDIO_CLK1 pin is configured at 11.2896MHz in this
series. See patch 22/25:

+	versa3: clock-generator@68 {
+		compatible = "renesas,5l35023";
+		reg = <0x68>;
+		clocks = <&x3_clk>;
+		#clock-cells = <1>;
+		assigned-clocks = <&versa3 0>,
+				  <&versa3 1>,
+				  *<&versa3 2>*,
+				  <&versa3 3>,
+				  <&versa3 4>,
+				  <&versa3 5>;
+		assigned-clock-rates = <24000000>,
+				       <12288000>,
+				       *<11289600>*,
+				       <25000000>,
+				       <100000000>,
+				       <100000000>;
+		renesas,settings = [
+		  80 00 11 19 4c 42 dc 2f 06 7d 20 1a 5f 1e f2 27
+		  00 40 00 00 00 00 00 00 06 0c 19 02 3f f0 90 86
+		  a0 80 30 30 9c
+		];
+	};

Thank you,
Claudiu Beznea

> 
> Cheers,
> Biju
> 
> 
>>
>> Thank you,
>> Claudiu Beznea
>>
>>>
>>> Maybe add audio_clk1, so that it described properly in clock tree??
>>>
>>> Cheers,
>>> Biju
>>>
>>>> +&audio_clk2 {
>>>> +	clock-frequency = <12288000>;
>>>> +	status = "okay";
>>>> +};
>>>> +
>>>>  &i2c0 {
>>>>  	status = "okay";
>>>>
>>>> @@ -94,6 +99,11 @@ da7212: codec@1a {  };
>>>>
>>>>  &pinctrl {
>>>> +	audio_clock_pins: audio-clock {
>>>> +		pins = "AUDIO_CLK1", "AUDIO_CLK2";
>>>> +		input-enable;
>>>> +	};
>>>> +
>>>>  	key-1-gpio-hog {
>>>>  		gpio-hog;
>>>>  		gpios = <RZG2L_GPIO(18, 0) GPIO_ACTIVE_LOW>; @@ -151,6 +161,13 @@ cd {
>>>>  			pinmux = <RZG2L_PORT_PINMUX(0, 2, 1)>; /* SD1_CD */
>>>>  		};
>>>>  	};
>>>> +
>>>> +	ssi3_pins: ssi3 {
>>>> +		pinmux = <RZG2L_PORT_PINMUX(18, 2, 8)>, /* BCK */
>>>> +			 <RZG2L_PORT_PINMUX(18, 3, 8)>, /* RCK */
>>>> +			 <RZG2L_PORT_PINMUX(18, 4, 8)>, /* TXD */
>>>> +			 <RZG2L_PORT_PINMUX(18, 5, 8)>; /* RXD */
>>>> +	};
>>>>  };
>>>>
>>>>  &scif0 {
>>>> @@ -171,3 +188,12 @@ &sdhi1 {
>>>>  	max-frequency = <125000000>;
>>>>  	status = "okay";
>>>>  };
>>>> +
>>>> +&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";
>>>> +};
>>>> --
>>>> 2.39.2
>>>