Skip to contentSkip to conversation

backlight: lp855x: Fixes after c1ff7da03e16

PATCH Applied Artur Weber 3 participants 6 messages Jul 14, 2023 – Jul 28, 2023

Subsystems (3)
Framebuffer layer PWM Backlight class/subsystem
lore
Share & tools
lore

Artur Weber

author
#0 lore raw

cover backlight: lp855x: Fixes after c1ff7da03e16

Two small fixes after commit c1ff7da03e16 ("video: backlight: lp855x:
Get PWM for PWM mode during probe"), stemming from a review[1] by Uwe Kleine-König.

[1] https://lore.kernel.org/all/[email protected]/

Signed-off-by Artur Weber <[email protected]>

Artur Weber (2):
backlight: lp855x: Initialize PWM state on first brightness change
backlight: lp855x: Catch errors when changing brightness

 drivers/video/backlight/lp855x_bl.c | 33 +++++++++++++++++------------
 1 file changed, 20 insertions(+), 13 deletions(-)

base-commit: 7fcd473a6455450428795d20db7afd2691c92336

Signature · 1 line
2.41.0
author
#1 lore raw

PATCH 1/2 backlight: lp855x: Initialize PWM state on first brightness change

As pointed out by Uwe Kleine-König[1], the changes introduced in commit c1ff7da03e16 ("video: backlight: lp855x: Get PWM for PWM mode during probe") caused the PWM state set up by the bootloader to be re-set when the driver is probed. This differs from the behavior from before that patch, where the PWM state would be initialized on the first brightness change.

Fix this by moving the PWM state initialization into the PWM control function. Add a new variable, needs_pwm_init, to the device info struct to allow us to check whether we need the initialization, or whether it has already been done.

[1] https://lore.kernel.org/lkml/[email protected]/

Fixes c1ff7da03e16 ("video: backlight: lp855x: Get PWM for PWM mode during probe")
Signed-off-by Artur Weber <[email protected]>
---
 drivers/video/backlight/lp855x_bl.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)
drivers/video/backlight/lp855x_bl.c +12 -8
index 1c9e921bca14..349ec324bc1e 100644
@@ -71,6 +71,7 @@ struct lp855x {
 	struct device *dev;
 	struct lp855x_platform_data *pdata;
 	struct pwm_device *pwm;
+	bool needs_pwm_init;
 	struct regulator *supply;	/* regulator for VDD input */
 	struct regulator *enable;	/* regulator for EN/VDDIO input */
 };

@@ -220,7 +221,15 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
 {
 	struct pwm_state state;
 
-	pwm_get_state(lp->pwm, &state);
+	if (lp->needs_pwm_init) {
+		pwm_init_state(lp->pwm, &state);
+		/* Legacy platform data compatibility */
+		if (lp->pdata->period_ns > 0)
+			state.period = lp->pdata->period_ns;
+		lp->needs_pwm_init = false;
+	} else {
+		pwm_get_state(lp->pwm, &state);
+	}
 
 	state.duty_cycle = div_u64(br * state.period, max_br);
 	state.enabled = state.duty_cycle;

@@ -387,7 +396,6 @@ static int lp855x_probe(struct i2c_client *cl)
 	const struct i2c_device_id *id = i2c_client_get_device_id(cl);
 	const struct acpi_device_id *acpi_id = NULL;
 	struct device *dev = &cl->dev;
-	struct pwm_state pwmstate;
 	struct lp855x *lp;
 	int ret;
 

@@ -470,15 +478,11 @@ static int lp855x_probe(struct i2c_client *cl)
 		else
 			return dev_err_probe(dev, ret, "getting PWM\n");
 
+		lp->needs_pwm_init = false;
 		lp->mode = REGISTER_BASED;
 		dev_dbg(dev, "mode: register based\n");
 	} else {
-		pwm_init_state(lp->pwm, &pwmstate);
-		/* Legacy platform data compatibility */
-		if (lp->pdata->period_ns > 0)
-			pwmstate.period = lp->pdata->period_ns;
-		pwm_apply_state(lp->pwm, &pwmstate);
-
+		lp->needs_pwm_init = true;
 		lp->mode = PWM_BASED;
 		dev_dbg(dev, "mode: PWM based\n");
 	}
Signature · 1 line
2.41.0
Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] backlight: lp855x: Initialize PWM state on first brightness change

Quoted 12 lines · Artur Weber

As pointed out by Uwe Kleine-König[1], the changes introduced in commit c1ff7da03e16 ("video: backlight: lp855x: Get PWM for PWM mode during probe") caused the PWM state set up by the bootloader to be re-set when the driver is probed. This differs from the behavior from before that patch, where the PWM state would be initialized on the first brightness change.

Fix this by moving the PWM state initialization into the PWM control function. Add a new variable, needs_pwm_init, to the device info struct to allow us to check whether we need the initialization, or whether it has already been done.

Quoted 4 lines

[1] https://lore.kernel.org/lkml/[email protected]/

Fixes c1ff7da03e16 ("video: backlight: lp855x: Get PWM for PWM mode during probe")
Signed-off-by Artur Weber <[email protected]>
Reviewed-by Daniel Thompson <[email protected]>
author
#3 lore raw

PATCH 2/2 backlight: lp855x: Catch errors when changing brightness

In reply to Artur Weber

The lp855x_bl_update_status function's return type is int, but it always returns 0, without checking for the results of the write_byte/pwm_ctrl functions called within.

Make this function return the return values of the functions it calls, and modify the lp855x_pwm_ctrl function to return errors.

Signed-off-by Artur Weber <[email protected]>
---
 drivers/video/backlight/lp855x_bl.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
drivers/video/backlight/lp855x_bl.c +8 -5
index 349ec324bc1e..61a7f45bfad8 100644
@@ -217,7 +217,7 @@ static int lp855x_configure(struct lp855x *lp)
 	return ret;
 }
 
-static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
+static int lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
 {
 	struct pwm_state state;
 

@@ -234,23 +234,26 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
 	state.duty_cycle = div_u64(br * state.period, max_br);
 	state.enabled = state.duty_cycle;
 
-	pwm_apply_state(lp->pwm, &state);
+	return pwm_apply_state(lp->pwm, &state);
 }
 
 static int lp855x_bl_update_status(struct backlight_device *bl)
 {
 	struct lp855x *lp = bl_get_data(bl);
 	int brightness = bl->props.brightness;
+	int ret;
 
 	if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
 		brightness = 0;
 
 	if (lp->mode == PWM_BASED)
-		lp855x_pwm_ctrl(lp, brightness, bl->props.max_brightness);
+		ret = lp855x_pwm_ctrl(lp, brightness,
+				      bl->props.max_brightness);
 	else if (lp->mode == REGISTER_BASED)
-		lp855x_write_byte(lp, lp->cfg->reg_brightness, (u8)brightness);
+		ret = lp855x_write_byte(lp, lp->cfg->reg_brightness,
+					(u8)brightness);
 
-	return 0;
+	return ret;
 }
 
 static const struct backlight_ops lp855x_bl_ops = {
Signature · 1 line
2.41.0
Reviewed-by on v1 2/2
#4 lore raw

Re: [PATCH 2/2] backlight: lp855x: Catch errors when changing brightness

Quoted 8 lines · Artur Weber

The lp855x_bl_update_status function's return type is int, but it always returns 0, without checking for the results of the write_byte/pwm_ctrl functions called within.

Make this function return the return values of the functions it calls, and modify the lp855x_pwm_ctrl function to return errors.

Signed-off-by Artur Weber <[email protected]>
Reviewed-by Daniel Thompson <[email protected]>
maintainer Applied
#5 lore raw
In reply to Artur Weber
Quoted 9 lines · Artur Weber

Two small fixes after commit c1ff7da03e16 ("video: backlight: lp855x:
Get PWM for PWM mode during probe"), stemming from a review[1] by Uwe Kleine-König.

[1] https://lore.kernel.org/all/[email protected]/

Signed-off-by Artur Weber <[email protected]>

[...]

Applied, thanks!

[1/2] backlight: lp855x: Initialize PWM state on first brightness change

      commit: 4c09e20b3c85f60353ace21092e34f35f5e3ab00
[2/2] backlight: lp855x: Catch errors when changing brightness
      commit: 5145531be5fbad0e914d1dc1cbd392d7b756abaa

--
Lee Jones [李琼斯]