* [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor
@ 2017-05-30 11:48 Linus Walleij
2017-05-30 11:48 ` [PATCH 2/2 v2] backlight: gpio: delete pdata inversion Linus Walleij
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Linus Walleij @ 2017-05-30 11:48 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, linux-kernel
Cc: Laurent Pinchart, Linus Walleij
This driver is predominantly used by device tree systems, all
of which can deal with modern GPIO descriptors. The legacy
GPIO API is only used by one SH board so make the GPIO
descriptor the default way to deal with it.
As an intended side effect we do not need to look around in
the device tree for the inversion flag since the GPIO
descriptors will intrinsically deal with this.
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Assign flags value using the ternary operator.
---
drivers/video/backlight/gpio_backlight.c | 71 +++++++++++++++++++-------------
1 file changed, 43 insertions(+), 28 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
index 18134416b154..5ffaff1e4142 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -9,7 +9,8 @@
#include <linux/backlight.h>
#include <linux/err.h>
#include <linux/fb.h>
-#include <linux/gpio.h>
+#include <linux/gpio.h> /* Only for legacy support */
+#include <linux/gpio/consumer.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -23,7 +24,7 @@ struct gpio_backlight {
struct device *dev;
struct device *fbdev;
- int gpio;
+ struct gpio_desc *gpiod;
int active;
int def_value;
};
@@ -38,8 +39,8 @@ static int gpio_backlight_update_status(struct backlight_device *bl)
bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
brightness = 0;
- gpio_set_value_cansleep(gbl->gpio,
- brightness ? gbl->active : !gbl->active);
+ gpiod_set_value_cansleep(gbl->gpiod,
+ brightness ? gbl->active : !gbl->active);
return 0;
}
@@ -61,23 +62,27 @@ static const struct backlight_ops gpio_backlight_ops = {
static int gpio_backlight_probe_dt(struct platform_device *pdev,
struct gpio_backlight *gbl)
{
- struct device_node *np = pdev->dev.of_node;
- enum of_gpio_flags gpio_flags;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ enum gpiod_flags flags;
+ int ret;
+
+ gbl->def_value = of_property_read_bool(np, "default-on");
+ flags = gbl->def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
+ /* GPIO descriptors keep track of inversion */
+ gbl->active = 1;
- gbl->gpio = of_get_gpio_flags(np, 0, &gpio_flags);
+ gbl->gpiod = devm_gpiod_get(dev, NULL, flags);
+ if (IS_ERR(gbl->gpiod)) {
+ ret = PTR_ERR(gbl->gpiod);
- if (!gpio_is_valid(gbl->gpio)) {
- if (gbl->gpio != -EPROBE_DEFER) {
- dev_err(&pdev->dev,
+ if (ret != -EPROBE_DEFER) {
+ dev_err(dev,
"Error: The gpios parameter is missing or invalid.\n");
}
- return gbl->gpio;
+ return ret;
}
- gbl->active = (gpio_flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
-
- gbl->def_value = of_property_read_bool(np, "default-on");
-
return 0;
}
@@ -89,7 +94,6 @@ static int gpio_backlight_probe(struct platform_device *pdev)
struct backlight_device *bl;
struct gpio_backlight *gbl;
struct device_node *np = pdev->dev.of_node;
- unsigned long flags = GPIOF_DIR_OUT;
int ret;
if (!pdata && !np) {
@@ -109,22 +113,33 @@ static int gpio_backlight_probe(struct platform_device *pdev)
if (ret)
return ret;
} else {
+ /*
+ * Legacy platform data GPIO retrieveal. Do not expand
+ * the use of this code path, currently only used by one
+ * SH board.
+ */
+ unsigned long flags = GPIOF_DIR_OUT;
+
gbl->fbdev = pdata->fbdev;
- gbl->gpio = pdata->gpio;
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;
- ret = devm_gpio_request_one(gbl->dev, gbl->gpio, flags,
- pdata ? pdata->name : "backlight");
- if (ret < 0) {
- dev_err(&pdev->dev, "unable to request GPIO\n");
- return ret;
+ if (gbl->active)
+ flags |= gbl->def_value ?
+ GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
+ else
+ flags |= gbl->def_value ?
+ GPIOF_INIT_LOW : GPIOF_INIT_HIGH;
+
+ ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
+ pdata ? pdata->name : "backlight");
+ if (ret < 0) {
+ dev_err(&pdev->dev, "unable to request GPIO\n");
+ return ret;
+ }
+ gbl->gpiod = gpio_to_desc(pdata->gpio);
+ if (!gbl->gpiod)
+ return -EINVAL;
}
memset(&props, 0, sizeof(props));
--
2.9.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/2 v2] backlight: gpio: delete pdata inversion
2017-05-30 11:48 [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor Linus Walleij
@ 2017-05-30 11:48 ` Linus Walleij
2017-08-01 10:28 ` Laurent Pinchart
2017-08-07 16:12 ` Lee Jones
2017-08-01 8:20 ` [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor Linus Walleij
2017-08-07 16:12 ` Lee Jones
2 siblings, 2 replies; 8+ messages in thread
From: Linus Walleij @ 2017-05-30 11:48 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, linux-kernel
Cc: Laurent Pinchart, 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.
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Rebase on the v2 of patch [1/2]
---
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 5ffaff1e4142..e470da95d806 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;
}
@@ -69,8 +67,6 @@ static int gpio_backlight_probe_dt(struct platform_device *pdev,
gbl->def_value = of_property_read_bool(np, "default-on");
flags = gbl->def_value ? GPIOD_OUT_HIGH : 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)) {
@@ -121,15 +117,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.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/2 v2] backlight: gpio: delete pdata inversion
2017-05-30 11:48 ` [PATCH 2/2 v2] backlight: gpio: delete pdata inversion Linus Walleij
@ 2017-08-01 10:28 ` Laurent Pinchart
2017-08-07 16:12 ` Lee Jones
1 sibling, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2017-08-01 10:28 UTC (permalink / raw)
To: Linus Walleij; +Cc: Lee Jones, Daniel Thompson, Jingoo Han, linux-kernel
Hi Linus,
Thank you for the patch.
On Tuesday 30 May 2017 13:48:22 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.
>
> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> ChangeLog v1->v2:
> - Rebase on the v2 of patch [1/2]
> ---
> 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 5ffaff1e4142..e470da95d806
> 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;
> }
> @@ -69,8 +67,6 @@ static int gpio_backlight_probe_dt(struct platform_device
> *pdev,
>
> gbl->def_value = of_property_read_bool(np, "default-on");
> flags = gbl->def_value ? GPIOD_OUT_HIGH : 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)) {
> @@ -121,15 +117,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;
> };
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/2 v2] backlight: gpio: delete pdata inversion
2017-05-30 11:48 ` [PATCH 2/2 v2] backlight: gpio: delete pdata inversion Linus Walleij
2017-08-01 10:28 ` Laurent Pinchart
@ 2017-08-07 16:12 ` Lee Jones
1 sibling, 0 replies; 8+ messages in thread
From: Lee Jones @ 2017-08-07 16:12 UTC (permalink / raw)
To: Linus Walleij; +Cc: Daniel Thompson, Jingoo Han, linux-kernel, Laurent Pinchart
On Tue, 30 May 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.
>
> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Rebase on the v2 of patch [1/2]
> ---
> 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 5ffaff1e4142..e470da95d806 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;
> }
> @@ -69,8 +67,6 @@ static int gpio_backlight_probe_dt(struct platform_device *pdev,
>
> gbl->def_value = of_property_read_bool(np, "default-on");
> flags = gbl->def_value ? GPIOD_OUT_HIGH : 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)) {
> @@ -121,15 +117,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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor
2017-05-30 11:48 [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor Linus Walleij
2017-05-30 11:48 ` [PATCH 2/2 v2] backlight: gpio: delete pdata inversion Linus Walleij
@ 2017-08-01 8:20 ` Linus Walleij
2017-08-01 10:29 ` Laurent Pinchart
2017-08-07 7:53 ` Lee Jones
2017-08-07 16:12 ` Lee Jones
2 siblings, 2 replies; 8+ messages in thread
From: Linus Walleij @ 2017-08-01 8:20 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, linux-kernel
Cc: Laurent Pinchart, Linus Walleij
On Tue, May 30, 2017 at 1:48 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> This driver is predominantly used by device tree systems, all
> of which can deal with modern GPIO descriptors. The legacy
> GPIO API is only used by one SH board so make the GPIO
> descriptor the default way to deal with it.
>
> As an intended side effect we do not need to look around in
> the device tree for the inversion flag since the GPIO
> descriptors will intrinsically deal with this.
>
> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Assign flags value using the ternary operator.
This patch seems to have been inadvertedly dropped?
Can it be applied, along with 2/2?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor
2017-08-01 8:20 ` [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor Linus Walleij
@ 2017-08-01 10:29 ` Laurent Pinchart
2017-08-07 7:53 ` Lee Jones
1 sibling, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2017-08-01 10:29 UTC (permalink / raw)
To: Linus Walleij; +Cc: Lee Jones, Daniel Thompson, Jingoo Han, linux-kernel
Hi Linus,
Thank you for the patch.
On Tuesday 01 Aug 2017 10:20:32 Linus Walleij wrote:
> On Tue, May 30, 2017 at 1:48 PM, Linus Walleij <linus.walleij@linaro.org>
wrote:
> > This driver is predominantly used by device tree systems, all
> > of which can deal with modern GPIO descriptors. The legacy
> > GPIO API is only used by one SH board so make the GPIO
> > descriptor the default way to deal with it.
> >
> > As an intended side effect we do not need to look around in
> > the device tree for the inversion flag since the GPIO
> > descriptors will intrinsically deal with this.
> >
> > Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > ChangeLog v1->v2:
> > - Assign flags value using the ternary operator.
>
> This patch seems to have been inadvertedly dropped?
>
> Can it be applied, along with 2/2?
>
> Yours,
> Linus Walleij
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor
2017-08-01 8:20 ` [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor Linus Walleij
2017-08-01 10:29 ` Laurent Pinchart
@ 2017-08-07 7:53 ` Lee Jones
1 sibling, 0 replies; 8+ messages in thread
From: Lee Jones @ 2017-08-07 7:53 UTC (permalink / raw)
To: Linus Walleij; +Cc: Daniel Thompson, Jingoo Han, linux-kernel, Laurent Pinchart
On Tue, 01 Aug 2017, Linus Walleij wrote:
> On Tue, May 30, 2017 at 1:48 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> > This driver is predominantly used by device tree systems, all
> > of which can deal with modern GPIO descriptors. The legacy
> > GPIO API is only used by one SH board so make the GPIO
> > descriptor the default way to deal with it.
> >
> > As an intended side effect we do not need to look around in
> > the device tree for the inversion flag since the GPIO
> > descriptors will intrinsically deal with this.
> >
> > Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > ChangeLog v1->v2:
> > - Assign flags value using the ternary operator.
>
> This patch seems to have been inadvertedly dropped?
>
> Can it be applied, along with 2/2?
The patch hasn't been dropped. It just hasn't been picked up yet,
since I have been on vacation. This is my first day back.
I am conducting a first (quick) pass of my inbox now. Will hoover up
on my second (thorough) pass later today.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor
2017-05-30 11:48 [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor Linus Walleij
2017-05-30 11:48 ` [PATCH 2/2 v2] backlight: gpio: delete pdata inversion Linus Walleij
2017-08-01 8:20 ` [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor Linus Walleij
@ 2017-08-07 16:12 ` Lee Jones
2 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2017-08-07 16:12 UTC (permalink / raw)
To: Linus Walleij; +Cc: Daniel Thompson, Jingoo Han, linux-kernel, Laurent Pinchart
On Tue, 30 May 2017, Linus Walleij wrote:
> This driver is predominantly used by device tree systems, all
> of which can deal with modern GPIO descriptors. The legacy
> GPIO API is only used by one SH board so make the GPIO
> descriptor the default way to deal with it.
>
> As an intended side effect we do not need to look around in
> the device tree for the inversion flag since the GPIO
> descriptors will intrinsically deal with this.
>
> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Assign flags value using the ternary operator.
> ---
> drivers/video/backlight/gpio_backlight.c | 71 +++++++++++++++++++-------------
> 1 file changed, 43 insertions(+), 28 deletions(-)
Applied, thanks.
> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
> index 18134416b154..5ffaff1e4142 100644
> --- a/drivers/video/backlight/gpio_backlight.c
> +++ b/drivers/video/backlight/gpio_backlight.c
> @@ -9,7 +9,8 @@
> #include <linux/backlight.h>
> #include <linux/err.h>
> #include <linux/fb.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio.h> /* Only for legacy support */
> +#include <linux/gpio/consumer.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> @@ -23,7 +24,7 @@ struct gpio_backlight {
> struct device *dev;
> struct device *fbdev;
>
> - int gpio;
> + struct gpio_desc *gpiod;
> int active;
> int def_value;
> };
> @@ -38,8 +39,8 @@ static int gpio_backlight_update_status(struct backlight_device *bl)
> bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> brightness = 0;
>
> - gpio_set_value_cansleep(gbl->gpio,
> - brightness ? gbl->active : !gbl->active);
> + gpiod_set_value_cansleep(gbl->gpiod,
> + brightness ? gbl->active : !gbl->active);
>
> return 0;
> }
> @@ -61,23 +62,27 @@ static const struct backlight_ops gpio_backlight_ops = {
> static int gpio_backlight_probe_dt(struct platform_device *pdev,
> struct gpio_backlight *gbl)
> {
> - struct device_node *np = pdev->dev.of_node;
> - enum of_gpio_flags gpio_flags;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + enum gpiod_flags flags;
> + int ret;
> +
> + gbl->def_value = of_property_read_bool(np, "default-on");
> + flags = gbl->def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
> + /* GPIO descriptors keep track of inversion */
> + gbl->active = 1;
>
> - gbl->gpio = of_get_gpio_flags(np, 0, &gpio_flags);
> + gbl->gpiod = devm_gpiod_get(dev, NULL, flags);
> + if (IS_ERR(gbl->gpiod)) {
> + ret = PTR_ERR(gbl->gpiod);
>
> - if (!gpio_is_valid(gbl->gpio)) {
> - if (gbl->gpio != -EPROBE_DEFER) {
> - dev_err(&pdev->dev,
> + if (ret != -EPROBE_DEFER) {
> + dev_err(dev,
> "Error: The gpios parameter is missing or invalid.\n");
> }
> - return gbl->gpio;
> + return ret;
> }
>
> - gbl->active = (gpio_flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
> -
> - gbl->def_value = of_property_read_bool(np, "default-on");
> -
> return 0;
> }
>
> @@ -89,7 +94,6 @@ static int gpio_backlight_probe(struct platform_device *pdev)
> struct backlight_device *bl;
> struct gpio_backlight *gbl;
> struct device_node *np = pdev->dev.of_node;
> - unsigned long flags = GPIOF_DIR_OUT;
> int ret;
>
> if (!pdata && !np) {
> @@ -109,22 +113,33 @@ static int gpio_backlight_probe(struct platform_device *pdev)
> if (ret)
> return ret;
> } else {
> + /*
> + * Legacy platform data GPIO retrieveal. Do not expand
> + * the use of this code path, currently only used by one
> + * SH board.
> + */
> + unsigned long flags = GPIOF_DIR_OUT;
> +
> gbl->fbdev = pdata->fbdev;
> - gbl->gpio = pdata->gpio;
> 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;
>
> - ret = devm_gpio_request_one(gbl->dev, gbl->gpio, flags,
> - pdata ? pdata->name : "backlight");
> - if (ret < 0) {
> - dev_err(&pdev->dev, "unable to request GPIO\n");
> - return ret;
> + if (gbl->active)
> + flags |= gbl->def_value ?
> + GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
> + else
> + flags |= gbl->def_value ?
> + GPIOF_INIT_LOW : GPIOF_INIT_HIGH;
> +
> + ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
> + pdata ? pdata->name : "backlight");
> + if (ret < 0) {
> + dev_err(&pdev->dev, "unable to request GPIO\n");
> + return ret;
> + }
> + gbl->gpiod = gpio_to_desc(pdata->gpio);
> + if (!gbl->gpiod)
> + return -EINVAL;
> }
>
> memset(&props, 0, sizeof(props));
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-08-07 16:12 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-30 11:48 [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor Linus Walleij
2017-05-30 11:48 ` [PATCH 2/2 v2] backlight: gpio: delete pdata inversion Linus Walleij
2017-08-01 10:28 ` Laurent Pinchart
2017-08-07 16:12 ` Lee Jones
2017-08-01 8:20 ` [PATCH 1/2 v2] backlight: gpio: Convert to use GPIO descriptor Linus Walleij
2017-08-01 10:29 ` Laurent Pinchart
2017-08-07 7:53 ` Lee Jones
2017-08-07 16:12 ` Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).