| Message ID | 20170717212811.25374-1-enric.balletbo@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 17/07/17 22:28, Enric Balletbo i Serra wrote: > Before this patch the enable signal was set before the PWM signal and > vice-versa on power off. I guess that this sequence is wrong, at least, > it is on the different panels datasheets that I checked, so I inverted > the sequence to follow the specs. Could you list the part numbers for the panels you checked? Getting that in the git history would be really helpful for future archaeologists (including me). Also whilst changing the header I'd also say that "I guess that" does not inspire much confidence. It sounds like you have done some homework here... surely you've moved past guess work! Daniel. > > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> > --- > Changes since v2: > - Add this as a separate patch (Thierry Reding) > Changes since v1: > - None > > drivers/video/backlight/pwm_bl.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c > index 002f1ce..909a686 100644 > --- a/drivers/video/backlight/pwm_bl.c > +++ b/drivers/video/backlight/pwm_bl.c > @@ -54,10 +54,11 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness) > if (err < 0) > dev_err(pb->dev, "failed to enable power supply\n"); > > + pwm_enable(pb->pwm); > + > if (pb->enable_gpio) > gpiod_set_value_cansleep(pb->enable_gpio, 1); > > - pwm_enable(pb->pwm); > pb->enabled = true; > } > > @@ -66,12 +67,12 @@ static void pwm_backlight_power_off(struct pwm_bl_data *pb) > if (!pb->enabled) > return; > > - pwm_config(pb->pwm, 0, pb->period); > - pwm_disable(pb->pwm); > - > if (pb->enable_gpio) > gpiod_set_value_cansleep(pb->enable_gpio, 0); > > + pwm_config(pb->pwm, 0, pb->period); > + pwm_disable(pb->pwm); > + > regulator_disable(pb->power_supply); > pb->enabled = false; > } >
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index 002f1ce..909a686 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -54,10 +54,11 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness) if (err < 0) dev_err(pb->dev, "failed to enable power supply\n"); + pwm_enable(pb->pwm); + if (pb->enable_gpio) gpiod_set_value_cansleep(pb->enable_gpio, 1); - pwm_enable(pb->pwm); pb->enabled = true; } @@ -66,12 +67,12 @@ static void pwm_backlight_power_off(struct pwm_bl_data *pb) if (!pb->enabled) return; - pwm_config(pb->pwm, 0, pb->period); - pwm_disable(pb->pwm); - if (pb->enable_gpio) gpiod_set_value_cansleep(pb->enable_gpio, 0); + pwm_config(pb->pwm, 0, pb->period); + pwm_disable(pb->pwm); + regulator_disable(pb->power_supply); pb->enabled = false; }
Before this patch the enable signal was set before the PWM signal and vice-versa on power off. I guess that this sequence is wrong, at least, it is on the different panels datasheets that I checked, so I inverted the sequence to follow the specs. Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> --- Changes since v2: - Add this as a separate patch (Thierry Reding) Changes since v1: - None drivers/video/backlight/pwm_bl.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)