Re: [PATCH 1/8] dt-bindings: embedded-controller: qcom,hamoa-crd-ec: Add qcom,tsens
Anvesh Jain P <[email protected]> Wed, 5 Aug 2026 15:24:29 +0530
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree |
|---|---|
| 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