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

Anvesh Jain P <[email protected]>
Newsgroups org.kernel.vger.linux-devicetree,dev.linux.lists.sashiko-reviews
Message-ID <[email protected]>

On 7/28/2026 11:24 PM, [email protected] 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 <[email protected]>
> 
> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.