* [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness
[not found] <1382462819-28576-1-git-send-email-jhovold@gmail.com>
@ 2013-10-22 17:26 ` Johan Hovold
2013-10-23 1:20 ` Jingoo Han
0 siblings, 1 reply; 14+ messages in thread
From: Johan Hovold @ 2013-10-22 17:26 UTC (permalink / raw)
To: Richard Purdie, Jingoo Han
Cc: Nicolas Ferre, linux-fbdev, linux-kernel, Johan Hovold, stable
The driver supports 16-bit brightness values, but the value returned
from get_brightness was truncated to eight bits.
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 66885fb..8aac273 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -70,7 +70,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
{
struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
- u8 intensity;
+ u32 intensity;
if (pwmbl->pdata->pwm_active_low) {
intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
@@ -80,7 +80,7 @@ static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
}
- return intensity;
+ return (u16)intensity;
}
static int atmel_pwm_bl_init_pwm(struct atmel_pwm_bl *pwmbl)
--
1.8.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness
2013-10-22 17:26 ` [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness Johan Hovold
@ 2013-10-23 1:20 ` Jingoo Han
2013-10-23 8:51 ` Johan Hovold
0 siblings, 1 reply; 14+ messages in thread
From: Jingoo Han @ 2013-10-23 1:20 UTC (permalink / raw)
To: 'Johan Hovold'
Cc: 'Richard Purdie', 'Nicolas Ferre', linux-fbdev,
linux-kernel, stable, 'Jingoo Han'
On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
>
> The driver supports 16-bit brightness values, but the value returned
> from get_brightness was truncated to eight bits.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Johan Hovold <jhovold@gmail.com>
> ---
> drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
> index 66885fb..8aac273 100644
> --- a/drivers/video/backlight/atmel-pwm-bl.c
> +++ b/drivers/video/backlight/atmel-pwm-bl.c
> @@ -70,7 +70,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
> static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
> {
> struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
> - u8 intensity;
> + u32 intensity;
>
> if (pwmbl->pdata->pwm_active_low) {
> intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
> @@ -80,7 +80,7 @@ static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
> pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
> }
>
> - return intensity;
> + return (u16)intensity;
However, atmel_pwm_bl_get_intensity() should return 'int',
instead of 'u16'. Also, pwm_channel_readl() returns 'u32'.
Then, how about the following?
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -70,17 +70,17 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
{
struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
- u8 intensity;
+ u16 intensity;
if (pwmbl->pdata->pwm_active_low) {
- intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
+ intensity = (u16) pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
pwmbl->pdata->pwm_duty_min;
} else {
- intensity = pwmbl->pdata->pwm_duty_max -
+ intensity = (u16) pwmbl->pdata->pwm_duty_max -
pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
}
- return intensity;
+ return (int)intensity;
}
Best regards,
Jingoo Han
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness
2013-10-23 1:20 ` Jingoo Han
@ 2013-10-23 8:51 ` Johan Hovold
2013-10-23 9:35 ` Jingoo Han
0 siblings, 1 reply; 14+ messages in thread
From: Johan Hovold @ 2013-10-23 8:51 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Johan Hovold', 'Richard Purdie',
'Nicolas Ferre', linux-fbdev, linux-kernel, stable
On Wed, Oct 23, 2013 at 10:20:59AM +0900, Jingoo Han wrote:
> On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
> >
> > The driver supports 16-bit brightness values, but the value returned
> > from get_brightness was truncated to eight bits.
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Johan Hovold <jhovold@gmail.com>
> > ---
> > drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
> > index 66885fb..8aac273 100644
> > --- a/drivers/video/backlight/atmel-pwm-bl.c
> > +++ b/drivers/video/backlight/atmel-pwm-bl.c
> > @@ -70,7 +70,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
> > static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
> > {
> > struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
> > - u8 intensity;
> > + u32 intensity;
> >
> > if (pwmbl->pdata->pwm_active_low) {
> > intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
> > @@ -80,7 +80,7 @@ static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
> > pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
> > }
> >
> > - return intensity;
> > + return (u16)intensity;
>
> However, atmel_pwm_bl_get_intensity() should return 'int',
> instead of 'u16'.
Yes, but the cast to int is implicit. Perhaps
return (intensity & 0xffff);
(or just a comment) would make it more clear why the cast is there.
> Also, pwm_channel_readl() returns 'u32'.
Yes, (and only the 16 least-significant bits are used). That and the
fact that the platform-data limits are currently unsigned long (I was
considering fixing this later) was why I preferred keeping all register
value manipulation in u32 and do a single cast at the end.
> Then, how about the following?
>
> --- a/drivers/video/backlight/atmel-pwm-bl.c
> +++ b/drivers/video/backlight/atmel-pwm-bl.c
> @@ -70,17 +70,17 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
> static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
> {
> struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
> - u8 intensity;
> + u16 intensity;
>
> if (pwmbl->pdata->pwm_active_low) {
> - intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
> + intensity = (u16) pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
> pwmbl->pdata->pwm_duty_min;
This would actually introduce a new conversion warning (unless you add
parentheses as well) as pwm_duty_min is unsigned long. Same below.
> } else {
> - intensity = pwmbl->pdata->pwm_duty_max -
> + intensity = (u16) pwmbl->pdata->pwm_duty_max -
> pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
> }
>
> - return intensity;
> + return (int)intensity;
> }
However, if you feel strongly about this, I'll respin the series (a later
patch would likely no longer apply), use u16 and add casts to the two
assignments.
Thanks,
Johan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness
2013-10-23 8:51 ` Johan Hovold
@ 2013-10-23 9:35 ` Jingoo Han
0 siblings, 0 replies; 14+ messages in thread
From: Jingoo Han @ 2013-10-23 9:35 UTC (permalink / raw)
To: 'Johan Hovold'
Cc: 'Richard Purdie', 'Nicolas Ferre', linux-fbdev,
linux-kernel, stable, 'Jingoo Han'
On Wednesday, October 23, 2013 5:51 PM, Johan Hovold wrote:
> On Wed, Oct 23, 2013 at 10:20:59AM +0900, Jingoo Han wrote:
> > On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
> > >
> > > The driver supports 16-bit brightness values, but the value returned
> > > from get_brightness was truncated to eight bits.
> > >
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Johan Hovold <jhovold@gmail.com>
> > > ---
> > > drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
> > > index 66885fb..8aac273 100644
> > > --- a/drivers/video/backlight/atmel-pwm-bl.c
> > > +++ b/drivers/video/backlight/atmel-pwm-bl.c
> > > @@ -70,7 +70,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
> > > static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
> > > {
> > > struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
> > > - u8 intensity;
> > > + u32 intensity;
> > >
> > > if (pwmbl->pdata->pwm_active_low) {
> > > intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
> > > @@ -80,7 +80,7 @@ static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
> > > pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
> > > }
> > >
> > > - return intensity;
> > > + return (u16)intensity;
> >
> > However, atmel_pwm_bl_get_intensity() should return 'int',
> > instead of 'u16'.
>
> Yes, but the cast to int is implicit. Perhaps
>
> return (intensity & 0xffff);
>
> (or just a comment) would make it more clear why the cast is there.
>
> > Also, pwm_channel_readl() returns 'u32'.
>
> Yes, (and only the 16 least-significant bits are used). That and the
> fact that the platform-data limits are currently unsigned long (I was
> considering fixing this later) was why I preferred keeping all register
> value manipulation in u32 and do a single cast at the end.
>
> > Then, how about the following?
> >
> > --- a/drivers/video/backlight/atmel-pwm-bl.c
> > +++ b/drivers/video/backlight/atmel-pwm-bl.c
> > @@ -70,17 +70,17 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
> > static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
> > {
> > struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
> > - u8 intensity;
> > + u16 intensity;
> >
> > if (pwmbl->pdata->pwm_active_low) {
> > - intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
> > + intensity = (u16) pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
> > pwmbl->pdata->pwm_duty_min;
>
> This would actually introduce a new conversion warning (unless you add
> parentheses as well) as pwm_duty_min is unsigned long. Same below.
>
> > } else {
> > - intensity = pwmbl->pdata->pwm_duty_max -
> > + intensity = (u16) pwmbl->pdata->pwm_duty_max -
> > pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
> > }
> >
> > - return intensity;
> > + return (int)intensity;
> > }
>
> However, if you feel strongly about this, I'll respin the series (a later
> patch would likely no longer apply), use u16 and add casts to the two
> assignments.
Thank you for your kind and detailed description. :-)
OK, I have no objection on your original patch.
Would you re-send these patches with my Acked-by with CC'ing Andrew Morton,
Tomi Valkeinen? Then, these patches can be merged through mm-tree.
Best regards,
Jingoo Han
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness
[not found] <1382522143-32072-1-git-send-email-jhovold@gmail.com>
@ 2013-10-23 9:55 ` Johan Hovold
2013-10-25 11:08 ` Jean-Christophe PLAGNIOL-VILLARD
2013-10-23 9:55 ` [PATCH 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove Johan Hovold
[not found] ` <1383064064-4983-1-git-send-email-jhovold@gmail.com>
2 siblings, 1 reply; 14+ messages in thread
From: Johan Hovold @ 2013-10-23 9:55 UTC (permalink / raw)
To: Richard Purdie, Jingoo Han
Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold, stable
The driver supports 16-bit brightness values, but the value returned
from get_brightness was truncated to eight bits.
Cc: stable@vger.kernel.org
Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 66885fb..8aac273 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -70,7 +70,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
{
struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
- u8 intensity;
+ u32 intensity;
if (pwmbl->pdata->pwm_active_low) {
intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
@@ -80,7 +80,7 @@ static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
}
- return intensity;
+ return (u16)intensity;
}
static int atmel_pwm_bl_init_pwm(struct atmel_pwm_bl *pwmbl)
--
1.8.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove
[not found] <1382522143-32072-1-git-send-email-jhovold@gmail.com>
2013-10-23 9:55 ` [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness Johan Hovold
@ 2013-10-23 9:55 ` Johan Hovold
2013-10-25 11:09 ` Jean-Christophe PLAGNIOL-VILLARD
[not found] ` <1383064064-4983-1-git-send-email-jhovold@gmail.com>
2 siblings, 1 reply; 14+ messages in thread
From: Johan Hovold @ 2013-10-23 9:55 UTC (permalink / raw)
To: Richard Purdie, Jingoo Han
Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold, stable
Make sure to honour gpio polarity also at remove so that the backlight
is actually disabled on boards with active-low enable pin.
Cc: stable@vger.kernel.org
Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/video/backlight/atmel-pwm-bl.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 8aac273..3cb0094 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -205,8 +205,10 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
{
struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
- if (pwmbl->gpio_on != -1)
- gpio_set_value(pwmbl->gpio_on, 0);
+ if (pwmbl->gpio_on != -1) {
+ gpio_set_value(pwmbl->gpio_on,
+ 0 ^ pwmbl->pdata->on_active_low);
+ }
pwm_channel_disable(&pwmbl->pwmc);
pwm_channel_free(&pwmbl->pwmc);
--
1.8.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness
2013-10-23 9:55 ` [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness Johan Hovold
@ 2013-10-25 11:08 ` Jean-Christophe PLAGNIOL-VILLARD
2013-10-29 13:32 ` Johan Hovold
0 siblings, 1 reply; 14+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-25 11:08 UTC (permalink / raw)
To: Johan Hovold
Cc: Richard Purdie, Jingoo Han, Nicolas Ferre, Tomi Valkeinen,
Andrew Morton, linux-fbdev, linux-kernel, stable
On 11:55 Wed 23 Oct , Johan Hovold wrote:
> The driver supports 16-bit brightness values, but the value returned
> from get_brightness was truncated to eight bits.
>
> Cc: stable@vger.kernel.org
> Acked-by: Jingoo Han <jg1.han@samsung.com>
> Signed-off-by: Johan Hovold <jhovold@gmail.com>
> ---
> drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
> index 66885fb..8aac273 100644
> --- a/drivers/video/backlight/atmel-pwm-bl.c
> +++ b/drivers/video/backlight/atmel-pwm-bl.c
> @@ -70,7 +70,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
> static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
> {
> struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
> - u8 intensity;
> + u32 intensity;
>
> if (pwmbl->pdata->pwm_active_low) {
> intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
> @@ -80,7 +80,7 @@ static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
> pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
> }
>
> - return intensity;
> + return (u16)intensity;
no cast mask it
> }
>
> static int atmel_pwm_bl_init_pwm(struct atmel_pwm_bl *pwmbl)
> --
> 1.8.4
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove
2013-10-23 9:55 ` [PATCH 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove Johan Hovold
@ 2013-10-25 11:09 ` Jean-Christophe PLAGNIOL-VILLARD
2013-10-29 13:08 ` Johan Hovold
0 siblings, 1 reply; 14+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-10-25 11:09 UTC (permalink / raw)
To: Johan Hovold
Cc: Richard Purdie, Jingoo Han, Nicolas Ferre, Tomi Valkeinen,
Andrew Morton, linux-fbdev, linux-kernel, stable
On 11:55 Wed 23 Oct , Johan Hovold wrote:
> Make sure to honour gpio polarity also at remove so that the backlight
> is actually disabled on boards with active-low enable pin.
>
> Cc: stable@vger.kernel.org
> Acked-by: Jingoo Han <jg1.han@samsung.com>
> Signed-off-by: Johan Hovold <jhovold@gmail.com>
> ---
> drivers/video/backlight/atmel-pwm-bl.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
> index 8aac273..3cb0094 100644
> --- a/drivers/video/backlight/atmel-pwm-bl.c
> +++ b/drivers/video/backlight/atmel-pwm-bl.c
> @@ -205,8 +205,10 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
> {
> struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
>
> - if (pwmbl->gpio_on != -1)
> - gpio_set_value(pwmbl->gpio_on, 0);
> + if (pwmbl->gpio_on != -1) {
here we need to use gpio_is_valid
> + gpio_set_value(pwmbl->gpio_on,
> + 0 ^ pwmbl->pdata->on_active_low);
> + }
> pwm_channel_disable(&pwmbl->pwmc);
> pwm_channel_free(&pwmbl->pwmc);
>
> --
> 1.8.4
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove
2013-10-25 11:09 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2013-10-29 13:08 ` Johan Hovold
0 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2013-10-29 13:08 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD
Cc: Johan Hovold, Richard Purdie, Jingoo Han, Nicolas Ferre,
Tomi Valkeinen, Andrew Morton, linux-fbdev, linux-kernel, stable
On Fri, Oct 25, 2013 at 01:09:28PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:55 Wed 23 Oct , Johan Hovold wrote:
> > Make sure to honour gpio polarity also at remove so that the backlight
> > is actually disabled on boards with active-low enable pin.
> >
> > Cc: stable@vger.kernel.org
> > Acked-by: Jingoo Han <jg1.han@samsung.com>
> > Signed-off-by: Johan Hovold <jhovold@gmail.com>
> > ---
> > drivers/video/backlight/atmel-pwm-bl.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
> > index 8aac273..3cb0094 100644
> > --- a/drivers/video/backlight/atmel-pwm-bl.c
> > +++ b/drivers/video/backlight/atmel-pwm-bl.c
> > @@ -205,8 +205,10 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
> > {
> > struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
> >
> > - if (pwmbl->gpio_on != -1)
> > - gpio_set_value(pwmbl->gpio_on, 0);
> > + if (pwmbl->gpio_on != -1) {
> here we need to use gpio_is_valid
That is a different issue which is fixed by patch 7/9. It doesn't make
sense to only change one of the gpio-validity tests.
Johan
> > + gpio_set_value(pwmbl->gpio_on,
> > + 0 ^ pwmbl->pdata->on_active_low);
> > + }
> > pwm_channel_disable(&pwmbl->pwmc);
> > pwm_channel_free(&pwmbl->pwmc);
> >
> > --
> > 1.8.4
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness
2013-10-25 11:08 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2013-10-29 13:32 ` Johan Hovold
0 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2013-10-29 13:32 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD
Cc: Johan Hovold, Richard Purdie, Jingoo Han, Nicolas Ferre,
Tomi Valkeinen, Andrew Morton, linux-fbdev, linux-kernel, stable
On Fri, Oct 25, 2013 at 01:08:36PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:55 Wed 23 Oct , Johan Hovold wrote:
> > The driver supports 16-bit brightness values, but the value returned
> > from get_brightness was truncated to eight bits.
> >
> > Cc: stable@vger.kernel.org
> > Acked-by: Jingoo Han <jg1.han@samsung.com>
> > Signed-off-by: Johan Hovold <jhovold@gmail.com>
> > ---
> > drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
> > index 66885fb..8aac273 100644
> > --- a/drivers/video/backlight/atmel-pwm-bl.c
> > +++ b/drivers/video/backlight/atmel-pwm-bl.c
> > @@ -70,7 +70,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
> > static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
> > {
> > struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
> > - u8 intensity;
> > + u32 intensity;
> >
> > if (pwmbl->pdata->pwm_active_low) {
> > intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
> > @@ -80,7 +80,7 @@ static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
> > pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
> > }
> >
> > - return intensity;
> > + return (u16)intensity;
> no cast mask it
Yeah, perhaps that is better. I discussed this a bit with Jingoo Han in
the thread of the original post. I'll respin.
Thanks,
Johan
> > }
> >
> > static int atmel_pwm_bl_init_pwm(struct atmel_pwm_bl *pwmbl)
> > --
> > 1.8.4
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/9] backlight: atmel-pwm-bl: fix reported brightness
[not found] ` <1383064064-4983-1-git-send-email-jhovold@gmail.com>
@ 2013-10-29 16:27 ` Johan Hovold
2013-10-29 16:27 ` [PATCH v2 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove Johan Hovold
[not found] ` <1383242264-7652-1-git-send-email-jhovold@gmail.com>
2 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2013-10-29 16:27 UTC (permalink / raw)
To: Richard Purdie, Jingoo Han
Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold, stable
The driver supports 16-bit brightness values, but the value returned
from get_brightness was truncated to eight bits.
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 66885fb..0971a8e 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -70,7 +70,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
{
struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
- u8 intensity;
+ u32 intensity;
if (pwmbl->pdata->pwm_active_low) {
intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
@@ -80,7 +80,7 @@ static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
}
- return intensity;
+ return intensity & 0xffff;
}
static int atmel_pwm_bl_init_pwm(struct atmel_pwm_bl *pwmbl)
--
1.8.4.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove
[not found] ` <1383064064-4983-1-git-send-email-jhovold@gmail.com>
2013-10-29 16:27 ` [PATCH v2 1/9] backlight: atmel-pwm-bl: fix reported brightness Johan Hovold
@ 2013-10-29 16:27 ` Johan Hovold
[not found] ` <1383242264-7652-1-git-send-email-jhovold@gmail.com>
2 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2013-10-29 16:27 UTC (permalink / raw)
To: Richard Purdie, Jingoo Han
Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold, stable
Make sure to honour gpio polarity also at remove so that the backlight
is actually disabled on boards with active-low enable pin.
Cc: stable@vger.kernel.org
Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/video/backlight/atmel-pwm-bl.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 0971a8e..e21beb6 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -205,8 +205,10 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
{
struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
- if (pwmbl->gpio_on != -1)
- gpio_set_value(pwmbl->gpio_on, 0);
+ if (pwmbl->gpio_on != -1) {
+ gpio_set_value(pwmbl->gpio_on,
+ 0 ^ pwmbl->pdata->on_active_low);
+ }
pwm_channel_disable(&pwmbl->pwmc);
pwm_channel_free(&pwmbl->pwmc);
--
1.8.4.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 1/9] backlight: atmel-pwm-bl: fix reported brightness
[not found] ` <1383242264-7652-1-git-send-email-jhovold@gmail.com>
@ 2013-10-31 17:57 ` Johan Hovold
2013-10-31 17:57 ` [PATCH v3 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove Johan Hovold
1 sibling, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2013-10-31 17:57 UTC (permalink / raw)
To: Richard Purdie, Jingoo Han
Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold, stable
The driver supports 16-bit brightness values, but the value returned
from get_brightness was truncated to eight bits.
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 66885fb..0971a8e 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -70,7 +70,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
{
struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
- u8 intensity;
+ u32 intensity;
if (pwmbl->pdata->pwm_active_low) {
intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
@@ -80,7 +80,7 @@ static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
}
- return intensity;
+ return intensity & 0xffff;
}
static int atmel_pwm_bl_init_pwm(struct atmel_pwm_bl *pwmbl)
--
1.8.4.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove
[not found] ` <1383242264-7652-1-git-send-email-jhovold@gmail.com>
2013-10-31 17:57 ` [PATCH v3 1/9] backlight: atmel-pwm-bl: fix reported brightness Johan Hovold
@ 2013-10-31 17:57 ` Johan Hovold
1 sibling, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2013-10-31 17:57 UTC (permalink / raw)
To: Richard Purdie, Jingoo Han
Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold, stable
Make sure to honour gpio polarity also at remove so that the backlight
is actually disabled on boards with active-low enable pin.
Cc: stable@vger.kernel.org
Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/video/backlight/atmel-pwm-bl.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 0971a8e..e21beb6 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -205,8 +205,10 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
{
struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
- if (pwmbl->gpio_on != -1)
- gpio_set_value(pwmbl->gpio_on, 0);
+ if (pwmbl->gpio_on != -1) {
+ gpio_set_value(pwmbl->gpio_on,
+ 0 ^ pwmbl->pdata->on_active_low);
+ }
pwm_channel_disable(&pwmbl->pwmc);
pwm_channel_free(&pwmbl->pwmc);
--
1.8.4.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-10-31 17:57 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1382522143-32072-1-git-send-email-jhovold@gmail.com>
2013-10-23 9:55 ` [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness Johan Hovold
2013-10-25 11:08 ` Jean-Christophe PLAGNIOL-VILLARD
2013-10-29 13:32 ` Johan Hovold
2013-10-23 9:55 ` [PATCH 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove Johan Hovold
2013-10-25 11:09 ` Jean-Christophe PLAGNIOL-VILLARD
2013-10-29 13:08 ` Johan Hovold
[not found] ` <1383064064-4983-1-git-send-email-jhovold@gmail.com>
2013-10-29 16:27 ` [PATCH v2 1/9] backlight: atmel-pwm-bl: fix reported brightness Johan Hovold
2013-10-29 16:27 ` [PATCH v2 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove Johan Hovold
[not found] ` <1383242264-7652-1-git-send-email-jhovold@gmail.com>
2013-10-31 17:57 ` [PATCH v3 1/9] backlight: atmel-pwm-bl: fix reported brightness Johan Hovold
2013-10-31 17:57 ` [PATCH v3 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove Johan Hovold
[not found] <1382462819-28576-1-git-send-email-jhovold@gmail.com>
2013-10-22 17:26 ` [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness Johan Hovold
2013-10-23 1:20 ` Jingoo Han
2013-10-23 8:51 ` Johan Hovold
2013-10-23 9:35 ` Jingoo Han
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox