* [PATCH 0/2] hwmon: (pwm-fan) add option to stop fan on shutdown @ 2019-12-22 14:10 Akinobu Mita 2019-12-22 14:10 ` [PATCH 1/2] " Akinobu Mita 2019-12-22 14:10 ` [PATCH 2/2] dt-bindings: hwmon: (pwm-fan) add disable-state-shutdown property Akinobu Mita 0 siblings, 2 replies; 6+ messages in thread From: Akinobu Mita @ 2019-12-22 14:10 UTC (permalink / raw) To: linux-hwmon, devicetree Cc: Akinobu Mita, Rob Herring, Mark Rutland, Kamil Debski, Bartlomiej Zolnierkiewicz, Guenter Roeck The pwm-fan driver leaves the fun running when shutting down the system. (On the other hand the gpio-fan driver stops it.) This adds an optional property "disable-state-shutdown" in case someone wants to stop the fun running when shutting down the system. Akinobu Mita (2): hwmon: (pwm-fan) add option to stop fan on shutdown dt-bindings: hwmon: (pwm-fan) add disable-state-shutdown property .../devicetree/bindings/hwmon/pwm-fan.txt | 2 ++ drivers/hwmon/pwm-fan.c | 24 ++++++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) Cc: Rob Herring <robh+dt@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Kamil Debski <kamil@wypas.org> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Cc: Guenter Roeck <linux@roeck-us.net> -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] hwmon: (pwm-fan) add option to stop fan on shutdown 2019-12-22 14:10 [PATCH 0/2] hwmon: (pwm-fan) add option to stop fan on shutdown Akinobu Mita @ 2019-12-22 14:10 ` Akinobu Mita 2020-01-08 16:13 ` Rob Herring 2019-12-22 14:10 ` [PATCH 2/2] dt-bindings: hwmon: (pwm-fan) add disable-state-shutdown property Akinobu Mita 1 sibling, 1 reply; 6+ messages in thread From: Akinobu Mita @ 2019-12-22 14:10 UTC (permalink / raw) To: linux-hwmon, devicetree Cc: Akinobu Mita, Rob Herring, Mark Rutland, Kamil Debski, Bartlomiej Zolnierkiewicz, Guenter Roeck The pwm-fan driver leaves the fun running when shutting down the system. (On the other hand the gpio-fan driver stops it.) This adds an optional property "disable-state-shutdown" in case someone wants to stop the fun running when shutting down the system. Cc: Rob Herring <robh+dt@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Kamil Debski <kamil@wypas.org> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Cc: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> --- drivers/hwmon/pwm-fan.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c index 42ffd2e..8775d37 100644 --- a/drivers/hwmon/pwm-fan.c +++ b/drivers/hwmon/pwm-fan.c @@ -33,6 +33,7 @@ struct pwm_fan_ctx { u8 pulses_per_revolution; ktime_t sample_start; struct timer_list rpm_timer; + bool disable_state_shutdown; unsigned int pwm_value; unsigned int pwm_fan_state; @@ -292,6 +293,9 @@ static int pwm_fan_probe(struct platform_device *pdev) mutex_init(&ctx->lock); + ctx->disable_state_shutdown = + of_property_read_bool(dev->of_node, "disable-state-shutdown"); + ctx->pwm = devm_of_pwm_get(dev, dev->of_node, NULL); if (IS_ERR(ctx->pwm)) { ret = PTR_ERR(ctx->pwm); @@ -390,8 +394,7 @@ static int pwm_fan_probe(struct platform_device *pdev) return 0; } -#ifdef CONFIG_PM_SLEEP -static int pwm_fan_suspend(struct device *dev) +static int pwm_fan_disable(struct device *dev) { struct pwm_fan_ctx *ctx = dev_get_drvdata(dev); struct pwm_args args; @@ -418,6 +421,22 @@ static int pwm_fan_suspend(struct device *dev) return 0; } +static void pwm_fan_shutdown(struct platform_device *pdev) +{ + struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev); + + if (!ctx->disable_state_shutdown) + return; + + pwm_fan_disable(&pdev->dev); +} + +#ifdef CONFIG_PM_SLEEP +static int pwm_fan_suspend(struct device *dev) +{ + return pwm_fan_disable(dev); +} + static int pwm_fan_resume(struct device *dev) { struct pwm_fan_ctx *ctx = dev_get_drvdata(dev); @@ -455,6 +474,7 @@ MODULE_DEVICE_TABLE(of, of_pwm_fan_match); static struct platform_driver pwm_fan_driver = { .probe = pwm_fan_probe, + .shutdown = pwm_fan_shutdown, .driver = { .name = "pwm-fan", .pm = &pwm_fan_pm, -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] hwmon: (pwm-fan) add option to stop fan on shutdown 2019-12-22 14:10 ` [PATCH 1/2] " Akinobu Mita @ 2020-01-08 16:13 ` Rob Herring 2020-01-10 5:06 ` Akinobu Mita 0 siblings, 1 reply; 6+ messages in thread From: Rob Herring @ 2020-01-08 16:13 UTC (permalink / raw) To: Akinobu Mita Cc: linux-hwmon, devicetree, Mark Rutland, Kamil Debski, Bartlomiej Zolnierkiewicz, Guenter Roeck On Sun, Dec 22, 2019 at 11:10:22PM +0900, Akinobu Mita wrote: > The pwm-fan driver leaves the fun running when shutting down the system. > (On the other hand the gpio-fan driver stops it.) Seemms like we should have consistent behavior independent of what the underlying implementation uses. Is there actually a case you'd want to leave the fan on? It seems strange to disable in suspend and leave on in shutdown. Wouldn't the shutdown state depend if the PWM off state is high or low? IIRC, i.MX PWM has a quirk that the PWM disabled state is high. Doesn't it also depend on what the PWM driver does in shutdown? Rob > > This adds an optional property "disable-state-shutdown" in case someone > wants to stop the fun running when shutting down the system. > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Kamil Debski <kamil@wypas.org> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Cc: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > --- > drivers/hwmon/pwm-fan.c | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c > index 42ffd2e..8775d37 100644 > --- a/drivers/hwmon/pwm-fan.c > +++ b/drivers/hwmon/pwm-fan.c > @@ -33,6 +33,7 @@ struct pwm_fan_ctx { > u8 pulses_per_revolution; > ktime_t sample_start; > struct timer_list rpm_timer; > + bool disable_state_shutdown; > > unsigned int pwm_value; > unsigned int pwm_fan_state; > @@ -292,6 +293,9 @@ static int pwm_fan_probe(struct platform_device *pdev) > > mutex_init(&ctx->lock); > > + ctx->disable_state_shutdown = > + of_property_read_bool(dev->of_node, "disable-state-shutdown"); > + > ctx->pwm = devm_of_pwm_get(dev, dev->of_node, NULL); > if (IS_ERR(ctx->pwm)) { > ret = PTR_ERR(ctx->pwm); > @@ -390,8 +394,7 @@ static int pwm_fan_probe(struct platform_device *pdev) > return 0; > } > > -#ifdef CONFIG_PM_SLEEP > -static int pwm_fan_suspend(struct device *dev) > +static int pwm_fan_disable(struct device *dev) > { > struct pwm_fan_ctx *ctx = dev_get_drvdata(dev); > struct pwm_args args; > @@ -418,6 +421,22 @@ static int pwm_fan_suspend(struct device *dev) > return 0; > } > > +static void pwm_fan_shutdown(struct platform_device *pdev) > +{ > + struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev); > + > + if (!ctx->disable_state_shutdown) > + return; > + > + pwm_fan_disable(&pdev->dev); > +} > + > +#ifdef CONFIG_PM_SLEEP > +static int pwm_fan_suspend(struct device *dev) > +{ > + return pwm_fan_disable(dev); > +} > + > static int pwm_fan_resume(struct device *dev) > { > struct pwm_fan_ctx *ctx = dev_get_drvdata(dev); > @@ -455,6 +474,7 @@ MODULE_DEVICE_TABLE(of, of_pwm_fan_match); > > static struct platform_driver pwm_fan_driver = { > .probe = pwm_fan_probe, > + .shutdown = pwm_fan_shutdown, > .driver = { > .name = "pwm-fan", > .pm = &pwm_fan_pm, > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] hwmon: (pwm-fan) add option to stop fan on shutdown 2020-01-08 16:13 ` Rob Herring @ 2020-01-10 5:06 ` Akinobu Mita 2020-01-13 16:03 ` Rob Herring 0 siblings, 1 reply; 6+ messages in thread From: Akinobu Mita @ 2020-01-10 5:06 UTC (permalink / raw) To: Rob Herring Cc: linux-hwmon, open list:OPEN FIRMWARE AND..., Mark Rutland, Kamil Debski, Bartlomiej Zolnierkiewicz, Guenter Roeck 2020年1月9日(木) 1:13 Rob Herring <robh@kernel.org>: > > On Sun, Dec 22, 2019 at 11:10:22PM +0900, Akinobu Mita wrote: > > The pwm-fan driver leaves the fun running when shutting down the system. > > (On the other hand the gpio-fan driver stops it.) > > Seemms like we should have consistent behavior independent of what the > underlying implementation uses. Is there actually a case you'd want to > leave the fan on? It seems strange to disable in suspend and leave on in > shutdown. I agree. I was trying to keep the current behavior unchanged, so I added "disable-state-shutdown" property. But I can't think of any case we want to leave the fun on in shutdown. So it's better to change the shutdown behavior and remove the option completely or add "retain-state-shutdown" property instead. (The "retain-state-shutdown" property is inspired by gpio-leds) > Wouldn't the shutdown state depend if the PWM off state is high or low? > IIRC, i.MX PWM has a quirk that the PWM disabled state is high. Doesn't It could be possible to affect the shutdown behavior for pwm-fan. There are three i.MX PWM drivers (pwm-imx1, pwm-imx27, and pwm-tpm). Do you remember which one actually have such limitation? Maybe it should be handled by the PWM controller/chip driver and PWM core. From the perspective of PWM user drivers for now, we have nothing to do other than setting duty cycle zero and then disable PWM for stopping the pwm-fan. > it also depend on what the PWM driver does in shutdown? I think so. But as far as I can see, no PWM drivers implement shutdown callback. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] hwmon: (pwm-fan) add option to stop fan on shutdown 2020-01-10 5:06 ` Akinobu Mita @ 2020-01-13 16:03 ` Rob Herring 0 siblings, 0 replies; 6+ messages in thread From: Rob Herring @ 2020-01-13 16:03 UTC (permalink / raw) To: Akinobu Mita Cc: Linux HWMON List, open list:OPEN FIRMWARE AND..., Mark Rutland, Kamil Debski, Bartlomiej Zolnierkiewicz, Guenter Roeck On Thu, Jan 9, 2020 at 11:07 PM Akinobu Mita <akinobu.mita@gmail.com> wrote: > > 2020年1月9日(木) 1:13 Rob Herring <robh@kernel.org>: > > > > On Sun, Dec 22, 2019 at 11:10:22PM +0900, Akinobu Mita wrote: > > > The pwm-fan driver leaves the fun running when shutting down the system. > > > (On the other hand the gpio-fan driver stops it.) > > > > Seemms like we should have consistent behavior independent of what the > > underlying implementation uses. Is there actually a case you'd want to > > leave the fan on? It seems strange to disable in suspend and leave on in > > shutdown. > > I agree. I was trying to keep the current behavior unchanged, so I added > "disable-state-shutdown" property. But I can't think of any case we want > to leave the fun on in shutdown. > > So it's better to change the shutdown behavior and remove the option > completely or add "retain-state-shutdown" property instead. > (The "retain-state-shutdown" property is inspired by gpio-leds) I would just turn off the fan in shutdown and see if anyone complains. > > Wouldn't the shutdown state depend if the PWM off state is high or low? > > IIRC, i.MX PWM has a quirk that the PWM disabled state is high. Doesn't > > It could be possible to affect the shutdown behavior for pwm-fan. > There are three i.MX PWM drivers (pwm-imx1, pwm-imx27, and pwm-tpm). > Do you remember which one actually have such limitation? No. I believe the fix was to use pinctrl modes to force the state. And I think the issue was for suspend rather than shutdown (but it seems unlikely you'd want the fan off in suspend and on in shutdown). > > Maybe it should be handled by the PWM controller/chip driver and PWM core. > From the perspective of PWM user drivers for now, we have nothing to do > other than setting duty cycle zero and then disable PWM for stopping the > pwm-fan. > > > it also depend on what the PWM driver does in shutdown? > > I think so. But as far as I can see, no PWM drivers implement shutdown > callback. Do they need to? I'd assume most SoC's are powered off or put in reset which makes the PWN pin go low. Rob ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] dt-bindings: hwmon: (pwm-fan) add disable-state-shutdown property 2019-12-22 14:10 [PATCH 0/2] hwmon: (pwm-fan) add option to stop fan on shutdown Akinobu Mita 2019-12-22 14:10 ` [PATCH 1/2] " Akinobu Mita @ 2019-12-22 14:10 ` Akinobu Mita 1 sibling, 0 replies; 6+ messages in thread From: Akinobu Mita @ 2019-12-22 14:10 UTC (permalink / raw) To: linux-hwmon, devicetree Cc: Akinobu Mita, Rob Herring, Mark Rutland, Kamil Debski, Bartlomiej Zolnierkiewicz, Guenter Roeck Document new disable-state-shutdown property that is introduced in case someone wants to stop the fun running when shutting down the system. Cc: Rob Herring <robh+dt@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Kamil Debski <kamil@wypas.org> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Cc: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> --- Documentation/devicetree/bindings/hwmon/pwm-fan.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt index 41b7676..1e25787 100644 --- a/Documentation/devicetree/bindings/hwmon/pwm-fan.txt +++ b/Documentation/devicetree/bindings/hwmon/pwm-fan.txt @@ -18,6 +18,8 @@ Optional properties: an integer (default is 2 interrupts per revolution). The value must be greater than zero. +- disable-state-shutdown: Disable the state of the PWM on shutdown. + Example: fan0: pwm-fan { compatible = "pwm-fan"; -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-13 16:03 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-22 14:10 [PATCH 0/2] hwmon: (pwm-fan) add option to stop fan on shutdown Akinobu Mita 2019-12-22 14:10 ` [PATCH 1/2] " Akinobu Mita 2020-01-08 16:13 ` Rob Herring 2020-01-10 5:06 ` Akinobu Mita 2020-01-13 16:03 ` Rob Herring 2019-12-22 14:10 ` [PATCH 2/2] dt-bindings: hwmon: (pwm-fan) add disable-state-shutdown property Akinobu Mita
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).