Re: [PATCH] thermal: spacemit: k1: disable hardware on driver remove

["Shuwei Wu" <shuwei.wu@mailbox.org>]

Hi Pei,

On Mon Jul 13, 2026 at 4:55 PM CST, Pei Xiao wrote:
>
>
> 在 2026/7/13 16:40, Troy Mitchell 写道:
>> On Mon Jul 13, 2026 at 1:32 AM PDT, Pei Xiao wrote:
>>> The driver uses devm_request_threaded_irq() but never disables the
>>> hardware interrupt enable, sensor enable, or sensor power bits when
>>> the driver is unbound.
>>>
>>> Add a devm action that clears K1_TSENSOR_INT_EN_REG,
>>> K1_TSENSOR_EN_ALL, and K1_TSENSOR_PCTRL_ENABLE on remove, mirroring
>>> what k1_tsensor_init() sets up.
>>>
>>> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
>>> ---
>>>  drivers/thermal/spacemit/k1_tsensor.c | 27 +++++++++++++++++++++++++++
>>>  1 file changed, 27 insertions(+)
>>>
>>> diff --git a/drivers/thermal/spacemit/k1_tsensor.c b/drivers/thermal/spacemit/k1_tsensor.c
>>> index 79222d233129..cf79dc7d403f 100644
>>> --- a/drivers/thermal/spacemit/k1_tsensor.c
>>> +++ b/drivers/thermal/spacemit/k1_tsensor.c
>>> @@ -199,6 +199,29 @@ static irqreturn_t k1_tsensor_irq_thread(int irq, void *data)
>>>  	return IRQ_HANDLED;
>>>  }
>>>  
>>> +static void k1_tsensor_shutdown(struct k1_tsensor *ts)
>>> +{
>>> +	u32 val;
>>> +
>>> +	/* Disable all interrupts */
>>> +	writel(0xffffffff, ts->base + K1_TSENSOR_INT_EN_REG);
>>> +
>>> +	/* Disable all sensor */
>>> +	val = readl(ts->base + K1_TSENSOR_EN_REG);
>>> +	val &= ~K1_TSENSOR_EN_ALL;
>>> +	writel(val, ts->base + K1_TSENSOR_EN_REG);
>>> +
>>> +	/* Power down the sensor module */
>>> +	val = readl(ts->base + K1_TSENSOR_PCTRL_REG);
>>> +	val &= ~K1_TSENSOR_PCTRL_ENABLE;
>> Why is only the ENABLE bit cleared here?
>> At init time the driver also sets:
>>     val |= K1_TSENSOR_PCTRL_RAW_SEL |
>>     K1_TSENSOR_PCTRL_TEMP_MODE |
>>     K1_TSENSOR_PCTRL_HW_AUTO_MODE |
>>     K1_TSENSOR_PCTRL_ENABLE;
>>
>> Should the other bits be cleared in shutdown as well, or is clearing ENABLE
>> alone?
> Hi Troy,
>  Maybe Wu(author) have someting to say about these regs.
>  Thanks.
>
> Pei.

Thanks for pointing this out.

I agree that the shutdown path should clear all control bits
set by k1_tsensor_init().

The managed cleanup will assert the reset, but the reset operation may fail
and its return value is ignored. Therefore, it is safer to disable these
settings explicitly before releasing the resources.

>
>
>>                             - Troy

-- 
Best regards,
Shuwei Wu