Re: [PATCH 1/8] dt-bindings: embedded-controller: qcom,hamoa-crd-ec: Add qcom,tsens

[Anvesh Jain P <anvesh.p@oss.qualcomm.com>]

On 7/28/2026 11:24 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The custom `qcom,tsens` property encodes a driver-specific software algorithm into Devicetree and redefines standard phandle specifier semantics to avoid standard sensor enumeration.
> --
> 
> --- Patch [1]: [PATCH 1/8] dt-bindings: embedded-controller: qcom,hamoa-crd-ec: Add qcom,tsens ---
> commit d7c232898faf92bef60389e6c46fc016182bd22a
> Author: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
> 
> dt-bindings: embedded-controller: qcom,hamoa-crd-ec: Add qcom,tsens
> 
> This commit adds the qcom,tsens property to the Hamoa-based boards embedded
> controller binding. It pairs tsens provider phandles with a count of leading
> sensor IDs, whose readings are averaged to compute the SoC junction
> temperature.
> 
>> diff --git a/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-crd-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-crd-ec.yaml
>> --- a/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-crd-ec.yaml
>> +++ b/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-crd-ec.yaml
> [ ... ]
>> @@ -32,11 +32,40 @@ properties:
>>    interrupts:
>>      maxItems: 1
>>  
>> +  # A generic "thermal-sensors" phandle only identifies individual sensor
>> +  # IDs, and thermal zones are a Linux-side grouping of those sensors, not
>> +  # a devicetree concept the EC cares about. What the driver actually needs
>> +  # is raw readings from a contiguous range of leading sensor IDs on each
>> +  # tsens provider, averaged together, so qcom,tsens pairs each provider
>> +  # phandle with a count instead of enumerating every sensor cell.
>> +  qcom,tsens:
>> +    description:
>> +      List of tsens providers and, for each, the number of leading sensor
>> +      IDs on that provider whose readings are averaged to compute the SoC
>> +      junction temperature reported to the EC for fan control. Only
>> +      applicable for Hamoa based devices.
> 
> [Severity: Medium]
> Does this property encode a driver-specific software algorithm into the
> device tree? Device tree is strictly designed to describe hardware topology,
> not software policies like averaging sensor readings.
> 
> By bypassing standard properties like thermal-sensors and redefining the
> phandle specifier semantics to represent a count rather than enumerating the
> specific hardware sensor IDs, does this create an incorrectly designed ABI?
> It might be better to rely on standard hardware enumeration and leave the
> averaging policy to the software drivers.
> [ ... ]
>

Ack, I'll drop qcom,tsens and switch to standard thermal-sensors, moving
the averaging logic into the driver where it belongs.

-- 
Best Regards,
Anvesh