diff mbox series

[U-Boot] video: backlight: fix pwm invertation

Message ID ae1ec2f3052437d037a9698a232f4d14bf6ff0cb.1561145026.git.marvin24@gmx.de
State Superseded
Delegated to: Anatolij Gustschin
Headers show
Series [U-Boot] video: backlight: fix pwm invertation | expand

Commit Message

Marc Dietrich June 21, 2019, 8:01 p.m. UTC
Fixes: 57e7775413 ("video: backlight: Parse PWM polarity cell")

set_pwm will always fail if pwm_set_invert is not implemented, leaving the
backlight dark. Fix this by calling pwm_set_invert only if pwm is inverted.

Signed-off-by: Marc Dietrich <marvin24@gmx.de>
---
 drivers/video/pwm_backlight.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

--
2.17.1

Comments

Marc Dietrich June 27, 2019, 7:51 p.m. UTC | #1
Am Freitag, 21. Juni 2019, 22:01:35 CEST schrieb Marc Dietrich:
> Fixes: 57e7775413 ("video: backlight: Parse PWM polarity cell")
> 
> set_pwm will always fail if pwm_set_invert is not implemented, leaving the
> backlight dark. Fix this by calling pwm_set_invert only if pwm is inverted.
> 
> Signed-off-by: Marc Dietrich <marvin24@gmx.de>

ping?

> ---
>  drivers/video/pwm_backlight.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/pwm_backlight.c b/drivers/video/pwm_backlight.c
> index a587977c22..26bcbeb42b 100644
> --- a/drivers/video/pwm_backlight.c
> +++ b/drivers/video/pwm_backlight.c
> @@ -61,12 +61,13 @@ static int set_pwm(struct pwm_backlight_priv *priv)
> 
>  	duty_cycle = priv->period_ns * (priv->cur_level - priv->min_level) /
>  		(priv->max_level - priv->min_level + 1);
> +
>  	ret = pwm_set_config(priv->pwm, priv->channel, priv->period_ns,
>  			     duty_cycle);
> -	if (ret)
> -		return log_ret(ret);
> 
> -	ret = pwm_set_invert(priv->pwm, priv->channel, priv->polarity);
> +	if (!ret && priv->polarity)
> +		ret = pwm_set_invert(priv->pwm, priv->channel, priv->polarity);
> +
>  	return log_ret(ret);
>  }
> 
> --
> 2.17.1
Nicolas Chauvet July 2, 2019, 6:12 a.m. UTC | #2
This patch fixes the panel on my device (paz00).
Can it be applied to 2019-07 ?

Thx in advance


Le jeu. 27 juin 2019 à 21:52, Marc Dietrich <marvin24@gmx.de> a écrit :
>
> Am Freitag, 21. Juni 2019, 22:01:35 CEST schrieb Marc Dietrich:
> > Fixes: 57e7775413 ("video: backlight: Parse PWM polarity cell")
> >
> > set_pwm will always fail if pwm_set_invert is not implemented, leaving the
> > backlight dark. Fix this by calling pwm_set_invert only if pwm is inverted.
> >
> > Signed-off-by: Marc Dietrich <marvin24@gmx.de>
>
> ping?
>
> > ---
> >  drivers/video/pwm_backlight.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/video/pwm_backlight.c b/drivers/video/pwm_backlight.c
> > index a587977c22..26bcbeb42b 100644
> > --- a/drivers/video/pwm_backlight.c
> > +++ b/drivers/video/pwm_backlight.c
> > @@ -61,12 +61,13 @@ static int set_pwm(struct pwm_backlight_priv *priv)
> >
> >       duty_cycle = priv->period_ns * (priv->cur_level - priv->min_level) /
> >               (priv->max_level - priv->min_level + 1);
> > +
> >       ret = pwm_set_config(priv->pwm, priv->channel, priv->period_ns,
> >                            duty_cycle);
> > -     if (ret)
> > -             return log_ret(ret);
> >
> > -     ret = pwm_set_invert(priv->pwm, priv->channel, priv->polarity);
> > +     if (!ret && priv->polarity)
> > +             ret = pwm_set_invert(priv->pwm, priv->channel, priv->polarity);
> > +
> >       return log_ret(ret);
> >  }
> >
> > --
> > 2.17.1
>
>
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot



--
-

Nicolas (kwizart)
Stefan Mavrodiev July 2, 2019, 7:40 a.m. UTC | #3
Hi,

On 6/27/19 9:56 PM, marvin24@posteo.de wrote:
> Am Freitag, 21. Juni 2019, 22:01:35 CEST schrieb Marc Dietrich:
>> Fixes: 57e7775413 ("video: backlight: Parse PWM polarity cell")
>>
>> set_pwm will always fail if pwm_set_invert is not implemented, leaving the
>> backlight dark. Fix this by calling pwm_set_invert only if pwm is inverted.
I'm not sure if this is true. pwm_set_invert() checks for .set_invert.
Which pwm driver you're using?


>>
>> Signed-off-by: Marc Dietrich <marvin24@gmx.de>
> ping?
>
>
>> ---
>>   drivers/video/pwm_backlight.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/pwm_backlight.c b/drivers/video/pwm_backlight.c
>> index a587977c22..26bcbeb42b 100644
>> --- a/drivers/video/pwm_backlight.c
>> +++ b/drivers/video/pwm_backlight.c
>> @@ -61,12 +61,13 @@ static int set_pwm(struct pwm_backlight_priv *priv)
>>
>>   	duty_cycle = priv->period_ns * (priv->cur_level - priv->min_level) /
>>   		(priv->max_level - priv->min_level + 1);
>> +
>>   	ret = pwm_set_config(priv->pwm, priv->channel, priv->period_ns,
>>   			     duty_cycle);
>> -	if (ret)
>> -		return log_ret(ret);
>>
>> -	ret = pwm_set_invert(priv->pwm, priv->channel, priv->polarity);
>> +	if (!ret && priv->polarity)
>> +		ret = pwm_set_invert(priv->pwm, priv->channel, priv->polarity);
This shouldn't be a problem. Rather the driver doesn't handle polarity 
correctly.
>> +
>>   	return log_ret(ret);
>>   }
>>
>> --
>> 2.17.1

P.S.

You should cc to all maintainers. You can use ./scripts/get_maintainer.pl.


Best regards,
Stefan Mavrodiev
Nicolas Chauvet July 2, 2019, 8:42 a.m. UTC | #4
Le mar. 2 juil. 2019 à 09:41, Stefan Mavrodiev <stefan@olimex.com> a écrit :
>
> Hi,
>
> On 6/27/19 9:56 PM, marvin24@posteo.de wrote:
> > Am Freitag, 21. Juni 2019, 22:01:35 CEST schrieb Marc Dietrich:
> >> Fixes: 57e7775413 ("video: backlight: Parse PWM polarity cell")
> >>
> >> set_pwm will always fail if pwm_set_invert is not implemented, leaving the
> >> backlight dark. Fix this by calling pwm_set_invert only if pwm is inverted.
> I'm not sure if this is true. pwm_set_invert() checks for .set_invert.
> Which pwm driver you're using?
This is drivers/pwm/pwm_tegra.c
Here .set_invert is not implemented.

Thx for the review.
Stefan Mavrodiev July 2, 2019, 9:05 a.m. UTC | #5
Hi,

On 7/2/19 11:42 AM, Nicolas Chauvet wrote:
> Le mar. 2 juil. 2019 à 09:41, Stefan Mavrodiev <stefan@olimex.com> a écrit :
>> Hi,
>>
>> On 6/27/19 9:56 PM, marvin24@posteo.de wrote:
>>> Am Freitag, 21. Juni 2019, 22:01:35 CEST schrieb Marc Dietrich:
>>>> Fixes: 57e7775413 ("video: backlight: Parse PWM polarity cell")
>>>>
>>>> set_pwm will always fail if pwm_set_invert is not implemented, leaving the
>>>> backlight dark. Fix this by calling pwm_set_invert only if pwm is inverted.
>> I'm not sure if this is true. pwm_set_invert() checks for .set_invert.
>> Which pwm driver you're using?
> This is drivers/pwm/pwm_tegra.c
> Here .set_invert is not implemented.
>
> Thx for the review.
>
You're right. Some drivers has #pwm-cells = <2> and in this case 
polarity is 0.

However your approach is wrong. You will break other drivers with 
#pwm-cells = <3>.
I think your patch should look something like this:

ret = pwm_set_invert(priv->pwm, priv->channel, priv->polarity);
if (ret == -ENOSYS)
     ret = 0;

return log_ret(ret);

This will not return error if set_inverted is not implemented.


Stefan
Stefan Mavrodiev July 2, 2019, 9:08 a.m. UTC | #6
On 7/2/19 12:05 PM, Stefan Mavrodiev wrote:
> Hi,
>
> On 7/2/19 11:42 AM, Nicolas Chauvet wrote:
>> Le mar. 2 juil. 2019 à 09:41, Stefan Mavrodiev <stefan@olimex.com> a 
>> écrit :
>>> Hi,
>>>
>>> On 6/27/19 9:56 PM, marvin24@posteo.de wrote:
>>>> Am Freitag, 21. Juni 2019, 22:01:35 CEST schrieb Marc Dietrich:
>>>>> Fixes: 57e7775413 ("video: backlight: Parse PWM polarity cell")
>>>>>
>>>>> set_pwm will always fail if pwm_set_invert is not implemented, 
>>>>> leaving the
>>>>> backlight dark. Fix this by calling pwm_set_invert only if pwm is 
>>>>> inverted.
>>> I'm not sure if this is true. pwm_set_invert() checks for .set_invert.
>>> Which pwm driver you're using?
>> This is drivers/pwm/pwm_tegra.c
>> Here .set_invert is not implemented.
>>
>> Thx for the review.
>>
> You're right. Some drivers has #pwm-cells = <2> and in this case 
> polarity is 0.
>
> However your approach is wrong. You will break other drivers with 
> #pwm-cells = <3>.
> I think your patch should look something like this:
>
> ret = pwm_set_invert(priv->pwm, priv->channel, priv->polarity);
> if (ret == -ENOSYS)
>     ret = 0;
>
> return log_ret(ret);


You should ask the custodians to review this.

>
> This will not return error if set_inverted is not implemented.

> Stefan
>
diff mbox series

Patch

diff --git a/drivers/video/pwm_backlight.c b/drivers/video/pwm_backlight.c
index a587977c22..26bcbeb42b 100644
--- a/drivers/video/pwm_backlight.c
+++ b/drivers/video/pwm_backlight.c
@@ -61,12 +61,13 @@  static int set_pwm(struct pwm_backlight_priv *priv)

 	duty_cycle = priv->period_ns * (priv->cur_level - priv->min_level) /
 		(priv->max_level - priv->min_level + 1);
+
 	ret = pwm_set_config(priv->pwm, priv->channel, priv->period_ns,
 			     duty_cycle);
-	if (ret)
-		return log_ret(ret);

-	ret = pwm_set_invert(priv->pwm, priv->channel, priv->polarity);
+	if (!ret && priv->polarity)
+		ret = pwm_set_invert(priv->pwm, priv->channel, priv->polarity);
+
 	return log_ret(ret);
 }