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