diff mbox series

[v2,1/2] hwmon: (pwm-fan) add option to leave fan on shutdown

Message ID 20241026080535.444903-2-akinobu.mita@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series hwmon: (pwm-fan) add option to leave fan on shutdown | expand

Commit Message

Akinobu Mita Oct. 26, 2024, 8:05 a.m. UTC
This adds an optional property "retain-state-shutdown" as requested by
Billy Tsai.

Billy said:
 "Our platform is BMC that will use a PWM-FAN driver to control the fan
 on the managed host. In our case, we do not want to stop the fan when
 the BMC is reboot, which may cause the temperature of the managed host
 not to be lowered."

Cc: Jean Delvare <jdelvare@suse.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Rob Herring <robh@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Billy Tsai <billy_tsai@aspeedtech.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/hwmon/pwm-fan.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Billy Tsai Oct. 29, 2024, 2:34 a.m. UTC | #1
> This adds an optional property "retain-state-shutdown" as requested by
> Billy Tsai.

> Billy said:
>  "Our platform is BMC that will use a PWM-FAN driver to control the fan
>  on the managed host. In our case, we do not want to stop the fan when
>  the BMC is reboot, which may cause the temperature of the managed host
>  not to be lowered."

I confirmed that it works properly.

Reviewed-by: Billy Tsai <billy_tsai@aspeedtech.com>
Tested-by: Billy Tsai <billy_tsai@aspeedtech.com>

Thanks
Akinobu Mita Oct. 30, 2024, 2:55 p.m. UTC | #2
2024年10月29日(火) 11:35 Billy Tsai <billy_tsai@aspeedtech.com>:
>
> > This adds an optional property "retain-state-shutdown" as requested by
> > Billy Tsai.
>
> > Billy said:
> >  "Our platform is BMC that will use a PWM-FAN driver to control the fan
> >  on the managed host. In our case, we do not want to stop the fan when
> >  the BMC is reboot, which may cause the temperature of the managed host
> >  not to be lowered."
>
> I confirmed that it works properly.
>
> Reviewed-by: Billy Tsai <billy_tsai@aspeedtech.com>
> Tested-by: Billy Tsai <billy_tsai@aspeedtech.com>

Thank you very much for testing.
However, I plan to change this option to device attribute
(/sys/class/hwmon/hwmonX/retain-state-shutdown) instead of the device tree.
Guenter Roeck Oct. 30, 2024, 3:02 p.m. UTC | #3
On 10/30/24 07:55, Akinobu Mita wrote:
> 2024年10月29日(火) 11:35 Billy Tsai <billy_tsai@aspeedtech.com>:
>>
>>> This adds an optional property "retain-state-shutdown" as requested by
>>> Billy Tsai.
>>
>>> Billy said:
>>>   "Our platform is BMC that will use a PWM-FAN driver to control the fan
>>>   on the managed host. In our case, we do not want to stop the fan when
>>>   the BMC is reboot, which may cause the temperature of the managed host
>>>   not to be lowered."
>>
>> I confirmed that it works properly.
>>
>> Reviewed-by: Billy Tsai <billy_tsai@aspeedtech.com>
>> Tested-by: Billy Tsai <billy_tsai@aspeedtech.com>
> 
> Thank you very much for testing.
> However, I plan to change this option to device attribute
> (/sys/class/hwmon/hwmonX/retain-state-shutdown) instead of the device tree.


No, I won't acccept that.

Guenter
Akinobu Mita Oct. 30, 2024, 3:18 p.m. UTC | #4
2024年10月31日(木) 0:02 Guenter Roeck <linux@roeck-us.net>:
>
> On 10/30/24 07:55, Akinobu Mita wrote:
> > 2024年10月29日(火) 11:35 Billy Tsai <billy_tsai@aspeedtech.com>:
> >>
> >>> This adds an optional property "retain-state-shutdown" as requested by
> >>> Billy Tsai.
> >>
> >>> Billy said:
> >>>   "Our platform is BMC that will use a PWM-FAN driver to control the fan
> >>>   on the managed host. In our case, we do not want to stop the fan when
> >>>   the BMC is reboot, which may cause the temperature of the managed host
> >>>   not to be lowered."
> >>
> >> I confirmed that it works properly.
> >>
> >> Reviewed-by: Billy Tsai <billy_tsai@aspeedtech.com>
> >> Tested-by: Billy Tsai <billy_tsai@aspeedtech.com>
> >
> > Thank you very much for testing.
> > However, I plan to change this option to device attribute
> > (/sys/class/hwmon/hwmonX/retain-state-shutdown) instead of the device tree.
>
>
> No, I won't acccept that.

Oops, I mean /sys/device/platform/pwm-fan.X/retain-state-shutdown
not /sys/class/hwmon/hwmonX/retain-state-shutdown.
Guenter Roeck Oct. 30, 2024, 3:30 p.m. UTC | #5
On 10/30/24 08:18, Akinobu Mita wrote:
> 2024年10月31日(木) 0:02 Guenter Roeck <linux@roeck-us.net>:
>>
>> On 10/30/24 07:55, Akinobu Mita wrote:
>>> 2024年10月29日(火) 11:35 Billy Tsai <billy_tsai@aspeedtech.com>:
>>>>
>>>>> This adds an optional property "retain-state-shutdown" as requested by
>>>>> Billy Tsai.
>>>>
>>>>> Billy said:
>>>>>    "Our platform is BMC that will use a PWM-FAN driver to control the fan
>>>>>    on the managed host. In our case, we do not want to stop the fan when
>>>>>    the BMC is reboot, which may cause the temperature of the managed host
>>>>>    not to be lowered."
>>>>
>>>> I confirmed that it works properly.
>>>>
>>>> Reviewed-by: Billy Tsai <billy_tsai@aspeedtech.com>
>>>> Tested-by: Billy Tsai <billy_tsai@aspeedtech.com>
>>>
>>> Thank you very much for testing.
>>> However, I plan to change this option to device attribute
>>> (/sys/class/hwmon/hwmonX/retain-state-shutdown) instead of the device tree.
>>
>>
>> No, I won't acccept that.
> 
> Oops, I mean /sys/device/platform/pwm-fan.X/retain-state-shutdown
> not /sys/class/hwmon/hwmonX/retain-state-shutdown.

That does not make a difference. It still very much looks like a system property
to me, and making it a device property to be set by userspace just looks like
a workaround to address refusal by dt maintainers to accept the system property.

You'll have to make the case for why this is _not_ a system property for me
to accept it.

Guenter
diff mbox series

Patch

diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index c434db4656e7..dcb48a41f9f0 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -51,6 +51,7 @@  struct pwm_fan_ctx {
 	u32 *pulses_per_revolution;
 	ktime_t sample_start;
 	struct timer_list rpm_timer;
+	bool retain_state_shutdown;
 
 	unsigned int pwm_value;
 	unsigned int pwm_fan_state;
@@ -490,6 +491,8 @@  static int pwm_fan_probe(struct platform_device *pdev)
 
 	mutex_init(&ctx->lock);
 
+	ctx->retain_state_shutdown = device_property_read_bool(dev, "retain-state-shutdown");
+
 	ctx->dev = &pdev->dev;
 	ctx->pwm = devm_pwm_get(dev, NULL);
 	if (IS_ERR(ctx->pwm))
@@ -655,7 +658,8 @@  static void pwm_fan_shutdown(struct platform_device *pdev)
 {
 	struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev);
 
-	pwm_fan_cleanup(ctx);
+	if (!ctx->retain_state_shutdown)
+		pwm_fan_cleanup(ctx);
 }
 
 static int pwm_fan_suspend(struct device *dev)