| Message ID | 1493039598-25881-1-git-send-email-b.zolnierkie@samsung.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 04/24/2017 06:13 AM, Bartlomiej Zolnierkiewicz wrote: > Switch pwm-fan driver to new atomic PWM API. > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- > Depends on "[PATCH v2 0/3] pwm: pwm-samsung: fix suspend/resume support" > patchset (https://www.spinics.net/lists/kernel/msg2495209.html). > Why would this driver, which does not depend on any Samsung code, depend on a Samsung specific fix ? Was this tested with hardware ? Thanks, Guenter > drivers/hwmon/pwm-fan.c | 68 +++++++++++++++++++------------------------------ > 1 file changed, 26 insertions(+), 42 deletions(-) > > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c > index 9dc40f3..4ac7700 100644 > --- a/drivers/hwmon/pwm-fan.c > +++ b/drivers/hwmon/pwm-fan.c > @@ -40,31 +40,22 @@ struct pwm_fan_ctx { > > static int __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm) > { > - struct pwm_args pargs; > - unsigned long duty; > + unsigned long period; > int ret = 0; > - > - pwm_get_args(ctx->pwm, &pargs); > + struct pwm_state state = { }; > > mutex_lock(&ctx->lock); > if (ctx->pwm_value == pwm) > goto exit_set_pwm_err; > > - duty = DIV_ROUND_UP(pwm * (pargs.period - 1), MAX_PWM); > - ret = pwm_config(ctx->pwm, duty, pargs.period); > - if (ret) > - goto exit_set_pwm_err; > - > - if (pwm == 0) > - pwm_disable(ctx->pwm); > + pwm_init_state(ctx->pwm, &state); > + period = ctx->pwm->args.period; > + state.duty_cycle = DIV_ROUND_UP(pwm * (period - 1), MAX_PWM); > + state.enabled = pwm ? true : false; > > - if (ctx->pwm_value == 0) { > - ret = pwm_enable(ctx->pwm); > - if (ret) > - goto exit_set_pwm_err; > - } > - > - ctx->pwm_value = pwm; > + ret = pwm_apply_state(ctx->pwm, &state); > + if (!ret) > + ctx->pwm_value = pwm; > exit_set_pwm_err: > mutex_unlock(&ctx->lock); > return ret; > @@ -218,10 +209,9 @@ static int pwm_fan_probe(struct platform_device *pdev) > { > struct thermal_cooling_device *cdev; > struct pwm_fan_ctx *ctx; > - struct pwm_args pargs; > struct device *hwmon; > - int duty_cycle; > int ret; > + struct pwm_state state = { }; > > ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); > if (!ctx) > @@ -237,28 +227,16 @@ static int pwm_fan_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, ctx); > > - /* > - * FIXME: pwm_apply_args() should be removed when switching to the > - * atomic PWM API. > - */ > - pwm_apply_args(ctx->pwm); > - > - /* Set duty cycle to maximum allowed */ > - pwm_get_args(ctx->pwm, &pargs); > - > - duty_cycle = pargs.period - 1; > ctx->pwm_value = MAX_PWM; > > - ret = pwm_config(ctx->pwm, duty_cycle, pargs.period); > - if (ret) { > - dev_err(&pdev->dev, "Failed to configure PWM\n"); > - return ret; > - } > + /* Set duty cycle to maximum allowed and enable PWM output */ > + pwm_init_state(ctx->pwm, &state); > + state.duty_cycle = ctx->pwm->args.period - 1; > + state.enabled = true; > > - /* Enbale PWM output */ > - ret = pwm_enable(ctx->pwm); > + ret = pwm_apply_state(ctx->pwm, &state); > if (ret) { > - dev_err(&pdev->dev, "Failed to enable PWM\n"); > + dev_err(&pdev->dev, "Failed to configure PWM\n"); > return ret; > } > > @@ -266,8 +244,8 @@ static int pwm_fan_probe(struct platform_device *pdev) > ctx, pwm_fan_groups); > if (IS_ERR(hwmon)) { > dev_err(&pdev->dev, "Failed to register hwmon device\n"); > - pwm_disable(ctx->pwm); > - return PTR_ERR(hwmon); > + ret = PTR_ERR(hwmon); > + goto err_pwm_disable; > } > > ret = pwm_fan_of_get_cooling_data(&pdev->dev, ctx); > @@ -282,14 +260,20 @@ static int pwm_fan_probe(struct platform_device *pdev) > if (IS_ERR(cdev)) { > dev_err(&pdev->dev, > "Failed to register pwm-fan as cooling device"); > - pwm_disable(ctx->pwm); > - return PTR_ERR(cdev); > + ret = PTR_ERR(cdev); > + goto err_pwm_disable; > } > ctx->cdev = cdev; > thermal_cdev_update(cdev); > } > > return 0; > + > +err_pwm_disable: > + state.enabled = false; > + pwm_apply_state(ctx->pwm, &state); > + > + return ret; > } > > static int pwm_fan_remove(struct platform_device *pdev) > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Monday, April 24, 2017 06:26:12 AM Guenter Roeck wrote: > On 04/24/2017 06:13 AM, Bartlomiej Zolnierkiewicz wrote: > > Switch pwm-fan driver to new atomic PWM API. > > > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > --- > > Depends on "[PATCH v2 0/3] pwm: pwm-samsung: fix suspend/resume support" > > patchset (https://www.spinics.net/lists/kernel/msg2495209.html). > > > > Why would this driver, which does not depend on any Samsung code, > depend on a Samsung specific fix ? The above patchset contains pwm-fan specific patch: "[PATCH v2 3/3] hwmon: pwm-fan: remove no longer needed suspend/resume code" (https://www.spinics.net/lists/kernel/msg2495210.html). It seems that the patchset name itself could be better, sorry for this. > Was this tested with hardware ? Yes, on Exynos5422 based Odroid-XU3 board. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 24, 2017 at 03:13:17PM +0200, Bartlomiej Zolnierkiewicz wrote: > Switch pwm-fan driver to new atomic PWM API. > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- > Depends on "[PATCH v2 0/3] pwm: pwm-samsung: fix suspend/resume support" > patchset (https://www.spinics.net/lists/kernel/msg2495209.html). I lost track where we are with this patch. The above series set did not make it, as far as I can see. Is this patch truly dependent on that series, or can it be applied separately ? It does apply cleanly. Thanks, Guenter > > drivers/hwmon/pwm-fan.c | 68 +++++++++++++++++++------------------------------ > 1 file changed, 26 insertions(+), 42 deletions(-) > > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c > index 9dc40f3..4ac7700 100644 > --- a/drivers/hwmon/pwm-fan.c > +++ b/drivers/hwmon/pwm-fan.c > @@ -40,31 +40,22 @@ struct pwm_fan_ctx { > > static int __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm) > { > - struct pwm_args pargs; > - unsigned long duty; > + unsigned long period; > int ret = 0; > - > - pwm_get_args(ctx->pwm, &pargs); > + struct pwm_state state = { }; > > mutex_lock(&ctx->lock); > if (ctx->pwm_value == pwm) > goto exit_set_pwm_err; > > - duty = DIV_ROUND_UP(pwm * (pargs.period - 1), MAX_PWM); > - ret = pwm_config(ctx->pwm, duty, pargs.period); > - if (ret) > - goto exit_set_pwm_err; > - > - if (pwm == 0) > - pwm_disable(ctx->pwm); > + pwm_init_state(ctx->pwm, &state); > + period = ctx->pwm->args.period; > + state.duty_cycle = DIV_ROUND_UP(pwm * (period - 1), MAX_PWM); > + state.enabled = pwm ? true : false; > > - if (ctx->pwm_value == 0) { > - ret = pwm_enable(ctx->pwm); > - if (ret) > - goto exit_set_pwm_err; > - } > - > - ctx->pwm_value = pwm; > + ret = pwm_apply_state(ctx->pwm, &state); > + if (!ret) > + ctx->pwm_value = pwm; > exit_set_pwm_err: > mutex_unlock(&ctx->lock); > return ret; > @@ -218,10 +209,9 @@ static int pwm_fan_probe(struct platform_device *pdev) > { > struct thermal_cooling_device *cdev; > struct pwm_fan_ctx *ctx; > - struct pwm_args pargs; > struct device *hwmon; > - int duty_cycle; > int ret; > + struct pwm_state state = { }; > > ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); > if (!ctx) > @@ -237,28 +227,16 @@ static int pwm_fan_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, ctx); > > - /* > - * FIXME: pwm_apply_args() should be removed when switching to the > - * atomic PWM API. > - */ > - pwm_apply_args(ctx->pwm); > - > - /* Set duty cycle to maximum allowed */ > - pwm_get_args(ctx->pwm, &pargs); > - > - duty_cycle = pargs.period - 1; > ctx->pwm_value = MAX_PWM; > > - ret = pwm_config(ctx->pwm, duty_cycle, pargs.period); > - if (ret) { > - dev_err(&pdev->dev, "Failed to configure PWM\n"); > - return ret; > - } > + /* Set duty cycle to maximum allowed and enable PWM output */ > + pwm_init_state(ctx->pwm, &state); > + state.duty_cycle = ctx->pwm->args.period - 1; > + state.enabled = true; > > - /* Enbale PWM output */ > - ret = pwm_enable(ctx->pwm); > + ret = pwm_apply_state(ctx->pwm, &state); > if (ret) { > - dev_err(&pdev->dev, "Failed to enable PWM\n"); > + dev_err(&pdev->dev, "Failed to configure PWM\n"); > return ret; > } > > @@ -266,8 +244,8 @@ static int pwm_fan_probe(struct platform_device *pdev) > ctx, pwm_fan_groups); > if (IS_ERR(hwmon)) { > dev_err(&pdev->dev, "Failed to register hwmon device\n"); > - pwm_disable(ctx->pwm); > - return PTR_ERR(hwmon); > + ret = PTR_ERR(hwmon); > + goto err_pwm_disable; > } > > ret = pwm_fan_of_get_cooling_data(&pdev->dev, ctx); > @@ -282,14 +260,20 @@ static int pwm_fan_probe(struct platform_device *pdev) > if (IS_ERR(cdev)) { > dev_err(&pdev->dev, > "Failed to register pwm-fan as cooling device"); > - pwm_disable(ctx->pwm); > - return PTR_ERR(cdev); > + ret = PTR_ERR(cdev); > + goto err_pwm_disable; > } > ctx->cdev = cdev; > thermal_cdev_update(cdev); > } > > return 0; > + > +err_pwm_disable: > + state.enabled = false; > + pwm_apply_state(ctx->pwm, &state); > + > + return ret; > } > > static int pwm_fan_remove(struct platform_device *pdev) -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday, June 02, 2017 06:23:53 AM Guenter Roeck wrote: > On Mon, Apr 24, 2017 at 03:13:17PM +0200, Bartlomiej Zolnierkiewicz wrote: > > Switch pwm-fan driver to new atomic PWM API. > > > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > --- > > Depends on "[PATCH v2 0/3] pwm: pwm-samsung: fix suspend/resume support" > > patchset (https://www.spinics.net/lists/kernel/msg2495209.html). > > I lost track where we are with this patch. The above series set did not make > it, as far as I can see. Is this patch truly dependent on that series, or > can it be applied separately ? It does apply cleanly. I think that it can be applied separately (the broken suspend/resume code will continue to use the old API until the previous patchset is finally merged and the code in question is removed altogether). Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 07, 2017 at 03:38:44PM +0200, Bartlomiej Zolnierkiewicz wrote: > On Friday, June 02, 2017 06:23:53 AM Guenter Roeck wrote: > > On Mon, Apr 24, 2017 at 03:13:17PM +0200, Bartlomiej Zolnierkiewicz wrote: > > > Switch pwm-fan driver to new atomic PWM API. > > > > > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > > --- > > > Depends on "[PATCH v2 0/3] pwm: pwm-samsung: fix suspend/resume support" > > > patchset (https://www.spinics.net/lists/kernel/msg2495209.html). > > > > I lost track where we are with this patch. The above series set did not make > > it, as far as I can see. Is this patch truly dependent on that series, or > > can it be applied separately ? It does apply cleanly. > > I think that it can be applied separately (the broken suspend/resume code > will continue to use the old API until the previous patchset is finally > merged and the code in question is removed altogether). > Ok. Applied to hwmon-next. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c index 9dc40f3..4ac7700 100644 --- a/drivers/hwmon/pwm-fan.c +++ b/drivers/hwmon/pwm-fan.c @@ -40,31 +40,22 @@ struct pwm_fan_ctx { static int __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm) { - struct pwm_args pargs; - unsigned long duty; + unsigned long period; int ret = 0; - - pwm_get_args(ctx->pwm, &pargs); + struct pwm_state state = { }; mutex_lock(&ctx->lock); if (ctx->pwm_value == pwm) goto exit_set_pwm_err; - duty = DIV_ROUND_UP(pwm * (pargs.period - 1), MAX_PWM); - ret = pwm_config(ctx->pwm, duty, pargs.period); - if (ret) - goto exit_set_pwm_err; - - if (pwm == 0) - pwm_disable(ctx->pwm); + pwm_init_state(ctx->pwm, &state); + period = ctx->pwm->args.period; + state.duty_cycle = DIV_ROUND_UP(pwm * (period - 1), MAX_PWM); + state.enabled = pwm ? true : false; - if (ctx->pwm_value == 0) { - ret = pwm_enable(ctx->pwm); - if (ret) - goto exit_set_pwm_err; - } - - ctx->pwm_value = pwm; + ret = pwm_apply_state(ctx->pwm, &state); + if (!ret) + ctx->pwm_value = pwm; exit_set_pwm_err: mutex_unlock(&ctx->lock); return ret; @@ -218,10 +209,9 @@ static int pwm_fan_probe(struct platform_device *pdev) { struct thermal_cooling_device *cdev; struct pwm_fan_ctx *ctx; - struct pwm_args pargs; struct device *hwmon; - int duty_cycle; int ret; + struct pwm_state state = { }; ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); if (!ctx) @@ -237,28 +227,16 @@ static int pwm_fan_probe(struct platform_device *pdev) platform_set_drvdata(pdev, ctx); - /* - * FIXME: pwm_apply_args() should be removed when switching to the - * atomic PWM API. - */ - pwm_apply_args(ctx->pwm); - - /* Set duty cycle to maximum allowed */ - pwm_get_args(ctx->pwm, &pargs); - - duty_cycle = pargs.period - 1; ctx->pwm_value = MAX_PWM; - ret = pwm_config(ctx->pwm, duty_cycle, pargs.period); - if (ret) { - dev_err(&pdev->dev, "Failed to configure PWM\n"); - return ret; - } + /* Set duty cycle to maximum allowed and enable PWM output */ + pwm_init_state(ctx->pwm, &state); + state.duty_cycle = ctx->pwm->args.period - 1; + state.enabled = true; - /* Enbale PWM output */ - ret = pwm_enable(ctx->pwm); + ret = pwm_apply_state(ctx->pwm, &state); if (ret) { - dev_err(&pdev->dev, "Failed to enable PWM\n"); + dev_err(&pdev->dev, "Failed to configure PWM\n"); return ret; } @@ -266,8 +244,8 @@ static int pwm_fan_probe(struct platform_device *pdev) ctx, pwm_fan_groups); if (IS_ERR(hwmon)) { dev_err(&pdev->dev, "Failed to register hwmon device\n"); - pwm_disable(ctx->pwm); - return PTR_ERR(hwmon); + ret = PTR_ERR(hwmon); + goto err_pwm_disable; } ret = pwm_fan_of_get_cooling_data(&pdev->dev, ctx); @@ -282,14 +260,20 @@ static int pwm_fan_probe(struct platform_device *pdev) if (IS_ERR(cdev)) { dev_err(&pdev->dev, "Failed to register pwm-fan as cooling device"); - pwm_disable(ctx->pwm); - return PTR_ERR(cdev); + ret = PTR_ERR(cdev); + goto err_pwm_disable; } ctx->cdev = cdev; thermal_cdev_update(cdev); } return 0; + +err_pwm_disable: + state.enabled = false; + pwm_apply_state(ctx->pwm, &state); + + return ret; } static int pwm_fan_remove(struct platform_device *pdev)
Switch pwm-fan driver to new atomic PWM API. Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> --- Depends on "[PATCH v2 0/3] pwm: pwm-samsung: fix suspend/resume support" patchset (https://www.spinics.net/lists/kernel/msg2495209.html). drivers/hwmon/pwm-fan.c | 68 +++++++++++++++++++------------------------------ 1 file changed, 26 insertions(+), 42 deletions(-)