Re: [PATCH] arm64: dts: rockchip: Fix broken tsadc pinctrl binding for rk3588

[Dragan Simic <dsimic@manjaro.org>]

Hello Alexey,

On 2025-01-28 11:30, Alexey Charkov wrote:
> On Tue, Jan 28, 2025 at 1:24 PM Dragan Simic <dsimic@manjaro.org> 
> wrote:
>> On 2025-01-26 15:25, Alexander Shiyan wrote:
>> >> > > I think it's actually better to accept the approach in Alexander's
>> >> > > patch, because the whole thing applies to other Rockchip SoCs as well,
>> >> > > not just to the RK3588(S).
>> >> >
>> >> > Anyway, I've just tried it after including the changes below, and
>> >> > while /sys/kernel/debug/pinctrl/pinctrl-handles shows the expected
>> >> > pinctrls under tsadc, the driver still doesn't seem to be triggering a
>> >> > PMIC reset. Weird. Any thoughts welcome.
>> >>
>> >> I found the culprit. "otpout" (or "default" if we follow Alexander's
>> >> suggested approach) pinctrl state should refer to the &tsadc_shut_org
>> >> config instead of &tsadc_shut - then the PMIC reset works.
>> >
>> > Great, I'll use this in v2.
>> 
>> Please, let's wait with the v2 until I go through the whole thing 
>> again
> 
> I, for one, would welcome a v2 that could be tested and confirmed
> working with and without driver changes. Especially given that:
>  - the changes are pretty small
>  - hardware docs say nothing about the difference between TSADC_SHUT
> vs. TSADC_SHUT_ORG, except that one is config #2 and the other is
> config #1
>  - none of the source trees I looked at seem to enable PMIC based
> resets on any RK3588-based boards, so these pinctrl configs appear to
> have never been tested in the wild for RK3588*
> 
> So trying and testing seems to be the only way to understand the best
> way forward. Unless, of course, someone from Rockchip can comment on
> how the hardware works with TSADC_SHUT vs. TSADC_SHUT_ORG.

Perhaps the best approach would be to have the v2 that works without
the driver changes, so it can be propagated into the stable kernels.

Then, a separate series, which I'll volunteer for, :) would introduce
the cleanups and any needed driver changes, which wouldn't be propagated
into the stable kernels.  That way, we'd have the minimal bugfixes in
stable kernels, and the nice cleanups in the latest kernel version.

Of course, detailed testing is mandatory.

>> which I expected to have done already, but had some other "IRL stuff"
>> that introduced a delay.