| Message ID | 20170530114822.24308-1-linus.walleij@linaro.org |
|---|---|
| State | Accepted |
| Commit | de7389003ab77fc63296b2adee4cd9c48dc41d5a |
| Headers | show |
| Series | [1/2,v2] backlight: gpio: Convert to use GPIO descriptor | expand |
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
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
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
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
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));