Re: [PATCH v2 3/3] thermal: qcom: add PM8901 PMIC temperature-alarm driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The driver disables the PMIC's hardware thermal auto-shutdown during probe but does not implement a `.shutdown` callback to restore it, leaving the device physically unprotected across warm reboots.
- [High] The driver overrides the PMIC's hardware thermal shutdown but fails to configure the thermal interrupts as system wakeup sources or restore the hardware shutdown during suspend, leaving the device unprotected while suspended.
--

commit ea0822f7dd85ca701edd835f63cf382da86b80eb
Author: Herman van Hazendonk <github.com@herrie.org>

thermal: qcom: add PM8901 PMIC temperature-alarm driver

Add a thermal-of sensor driver for the temperature-alarm block inside
the Qualcomm PM8901 PMIC. It exposes an over-temperature alarm at SSBI
offset 0x23/0x24 with three escalating stages.

> diff --git a/drivers/thermal/qcom/qcom-pm8901-tm.c b/drivers/thermal/qcom/qcom-pm8901-tm.c
> new file mode 100644
> index 0000000000000..b159e9917c2c9
> --- /dev/null
> +++ b/drivers/thermal/qcom/qcom-pm8901-tm.c

[ ... ]

> +	chip->tz_dev = devm_thermal_of_zone_register(&pdev->dev, 0, chip,
> +						     &pm8901_tm_zone_ops);
> +	if (IS_ERR(chip->tz_dev))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(chip->tz_dev),
> +				     "thermal zone register failed\n");
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL,
> +					pm8901_tm_isr, IRQF_ONESHOT,
> +					"pm8901-tm-alarm", chip);

[Severity: High]
Since the PMIC's hardware thermal shutdown is overridden by the software
driver during probe, should these thermal interrupts be configured as system
wakeup sources?

During system suspend, non-wakeup interrupts are disabled and the software
thermal core sleeps. Without wakeup enabled, could the device overheat while
suspended without the hardware or OS acting on it to prevent damage?

[ ... ]

> +static const struct of_device_id pm8901_tm_match_table[] = {
> +	{ .compatible = "qcom,pm8901-temp-alarm" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, pm8901_tm_match_table);
> +
> +static struct platform_driver pm8901_tm_driver = {
> +	.driver = {
> +		.name		= "pm8901-temp-alarm",
> +		.of_match_table	= pm8901_tm_match_table,
> +	},
> +	.probe	= pm8901_tm_probe,
> +};

[Severity: High]
Is a .shutdown callback needed in pm8901_tm_driver to restore the PMIC's
hardware thermal auto-shutdown?

While pm8901_tm_enable_sw_override() registers a devm action to restore it
on unbind, devm actions are not executed during the system shutdown or reboot
path in device_shutdown().

Could this leave the device in software-override mode across a warm reboot,
where it would be physically unprotected if it overheats during the bootloader
phase before the OS boots?

Additionally, does this driver need suspend and resume PM callbacks in
dev_pm_ops to temporarily re-enable the hardware shutdown while the software
thermal core is asleep?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1780197411.git.github.com@herrie.org?part=3