From: Markus Niebel <Markus.Niebel@xxxxxxxxxxxxxxx>
A pwm value equal to zero is meant to switch off the pwm
hence also switching off the fan. Currently the optional
regulator is always on. When using this driver on boards
with an inverted pwm signal polarity this can cause running
the fan at maximum speed when setting pwm to zero.
The proposed changes switch the regulator off, when PWM is
currently enabled but pwm is requested to set to zero
and switch der regulator on, when PWM is currently disabled
but pwm shall be set to a no zero value.
Add __set_pwm_and_regulator and rewrite __set_pwm to
handle regulator switching for the following conditions:
- probe: pwm duty -> max, pwm state is unkwown: use __set_pwm
and enable regulator separately to keep the devm action
- off: new pwm duty is zero, pwm currently enabled: disable
regulator
- on: new pwm duty is non zero, pwm currently disabled: enable
regulator
Signed-off-by: Markus Niebel <Markus.Niebel@xxxxxxxxxxxxxxx>
---
drivers/hwmon/pwm-fan.c | 144 ++++++++++++++++++++++++++--------------
1 file changed, 93 insertions(+), 51 deletions(-)
diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index f12b9a28a232..b47d59fbe836 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -97,18 +97,50 @@ static int __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
unsigned long period;
int ret = 0;
struct pwm_state *state = &ctx->pwm_state;
+ /* track changes of reg_en for error handling */
+ enum regulator_change {
+ untouched,
+ enabled,
+ disabled,
+ } reg_change = untouched;
mutex_lock(&ctx->lock);
+
if (ctx->pwm_value == pwm)
goto exit_set_pwm_err;
+ if (ctx->reg_en) {
+ if (pwm && !state->enabled) {
+ reg_change = enabled;
+ ret = regulator_enable(ctx->reg_en);
+ } else if (!pwm && state->enabled) {
+ reg_change = disabled;
+ ret = regulator_disable(ctx->reg_en);
+ }
+ if (ret)
+ goto exit_set_pwm_err;
+ }
+
period = state->period;
state->duty_cycle = DIV_ROUND_UP(pwm * (period - 1), MAX_PWM);
state->enabled = pwm ? true : false;
ret = pwm_apply_state(ctx->pwm, state);
- if (!ret)
+ if (!ret) {
ctx->pwm_value = pwm;
+ } else if (reg_change != untouched) {
+ /*
+ * revert regulator changes to keep consistency between
+ * pwm and regulator
+ */
+ int err;
+
+ if (reg_change == enabled)
+ err = regulator_disable(ctx->reg_en);
+ else if (reg_change == disabled)
+ err = regulator_enable(ctx->reg_en);
+ }
+
exit_set_pwm_err:
mutex_unlock(&ctx->lock);
return ret;
@@ -280,18 +312,50 @@ static int pwm_fan_of_get_cooling_data(struct device *dev,
return 0;
}
-static void pwm_fan_regulator_disable(void *data)
+/*
+ * disable fan and regulator
+ * if cleanup is true, disable pwm regardless of regulator disable result
+ * this makes the function dual use for unloading driver and suspend
+ */
+
+static int __pwm_fan_disable_or_cleanup(struct pwm_fan_ctx *ctx, bool cleanup)
{
- regulator_disable(data);
+ int ret;
+
+ if (ctx->pwm_value) {
+ /* keep ctx->pwm_state unmodified for pwm_fan_resume() */
+ struct pwm_state state = ctx->pwm_state;
+
+ /* regulator is only enabled if pwm_value is not zero */
+ if (ctx->pwm_value && ctx->reg_en) {
+ ret = regulator_disable(ctx->reg_en);
+ if (ret) {
+ pr_err("Failed to disable fan supply: %d\n", ret);
+ if (!cleanup)
+ return ret;
+ }
+ }
+
+ state.duty_cycle = 0;
+ state.enabled = false;
+ ret = pwm_apply_state(ctx->pwm, &state);
+ }
+
+ return ret;
}
-static void pwm_fan_pwm_disable(void *__ctx)
+static void pwm_fan_cleanup(void *__ctx)
{
struct pwm_fan_ctx *ctx = __ctx;
-
- ctx->pwm_state.enabled = false;
- pwm_apply_state(ctx->pwm, &ctx->pwm_state);
del_timer_sync(&ctx->rpm_timer);
+ __pwm_fan_disable_or_cleanup(ctx, true);
+}
+
+static int pwm_fan_disable(void *__ctx)
+{
+ struct pwm_fan_ctx *ctx = __ctx;
+
+ return __pwm_fan_disable_or_cleanup(ctx, false);
}
static int pwm_fan_probe(struct platform_device *pdev)
@@ -324,19 +388,14 @@ static int pwm_fan_probe(struct platform_device *pdev)
return PTR_ERR(ctx->reg_en);
ctx->reg_en = NULL;
- } else {
- ret = regulator_enable(ctx->reg_en);
- if (ret) {
- dev_err(dev, "Failed to enable fan supply: %d\n", ret);
- return ret;
- }
- ret = devm_add_action_or_reset(dev, pwm_fan_regulator_disable,
- ctx->reg_en);
- if (ret)
- return ret;
}
pwm_init_state(ctx->pwm, &ctx->pwm_state);
+ /*
+ * Ensure the PWM is switched on (including the regulator),
+ * independently from any previous PWM state
+ */
+ ctx->pwm_state.enabled = false;
/*
* __set_pwm assumes that MAX_PWM * (period - 1) fits into an unsigned
@@ -348,14 +407,17 @@ static int pwm_fan_probe(struct platform_device *pdev)
return -EINVAL;
}
- /* Set duty cycle to maximum allowed and enable PWM output */
+ /*
+ * Set duty cycle to maximum allowed and enable PWM output as well as
+ * the regulator. In case of error nothing is changed
+ */
ret = __set_pwm(ctx, MAX_PWM);
if (ret) {
dev_err(dev, "Failed to configure PWM: %d\n", ret);
return ret;
}
timer_setup(&ctx->rpm_timer, sample_timer, 0);
- ret = devm_add_action_or_reset(dev, pwm_fan_pwm_disable, ctx);
+ ret = devm_add_action_or_reset(dev, pwm_fan_cleanup, ctx);
if (ret)
return ret;
@@ -461,42 +523,22 @@ static int pwm_fan_probe(struct platform_device *pdev)
return 0;
}
-static int pwm_fan_disable(struct device *dev)
-{
- struct pwm_fan_ctx *ctx = dev_get_drvdata(dev);
- int ret;
-
- if (ctx->pwm_value) {
- /* keep ctx->pwm_state unmodified for pwm_fan_resume() */
- struct pwm_state state = ctx->pwm_state;
-
- state.duty_cycle = 0;
- state.enabled = false;
- ret = pwm_apply_state(ctx->pwm, &state);
- if (ret < 0)
- return ret;
- }
-
- if (ctx->reg_en) {
- ret = regulator_disable(ctx->reg_en);
- if (ret) {
- dev_err(dev, "Failed to disable fan supply: %d\n", ret);
- return ret;
- }
- }
-
- return 0;
-}
-
static void pwm_fan_shutdown(struct platform_device *pdev)
{
- pwm_fan_disable(&pdev->dev);
+ struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev);
+
+ pwm_fan_cleanup(ctx);
}
#ifdef CONFIG_PM_SLEEP
static int pwm_fan_suspend(struct device *dev)
{
- return pwm_fan_disable(dev);
+ struct pwm_fan_ctx *ctx = dev_get_drvdata(dev);
+ int ret;
+
+ ret = pwm_fan_disable(ctx);
+
+ return ret;
}
static int pwm_fan_resume(struct device *dev)
@@ -504,6 +546,9 @@ static int pwm_fan_resume(struct device *dev)
struct pwm_fan_ctx *ctx = dev_get_drvdata(dev);
int ret;
+ if (ctx->pwm_value == 0)
+ return 0;
+
if (ctx->reg_en) {
ret = regulator_enable(ctx->reg_en);
if (ret) {
@@ -512,9 +557,6 @@ static int pwm_fan_resume(struct device *dev)
}
}
- if (ctx->pwm_value == 0)
- return 0;
-
return pwm_apply_state(ctx->pwm, &ctx->pwm_state);
}
#endif
--
2.25.1
