diff mbox series

arm64: dts: rockchip: Fix broken tsadc pinctrl binding for rk3588

Message ID 20250124052611.3705-1-eagle.alexander923@gmail.com (mailing list archive)
State New
Headers show
Series arm64: dts: rockchip: Fix broken tsadc pinctrl binding for rk3588 | expand

Commit Message

Alexander Shiyan Jan. 24, 2025, 5:26 a.m. UTC
There is no pinctrl "gpio" and "otpout" (probably designed as "output")
handling in the tsadc driver.
Let's use proper binding "default" and "sleep".

Fixes: 32641b8ab1a5 ("arm64: dts: rockchip: add rk3588 thermal sensor")
Cc: stable@vger.kernel.org
Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Alexey Charkov Jan. 24, 2025, 8:33 a.m. UTC | #1
Hi Alexander,

On Fri, Jan 24, 2025 at 9:26 AM Alexander Shiyan
<eagle.alexander923@gmail.com> 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?

Best regards,
Alexey
Dragan Simic Jan. 24, 2025, 10:06 a.m. UTC | #2
Hello Alexey,

On 2025-01-24 09:33, Alexey Charkov wrote:
> On Fri, Jan 24, 2025 at 9:26 AM Alexander Shiyan
> <eagle.alexander923@gmail.com> 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);
+
         for (i = 0; i < thermal->chip->chn_num; i++) {
                 error = rockchip_thermal_register_sensor(pdev, thermal,
                                                 &thermal->sensors[i],

If you could test it, please, it would be great, and I'd prepare the
proper patch tomorrow or so.
Alexey Charkov Jan. 24, 2025, 10:25 a.m. UTC | #3
On Fri, Jan 24, 2025 at 2:06 PM Dragan Simic <dsimic@manjaro.org> 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@gmail.com> 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
Dragan Simic Jan. 24, 2025, 10:37 a.m. UTC | #4
On 2025-01-24 11:25, Alexey Charkov wrote:
> On Fri, Jan 24, 2025 at 2:06 PM Dragan Simic <dsimic@manjaro.org> 
> wrote:
>> On 2025-01-24 09:33, Alexey Charkov wrote:
>> > On Fri, Jan 24, 2025 at 9:26 AM Alexander Shiyan
>> > <eagle.alexander923@gmail.com> 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.

You're right, but the "else block" is what the downstream driver does,
so I think it's better to simply stay on the safe side and follow that
logic in the upstream driver.  Is it really needed?  Perhaps not, but
it also shouldn't hurt.

> Will try and revert.

Awesome, thanks!

> 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.

Indeed, "otpout" wasn't a typo, and I've already addressed that in my
comments to Alexander's patch.  Will send that response a bit later.

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).
Dragan Simic Jan. 24, 2025, 10:38 a.m. UTC | #5
Hello Alexander,

On 2025-01-24 06:26, Alexander Shiyan 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".
> 
> Fixes: 32641b8ab1a5 ("arm64: dts: rockchip: add rk3588 thermal sensor")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> index a337f3fb8377..f141065eb69d 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> @@ -2667,9 +2667,9 @@ tsadc: tsadc@fec00000 {
>  		rockchip,hw-tshut-temp = <120000>;
>  		rockchip,hw-tshut-mode = <0>; /* tshut mode 0:CRU 1:GPIO */
>  		rockchip,hw-tshut-polarity = <0>; /* tshut polarity 0:LOW 1:HIGH */
> -		pinctrl-0 = <&tsadc_gpio_func>;
> -		pinctrl-1 = <&tsadc_shut>;
> -		pinctrl-names = "gpio", "otpout";
> +		pinctrl-0 = <&tsadc_shut>;
> +		pinctrl-1 = <&tsadc_gpio_func>;
> +		pinctrl-names = "default", "sleep";
>  		#thermal-sensor-cells = <1>;
>  		status = "disabled";
>  	};

Thanks for the patch, it's looking good to me.  The old values
for the pinctrl names are leftovers back from the import of the
downstream kernel code, while the new values follow the expected
pinctrl naming scheme.  The resulting behavior follows, almost
entirely, the behavior found in the downstream kernel code.

Actually, there's some rather critical discrepancy between the
upstream TSADC driver and it's downstream cousin, as already
described in earlier responses from Alexey and me.  However,
those issues have to be addressed in a separate patch, while
this patch, to me, remains fine on its own.

My only suggestions would be to adjust both the patch summary
and the description not to use word "binding", because that
technically isn't fixed here, but to use "pinctrl names" instead.
Also, please note that the downstream kernel uses "otpout" as
a pinctrl name, [1] so the assumption about "output" in the
patch description should be removed.

With the suggestions from above addressed in the v2, please feel
free to include my

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

[1] 
https://raw.githubusercontent.com/rockchip-linux/kernel/refs/heads/develop-5.10/drivers/thermal/rockchip_thermal.c
Dragan Simic Jan. 24, 2025, 10:45 a.m. UTC | #6
On 2025-01-24 11:37, Dragan Simic wrote:
> On 2025-01-24 11:25, Alexey Charkov wrote:
>> On Fri, Jan 24, 2025 at 2:06 PM Dragan Simic <dsimic@manjaro.org> 
>> wrote:
>>> On 2025-01-24 09:33, Alexey Charkov wrote:
>>> > On Fri, Jan 24, 2025 at 9:26 AM Alexander Shiyan
>>> > <eagle.alexander923@gmail.com> 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.
> 
> You're right, but the "else block" is what the downstream driver does,
> so I think it's better to simply stay on the safe side and follow that
> logic in the upstream driver.  Is it really needed?  Perhaps not, but
> it also shouldn't hurt.
> 
>> Will try and revert.
> 
> Awesome, thanks!

Actually...  Revert or report? :)

>> 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.
> 
> Indeed, "otpout" wasn't a typo, and I've already addressed that in my
> comments to Alexander's patch.  Will send that response a bit later.
> 
> 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).
Alexey Charkov Jan. 24, 2025, 5:23 p.m. UTC | #7
On Fri, Jan 24, 2025 at 2:37 PM Dragan Simic <dsimic@manjaro.org> wrote:
>
> On 2025-01-24 11:25, Alexey Charkov wrote:
> > On Fri, Jan 24, 2025 at 2:06 PM Dragan Simic <dsimic@manjaro.org>
> > wrote:
> >> On 2025-01-24 09:33, Alexey Charkov wrote:
> >> > On Fri, Jan 24, 2025 at 9:26 AM Alexander Shiyan
> >> > <eagle.alexander923@gmail.com> 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.
>
> You're right, but the "else block" is what the downstream driver does,

Does it though? It only handles the TSHUT_MODE_GPIO case as far as I
can tell (or TSHUT_MODE_OTP in downstream driver lingo) [1]

[1] https://github.com/radxa/kernel/blob/edb3eeeaa4643ecac6f4185d6d391c574735fca1/drivers/thermal/rockchip_thermal.c#L2564

> so I think it's better to simply stay on the safe side and follow that
> logic in the upstream driver.  Is it really needed?  Perhaps not, but
> it also shouldn't hurt.
>
> > Will try and revert.
>
> Awesome, thanks!
>
> > 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.
>
> Indeed, "otpout" wasn't a typo, and I've already addressed that in my
> comments to Alexander's patch.  Will send that response a bit later.
>
> 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.

Best regards,
Alexey

rock-5c ~ # cat /sys/kernel/debug/pinctrl/pinctrl-handles
Requested pin control handlers their pinmux maps:
[...]
device: fec00000.tsadc current state: default
 state: default
   type: MUX_GROUP controller rockchip-pinctrl group: tsadc-shut (84)
function: tsadc (84)
   type: CONFIGS_PIN controller rockchip-pinctrl pin gpio0-1 (1)config 00000001
 state: sleep
   type: MUX_GROUP controller rockchip-pinctrl group: tsadc-gpio-func
(95) function: gpio-func (97)
   type: CONFIGS_PIN controller rockchip-pinctrl pin gpio0-1 (1)config 00000001
[...]

rock-5c ~ # gpioinfo 0
gpiochip0 - 32 lines:
       line   0:      unnamed "regulator-vdd-3v3" output active-high [used]
       line   1:      unnamed       unused   input  active-high
       line   2:      unnamed       unused   input  active-high
       line   3:      unnamed       unused   input  active-high
       line   4:      unnamed       unused   input  active-high
       line   5:      unnamed       unused   input  active-high
       line   6:      unnamed       unused   input  active-high
       line   7:      unnamed       unused   input  active-high
       line   8:      unnamed       unused   input  active-high
       line   9:      unnamed       unused   input  active-high
       line  10:      unnamed       unused   input  active-high
       line  11:      unnamed       unused   input  active-high
       line  12:      unnamed       unused   input  active-high
       line  13:      unnamed       unused   input  active-high
       line  14:      unnamed       unused   input  active-high
       line  15:      unnamed       unused   input  active-high
       line  16:      unnamed       unused   input  active-high
       line  17:      unnamed       unused   input  active-high
       line  18:      unnamed       unused   input  active-high
       line  19:      unnamed       unused   input  active-high
       line  20:      unnamed       unused   input  active-high
       line  21:      unnamed "regulator-pcie2x1l2-3v3" output
active-high [used]
       line  22:      unnamed       unused   input  active-high
       line  23:      unnamed       unused   input  active-high
       line  24:      unnamed       unused   input  active-high
       line  25:      unnamed       unused   input  active-high
       line  26:      unnamed       unused   input  active-high
       line  27:      unnamed       unused   input  active-high
       line  28:      unnamed "regulator-vcc5v0-usb-otg0" output
active-high [used]
       line  29:      unnamed       unused   input  active-high
       line  30:      unnamed       unused   input  active-high
       line  31:      unnamed       unused   input  active-high

(note line 1: unused above)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts
b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts
index 6e56d7704cbe..e8c4d9b3f828 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts
@@ -873,6 +873,9 @@ regulator-state-mem {
};

&tsadc {
+       rockchip,hw-tshut-temp = <65000>;
+       rockchip,hw-tshut-mode = <1>; /* tshut mode 0:CRU 1:GPIO */
+       rockchip,hw-tshut-polarity = <0>; /* tshut polarity 0:LOW 1:HIGH */
       status = "okay";
};

diff --git a/drivers/thermal/rockchip_thermal.c
b/drivers/thermal/rockchip_thermal.c
index f551df48eef9..4f474906b2b0 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -1568,6 +1568,9 @@ 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(&pdev->dev);
+
       for (i = 0; i < thermal->chip->chn_num; i++) {
               error = rockchip_thermal_register_sensor(pdev, thermal,
                                               &thermal->sensors[i],
@@ -1614,6 +1617,9 @@ static void rockchip_thermal_remove(struct
platform_device *pdev)
       }

       thermal->chip->control(thermal->regs, false);
+
+       if (thermal->tshut_mode == TSHUT_MODE_GPIO)
+               pinctrl_pm_select_sleep_state(&pdev->dev);
}

static int __maybe_unused rockchip_thermal_suspend(struct device *dev)
@@ -1629,7 +1635,8 @@ static int __maybe_unused
rockchip_thermal_suspend(struct device *dev)
       clk_disable(thermal->pclk);
       clk_disable(thermal->clk);

-       pinctrl_pm_select_sleep_state(dev);
+       if (thermal->tshut_mode == TSHUT_MODE_GPIO)
+               pinctrl_pm_select_sleep_state(dev);

       return 0;
}
@@ -1674,7 +1681,8 @@ static int __maybe_unused
rockchip_thermal_resume(struct device *dev)
       for (i = 0; i < thermal->chip->chn_num; i++)
               rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);

-       pinctrl_pm_select_default_state(dev);
+       if (thermal->tshut_mode == TSHUT_MODE_GPIO)
+               pinctrl_pm_select_default_state(dev);

       return 0;
}
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index a337f3fb8377..f141065eb69d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -2667,9 +2667,9 @@  tsadc: tsadc@fec00000 {
 		rockchip,hw-tshut-temp = <120000>;
 		rockchip,hw-tshut-mode = <0>; /* tshut mode 0:CRU 1:GPIO */
 		rockchip,hw-tshut-polarity = <0>; /* tshut polarity 0:LOW 1:HIGH */
-		pinctrl-0 = <&tsadc_gpio_func>;
-		pinctrl-1 = <&tsadc_shut>;
-		pinctrl-names = "gpio", "otpout";
+		pinctrl-0 = <&tsadc_shut>;
+		pinctrl-1 = <&tsadc_gpio_func>;
+		pinctrl-names = "default", "sleep";
 		#thermal-sensor-cells = <1>;
 		status = "disabled";
 	};