[PATCH 2/2] backlight: gpio: delete pdata inversion

[PATCH 2/2] backlight: gpio: delete pdata inversion

[Linus Walleij]

The option to invert the output of the GPIO (active low) is
not used by the only platform still using platform data to
set up a GPIO backlight (one SH board). Delete the option
as we do not expect to expand the use of board files for
this driver, and GPIO descriptors intrinsically keep track
of any signal inversion.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/video/backlight/gpio_backlight.c     | 15 ++-------------
 include/linux/platform_data/gpio_backlight.h |  1 -
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
index 17dd8071ad4d..8c19dc6c9236 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -25,7 +25,6 @@ struct gpio_backlight {
 	struct device *fbdev;
 
 	struct gpio_desc *gpiod;
-	int active;
 	int def_value;
 };
 
@@ -39,8 +38,7 @@ static int gpio_backlight_update_status(struct backlight_device *bl)
 	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
 		brightness = 0;
 
-	gpiod_set_value_cansleep(gbl->gpiod,
-				 brightness ? gbl->active : !gbl->active);
+	gpiod_set_value_cansleep(gbl->gpiod, brightness);
 
 	return 0;
 }
@@ -72,8 +70,6 @@ static int gpio_backlight_probe_dt(struct platform_device *pdev,
 		flags = GPIOD_OUT_HIGH;
 	else
 		flags = GPIOD_OUT_LOW;
-	/* GPIO descriptors keep track of inversion */
-	gbl->active = 1;
 
 	gbl->gpiod = devm_gpiod_get(dev, NULL, flags);
 	if (IS_ERR(gbl->gpiod)) {
@@ -124,15 +120,8 @@ static int gpio_backlight_probe(struct platform_device *pdev)
 		unsigned long flags = GPIOF_DIR_OUT;
 
 		gbl->fbdev = pdata->fbdev;
-		gbl->active = pdata->active_low ? 0 : 1;
 		gbl->def_value = pdata->def_value;
-
-		if (gbl->active)
-			flags |= gbl->def_value ?
-				GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
-		else
-			flags |= gbl->def_value ?
-				GPIOF_INIT_LOW : GPIOF_INIT_HIGH;
+		flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
 
 		ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
 					    pdata ? pdata->name : "backlight");
diff --git a/include/linux/platform_data/gpio_backlight.h b/include/linux/platform_data/gpio_backlight.h
index 5ae0d9c80d4d..683d90453c41 100644
--- a/include/linux/platform_data/gpio_backlight.h
+++ b/include/linux/platform_data/gpio_backlight.h
@@ -14,7 +14,6 @@ struct gpio_backlight_platform_data {
 	struct device *fbdev;
 	int gpio;
 	int def_value;
-	bool active_low;
 	const char *name;
 };
 
-- 
2.9.3

Re: [PATCH 2/2] backlight: gpio: delete pdata inversion

[Linus Walleij]

On Sun, Apr 30, 2017 at 10:17 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:

> The option to invert the output of the GPIO (active low) is
> not used by the only platform still using platform data to
> set up a GPIO backlight (one SH board). Delete the option
> as we do not expect to expand the use of board files for
> this driver, and GPIO descriptors intrinsically keep track
> of any signal inversion.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

No reaction on this patch since 21 days so sending an
unsolicited ping.

Yours,
Linus Walleij

Re: [PATCH 2/2] backlight: gpio: delete pdata inversion

[Daniel Thompson]

On 30/04/17 09:17, Linus Walleij wrote:
> The option to invert the output of the GPIO (active low) is
> not used by the only platform still using platform data to
> set up a GPIO backlight (one SH board). Delete the option
> as we do not expect to expand the use of board files for
> this driver, and GPIO descriptors intrinsically keep track
> of any signal inversion.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

I note that only ~40% of Linux architectures currently have DT support 
at this point... There are still a few "embedded" ones that make do with 
platform bus (although they are growing increasingly niche).

Having said that I can't argue with "it's not used anywhere" (I've only 
really posted the above paragraph as a hint for future archeology... 
since it might be me doing the digging):
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>


Daniel.


> ---
>   drivers/video/backlight/gpio_backlight.c     | 15 ++-------------
>   include/linux/platform_data/gpio_backlight.h |  1 -
>   2 files changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
> index 17dd8071ad4d..8c19dc6c9236 100644
> --- a/drivers/video/backlight/gpio_backlight.c
> +++ b/drivers/video/backlight/gpio_backlight.c
> @@ -25,7 +25,6 @@ struct gpio_backlight {
>   	struct device *fbdev;
>   
>   	struct gpio_desc *gpiod;
> -	int active;
>   	int def_value;
>   };
>   
> @@ -39,8 +38,7 @@ static int gpio_backlight_update_status(struct backlight_device *bl)
>   	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
>   		brightness = 0;
>   
> -	gpiod_set_value_cansleep(gbl->gpiod,
> -				 brightness ? gbl->active : !gbl->active);
> +	gpiod_set_value_cansleep(gbl->gpiod, brightness);
>   
>   	return 0;
>   }
> @@ -72,8 +70,6 @@ static int gpio_backlight_probe_dt(struct platform_device *pdev,
>   		flags = GPIOD_OUT_HIGH;
>   	else
>   		flags = GPIOD_OUT_LOW;
> -	/* GPIO descriptors keep track of inversion */
> -	gbl->active = 1;
>   
>   	gbl->gpiod = devm_gpiod_get(dev, NULL, flags);
>   	if (IS_ERR(gbl->gpiod)) {
> @@ -124,15 +120,8 @@ static int gpio_backlight_probe(struct platform_device *pdev)
>   		unsigned long flags = GPIOF_DIR_OUT;
>   
>   		gbl->fbdev = pdata->fbdev;
> -		gbl->active = pdata->active_low ? 0 : 1;
>   		gbl->def_value = pdata->def_value;
> -
> -		if (gbl->active)
> -			flags |= gbl->def_value ?
> -				GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
> -		else
> -			flags |= gbl->def_value ?
> -				GPIOF_INIT_LOW : GPIOF_INIT_HIGH;
> +		flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
>   
>   		ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
>   					    pdata ? pdata->name : "backlight");
> diff --git a/include/linux/platform_data/gpio_backlight.h b/include/linux/platform_data/gpio_backlight.h
> index 5ae0d9c80d4d..683d90453c41 100644
> --- a/include/linux/platform_data/gpio_backlight.h
> +++ b/include/linux/platform_data/gpio_backlight.h
> @@ -14,7 +14,6 @@ struct gpio_backlight_platform_data {
>   	struct device *fbdev;
>   	int gpio;
>   	int def_value;
> -	bool active_low;
>   	const char *name;
>   };
>   
> 

Re: [PATCH 2/2] backlight: gpio: delete pdata inversion

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3619 bytes --]

On Tue, May 23, 2017 at 03:15:01PM +0100, Daniel Thompson wrote:
> On 30/04/17 09:17, Linus Walleij wrote:
> > The option to invert the output of the GPIO (active low) is
> > not used by the only platform still using platform data to
> > set up a GPIO backlight (one SH board). Delete the option
> > as we do not expect to expand the use of board files for
> > this driver, and GPIO descriptors intrinsically keep track
> > of any signal inversion.
> > 
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> 
> I note that only ~40% of Linux architectures currently have DT support at
> this point... There are still a few "embedded" ones that make do with
> platform bus (although they are growing increasingly niche).
> 
> Having said that I can't argue with "it's not used anywhere" (I've only
> really posted the above paragraph as a hint for future archeology... since
> it might be me doing the digging):
> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>

Just for the record, if somebody needs to use board files, the
gpio subsystem provides gpiod_lookup_table instead, as described
in: Documentation/gpio/board.txt.

-- Sebastian

> > ---
> >   drivers/video/backlight/gpio_backlight.c     | 15 ++-------------
> >   include/linux/platform_data/gpio_backlight.h |  1 -
> >   2 files changed, 2 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
> > index 17dd8071ad4d..8c19dc6c9236 100644
> > --- a/drivers/video/backlight/gpio_backlight.c
> > +++ b/drivers/video/backlight/gpio_backlight.c
> > @@ -25,7 +25,6 @@ struct gpio_backlight {
> >   	struct device *fbdev;
> >   	struct gpio_desc *gpiod;
> > -	int active;
> >   	int def_value;
> >   };
> > @@ -39,8 +38,7 @@ static int gpio_backlight_update_status(struct backlight_device *bl)
> >   	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> >   		brightness = 0;
> > -	gpiod_set_value_cansleep(gbl->gpiod,
> > -				 brightness ? gbl->active : !gbl->active);
> > +	gpiod_set_value_cansleep(gbl->gpiod, brightness);
> >   	return 0;
> >   }
> > @@ -72,8 +70,6 @@ static int gpio_backlight_probe_dt(struct platform_device *pdev,
> >   		flags = GPIOD_OUT_HIGH;
> >   	else
> >   		flags = GPIOD_OUT_LOW;
> > -	/* GPIO descriptors keep track of inversion */
> > -	gbl->active = 1;
> >   	gbl->gpiod = devm_gpiod_get(dev, NULL, flags);
> >   	if (IS_ERR(gbl->gpiod)) {
> > @@ -124,15 +120,8 @@ static int gpio_backlight_probe(struct platform_device *pdev)
> >   		unsigned long flags = GPIOF_DIR_OUT;
> >   		gbl->fbdev = pdata->fbdev;
> > -		gbl->active = pdata->active_low ? 0 : 1;
> >   		gbl->def_value = pdata->def_value;
> > -
> > -		if (gbl->active)
> > -			flags |= gbl->def_value ?
> > -				GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
> > -		else
> > -			flags |= gbl->def_value ?
> > -				GPIOF_INIT_LOW : GPIOF_INIT_HIGH;
> > +		flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
> >   		ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
> >   					    pdata ? pdata->name : "backlight");
> > diff --git a/include/linux/platform_data/gpio_backlight.h b/include/linux/platform_data/gpio_backlight.h
> > index 5ae0d9c80d4d..683d90453c41 100644
> > --- a/include/linux/platform_data/gpio_backlight.h
> > +++ b/include/linux/platform_data/gpio_backlight.h
> > @@ -14,7 +14,6 @@ struct gpio_backlight_platform_data {
> >   	struct device *fbdev;
> >   	int gpio;
> >   	int def_value;
> > -	bool active_low;
> >   	const char *name;
> >   };
> > 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] backlight: gpio: delete pdata inversion

[Lee Jones]

On Sun, 30 Apr 2017, Linus Walleij wrote:

> The option to invert the output of the GPIO (active low) is
> not used by the only platform still using platform data to
> set up a GPIO backlight (one SH board). Delete the option
> as we do not expect to expand the use of board files for
> this driver, and GPIO descriptors intrinsically keep track
> of any signal inversion.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/video/backlight/gpio_backlight.c     | 15 ++-------------
>  include/linux/platform_data/gpio_backlight.h |  1 -
>  2 files changed, 2 insertions(+), 14 deletions(-)

Applied, thanks.

> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
> index 17dd8071ad4d..8c19dc6c9236 100644
> --- a/drivers/video/backlight/gpio_backlight.c
> +++ b/drivers/video/backlight/gpio_backlight.c
> @@ -25,7 +25,6 @@ struct gpio_backlight {
>  	struct device *fbdev;
>  
>  	struct gpio_desc *gpiod;
> -	int active;
>  	int def_value;
>  };
>  
> @@ -39,8 +38,7 @@ static int gpio_backlight_update_status(struct backlight_device *bl)
>  	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
>  		brightness = 0;
>  
> -	gpiod_set_value_cansleep(gbl->gpiod,
> -				 brightness ? gbl->active : !gbl->active);
> +	gpiod_set_value_cansleep(gbl->gpiod, brightness);
>  
>  	return 0;
>  }
> @@ -72,8 +70,6 @@ static int gpio_backlight_probe_dt(struct platform_device *pdev,
>  		flags = GPIOD_OUT_HIGH;
>  	else
>  		flags = GPIOD_OUT_LOW;
> -	/* GPIO descriptors keep track of inversion */
> -	gbl->active = 1;
>  
>  	gbl->gpiod = devm_gpiod_get(dev, NULL, flags);
>  	if (IS_ERR(gbl->gpiod)) {
> @@ -124,15 +120,8 @@ static int gpio_backlight_probe(struct platform_device *pdev)
>  		unsigned long flags = GPIOF_DIR_OUT;
>  
>  		gbl->fbdev = pdata->fbdev;
> -		gbl->active = pdata->active_low ? 0 : 1;
>  		gbl->def_value = pdata->def_value;
> -
> -		if (gbl->active)
> -			flags |= gbl->def_value ?
> -				GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
> -		else
> -			flags |= gbl->def_value ?
> -				GPIOF_INIT_LOW : GPIOF_INIT_HIGH;
> +		flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
>  
>  		ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
>  					    pdata ? pdata->name : "backlight");
> diff --git a/include/linux/platform_data/gpio_backlight.h b/include/linux/platform_data/gpio_backlight.h
> index 5ae0d9c80d4d..683d90453c41 100644
> --- a/include/linux/platform_data/gpio_backlight.h
> +++ b/include/linux/platform_data/gpio_backlight.h
> @@ -14,7 +14,6 @@ struct gpio_backlight_platform_data {
>  	struct device *fbdev;
>  	int gpio;
>  	int def_value;
> -	bool active_low;
>  	const char *name;
>  };
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/2] backlight: gpio: delete pdata inversion

[Lee Jones]

On Tue, 30 May 2017, Lee Jones wrote:

> On Sun, 30 Apr 2017, Linus Walleij wrote:
> 
> > The option to invert the output of the GPIO (active low) is
> > not used by the only platform still using platform data to
> > set up a GPIO backlight (one SH board). Delete the option
> > as we do not expect to expand the use of board files for
> > this driver, and GPIO descriptors intrinsically keep track
> > of any signal inversion.
> > 
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> >  drivers/video/backlight/gpio_backlight.c     | 15 ++-------------
> >  include/linux/platform_data/gpio_backlight.h |  1 -
> >  2 files changed, 2 insertions(+), 14 deletions(-)
> 
> Applied, thanks.

Change of plan.

This patch depends on:

 "backlight: gpio: Convert to use GPIO descriptor"

... which is expecting an update.

> > diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
> > index 17dd8071ad4d..8c19dc6c9236 100644
> > --- a/drivers/video/backlight/gpio_backlight.c
> > +++ b/drivers/video/backlight/gpio_backlight.c
> > @@ -25,7 +25,6 @@ struct gpio_backlight {
> >  	struct device *fbdev;
> >  
> >  	struct gpio_desc *gpiod;
> > -	int active;
> >  	int def_value;
> >  };
> >  
> > @@ -39,8 +38,7 @@ static int gpio_backlight_update_status(struct backlight_device *bl)
> >  	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> >  		brightness = 0;
> >  
> > -	gpiod_set_value_cansleep(gbl->gpiod,
> > -				 brightness ? gbl->active : !gbl->active);
> > +	gpiod_set_value_cansleep(gbl->gpiod, brightness);
> >  
> >  	return 0;
> >  }
> > @@ -72,8 +70,6 @@ static int gpio_backlight_probe_dt(struct platform_device *pdev,
> >  		flags = GPIOD_OUT_HIGH;
> >  	else
> >  		flags = GPIOD_OUT_LOW;
> > -	/* GPIO descriptors keep track of inversion */
> > -	gbl->active = 1;
> >  
> >  	gbl->gpiod = devm_gpiod_get(dev, NULL, flags);
> >  	if (IS_ERR(gbl->gpiod)) {
> > @@ -124,15 +120,8 @@ static int gpio_backlight_probe(struct platform_device *pdev)
> >  		unsigned long flags = GPIOF_DIR_OUT;
> >  
> >  		gbl->fbdev = pdata->fbdev;
> > -		gbl->active = pdata->active_low ? 0 : 1;
> >  		gbl->def_value = pdata->def_value;
> > -
> > -		if (gbl->active)
> > -			flags |= gbl->def_value ?
> > -				GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
> > -		else
> > -			flags |= gbl->def_value ?
> > -				GPIOF_INIT_LOW : GPIOF_INIT_HIGH;
> > +		flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
> >  
> >  		ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
> >  					    pdata ? pdata->name : "backlight");
> > diff --git a/include/linux/platform_data/gpio_backlight.h b/include/linux/platform_data/gpio_backlight.h
> > index 5ae0d9c80d4d..683d90453c41 100644
> > --- a/include/linux/platform_data/gpio_backlight.h
> > +++ b/include/linux/platform_data/gpio_backlight.h
> > @@ -14,7 +14,6 @@ struct gpio_backlight_platform_data {
> >  	struct device *fbdev;
> >  	int gpio;
> >  	int def_value;
> > -	bool active_low;
> >  	const char *name;
> >  };
> >  
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/2] backlight: gpio: delete pdata inversion

[Linus Walleij]

On Tue, May 30, 2017 at 10:29 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 30 May 2017, Lee Jones wrote:
>
>> On Sun, 30 Apr 2017, Linus Walleij wrote:
>>
>> > The option to invert the output of the GPIO (active low) is
>> > not used by the only platform still using platform data to
>> > set up a GPIO backlight (one SH board). Delete the option
>> > as we do not expect to expand the use of board files for
>> > this driver, and GPIO descriptors intrinsically keep track
>> > of any signal inversion.
>> >
>> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> > ---
>> >  drivers/video/backlight/gpio_backlight.c     | 15 ++-------------
>> >  include/linux/platform_data/gpio_backlight.h |  1 -
>> >  2 files changed, 2 insertions(+), 14 deletions(-)
>>
>> Applied, thanks.
>
> Change of plan.
>
> This patch depends on:
>
>  "backlight: gpio: Convert to use GPIO descriptor"

Oh, sorry for sluggishness.

I've updated it, resending a v2 now.

Yours,
Linus Walleij