| Message ID | 20220523110513.407516-4-alexander.stein@ew.tq-group.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: pwm-fan: switch regulator dynamically | expand |
On Mon, May 23, 2022 at 01:05:10PM +0200, Alexander Stein wrote: > This handles enabling/disabling the regulator in a single function, while > keeping the enables/disabled balanced. This is a preparation when > regulator is switched from different code paths. > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > --- > drivers/hwmon/pwm-fan.c | 52 +++++++++++++++++++++++++++++------------ > 1 file changed, 37 insertions(+), 15 deletions(-) > > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c > index 96b10d422828..04af24268963 100644 > --- a/drivers/hwmon/pwm-fan.c > +++ b/drivers/hwmon/pwm-fan.c > @@ -35,6 +35,7 @@ struct pwm_fan_ctx { > struct pwm_device *pwm; > struct pwm_state pwm_state; > struct regulator *reg_en; > + bool regulator_enabled; > bool enabled; > > int tach_count; > @@ -85,6 +86,29 @@ static void sample_timer(struct timer_list *t) > mod_timer(&ctx->rpm_timer, jiffies + HZ); > } > > +static int pwm_fan_switch_power(struct pwm_fan_ctx *ctx, bool on) > +{ > + int ret = 0; > + > + if (!ctx->reg_en) > + return ret; > + > + if (ctx->regulator_enabled && on) { > + ret = 0; ret is already 0 here. > + } else if (!ctx->regulator_enabled && on) { > + ret = regulator_enable(ctx->reg_en); > + if (ret == 0) > + ctx->regulator_enabled = true; > + } else if (ctx->regulator_enabled && !on) { > + ret = regulator_disable(ctx->reg_en); > + if (ret == 0) > + ctx->regulator_enabled = false; > + } else if (!ctx->regulator_enabled && !on) { > + ret = 0; ret is already 0 here. > + } > + return ret; > +} > + > static int pwm_fan_power_on(struct pwm_fan_ctx *ctx) > { > struct pwm_state *state = &ctx->pwm_state; > @@ -316,7 +340,9 @@ static int pwm_fan_of_get_cooling_data(struct device *dev, > > static void pwm_fan_regulator_disable(void *data) > { > - regulator_disable(data); > + struct pwm_fan_ctx *ctx = data; > + > + pwm_fan_switch_power(ctx, false); You can directly pass 'data' as argument here; there is no need for the extra variable. > } > > static void pwm_fan_pwm_disable(void *__ctx) > @@ -360,13 +386,13 @@ static int pwm_fan_probe(struct platform_device *pdev) > > ctx->reg_en = NULL; > } else { > - ret = regulator_enable(ctx->reg_en); > + ret = pwm_fan_switch_power(ctx, true); > 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); > + ctx); > if (ret) > return ret; > } > @@ -512,12 +538,10 @@ static int pwm_fan_disable(struct device *dev) > 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; > - } > + ret = pwm_fan_switch_power(ctx, false); > + if (ret) { > + dev_err(dev, "Failed to disable fan supply: %d\n", ret); > + return ret; > } > > return 0; > @@ -539,12 +563,10 @@ static int pwm_fan_resume(struct device *dev) > struct pwm_fan_ctx *ctx = dev_get_drvdata(dev); > int ret; > > - if (ctx->reg_en) { > - ret = regulator_enable(ctx->reg_en); > - if (ret) { > - dev_err(dev, "Failed to enable fan supply: %d\n", ret); > - return ret; > - } > + ret = pwm_fan_switch_power(ctx, true); > + if (ret) { > + dev_err(dev, "Failed to enable fan supply: %d\n", ret); > + return ret; > } > > if (ctx->pwm_value == 0)
Hello Guenter, thanks for your review. Am Dienstag, 30. August 2022, 15:50:13 CEST schrieb Guenter Roeck: > On Mon, May 23, 2022 at 01:05:10PM +0200, Alexander Stein wrote: > > This handles enabling/disabling the regulator in a single function, while > > keeping the enables/disabled balanced. This is a preparation when > > regulator is switched from different code paths. > > > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > > --- > > > > drivers/hwmon/pwm-fan.c | 52 +++++++++++++++++++++++++++++------------ > > 1 file changed, 37 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c > > index 96b10d422828..04af24268963 100644 > > --- a/drivers/hwmon/pwm-fan.c > > +++ b/drivers/hwmon/pwm-fan.c > > @@ -35,6 +35,7 @@ struct pwm_fan_ctx { > > > > struct pwm_device *pwm; > > struct pwm_state pwm_state; > > struct regulator *reg_en; > > > > + bool regulator_enabled; > > > > bool enabled; > > > > int tach_count; > > > > @@ -85,6 +86,29 @@ static void sample_timer(struct timer_list *t) > > > > mod_timer(&ctx->rpm_timer, jiffies + HZ); > > > > } > > > > +static int pwm_fan_switch_power(struct pwm_fan_ctx *ctx, bool on) > > +{ > > + int ret = 0; > > + > > + if (!ctx->reg_en) > > + return ret; > > + > > + if (ctx->regulator_enabled && on) { > > + ret = 0; > > ret is already 0 here. > > > + } else if (!ctx->regulator_enabled && on) { > > + ret = regulator_enable(ctx->reg_en); > > + if (ret == 0) > > + ctx->regulator_enabled = true; > > + } else if (ctx->regulator_enabled && !on) { > > + ret = regulator_disable(ctx->reg_en); > > + if (ret == 0) > > + ctx->regulator_enabled = false; > > + } else if (!ctx->regulator_enabled && !on) { > > + ret = 0; > > ret is already 0 here. Ok, I'll remove both branches, setting ret to 0 again. I just wanted to keep the branches for all possibilities, but no heard feelings here. > > + } > > + return ret; > > +} > > + > > > > static int pwm_fan_power_on(struct pwm_fan_ctx *ctx) > > { > > > > struct pwm_state *state = &ctx->pwm_state; > > > > @@ -316,7 +340,9 @@ static int pwm_fan_of_get_cooling_data(struct device > > *dev,> > > static void pwm_fan_regulator_disable(void *data) > > { > > > > - regulator_disable(data); > > + struct pwm_fan_ctx *ctx = data; > > + > > + pwm_fan_switch_power(ctx, false); > > You can directly pass 'data' as argument here; there is no need > for the extra variable. Nice, thanks. Will do so. Best regards, Alexander > > } > > > > static void pwm_fan_pwm_disable(void *__ctx) > > > > @@ -360,13 +386,13 @@ static int pwm_fan_probe(struct platform_device > > *pdev)> > > ctx->reg_en = NULL; > > > > } else { > > > > - ret = regulator_enable(ctx->reg_en); > > + ret = pwm_fan_switch_power(ctx, true); > > > > 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); > > + ctx); > > > > if (ret) > > > > return ret; > > > > } > > > > @@ -512,12 +538,10 @@ static int pwm_fan_disable(struct device *dev) > > > > 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; > > - } > > + ret = pwm_fan_switch_power(ctx, false); > > + if (ret) { > > + dev_err(dev, "Failed to disable fan supply: %d\n", ret); > > + return ret; > > > > } > > > > return 0; > > > > @@ -539,12 +563,10 @@ static int pwm_fan_resume(struct device *dev) > > > > struct pwm_fan_ctx *ctx = dev_get_drvdata(dev); > > int ret; > > > > - if (ctx->reg_en) { > > - ret = regulator_enable(ctx->reg_en); > > - if (ret) { > > - dev_err(dev, "Failed to enable fan supply: %d\n", ret); > > - return ret; > > - } > > + ret = pwm_fan_switch_power(ctx, true); > > + if (ret) { > > + dev_err(dev, "Failed to enable fan supply: %d\n", ret); > > + return ret; > > > > } > > > > if (ctx->pwm_value == 0)
diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c index 96b10d422828..04af24268963 100644 --- a/drivers/hwmon/pwm-fan.c +++ b/drivers/hwmon/pwm-fan.c @@ -35,6 +35,7 @@ struct pwm_fan_ctx { struct pwm_device *pwm; struct pwm_state pwm_state; struct regulator *reg_en; + bool regulator_enabled; bool enabled; int tach_count; @@ -85,6 +86,29 @@ static void sample_timer(struct timer_list *t) mod_timer(&ctx->rpm_timer, jiffies + HZ); } +static int pwm_fan_switch_power(struct pwm_fan_ctx *ctx, bool on) +{ + int ret = 0; + + if (!ctx->reg_en) + return ret; + + if (ctx->regulator_enabled && on) { + ret = 0; + } else if (!ctx->regulator_enabled && on) { + ret = regulator_enable(ctx->reg_en); + if (ret == 0) + ctx->regulator_enabled = true; + } else if (ctx->regulator_enabled && !on) { + ret = regulator_disable(ctx->reg_en); + if (ret == 0) + ctx->regulator_enabled = false; + } else if (!ctx->regulator_enabled && !on) { + ret = 0; + } + return ret; +} + static int pwm_fan_power_on(struct pwm_fan_ctx *ctx) { struct pwm_state *state = &ctx->pwm_state; @@ -316,7 +340,9 @@ static int pwm_fan_of_get_cooling_data(struct device *dev, static void pwm_fan_regulator_disable(void *data) { - regulator_disable(data); + struct pwm_fan_ctx *ctx = data; + + pwm_fan_switch_power(ctx, false); } static void pwm_fan_pwm_disable(void *__ctx) @@ -360,13 +386,13 @@ static int pwm_fan_probe(struct platform_device *pdev) ctx->reg_en = NULL; } else { - ret = regulator_enable(ctx->reg_en); + ret = pwm_fan_switch_power(ctx, true); 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); + ctx); if (ret) return ret; } @@ -512,12 +538,10 @@ static int pwm_fan_disable(struct device *dev) 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; - } + ret = pwm_fan_switch_power(ctx, false); + if (ret) { + dev_err(dev, "Failed to disable fan supply: %d\n", ret); + return ret; } return 0; @@ -539,12 +563,10 @@ static int pwm_fan_resume(struct device *dev) struct pwm_fan_ctx *ctx = dev_get_drvdata(dev); int ret; - if (ctx->reg_en) { - ret = regulator_enable(ctx->reg_en); - if (ret) { - dev_err(dev, "Failed to enable fan supply: %d\n", ret); - return ret; - } + ret = pwm_fan_switch_power(ctx, true); + if (ret) { + dev_err(dev, "Failed to enable fan supply: %d\n", ret); + return ret; } if (ctx->pwm_value == 0)
This handles enabling/disabling the regulator in a single function, while keeping the enables/disabled balanced. This is a preparation when regulator is switched from different code paths. Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> --- drivers/hwmon/pwm-fan.c | 52 +++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 15 deletions(-)