[PATCH v5 2/2] hwmon: emc2305: Support configurable fan PWM at shutdown

[Guenter Roeck]

On 3/25/26 09:04, florin.leotescu at oss.nxp.com wrote:
> From: Florin Leotescu <florin.leotescu at 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 at nxp.com>

Sashiko has a couple of concerns.

https://sashiko.dev/#/patchset/20260325160444.1736866-1-florin.leotescu%40oss.nxp.com

I don't know about the shutdown problem. If that is real, a lot of drivers
have the same problem. Don't bother about it right now.

However, the range check should indeed be a separate patch.

Thanks,
Guenter

> ---
>   drivers/hwmon/emc2305.c | 41 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)
> 
> diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
> index 64b213e1451e..722ec1d054f8 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;
>   
> @@ -548,6 +552,12 @@ static int emc2305_of_parse_pwm_child(struct device *dev,
>   		return ret;
>   	}
>   
> +	if (ch >= EMC2305_PWM_MAX) {
> +		dev_err(dev, "invalid reg %u of node %pOF (valid range 0-%u)\n", ch, child,
> +			EMC2305_PWM_MAX - 1);
> +		return -EINVAL;
> +	}
> +
>   	ret = of_parse_phandle_with_args(child, "pwms", "#pwm-cells", 0, &args);
>   
>   	if (ret)
> @@ -579,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;
>   }
>   
> @@ -631,6 +651,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);
> @@ -714,6 +737,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", },
>   	{},
> @@ -726,6 +766,7 @@ static struct i2c_driver emc2305_driver = {
>   		.of_match_table = of_emc2305_match_table,
>   	},
>   	.probe = emc2305_probe,
> +	.shutdown = emc2305_shutdown,
>   	.id_table = emc2305_ids,
>   };
>