Re: [PATCH v2] arm64: dts: qcom: Add PMIC thermal support for Agatti QRB2210-RB1 platform

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] arm64: dts: qcom: missing polling-delay on qrb2210-rb1 ADC thermal zones
--

commit 38fb45777755ec6c570eb782acfb19540ef209d8
Author: Raj Aryan <raj.aryan@oss.qualcomm.com>

arm64: dts: qcom: Add PMIC thermal support for Agatti QRB2210-RB1 platform

This commit adds PM4125 ADC channels, generic-adc-thermal bridge nodes, and
thermal zones for the msm, quiet, and xo thermistors on the Agatti
QRB2210-RB1 platform.

> diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
> index bf6fb12ad9906..17bc2b984c75f 100644
> --- a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
> +++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts

[ ... ]

> @@ -33,6 +34,68 @@ clk40m: can-clk {
>  		};
>  	};
>  
> +	msm_therm_bridge: msm-therm-bridge {
> +		compatible = "generic-adc-thermal";
> +		io-channels = <&pm4125_adc ADC5_AMUX_THM3_100K_PU>;
> +		io-channel-names = "sensor-channel";
> +		#thermal-sensor-cells = <0>;
> +	};
> +
> +	quiet_therm_bridge: quiet-therm-bridge {
> +		compatible = "generic-adc-thermal";
> +		io-channels = <&pm4125_adc ADC5_AMUX_THM2_100K_PU>;
> +		io-channel-names = "sensor-channel";
> +		#thermal-sensor-cells = <0>;
> +	};
> +
> +	xo_therm_bridge: xo-therm-bridge {
> +		compatible = "generic-adc-thermal";
> +		io-channels = <&pm4125_adc ADC5_XO_THERM_100K_PU>;
> +		io-channel-names = "sensor-channel";
> +		#thermal-sensor-cells = <0>;
> +	};
> +
> +	thermal-zones {
> +		msm-thermal {
> +			polling-delay-passive = <2000>;
> +			thermal-sensors = <&msm_therm_bridge>;
> +
> +			trips {
> +				active-config0 {
> +					temperature = <80000>;
> +					hysteresis = <2000>;
> +					type = "hot";
> +				};
> +			};
> +		};
> +
> +		quiet-thermal {
> +			polling-delay-passive = <2000>;
> +			thermal-sensors = <&quiet_therm_bridge>;
> +
> +			trips {
> +				active-config0 {
> +					temperature = <80000>;
> +					hysteresis = <2000>;
> +					type = "hot";
> +				};
> +			};
> +		};
> +
> +		xo-thermal {
> +			polling-delay-passive = <2000>;
> +			thermal-sensors = <&xo_therm_bridge>;
> +
> +			trips {
> +				active-config0 {
> +					temperature = <80000>;
> +					hysteresis = <2000>;
> +					type = "hot";
> +				};
> +			};
> +		};
> +	};

[Severity: High]
Will the msm-thermal, quiet-thermal, and xo-thermal zones ever be polled at
runtime without a polling-delay property?

Each zone is bound to a generic-adc-thermal bridge node, and gadc_thermal_ops
in drivers/thermal/thermal-generic-adc.c only provides .get_temp without
.set_trips or interrupt-driven trip support:

drivers/thermal/thermal-generic-adc.c:
static const struct thermal_zone_device_ops gadc_thermal_ops = {
	.get_temp = gadc_thermal_get_temp,
};

When thermal_of_zone_register() calls thermal_of_monitor_init() in
drivers/thermal/thermal_of.c, omitting polling-delay sets *delay to 0
(leaving tz->polling_delay_jiffies = 0):

drivers/thermal/thermal_of.c:thermal_of_monitor_init() {
	...
	ret = of_property_read_u32(np, "polling-delay", delay);
	if (ret == -EINVAL) {
		*delay = 0;
	}
	...
}

In monitor_thermal_zone() in drivers/thermal/thermal_core.c, periodic polling
is only scheduled if tz->passive > 0 or tz->polling_delay_jiffies is
non-zero:

drivers/thermal/thermal_core.c:monitor_thermal_zone() {
	if (tz->passive > 0 && tz->passive_delay_jiffies)
		thermal_zone_device_set_polling(tz, tz->passive_delay_jiffies);
	else if (tz->polling_delay_jiffies)
		thermal_zone_device_set_polling(tz, tz->polling_delay_jiffies);
}

Because these three thermal zones only define trips with type = "hot" and no
passive trips, tz->passive remains 0 and polling-delay-passive is never
activated. As a result, after the initial registration check, tz->poll_queue
is never queued and the active-config0 hot trips at 80000 mC cannot be
detected.

Should these thermal zones define polling-delay (or set polling-delay-passive
to <0> and use polling-delay = <2000>) so the ADC channels are periodically
sampled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-pm4125-adc-thermal-v2-1-8f405d9dac96@oss.qualcomm.com?part=1