[PATCH] dt-bindings: hwmon: microchip,emc2305: add vcc-supply and #cooling-cells

[PATCH] dt-bindings: hwmon: microchip,emc2305: add vcc-supply and #cooling-cells

[Vincent Jardin via B4 Relay]

From: Vincent Jardin <vjardin@free.fr>

Both properties are missing, so a board that supplies either fails with
dtbs_check:

  fan-controller@2c (microchip,emc2305): '#cooling-cells', 'vcc-supply'
  do not match any of the regexes: '^fan@[0-4]$', '^pinctrl-[0-9]+$'

The #cooling-cells property belongs to the controller node, which is
where the driver actually registers.

Two cells is what that registration needs: a cooling-maps entry
references the fan as <&emc2305 min max>.

Two cells also documents what the driver supports today. The
pwm-separate path registers one cooling device per PWM against the same
node, and they all get cdev_id 0, so no cell count can tell them apart.
That is a driver issue, noted here but not addressed.

The examples in this binding put that property on the fan@N subnodes
instead, which does not match what the driver does: they are left alone
here rather than changed blind.

vcc-supply describes the chip's own rail, which matters on boards where
the sensor sits behind a switched supply.

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
index 8c2548539d7fd..47640214de100 100644
--- a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
+++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
@@ -44,6 +44,11 @@ properties:
            - 0 (Open-Drain)
            - 1 (Push-Pull)
 
+  vcc-supply: true
+
+  "#cooling-cells":
+    const: 2
+
 patternProperties:
   '^fan@[0-4]$':
     $ref: fan-common.yaml#

---
base-commit: 0a0d1d55dad570724bf8c7ea83409639cfb4be9b
change-id: 20260824-for-upstream-dt-emc2305-vcc-cooling-251655c99ef8

Best regards,
-- 
Vincent Jardin <vjardin@free.fr>

Re: [PATCH] dt-bindings: hwmon: microchip,emc2305: add vcc-supply and #cooling-cells

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The commit message makes materially false claims about the driver's device tree parsing logic to justify leaving the binding examples unchanged.

Pre-existing issues:
- [High] The driver fails to use `hwmon_lock()` or any other locking to serialize accesses between hwmon sysfs and thermal subsystem operations, leading to race conditions and potential I2C corruption.
--

--- Patch [1]: [PATCH] dt-bindings: hwmon: microchip,emc2305: add vcc-supply and #cooling-cells ---
commit 2e59e1a6682ca5d565101743c080415ed986b896
Author: Vincent Jardin <vjardin@free.fr>

dt-bindings: hwmon: microchip,emc2305: add vcc-supply and #cooling-cells

This commit adds the missing vcc-supply and #cooling-cells properties to the
microchip,emc2305 device tree binding to resolve schema validation errors.
It places #cooling-cells at the controller node level.

> The #cooling-cells property belongs to the controller node, which is
> where the driver actually registers.
>
> Two cells is what that registration needs: a cooling-maps entry
> references the fan as <&emc2305 min max>.
>
> Two cells also documents what the driver supports today. The
> pwm-separate path registers one cooling device per PWM against the same
> node, and they all get cdev_id 0, so no cell count can tell them apart.
> That is a driver issue, noted here but not addressed.
>
> The examples in this binding put that property on the fan@N subnodes
> instead, which does not match what the driver does: they are left alone
> here rather than changed blind.

[Severity: Medium]
Is this description accurate regarding where the driver registers?

Looking at emc2305_probe() in drivers/hwmon/emc2305.c, when pwm_childs > 0,
the driver iterates through the subnodes and passes the child subnode directly
to devm_thermal_of_child_cooling_device_register():

drivers/hwmon/emc2305.c:emc2305_probe() {
    ...
    ret = emc2305_set_single_tz(dev, child, ch + 1);
    ...
}

It appears the examples in the binding are actually correct in placing
the #cooling-cells property on the fan@N subnodes when subnodes are present.

Could the commit message be updated to reflect this?

[Severity: High]
This is a pre-existing issue, but does the driver lack locking between
thermal subsystem and sysfs operations?

Looking at drivers/hwmon/emc2305.c, the thermal subsystem callback reads and
modifies shared state and performs I2C operations without acquiring any locks:

drivers/hwmon/emc2305.c:__emc2305_set_cur_state() {
    ...
    data->cdev_data[cdev_idx].cur_state = state;
    if (data->pwm_separate) {
        ret = i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_DRIVE(cdev_idx), val);
    ...
}

Concurrently, the hwmon sysfs callback modifies the hwmon state and triggers
I2C updates:

drivers/hwmon/emc2305.c:emc2305_write() {
    ...
    data->cdev_data[cdev_idx].last_hwmon_state =
        EMC2305_PWM_DUTY2STATE(val, data->max_state,
                               EMC2305_FAN_MAX);
    ...
}

Because the driver uses devm_thermal_of_child_cooling_device_register() for
its thermal device, the hwmon core lock is not automatically held. The hwmon
subsystem API requires drivers to implement their own locking (like hwmon_lock())
for manually registered attributes.

Can this lead to race conditions accessing shared state and potential
interleaving of I2C operations to the same chip?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-for-upstream-dt-emc2305-vcc-cooling-v1-1-cd3b53dfb5d4@free.fr?part=1

Re: [PATCH] dt-bindings: hwmon: microchip,emc2305: add vcc-supply and #cooling-cells

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2333 bytes --]

On Mon, Aug 24, 2026 at 05:42:59PM +0200, Vincent Jardin via B4 Relay wrote:
> From: Vincent Jardin <vjardin@free.fr>
> 
> Both properties are missing, so a board that supplies either fails with
> dtbs_check:
> 
>   fan-controller@2c (microchip,emc2305): '#cooling-cells', 'vcc-supply'
>   do not match any of the regexes: '^fan@[0-4]$', '^pinctrl-[0-9]+$'
> 
> The #cooling-cells property belongs to the controller node, which is
> where the driver actually registers.
> 
> Two cells is what that registration needs: a cooling-maps entry
> references the fan as <&emc2305 min max>.
> 
> Two cells also documents what the driver supports today. The
> pwm-separate path registers one cooling device per PWM against the same
> node, and they all get cdev_id 0, so no cell count can tell them apart.
> That is a driver issue, noted here but not addressed.
> 
> The examples in this binding put that property on the fan@N subnodes
> instead, which does not match what the driver does: they are left alone
> here rather than changed blind.

That seems correct though and putting it in the device node when there's 4
fans that could interact with different zones seems wrong. Shouldn't the
driver be fixed instead?

> vcc-supply describes the chip's own rail, which matters on boards where
> the sensor sits behind a switched supply.
> 
> Signed-off-by: Vincent Jardin <vjardin@free.fr>
> ---
>  Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> index 8c2548539d7fd..47640214de100 100644
> --- a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> @@ -44,6 +44,11 @@ properties:
>             - 0 (Open-Drain)
>             - 1 (Push-Pull)
>  
> +  vcc-supply: true
> +
> +  "#cooling-cells":
> +    const: 2
> +
>  patternProperties:
>    '^fan@[0-4]$':
>      $ref: fan-common.yaml#
> 
> ---
> base-commit: 0a0d1d55dad570724bf8c7ea83409639cfb4be9b
> change-id: 20260824-for-upstream-dt-emc2305-vcc-cooling-251655c99ef8
> 
> Best regards,
> -- 
> Vincent Jardin <vjardin@free.fr>
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: hwmon: microchip,emc2305: add vcc-supply and #cooling-cells

[Guenter Roeck]

On 8/24/26 10:00, Conor Dooley wrote:
> On Mon, Aug 24, 2026 at 05:42:59PM +0200, Vincent Jardin via B4 Relay wrote:
>> From: Vincent Jardin <vjardin@free.fr>
>>
>> Both properties are missing, so a board that supplies either fails with
>> dtbs_check:
>>
>>    fan-controller@2c (microchip,emc2305): '#cooling-cells', 'vcc-supply'
>>    do not match any of the regexes: '^fan@[0-4]$', '^pinctrl-[0-9]+$'
>>
>> The #cooling-cells property belongs to the controller node, which is
>> where the driver actually registers.
>>
>> Two cells is what that registration needs: a cooling-maps entry
>> references the fan as <&emc2305 min max>.
>>
>> Two cells also documents what the driver supports today. The
>> pwm-separate path registers one cooling device per PWM against the same
>> node, and they all get cdev_id 0, so no cell count can tell them apart.
>> That is a driver issue, noted here but not addressed.
>>
>> The examples in this binding put that property on the fan@N subnodes
>> instead, which does not match what the driver does: they are left alone
>> here rather than changed blind.
> 
> That seems correct though and putting it in the device node when there's 4
> fans that could interact with different zones seems wrong. Shouldn't the
> driver be fixed instead?
> 

I am a bit (or, rather, more than a bit) concerned about Sashiko's feedback:

New issues:
- [Medium] The commit message makes materially false claims about the driver's
   device tree parsing logic to justify leaving the binding examples unchanged.

Guenter

>> vcc-supply describes the chip's own rail, which matters on boards where
>> the sensor sits behind a switched supply.
>>
>> Signed-off-by: Vincent Jardin <vjardin@free.fr>
>> ---
>>   Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
>> index 8c2548539d7fd..47640214de100 100644
>> --- a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
>> @@ -44,6 +44,11 @@ properties:
>>              - 0 (Open-Drain)
>>              - 1 (Push-Pull)
>>   
>> +  vcc-supply: true
>> +
>> +  "#cooling-cells":
>> +    const: 2
>> +
>>   patternProperties:
>>     '^fan@[0-4]$':
>>       $ref: fan-common.yaml#
>>
>> ---
>> base-commit: 0a0d1d55dad570724bf8c7ea83409639cfb4be9b
>> change-id: 20260824-for-upstream-dt-emc2305-vcc-cooling-251655c99ef8
>>
>> Best regards,
>> -- 
>> Vincent Jardin <vjardin@free.fr>
>>
>>

Re: [PATCH] dt-bindings: hwmon: microchip,emc2305: add vcc-supply and #cooling-cells

[Vincent Jardin]

Hi Guenter,

> I am a bit (or, rather, more than a bit) concerned about Sashiko's feedback:
> 
> New issues:
> - [Medium] The commit message makes materially false claims about the driver's
>   device tree parsing logic to justify leaving the binding examples unchanged.

Sashiko is right on it ! I did miss read the code.

Best regards,
  Vincent

Re: [PATCH] dt-bindings: hwmon: microchip,emc2305: add vcc-supply and #cooling-cells

[Vincent Jardin]

Hi Conor,

> That seems correct though and putting it in the device node when there's 4
> fans that could interact with different zones seems wrong. Shouldn't the
> driver be fixed instead?

Hmm, I did re-read again the code and I am wrong.

I withdraw the patch instead of sending a v2.

Sorry for the noise,
  Vincent