[PATCH 0/9] backlight: atmel-pwm-bl: fixes and clean ups

[PATCH 0/9] backlight: atmel-pwm-bl: fixes and clean ups

[Johan Hovold]

These patches fix a few issues and clean up the atmel-pwm-bl driver
somewhat.

Johan

Johan Hovold (9):
  backlight: atmel-pwm-bl: fix reported brightness
  backlight: atmel-pwm-bl: fix gpio polarity in remove
  backlight: atmel-pwm-bl: fix module autoload
  backlight: atmel-pwm-bl: clean up probe error handling
  backlight: atmel-pwm-bl: clean up get_intensity
  backlight: atmel-pwm-bl: remove unused include
  backlight: atmel-pwm-bl: use gpio_is_valid
  backlight: atmel-pwm-bl: refactor gpio_on handling
  backlight: atmel-pwm-bl: use gpio_request_one

 drivers/video/backlight/atmel-pwm-bl.c | 86 ++++++++++++++++------------------
 1 file changed, 40 insertions(+), 46 deletions(-)

-- 
1.8.4

[PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness

[Johan Hovold]

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

Re: [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness

[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

Re: [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness

[Johan Hovold]

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
 

Re: [PATCH 1/9] backlight: atmel-pwm-bl: fix reported brightness

[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 

[PATCH 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove

[Johan Hovold]

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
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

Re: [PATCH 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove

[Jingoo Han]

On Wednesday, October 23, 2013 2:27 AM, 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
> Signed-off-by: Johan Hovold <jhovold@gmail.com>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/video/backlight/atmel-pwm-bl.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

[PATCH 3/9] backlight: atmel-pwm-bl: fix module autoload

[Johan Hovold]

Add missing module alias which is needed for module autoloading.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 3cb0094..cc5a5ed 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -229,3 +229,4 @@ module_platform_driver(atmel_pwm_bl_driver);
 MODULE_AUTHOR("Hans-Christian egtvedt <hans-christian.egtvedt@atmel.com>");
 MODULE_DESCRIPTION("Atmel PWM backlight driver");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:atmel-pwm-bl");
-- 
1.8.4

Re: [PATCH 3/9] backlight: atmel-pwm-bl: fix module autoload

[Jingoo Han]

On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
> 
> Add missing module alias which is needed for module autoloading.
> 
> Signed-off-by: Johan Hovold <jhovold@gmail.com>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/video/backlight/atmel-pwm-bl.c | 1 +
>  1 file changed, 1 insertion(+)

[PATCH 4/9] backlight: atmel-pwm-bl: clean up probe error handling

[Johan Hovold]

Clean up probe error handling by checking parameters before any
allocations and removing an obsolete error label. Also remove
unnecessary reset of private gpio number.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index cc5a5ed..52a8134 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -126,40 +126,33 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 	struct atmel_pwm_bl *pwmbl;
 	int retval;
 
+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata)
+		return -ENODEV;
+
+	if (pdata->pwm_compare_max < pdata->pwm_duty_max ||
+			pdata->pwm_duty_min > pdata->pwm_duty_max ||
+			pdata->pwm_frequency == 0)
+		return -EINVAL;
+
 	pwmbl = devm_kzalloc(&pdev->dev, sizeof(struct atmel_pwm_bl),
 				GFP_KERNEL);
 	if (!pwmbl)
 		return -ENOMEM;
 
 	pwmbl->pdev = pdev;
-
-	pdata = dev_get_platdata(&pdev->dev);
-	if (!pdata) {
-		retval = -ENODEV;
-		goto err_free_mem;
-	}
-
-	if (pdata->pwm_compare_max < pdata->pwm_duty_max ||
-			pdata->pwm_duty_min > pdata->pwm_duty_max ||
-			pdata->pwm_frequency == 0) {
-		retval = -EINVAL;
-		goto err_free_mem;
-	}
-
 	pwmbl->pdata = pdata;
 	pwmbl->gpio_on = pdata->gpio_on;
 
 	retval = pwm_channel_alloc(pdata->pwm_channel, &pwmbl->pwmc);
 	if (retval)
-		goto err_free_mem;
+		return retval;
 
 	if (pwmbl->gpio_on != -1) {
 		retval = devm_gpio_request(&pdev->dev, pwmbl->gpio_on,
 					"gpio_atmel_pwm_bl");
-		if (retval) {
-			pwmbl->gpio_on = -1;
+		if (retval)
 			goto err_free_pwm;
-		}
 
 		/* Turn display off by default. */
 		retval = gpio_direction_output(pwmbl->gpio_on,
@@ -197,7 +190,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 
 err_free_pwm:
 	pwm_channel_free(&pwmbl->pwmc);
-err_free_mem:
+
 	return retval;
 }
 
-- 
1.8.4

Re: [PATCH 4/9] backlight: atmel-pwm-bl: clean up probe error handling

[Jingoo Han]

On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
> 
> Clean up probe error handling by checking parameters before any
> allocations and removing an obsolete error label. Also remove
> unnecessary reset of private gpio number.
> 
> Signed-off-by: Johan Hovold <jhovold@gmail.com>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/video/backlight/atmel-pwm-bl.c | 31 ++++++++++++-------------------
>  1 file changed, 12 insertions(+), 19 deletions(-)

[PATCH 5/9] backlight: atmel-pwm-bl: clean up get_intensity

[Johan Hovold]

Clean up get_intensity to increase readability.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 52a8134..504061c 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -70,15 +70,14 @@ 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);
+	u32 cdty;
 	u32 intensity;
 
-	if (pwmbl->pdata->pwm_active_low) {
-		intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
-			pwmbl->pdata->pwm_duty_min;
-	} else {
-		intensity = pwmbl->pdata->pwm_duty_max -
-			pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
-	}
+	cdty = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
+	if (pwmbl->pdata->pwm_active_low)
+		intensity = cdty - pwmbl->pdata->pwm_duty_min;
+	else
+		intensity = pwmbl->pdata->pwm_duty_max - cdty;
 
 	return (u16)intensity;
 }
-- 
1.8.4

Re: [PATCH 5/9] backlight: atmel-pwm-bl: clean up get_intensity

[Jingoo Han]

On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
> 
> Clean up get_intensity to increase readability.
> 
> Signed-off-by: Johan Hovold <jhovold@gmail.com>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/video/backlight/atmel-pwm-bl.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)

[PATCH 6/9] backlight: atmel-pwm-bl: remove unused include

[Johan Hovold]

Remove unused include of clk.h.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 504061c..db68cab 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -12,7 +12,6 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/fb.h>
-#include <linux/clk.h>
 #include <linux/gpio.h>
 #include <linux/backlight.h>
 #include <linux/atmel_pwm.h>
-- 
1.8.4

Re: [PATCH 6/9] backlight: atmel-pwm-bl: remove unused include

[Jingoo Han]

On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
> 
> Remove unused include of clk.h.
> 
> Signed-off-by: Johan Hovold <jhovold@gmail.com>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/video/backlight/atmel-pwm-bl.c | 1 -
>  1 file changed, 1 deletion(-)

[PATCH 7/9] backlight: atmel-pwm-bl: use gpio_is_valid

[Johan Hovold]

Use gpio_is_valid rather than open coding the more restrictive != -1
test.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index db68cab..ffc30d2 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -48,7 +48,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 		pwm_duty = pwmbl->pdata->pwm_duty_min;
 
 	if (!intensity) {
-		if (pwmbl->gpio_on != -1) {
+		if (gpio_is_valid(pwmbl->gpio_on)) {
 			gpio_set_value(pwmbl->gpio_on,
 					0 ^ pwmbl->pdata->on_active_low);
 		}
@@ -57,7 +57,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 	} else {
 		pwm_channel_enable(&pwmbl->pwmc);
 		pwm_channel_writel(&pwmbl->pwmc, PWM_CUPD, pwm_duty);
-		if (pwmbl->gpio_on != -1) {
+		if (gpio_is_valid(pwmbl->gpio_on)) {
 			gpio_set_value(pwmbl->gpio_on,
 					1 ^ pwmbl->pdata->on_active_low);
 		}
@@ -146,7 +146,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 	if (retval)
 		return retval;
 
-	if (pwmbl->gpio_on != -1) {
+	if (gpio_is_valid(pwmbl->gpio_on)) {
 		retval = devm_gpio_request(&pdev->dev, pwmbl->gpio_on,
 					"gpio_atmel_pwm_bl");
 		if (retval)
@@ -196,7 +196,7 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
 {
 	struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
 
-	if (pwmbl->gpio_on != -1) {
+	if (gpio_is_valid(pwmbl->gpio_on)) {
 		gpio_set_value(pwmbl->gpio_on,
 					0 ^ pwmbl->pdata->on_active_low);
 	}
-- 
1.8.4

Re: [PATCH 7/9] backlight: atmel-pwm-bl: use gpio_is_valid

[Jingoo Han]

On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
> 
> Use gpio_is_valid rather than open coding the more restrictive != -1
> test.
> 
> Signed-off-by: Johan Hovold <jhovold@gmail.com>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/video/backlight/atmel-pwm-bl.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

[PATCH 8/9] backlight: atmel-pwm-bl: refactor gpio_on handling

[Johan Hovold]

Add helper function to control the gpio_on signal.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index ffc30d2..1277e0c 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -26,6 +26,14 @@ struct atmel_pwm_bl {
 	int					gpio_on;
 };
 
+static void atmel_pwm_bl_set_gpio_on(struct atmel_pwm_bl *pwmbl, int on)
+{
+	if (!gpio_is_valid(pwmbl->gpio_on))
+		return;
+
+	gpio_set_value(pwmbl->gpio_on, on ^ pwmbl->pdata->on_active_low);
+}
+
 static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 {
 	struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
@@ -48,19 +56,13 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 		pwm_duty = pwmbl->pdata->pwm_duty_min;
 
 	if (!intensity) {
-		if (gpio_is_valid(pwmbl->gpio_on)) {
-			gpio_set_value(pwmbl->gpio_on,
-					0 ^ pwmbl->pdata->on_active_low);
-		}
+		atmel_pwm_bl_set_gpio_on(pwmbl, 0);
 		pwm_channel_writel(&pwmbl->pwmc, PWM_CUPD, pwm_duty);
 		pwm_channel_disable(&pwmbl->pwmc);
 	} else {
 		pwm_channel_enable(&pwmbl->pwmc);
 		pwm_channel_writel(&pwmbl->pwmc, PWM_CUPD, pwm_duty);
-		if (gpio_is_valid(pwmbl->gpio_on)) {
-			gpio_set_value(pwmbl->gpio_on,
-					1 ^ pwmbl->pdata->on_active_low);
-		}
+		atmel_pwm_bl_set_gpio_on(pwmbl, 1);
 	}
 
 	return 0;
@@ -196,10 +198,7 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
 {
 	struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
 
-	if (gpio_is_valid(pwmbl->gpio_on)) {
-		gpio_set_value(pwmbl->gpio_on,
-					0 ^ pwmbl->pdata->on_active_low);
-	}
+	atmel_pwm_bl_set_gpio_on(pwmbl, 0);
 	pwm_channel_disable(&pwmbl->pwmc);
 	pwm_channel_free(&pwmbl->pwmc);
 
-- 
1.8.4

Re: [PATCH 8/9] backlight: atmel-pwm-bl: refactor gpio_on handling

[Jingoo Han]

On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
> 
> Add helper function to control the gpio_on signal.
> 
> Signed-off-by: Johan Hovold <jhovold@gmail.com>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/video/backlight/atmel-pwm-bl.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)

[PATCH 9/9] backlight: atmel-pwm-bl: use gpio_request_one

[Johan Hovold]

Use devm_gpio_request_one rather than requesting and setting direction
in two calls.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 1277e0c..5ea2517 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -124,6 +124,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 	const struct atmel_pwm_bl_platform_data *pdata;
 	struct backlight_device *bldev;
 	struct atmel_pwm_bl *pwmbl;
+	int flags;
 	int retval;
 
 	pdata = dev_get_platdata(&pdev->dev);
@@ -149,14 +150,14 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 		return retval;
 
 	if (gpio_is_valid(pwmbl->gpio_on)) {
-		retval = devm_gpio_request(&pdev->dev, pwmbl->gpio_on,
-					"gpio_atmel_pwm_bl");
-		if (retval)
-			goto err_free_pwm;
-
 		/* Turn display off by default. */
-		retval = gpio_direction_output(pwmbl->gpio_on,
-				0 ^ pdata->on_active_low);
+		if (pdata->on_active_low)
+			flags = GPIOF_OUT_INIT_HIGH;
+		else
+			flags = GPIOF_OUT_INIT_LOW;
+
+		retval = devm_gpio_request_one(&pdev->dev, pwmbl->gpio_on,
+						flags, "gpio_atmel_pwm_bl");
 		if (retval)
 			goto err_free_pwm;
 	}
-- 
1.8.4

Re: [PATCH 9/9] backlight: atmel-pwm-bl: use gpio_request_one

[Jingoo Han]

On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
> 
> Use devm_gpio_request_one rather than requesting and setting direction
> in two calls.
> 
> Signed-off-by: Johan Hovold <jhovold@gmail.com>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/video/backlight/atmel-pwm-bl.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)

Re: [PATCH 0/9] backlight: atmel-pwm-bl: fixes and clean ups

[Jingoo Han]

On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
> 
> These patches fix a few issues and clean up the atmel-pwm-bl driver
> somewhat.
> 
> Johan
> 
> Johan Hovold (9):
>   backlight: atmel-pwm-bl: fix reported brightness
>   backlight: atmel-pwm-bl: fix gpio polarity in remove
>   backlight: atmel-pwm-bl: fix module autoload
>   backlight: atmel-pwm-bl: clean up probe error handling
>   backlight: atmel-pwm-bl: clean up get_intensity
>   backlight: atmel-pwm-bl: remove unused include
>   backlight: atmel-pwm-bl: use gpio_is_valid
>   backlight: atmel-pwm-bl: refactor gpio_on handling
>   backlight: atmel-pwm-bl: use gpio_request_one

++cc Andrew Morton, Tomi Valkeinen, Jean-Christophe Plagniol-Villard

Hi Johan Hovold,

Currently, because there is no git tree for backlight,
backlight patches have been merged to mm-tree by Andrew Morton.
Please, add Andrew Morton to CC list.

Also, there is another way.
If Nicolas Ferre wants to merge these patches, the patches can be
merged through ATMEL-SoC tree with my Acked-by.

Best regards,
Jingoo Han

> 
>  drivers/video/backlight/atmel-pwm-bl.c | 86 ++++++++++++++++------------------
>  1 file changed, 40 insertions(+), 46 deletions(-)

Re: [PATCH 0/9] backlight: atmel-pwm-bl: fixes and clean ups

[Nicolas Ferre]

On 23/10/2013 02:19, Jingoo Han :
> On Wednesday, October 23, 2013 2:27 AM, Johan Hovold wrote:
>>
>> These patches fix a few issues and clean up the atmel-pwm-bl driver
>> somewhat.
>>
>> Johan
>>
>> Johan Hovold (9):
>>    backlight: atmel-pwm-bl: fix reported brightness
>>    backlight: atmel-pwm-bl: fix gpio polarity in remove
>>    backlight: atmel-pwm-bl: fix module autoload
>>    backlight: atmel-pwm-bl: clean up probe error handling
>>    backlight: atmel-pwm-bl: clean up get_intensity
>>    backlight: atmel-pwm-bl: remove unused include
>>    backlight: atmel-pwm-bl: use gpio_is_valid
>>    backlight: atmel-pwm-bl: refactor gpio_on handling
>>    backlight: atmel-pwm-bl: use gpio_request_one
>
> ++cc Andrew Morton, Tomi Valkeinen, Jean-Christophe Plagniol-Villard
>
> Hi Johan Hovold,
>
> Currently, because there is no git tree for backlight,
> backlight patches have been merged to mm-tree by Andrew Morton.
> Please, add Andrew Morton to CC list.
>
> Also, there is another way.
> If Nicolas Ferre wants to merge these patches, the patches can be
> merged through ATMEL-SoC tree with my Acked-by.

Hi,

As it's a driver without interaction with AT91 code, maybe routing this 
patch series through mm-tree is the way to go.

If you find any issue in the process, please tell me. I would be happy 
to ease the process.

Bye,


>>
>>   drivers/video/backlight/atmel-pwm-bl.c | 86 ++++++++++++++++------------------
>>   1 file changed, 40 insertions(+), 46 deletions(-)
>
>


-- 
Nicolas Ferre

[PATCH RESEND 0/9] backlight: atmel-pwm-bl: fixes and clean ups

[Johan Hovold]

These patches fix a few issues and clean up the atmel-pwm-bl driver
somewhat.

Resend with added ack from Jingo Han.

Johan

Johan Hovold (9):
  backlight: atmel-pwm-bl: fix reported brightness
  backlight: atmel-pwm-bl: fix gpio polarity in remove
  backlight: atmel-pwm-bl: fix module autoload
  backlight: atmel-pwm-bl: clean up probe error handling
  backlight: atmel-pwm-bl: clean up get_intensity
  backlight: atmel-pwm-bl: remove unused include
  backlight: atmel-pwm-bl: use gpio_is_valid
  backlight: atmel-pwm-bl: refactor gpio_on handling
  backlight: atmel-pwm-bl: use gpio_request_one

 drivers/video/backlight/atmel-pwm-bl.c | 86 ++++++++++++++++------------------
 1 file changed, 40 insertions(+), 46 deletions(-)

-- 
1.8.4

[PATCH 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove

[Johan Hovold]

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

Re: [PATCH 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove

[Jean-Christophe PLAGNIOL-VILLARD]

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
> 

Re: [PATCH 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove

[Johan Hovold]

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
> >