Re: [PATCH] arm64: dts: rockchip: fix probe of analog sound card on rock-3a

[Michael Riesch <michael.riesch@wolfvision.net>]

Hi Nicolas,

On 1/16/23 16:14, Nicolas Frattaroli wrote:
> On Monday, 16 January 2023 09:41:50 CET Michael Riesch wrote:
>> Hi Jonas,
>>
>> On 1/15/23 22:15, Jonas Karlman wrote:
>>> The following was observed on my Radxa ROCK 3 Model A board:
>>>   rockchip-pinctrl pinctrl: pin gpio1-9 already requested by
>>>   vcc-cam-regulator; cannot claim for fe410000.i2s ...
>>>   platform rk809-sound: deferred probe pending
>>>
>>> Fix this by supplying a board specific pinctrl with the i2s1 pins used
>>> by pmic codec according to the schematic [1].
>>>
>>> [1] https://dl.radxa.com/rock3/docs/hw/3a/ROCK-3A-V1.3-SCH.pdf
>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>
>> Makes sense to me, but...
>>
>>> ---
>>>
>>>  arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
>>> b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts index
>>> 00d873a03cfe..a149c8b83f94 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
>>> @@ -573,6 +573,8 @@ &i2s0_8ch {
>>>
>>>  };
>>>  
>>>  &i2s1_8ch {
>>>
>>> +	pinctrl-names = "default";
>>> +	pinctrl-0 = <&i2s1m0_sclktx &i2s1m0_lrcktx &i2s1m0_sdi0 
> &i2s1m0_sdo0>;
>>
>> ... shouldn't this include i2s1m0_mclk as well?
> 
> You can totally use i2s without an mclk, but I don't have a specific
> explanation as to why this is lacking in this particular pinctrl, as
> I cargo-culted it from downstream.
> 
>> For some reason this has been omitted in the pinctrl defined in
>> rk356x.dtsi. But then rk356x.dtsi also claims
>>  - both i2s1m0_sdo1 and i2s1m0_sdi3
>>  - both i2s1m0_sdo2 and i2s1m0_sdi2
>>  - both i2s1m0_sdo3 and i2s1m0_sdi1
>> which are mapped to the same respective pins. Therefore it seems that
>> there might be something wrong with this pinctrl altogether.
> 
> This is actually correct, I believe. The driver has a feature called
> "io multiplex" which fiddles with the GRF to dynamically set the pin
> directions depending on the hardware parameters. As it happens, for
> example sdo1 and sdi3 are on the same pin mux, and can be switched
> between like this.
> 
> I don't know how well that meshes with upstream's understanding of
> pinctrl but in this case the two functions really can use the same
> pin.

Ah, fancy stuff! Thanks for the clarification!

Best regards,
Michael

> 
>>
>> Cc: Nicolas Frattaroli, maybe he can provide some clarification.
>>
>> Best regards,
>> Michael
>>
>>>  	rockchip,trcm-sync-tx-only;
>>>  	status = "okay";
>>>  
>>>  };
> 
> Cheers,
> Nicolas Frattaroli
> 
> 
> 
>