| Message ID | YYLI/b7KcqM8wcEB@fedora (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5] backlight: lp855x: Switch to atomic PWM API | expand |
On Wed, Nov 03, 2021 at 02:38:05PM -0300, Maíra Canal wrote: > Remove legacy PWM interface (pwm_config, pwm_enable, pwm_disable) and > replace it for the atomic PWM API. > > Signed-off-by: Maíra Canal <maira.canal@usp.br> > --- > V1 -> V2: Initialize variable and simplify conditional loop > V2 -> V3: Fix assignment of NULL variable > V3 -> V4: Replace division for pwm_set_relative_duty_cycle > V4 -> V5: Fix overwrite of state.period > --- > drivers/video/backlight/lp855x_bl.c | 21 +++++++++------------ > 1 file changed, 9 insertions(+), 12 deletions(-) > > diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c > index e94932c69f54..e70a7b72dcf3 100644 > --- a/drivers/video/backlight/lp855x_bl.c > +++ b/drivers/video/backlight/lp855x_bl.c > @@ -233,9 +233,8 @@ static int lp855x_configure(struct lp855x *lp) > > static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br) > { > - unsigned int period = lp->pdata->period_ns; > - unsigned int duty = br * period / max_br; > struct pwm_device *pwm; > + struct pwm_state state; > > /* request pwm device with the consumer name */ > if (!lp->pwm) { > @@ -245,18 +244,16 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br) > > lp->pwm = pwm; > > - /* > - * FIXME: pwm_apply_args() should be removed when switching to > - * the atomic PWM API. > - */ > - pwm_apply_args(pwm); > + pwm_init_state(lp->pwm, &state); > + state.period = lp->pdata->period_ns; > + } else { > + pwm_get_state(lp->pwm, &state); > } > > - pwm_config(lp->pwm, duty, period); > - if (duty) > - pwm_enable(lp->pwm); > - else > - pwm_disable(lp->pwm); > + pwm_set_relative_duty_cycle(&state, br, max_br); > + state.enabled = state.duty_cycle; > + > + pwm_apply_state(lp->pwm, &state); Looks mostly right, but only on a deeper look into the driver. The reason is that in the unmodified code there is always explicitly period=lp->pdata->period_ns; while after your change the period is unset (and so the previously set period is used). So either mention in the change log that this driver doesn't modify the pwm settings in other places or preferably pick an equivalent conversion (plus maybe an optimisation in a separate patch). If you go the "equivalent conversion" path, please note that pwm_set_relative_duty_cycle() isn't equivalent to br * period / max_br, as it implements a different rounding. Best regards Uwe
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c index e94932c69f54..e70a7b72dcf3 100644 --- a/drivers/video/backlight/lp855x_bl.c +++ b/drivers/video/backlight/lp855x_bl.c @@ -233,9 +233,8 @@ static int lp855x_configure(struct lp855x *lp) static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br) { - unsigned int period = lp->pdata->period_ns; - unsigned int duty = br * period / max_br; struct pwm_device *pwm; + struct pwm_state state; /* request pwm device with the consumer name */ if (!lp->pwm) { @@ -245,18 +244,16 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br) lp->pwm = pwm; - /* - * FIXME: pwm_apply_args() should be removed when switching to - * the atomic PWM API. - */ - pwm_apply_args(pwm); + pwm_init_state(lp->pwm, &state); + state.period = lp->pdata->period_ns; + } else { + pwm_get_state(lp->pwm, &state); } - pwm_config(lp->pwm, duty, period); - if (duty) - pwm_enable(lp->pwm); - else - pwm_disable(lp->pwm); + pwm_set_relative_duty_cycle(&state, br, max_br); + state.enabled = state.duty_cycle; + + pwm_apply_state(lp->pwm, &state); } static int lp855x_bl_update_status(struct backlight_device *bl)
Remove legacy PWM interface (pwm_config, pwm_enable, pwm_disable) and replace it for the atomic PWM API. Signed-off-by: Maíra Canal <maira.canal@usp.br> --- V1 -> V2: Initialize variable and simplify conditional loop V2 -> V3: Fix assignment of NULL variable V3 -> V4: Replace division for pwm_set_relative_duty_cycle V4 -> V5: Fix overwrite of state.period --- drivers/video/backlight/lp855x_bl.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-)