Re: [PATCH v2 4/4] arm64: dts: qcom: hamoa-pmics: Add ADC support

[Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>]

On 6/14/2026 1:47 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Missing Thermal Sensor Provider for VADC causing thermal zones to indefinitely defer
> - [Low] Devicetree Naming Violation: Underscore in node name
> --
> 
> commit 7b5021a0220835eeee60f0b85faf679ed9b09977
> Author: Ayyagari Ushasreevalli <aushasre@qti.qualcomm.com>
> 
> arm64: dts: qcom: hamoa-pmics: Add ADC support
> 
> This commit adds the ADC node and defines channels for die temperature,
> VPH power, and system thermistors for PMK8550, PM8550VE*, and PMC8380*
> PMICs. It also defines several thermal zones corresponding to the off-PMIC
> system thermistors.
> 
>> diff --git a/arch/arm64/boot/dts/qcom/hamoa-pmics.dtsi b/arch/arm64/boot/dts/qcom/hamoa-pmics.dtsi
>> index 6a31a0adf8be4..2e746ede850f8 100644
>> --- a/arch/arm64/boot/dts/qcom/hamoa-pmics.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/hamoa-pmics.dtsi
> [ ... ]
>> @@ -189,6 +191,90 @@ trip1 {
>>  				};
>>  			};
>>  		};
>> +
>> +		sys-0-thermal {
>> +			polling-delay-passive = <0>;
>> +			thermal-sensors = <&pmk8550_vadc ADC5_GEN3_AMUX1_GPIO_100K_PU(1)>;
> [ ... ]
>> @@ -277,6 +363,142 @@ pmk8550_pwm: pwm {
>>  
>>  			status = "disabled";
>>  		};
>> +
>> +		pmk8550_vadc: adc@9000 {
>> +			compatible = "qcom,spmi-adc5-gen3";
>> +			reg = <0x9000>, <0x9100>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			interrupts = <0x0 0x90 0x1 IRQ_TYPE_EDGE_RISING>,
>> +				     <0x0 0x91 0x1 IRQ_TYPE_EDGE_RISING>;
>> +			#thermal-sensor-cells = <1>;
> 
> [Severity: Medium]
> Does the driver for pmk8550_vadc (qcom-spmi-adc5-gen3.c) actually register a
> thermal zone provider?
> 
> It looks like the driver currently registers IIO channels but delegates
> thermal monitor capabilities to an auxiliary device named adc5_tm_gen3,
> which currently has no driver in the kernel to bind to it.
> 
> Without a driver registering the thermal zones via
> devm_thermal_of_zone_register(), will the generic thermal framework
> indefinitely defer probing for these thermal zones, leaving the system
> thermistor monitoring uninitialized?

It is true that the auxiliary driver is not yet present in the code, so
the thermistor monitoring functionality would not work yet. The main
ADC driver functionality should still work.

I have this patch series for adding the auxiliary driver:
https://lore.kernel.org/all/20260705-gen3_adc_tm-v3-0-ac62f387dbce@oss.qualcomm.com/.
I can make this DT series dependent on that one when pushing the next version.


> 
> [ ... ]
>> @@ -301,6 +525,18 @@ pm8550_gpios: gpio@8800 {
>>  			#gpio-cells = <2>;
>>  			interrupt-controller;
>>  			#interrupt-cells = <2>;
>> +
>> +			sys_therm_0_gpio3: sys-therm_0-gpio3-state {
> 
> [Severity: Low]
> This isn't a bug, but does this node name violate devicetree conventions?
> 
> Devicetree specifications strongly discourage underscores in node names,
> recommending hyphens instead. While the node label (sys_therm_0_gpio3)
> can use underscores, should the node name be sys-therm-0-gpio3-state?

I'll update the names to use hyphens instead of underscores.

Thanks,
Jishnu

> 
> The sibling node sys-therm-1-gpio4-state correctly uses hyphens. Leaving
> the underscore might trigger a node_name_chars warning in dtc when compiled
> with W=1.
> 
>> +				pins = "gpio3";
>> +				function = PMIC_GPIO_FUNC_NORMAL;
>> +				bias-high-impedance;
>> +			};
>