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 <[email protected]>
> Signed-off-by: Linus Walleij <[email protected]>
> ---
> 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

Reply via email to