* [PATCH v7 1/3] hwmon: emc2305: Fix fan channel index handling
2026-04-29 6:59 [PATCH v7 0/3] Support configurable fan PWM at shutdown florin.leotescu
@ 2026-04-29 6:59 ` florin.leotescu
2026-04-29 7:48 ` sashiko-bot
2026-04-30 23:13 ` Guenter Roeck
2026-04-29 6:59 ` [PATCH v7 2/3] dt-bindings: hwmon: emc2305: Add fan-shutdown-percent property florin.leotescu
2026-04-29 6:59 ` [PATCH v7 3/3] hwmon: emc2305: Support configurable fan PWM at shutdown florin.leotescu
2 siblings, 2 replies; 13+ messages in thread
From: florin.leotescu @ 2026-04-29 6:59 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Shych, linux-hwmon, devicetree, linux-kernel
Cc: daniel.baluta, viorel.suman, linux-arm-kernel, imx, festevam,
Florin Leotescu
From: Florin Leotescu <florin.leotescu@nxp.com>
The fan channel index is used to access per-channel data structures.
Validate the index against the number of available channels
before use to prevent out-of-bounds access if an invalid
value is provided.
The thermal registration path currently uses a sequential child index,
which may not match the validated channel from DT. Use the DT "reg"
property when registering cooling devices to ensure consistent
channel handling
Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
---
drivers/hwmon/emc2305.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
index 64b213e1451e..f71a0e265924 100644
--- a/drivers/hwmon/emc2305.c
+++ b/drivers/hwmon/emc2305.c
@@ -548,6 +548,12 @@ static int emc2305_of_parse_pwm_child(struct device *dev,
return ret;
}
+ if (ch >= data->pwm_num) {
+ dev_err(dev, "invalid reg %u for node %pOF (valid range 0-%u)\n", ch, child,
+ data->pwm_num - 1);
+ return -EINVAL;
+ }
+
ret = of_parse_phandle_with_args(child, "pwms", "#pwm-cells", 0, &args);
if (ret)
@@ -612,6 +618,7 @@ static int emc2305_probe(struct i2c_client *client)
int ret;
int i;
int pwm_childs;
+ u32 ch;
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
return -ENODEV;
@@ -680,12 +687,18 @@ static int emc2305_probe(struct i2c_client *client)
if (IS_REACHABLE(CONFIG_THERMAL)) {
/* Parse and check for the available PWM child nodes */
if (pwm_childs > 0) {
- i = 0;
for_each_child_of_node_scoped(dev->of_node, child) {
- ret = emc2305_set_single_tz(dev, child, i);
+ ret = of_property_read_u32(child, "reg", &ch);
+ if (ret || ch >= data->pwm_num)
+ continue;
+
+ /*
+ * emc2305_set_single_tz() uses 0 for the common cooling
+ * device and 1..pwm_num for individual fan channels.
+ */
+ ret = emc2305_set_single_tz(dev, child, ch + 1);
if (ret != 0)
return ret;
- i++;
}
} else {
ret = emc2305_set_tz(dev);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v7 1/3] hwmon: emc2305: Fix fan channel index handling
2026-04-29 6:59 ` [PATCH v7 1/3] hwmon: emc2305: Fix fan channel index handling florin.leotescu
@ 2026-04-29 7:48 ` sashiko-bot
2026-04-30 23:13 ` Guenter Roeck
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-04-29 7:48 UTC (permalink / raw)
To: florin.leotescu; +Cc: linux-hwmon
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing hwmon locking serialization between sysfs operations and thermal cooling callbacks.
- [High] Incomplete channel separation causing independent thermal updates to silently alias to channel 0.
- [High] Division by zero in `emc2305_show_fan()` when hardware returns a small tachometer value.
--
commit 3aaf7523e2096af769a25d6788cd08adc306d559
Author: Florin Leotescu <florin.leotescu@nxp.com>
hwmon: emc2305: Fix fan channel index handling
The commit fixes the fan channel index used for accessing per-channel data
structures. It validates the index against the available channels and uses
the DT reg property for thermal registration.
> diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
> index 64b213e1451e..f71a0e265924 100644
> --- a/drivers/hwmon/emc2305.c
> +++ b/drivers/hwmon/emc2305.c
[ ... ]
> @@ -680,12 +687,18 @@ static int emc2305_probe(struct i2c_client *client)
> if (IS_REACHABLE(CONFIG_THERMAL)) {
> /* Parse and check for the available PWM child nodes */
> if (pwm_childs > 0) {
> - i = 0;
> for_each_child_of_node_scoped(dev->of_node, child) {
> - ret = emc2305_set_single_tz(dev, child, i);
> + ret = of_property_read_u32(child, "reg", &ch);
> + if (ret || ch >= data->pwm_num)
> + continue;
> +
> + /*
> + * emc2305_set_single_tz() uses 0 for the common cooling
> + * device and 1..pwm_num for individual fan channels.
> + */
> + ret = emc2305_set_single_tz(dev, child, ch + 1);
Since data->pwm_separate is hardcoded to false for all DT configurations,
doesn't emc2305_get_cdev_idx() unconditionally return 0 at runtime for all
registered cooling devices?
If the thermal subsystem independently updates a specific fan channel, does
this cause the driver to overwrite channel 0's internal state and broadcast
the PWM change to all hardware channels instead of separating them?
> if (ret != 0)
> return ret;
> - i++;
> }
> } else {
> ret = emc2305_set_tz(dev);
I also noticed a few other issues in this file that are not introduced by
this patch, but might be worth addressing.
Is there missing locking between hwmon sysfs operations and thermal cooling
callbacks? Both emc2305_write() and emc2305_set_cur_state() can concurrently
modify shared state and issue I2C writes:
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);
...
}
Does the absence of a mutex here create a data race that can corrupt the
driver's internal state?
Additionally, is there a potential division by zero in emc2305_show_fan()?
drivers/hwmon/emc2305.c:emc2305_show_fan() {
...
ret = i2c_smbus_read_word_swapped(client, EMC2305_REG_FAN_TACH(channel));
if (ret <= 0)
return ret;
ret = ret >> EMC2305_TACH_REGS_UNUSE_BITS;
ret = EMC2305_RPM_FACTOR / ret;
...
}
If the hardware returns a value between 1 and 7, shifting right by 3 makes
ret = 0. Doesn't the next line then divide by zero and trigger a panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260429065955.2113012-1-florin.leotescu@oss.nxp.com?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v7 1/3] hwmon: emc2305: Fix fan channel index handling
2026-04-29 6:59 ` [PATCH v7 1/3] hwmon: emc2305: Fix fan channel index handling florin.leotescu
2026-04-29 7:48 ` sashiko-bot
@ 2026-04-30 23:13 ` Guenter Roeck
1 sibling, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2026-04-30 23:13 UTC (permalink / raw)
To: florin.leotescu
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Shych,
linux-hwmon, devicetree, linux-kernel, daniel.baluta,
viorel.suman, linux-arm-kernel, imx, festevam, Florin Leotescu
On Wed, Apr 29, 2026 at 09:59:53AM +0300, florin.leotescu@oss.nxp.com wrote:
> From: Florin Leotescu <florin.leotescu@nxp.com>
>
> The fan channel index is used to access per-channel data structures.
> Validate the index against the number of available channels
> before use to prevent out-of-bounds access if an invalid
> value is provided.
>
> The thermal registration path currently uses a sequential child index,
> which may not match the validated channel from DT. Use the DT "reg"
> property when registering cooling devices to ensure consistent
> channel handling
>
> Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
The problems found by Sashiko are real, but they are pre-existing.
The locking issues are pre-existing, and data->pwm_separate is for
all practical purposes always false (there is no upstream user of
the platform data, so it may well be removed), meaning there is no
separation of pwm channels if the thermal subsystem is enabled.
Given that, applied.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v7 2/3] dt-bindings: hwmon: emc2305: Add fan-shutdown-percent property
2026-04-29 6:59 [PATCH v7 0/3] Support configurable fan PWM at shutdown florin.leotescu
2026-04-29 6:59 ` [PATCH v7 1/3] hwmon: emc2305: Fix fan channel index handling florin.leotescu
@ 2026-04-29 6:59 ` florin.leotescu
2026-04-29 8:09 ` sashiko-bot
` (2 more replies)
2026-04-29 6:59 ` [PATCH v7 3/3] hwmon: emc2305: Support configurable fan PWM at shutdown florin.leotescu
2 siblings, 3 replies; 13+ messages in thread
From: florin.leotescu @ 2026-04-29 6:59 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Shych, linux-hwmon, devicetree, linux-kernel
Cc: daniel.baluta, viorel.suman, linux-arm-kernel, imx, festevam,
Florin Leotescu
From: Florin Leotescu <florin.leotescu@nxp.com>
The EMC2305 fan controller supports multiple independent PWM fan
outputs. Some systems require fans to enter a defined safe state
during system shutdown or reboot handoff, until firmware or the next
boot stage reconfigures the controller.
Add an optional "fan-shutdown-percent" property to fan child nodes
allowing the PWM duty cycle applied during shutdown to be configured
per fan output.
Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
---
.../devicetree/bindings/hwmon/microchip,emc2305.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
index d3f06ebc19fa..8c2548539d7f 100644
--- a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
+++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
@@ -54,6 +54,12 @@ patternProperties:
The fan number used to determine the associated PWM channel.
maxItems: 1
+ fan-shutdown-percent:
+ description:
+ PWM duty cycle in percent applied to the fan during shutdown.
+ minimum: 0
+ maximum: 100
+
required:
- reg
@@ -80,12 +86,14 @@ examples:
fan@0 {
reg = <0x0>;
pwms = <&fan_controller 26000 PWM_POLARITY_INVERTED 1>;
+ fan-shutdown-percent = <100>;
#cooling-cells = <2>;
};
fan@1 {
reg = <0x1>;
pwms = <&fan_controller 26000 0 1>;
+ fan-shutdown-percent = <50>;
#cooling-cells = <2>;
};
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v7 2/3] dt-bindings: hwmon: emc2305: Add fan-shutdown-percent property
2026-04-29 6:59 ` [PATCH v7 2/3] dt-bindings: hwmon: emc2305: Add fan-shutdown-percent property florin.leotescu
@ 2026-04-29 8:09 ` sashiko-bot
2026-04-29 18:18 ` Conor Dooley
2026-04-30 23:14 ` Guenter Roeck
2 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-04-29 8:09 UTC (permalink / raw)
To: florin.leotescu; +Cc: linux-hwmon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `fan-shutdown-percent` property is redefined locally in the emc2305 binding instead of being moved to the common fan schema.
--
commit 0de7c28110f8526177da5d5c1903c8b485eec6ec
Author: Florin Leotescu <florin.leotescu@nxp.com>
dt-bindings: hwmon: emc2305: Add fan-shutdown-percent property
The EMC2305 fan controller supports multiple independent PWM fan
outputs. Some systems require fans to enter a defined safe state
during system shutdown or reboot handoff.
> diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> index d3f06ebc19fa..8c2548539d7f 100644
> --- a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> @@ -54,6 +54,12 @@ patternProperties:
> The fan number used to determine the associated PWM channel.
> maxItems: 1
>
> + fan-shutdown-percent:
This isn't a bug, but should this property be moved to the common fan
schema instead of being defined locally here?
The fan-shutdown-percent property already exists in
Documentation/devicetree/bindings/hwmon/pwm-fan.yaml to represent the same
hardware behavior. Unifying this property in fan-common.yaml would prevent
schema fragmentation and serve as a single source of truth, especially since
microchip,emc2305.yaml already references fan-common.yaml# for its child
nodes.
> + description:
> + PWM duty cycle in percent applied to the fan during shutdown.
> + minimum: 0
> + maximum: 100
> +
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260429065955.2113012-1-florin.leotescu@oss.nxp.com?part=2
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 2/3] dt-bindings: hwmon: emc2305: Add fan-shutdown-percent property
2026-04-29 6:59 ` [PATCH v7 2/3] dt-bindings: hwmon: emc2305: Add fan-shutdown-percent property florin.leotescu
2026-04-29 8:09 ` sashiko-bot
@ 2026-04-29 18:18 ` Conor Dooley
2026-04-30 12:02 ` Florin Leotescu
2026-04-30 23:14 ` Guenter Roeck
2 siblings, 1 reply; 13+ messages in thread
From: Conor Dooley @ 2026-04-29 18:18 UTC (permalink / raw)
To: florin.leotescu
Cc: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Shych, linux-hwmon, devicetree, linux-kernel,
daniel.baluta, viorel.suman, linux-arm-kernel, imx, festevam,
Florin Leotescu
[-- Attachment #1: Type: text/plain, Size: 2082 bytes --]
On Wed, Apr 29, 2026 at 09:59:54AM +0300, florin.leotescu@oss.nxp.com wrote:
> From: Florin Leotescu <florin.leotescu@nxp.com>
>
> The EMC2305 fan controller supports multiple independent PWM fan
> outputs. Some systems require fans to enter a defined safe state
> during system shutdown or reboot handoff, until firmware or the next
> boot stage reconfigures the controller.
>
> Add an optional "fan-shutdown-percent" property to fan child nodes
> allowing the PWM duty cycle applied during shutdown to be configured
> per fan output.
>
> Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
Why didn't you pick up my tag from here:
https://lore.kernel.org/all/20260407-slang-scoff-795164352c62@spud/
> ---
> .../devicetree/bindings/hwmon/microchip,emc2305.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> index d3f06ebc19fa..8c2548539d7f 100644
> --- a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> @@ -54,6 +54,12 @@ patternProperties:
> The fan number used to determine the associated PWM channel.
> maxItems: 1
>
> + fan-shutdown-percent:
> + description:
> + PWM duty cycle in percent applied to the fan during shutdown.
> + minimum: 0
> + maximum: 100
> +
> required:
> - reg
>
> @@ -80,12 +86,14 @@ examples:
> fan@0 {
> reg = <0x0>;
> pwms = <&fan_controller 26000 PWM_POLARITY_INVERTED 1>;
> + fan-shutdown-percent = <100>;
> #cooling-cells = <2>;
> };
>
> fan@1 {
> reg = <0x1>;
> pwms = <&fan_controller 26000 0 1>;
> + fan-shutdown-percent = <50>;
> #cooling-cells = <2>;
> };
>
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v7 2/3] dt-bindings: hwmon: emc2305: Add fan-shutdown-percent property
2026-04-29 18:18 ` Conor Dooley
@ 2026-04-30 12:02 ` Florin Leotescu
2026-04-30 18:53 ` Conor Dooley
0 siblings, 1 reply; 13+ messages in thread
From: Florin Leotescu @ 2026-04-30 12:02 UTC (permalink / raw)
To: Conor Dooley
Cc: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Shych, linux-hwmon, devicetree, linux-kernel,
daniel.baluta, viorel.suman, linux-arm-kernel, imx, festevam,
Florin Leotescu
On Wed, Apr 29, 2026 at 07:18:04PM +0100, Conor Dooley wrote:
> On Wed, Apr 29, 2026 at 09:59:54AM +0300, florin.leotescu@oss.nxp.com wrote:
> > From: Florin Leotescu <florin.leotescu@nxp.com>
> >
> > The EMC2305 fan controller supports multiple independent PWM fan
> > outputs. Some systems require fans to enter a defined safe state
> > during system shutdown or reboot handoff, until firmware or the next
> > boot stage reconfigures the controller.
> >
> > Add an optional "fan-shutdown-percent" property to fan child nodes
> > allowing the PWM duty cycle applied during shutdown to be configured
> > per fan output.
> >
> > Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
>
> Why didn't you pick up my tag from here:
> https://lore.kernel.org/all/20260407-slang-scoff-795164352c62@spud/
>
Apologies, I missed your Acked-by tag when preparing the series.
I will include it in the next revision.
> > ---
> > .../devicetree/bindings/hwmon/microchip,emc2305.yaml | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> > index d3f06ebc19fa..8c2548539d7f 100644
> > --- a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> > +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> > @@ -54,6 +54,12 @@ patternProperties:
> > The fan number used to determine the associated PWM channel.
> > maxItems: 1
> >
> > + fan-shutdown-percent:
> > + description:
> > + PWM duty cycle in percent applied to the fan during shutdown.
> > + minimum: 0
> > + maximum: 100
> > +
> > required:
> > - reg
> >
> > @@ -80,12 +86,14 @@ examples:
> > fan@0 {
> > reg = <0x0>;
> > pwms = <&fan_controller 26000 PWM_POLARITY_INVERTED 1>;
> > + fan-shutdown-percent = <100>;
> > #cooling-cells = <2>;
> > };
> >
> > fan@1 {
> > reg = <0x1>;
> > pwms = <&fan_controller 26000 0 1>;
> > + fan-shutdown-percent = <50>;
> > #cooling-cells = <2>;
> > };
> >
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v7 2/3] dt-bindings: hwmon: emc2305: Add fan-shutdown-percent property
2026-04-30 12:02 ` Florin Leotescu
@ 2026-04-30 18:53 ` Conor Dooley
0 siblings, 0 replies; 13+ messages in thread
From: Conor Dooley @ 2026-04-30 18:53 UTC (permalink / raw)
To: Florin Leotescu
Cc: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Shych, linux-hwmon, devicetree, linux-kernel,
daniel.baluta, viorel.suman, linux-arm-kernel, imx, festevam,
Florin Leotescu
[-- Attachment #1: Type: text/plain, Size: 2713 bytes --]
On Thu, Apr 30, 2026 at 03:02:43PM +0300, Florin Leotescu wrote:
> On Wed, Apr 29, 2026 at 07:18:04PM +0100, Conor Dooley wrote:
> > On Wed, Apr 29, 2026 at 09:59:54AM +0300, florin.leotescu@oss.nxp.com wrote:
> > > From: Florin Leotescu <florin.leotescu@nxp.com>
> > >
> > > The EMC2305 fan controller supports multiple independent PWM fan
> > > outputs. Some systems require fans to enter a defined safe state
> > > during system shutdown or reboot handoff, until firmware or the next
> > > boot stage reconfigures the controller.
> > >
> > > Add an optional "fan-shutdown-percent" property to fan child nodes
> > > allowing the PWM duty cycle applied during shutdown to be configured
> > > per fan output.
> > >
> > > Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
> >
> > Why didn't you pick up my tag from here:
> > https://lore.kernel.org/all/20260407-slang-scoff-795164352c62@spud/
> >
>
> Apologies, I missed your Acked-by tag when preparing the series.
> I will include it in the next revision.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
Don't resend unless there's something else wrong with the series.
>
> > > ---
> > > .../devicetree/bindings/hwmon/microchip,emc2305.yaml | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> > > index d3f06ebc19fa..8c2548539d7f 100644
> > > --- a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> > > +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> > > @@ -54,6 +54,12 @@ patternProperties:
> > > The fan number used to determine the associated PWM channel.
> > > maxItems: 1
> > >
> > > + fan-shutdown-percent:
> > > + description:
> > > + PWM duty cycle in percent applied to the fan during shutdown.
> > > + minimum: 0
> > > + maximum: 100
> > > +
> > > required:
> > > - reg
> > >
> > > @@ -80,12 +86,14 @@ examples:
> > > fan@0 {
> > > reg = <0x0>;
> > > pwms = <&fan_controller 26000 PWM_POLARITY_INVERTED 1>;
> > > + fan-shutdown-percent = <100>;
> > > #cooling-cells = <2>;
> > > };
> > >
> > > fan@1 {
> > > reg = <0x1>;
> > > pwms = <&fan_controller 26000 0 1>;
> > > + fan-shutdown-percent = <50>;
> > > #cooling-cells = <2>;
> > > };
> > >
> > > --
> > > 2.34.1
> > >
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 2/3] dt-bindings: hwmon: emc2305: Add fan-shutdown-percent property
2026-04-29 6:59 ` [PATCH v7 2/3] dt-bindings: hwmon: emc2305: Add fan-shutdown-percent property florin.leotescu
2026-04-29 8:09 ` sashiko-bot
2026-04-29 18:18 ` Conor Dooley
@ 2026-04-30 23:14 ` Guenter Roeck
2 siblings, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2026-04-30 23:14 UTC (permalink / raw)
To: florin.leotescu
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Shych,
linux-hwmon, devicetree, linux-kernel, daniel.baluta,
viorel.suman, linux-arm-kernel, imx, festevam, Florin Leotescu
On Wed, Apr 29, 2026 at 09:59:54AM +0300, florin.leotescu@oss.nxp.com wrote:
> From: Florin Leotescu <florin.leotescu@nxp.com>
>
> The EMC2305 fan controller supports multiple independent PWM fan
> outputs. Some systems require fans to enter a defined safe state
> during system shutdown or reboot handoff, until firmware or the next
> boot stage reconfigures the controller.
>
> Add an optional "fan-shutdown-percent" property to fan child nodes
> allowing the PWM duty cycle applied during shutdown to be configured
> per fan output.
>
> Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
Applied.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v7 3/3] hwmon: emc2305: Support configurable fan PWM at shutdown
2026-04-29 6:59 [PATCH v7 0/3] Support configurable fan PWM at shutdown florin.leotescu
2026-04-29 6:59 ` [PATCH v7 1/3] hwmon: emc2305: Fix fan channel index handling florin.leotescu
2026-04-29 6:59 ` [PATCH v7 2/3] dt-bindings: hwmon: emc2305: Add fan-shutdown-percent property florin.leotescu
@ 2026-04-29 6:59 ` florin.leotescu
2026-04-29 8:48 ` sashiko-bot
2026-04-30 23:17 ` Guenter Roeck
2 siblings, 2 replies; 13+ messages in thread
From: florin.leotescu @ 2026-04-29 6:59 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Shych, linux-hwmon, devicetree, linux-kernel
Cc: daniel.baluta, viorel.suman, linux-arm-kernel, imx, festevam,
Florin Leotescu
From: Florin Leotescu <florin.leotescu@nxp.com>
Some systems require fans to enter in a defined safe state during system
shutdown or reboot handoff.
Add support for the optional Device Tree property "fan-shutdown-percent"
to configure the shutdown PWM duty cycle per fan output.
If the property is present for a fan channel, the driver converts the
configured percentage value to the corresponding PWM duty cycle and
applies it during driver shutdown.
If the property is not present, the fan state remains unchanged.
Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
---
drivers/hwmon/emc2305.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
index f71a0e265924..c5a0b2146478 100644
--- a/drivers/hwmon/emc2305.c
+++ b/drivers/hwmon/emc2305.c
@@ -32,6 +32,7 @@
#define EMC2305_REG_DRIVE_PWM_OUT 0x2b
#define EMC2305_OPEN_DRAIN 0x0
#define EMC2305_PUSH_PULL 0x1
+#define EMC2305_PWM_SHUTDOWN_UNSET -1
#define EMC2305_PWM_DUTY2STATE(duty, max_state, pwm_max) \
DIV_ROUND_CLOSEST((duty) * (max_state), (pwm_max))
@@ -104,6 +105,7 @@ struct emc2305_cdev_data {
* @pwm_output_mask: PWM output mask
* @pwm_polarity_mask: PWM polarity mask
* @pwm_separate: separate PWM settings for every channel
+ * @pwm_shutdown: Set shutdown PWM.
* @pwm_min: array of minimum PWM per channel
* @pwm_freq: array of PWM frequency per channel
* @cdev_data: array of cooling devices data
@@ -116,6 +118,7 @@ struct emc2305_data {
u8 pwm_output_mask;
u8 pwm_polarity_mask;
bool pwm_separate;
+ s16 pwm_shutdown[EMC2305_PWM_MAX];
u8 pwm_min[EMC2305_PWM_MAX];
u16 pwm_freq[EMC2305_PWM_MAX];
struct emc2305_cdev_data cdev_data[EMC2305_PWM_MAX];
@@ -539,6 +542,7 @@ static int emc2305_of_parse_pwm_child(struct device *dev,
struct device_node *child,
struct emc2305_data *data)
{ u32 ch;
+ u32 pwm_shutdown_percent;
int ret;
struct of_phandle_args args;
@@ -585,6 +589,16 @@ static int emc2305_of_parse_pwm_child(struct device *dev,
}
of_node_put(args.np);
+
+ ret = of_property_read_u32(child, "fan-shutdown-percent",
+ &pwm_shutdown_percent);
+
+ if (!ret) {
+ pwm_shutdown_percent = clamp(pwm_shutdown_percent, 0, 100);
+ data->pwm_shutdown[ch] =
+ DIV_ROUND_CLOSEST(pwm_shutdown_percent * EMC2305_FAN_MAX, 100);
+ }
+
return 0;
}
@@ -638,6 +652,9 @@ static int emc2305_probe(struct i2c_client *client)
if (ret)
return ret;
+ for (i = 0; i < EMC2305_PWM_MAX; i++)
+ data->pwm_shutdown[i] = EMC2305_PWM_SHUTDOWN_UNSET;
+
pwm_childs = emc2305_probe_childs_from_dt(dev);
pdata = dev_get_platdata(&client->dev);
@@ -727,6 +744,23 @@ static int emc2305_probe(struct i2c_client *client)
return 0;
}
+static void emc2305_shutdown(struct i2c_client *client)
+{
+ int i;
+ int ret;
+ struct emc2305_data *data = i2c_get_clientdata(client);
+
+ for (i = 0; i < data->pwm_num; i++) {
+ if (data->pwm_shutdown[i] != EMC2305_PWM_SHUTDOWN_UNSET) {
+ ret = i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_DRIVE(i),
+ data->pwm_shutdown[i]);
+ if (ret < 0)
+ dev_warn(&client->dev,
+ "Failed to set shutdown PWM for ch %d\n", i);
+ }
+ }
+}
+
static const struct of_device_id of_emc2305_match_table[] = {
{ .compatible = "microchip,emc2305", },
{},
@@ -739,6 +773,7 @@ static struct i2c_driver emc2305_driver = {
.of_match_table = of_emc2305_match_table,
},
.probe = emc2305_probe,
+ .shutdown = emc2305_shutdown,
.id_table = emc2305_ids,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v7 3/3] hwmon: emc2305: Support configurable fan PWM at shutdown
2026-04-29 6:59 ` [PATCH v7 3/3] hwmon: emc2305: Support configurable fan PWM at shutdown florin.leotescu
@ 2026-04-29 8:48 ` sashiko-bot
2026-04-30 23:17 ` Guenter Roeck
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-04-29 8:48 UTC (permalink / raw)
To: florin.leotescu; +Cc: linux-hwmon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The emc2305_shutdown() callback sets the hardware to a safe shutdown state but does not prevent concurrent or subsequent modifications from the thermal framework or sysfs, and violates hwmon locking guidelines.
--
commit 13f5fb0080b01e93b042a4e45c254e10cd715c59
Author: Florin Leotescu <florin.leotescu@nxp.com>
hwmon: emc2305: Support configurable fan PWM at shutdown
This commit adds support for an optional "fan-shutdown-percent" device tree
property to configure the safe shutdown PWM duty cycle per fan output. It
introduces a shutdown callback to apply these configured states during
system handoff.
> diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
> index f71a0e265924..c5a0b2146478 100644
> --- a/drivers/hwmon/emc2305.c
> +++ b/drivers/hwmon/emc2305.c
[ ... ]
> @@ -727,6 +744,23 @@ static int emc2305_probe(struct i2c_client *client)
> return 0;
> }
>
> +static void emc2305_shutdown(struct i2c_client *client)
> +{
> + int i;
> + int ret;
> + struct emc2305_data *data = i2c_get_clientdata(client);
> +
> + for (i = 0; i < data->pwm_num; i++) {
> + if (data->pwm_shutdown[i] != EMC2305_PWM_SHUTDOWN_UNSET) {
> + ret = i2c_smbus_write_byte_data(client, EMC2305_REG_FAN_DRIVE(i),
> + data->pwm_shutdown[i]);
Does this hardware access need to be serialized? The hwmon subsystem
guidelines explicitly state that drivers must use hwmon_lock() and
hwmon_unlock() to serialize hardware operations for attributes registered
outside the hwmon core.
Also, can concurrent or subsequent updates overwrite this safe shutdown state?
During system shutdown, kernel workqueues remain active and can continue to
execute thermal zone polling. Because the driver does not track whether it is
shutting down, the thermal framework's set_cur_state callback could execute
and overwrite the safe shutdown PWM state with a temperature-based value.
Would it make sense to introduce a boolean flag (e.g., shutting_down)
protected by hwmon_lock() here, and check this flag in the thermal and sysfs
write paths to abort further updates?
> + if (ret < 0)
> + dev_warn(&client->dev,
> + "Failed to set shutdown PWM for ch %d\n", i);
> + }
> + }
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260429065955.2113012-1-florin.leotescu@oss.nxp.com?part=3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v7 3/3] hwmon: emc2305: Support configurable fan PWM at shutdown
2026-04-29 6:59 ` [PATCH v7 3/3] hwmon: emc2305: Support configurable fan PWM at shutdown florin.leotescu
2026-04-29 8:48 ` sashiko-bot
@ 2026-04-30 23:17 ` Guenter Roeck
1 sibling, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2026-04-30 23:17 UTC (permalink / raw)
To: florin.leotescu
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Shych,
linux-hwmon, devicetree, linux-kernel, daniel.baluta,
viorel.suman, linux-arm-kernel, imx, festevam, Florin Leotescu
On Wed, Apr 29, 2026 at 09:59:55AM +0300, florin.leotescu@oss.nxp.com wrote:
> From: Florin Leotescu <florin.leotescu@nxp.com>
>
> Some systems require fans to enter in a defined safe state during system
> shutdown or reboot handoff.
>
> Add support for the optional Device Tree property "fan-shutdown-percent"
> to configure the shutdown PWM duty cycle per fan output.
>
> If the property is present for a fan channel, the driver converts the
> configured percentage value to the corresponding PWM duty cycle and
> applies it during driver shutdown.
>
> If the property is not present, the fan state remains unchanged.
>
> Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
I don't know if Sashiko's concerns are real; if sysfs accesses are still
possible during shutdown, lots of drivers have problems. With that in mind,
applied.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 13+ messages in thread