On Fri, Jan 24, 2025 at 2:06 PM Dragan Simic <dsimic@xxxxxxxxxxx> wrote: > > Hello Alexey, > > On 2025-01-24 09:33, Alexey Charkov wrote: > > On Fri, Jan 24, 2025 at 9:26 AM Alexander Shiyan > > <eagle.alexander923@xxxxxxxxx> wrote: > >> > >> There is no pinctrl "gpio" and "otpout" (probably designed as > >> "output") > >> handling in the tsadc driver. > >> Let's use proper binding "default" and "sleep". > > > > This looks reasonable, however I've tried it on my Radxa Rock 5C and > > the driver still doesn't claim GPIO0 RK_PA1 even with this change. As > > a result, a simulated thermal runaway condition (I've changed the > > tshut temperature to 65000 and tshut mode to 1) doesn't trigger a PMIC > > reset, even though a direct `gpioset 0 1=0` does. > > > > Are any additional changes needed to the driver itself? > > I've been digging through this patch the whole TSADC/OTP thing in the > last couple of hours, and AFAIK some parts of the upstream driver are > still missing, in comparison with the downstream driver. > > I've got some small suggestions for the patch itself, but the issue > you observed is obviously of higher priority, and I've singled it out > as well while digging through the code. > > Could you, please, try the patch below quickly, to see is it going to > fix the issue you observed? I've got some "IRL stuff" to take care of > today, so I can't test it myself, and it would be great to know is it > the right path to the proper fix. > > diff --git i/drivers/thermal/rockchip_thermal.c > w/drivers/thermal/rockchip_thermal.c > index f551df48eef9..62f0e14a8d98 100644 > --- i/drivers/thermal/rockchip_thermal.c > +++ w/drivers/thermal/rockchip_thermal.c > @@ -1568,6 +1568,11 @@ static int rockchip_thermal_probe(struct > platform_device *pdev) > thermal->chip->initialize(thermal->grf, thermal->regs, > thermal->tshut_polarity); > > + if (thermal->tshut_mode == TSHUT_MODE_GPIO) > + pinctrl_select_default_state(dev); > + else > + pinctrl_select_sleep_state(dev); I believe no 'else' block is needed here, because if tshut_mode is not TSHUT_MODE_GPIO then the TSADC doesn't use this pin at all, so there's no reason for the driver to mess with its pinctrl state. I'd rather put a mirroring block to put the pin back to its 'sleep' state in the removal function for the TSHUT_MODE_GPIO case. Will try and revert. P.S. Just looked at the downstream driver, and it actually calls TSHUT_MODE_GPIO TSHUT_MODE_OTP instead, so it seems that "otpout" was not a typo in the first place. So maybe the right approach here is not to change the device tree but rather fix the "gpio" / "otpout" pinctrl state handling in the driver. Best, Alexey
