On Tue, Dec 03, 2019 at 01:31:43PM +0100, Linus Walleij wrote:
> The code in the Corgi backlight driver can be considerably
> simplified by moving to GPIO descriptors and lookup tables
> from the board files instead of passing GPIO numbers using
> the old API.
> 
> Make sure to encode inversion semantics for the Akita and
> Spitz platforms inside the GPIO lookup table and drop the
> custom inversion semantics from the driver.
> 
> All in-tree users are converted in this patch.
> 
> Cc: Andrea Adami <andrea.ad...@gmail.com>
> Cc: Robert Jarzmik <robert.jarz...@free.fr>
> Signed-off-by: Linus Walleij <linus.wall...@linaro.org>

Reviewed-by: Daniel Thompson <daniel.thomp...@linaro.org>

> ---
>  arch/arm/mach-pxa/corgi.c           | 12 ++++-
>  arch/arm/mach-pxa/spitz.c           | 34 +++++++++++----
>  drivers/video/backlight/corgi_lcd.c | 68 ++++++++---------------------
>  include/linux/spi/corgi_lcd.h       |  3 --
>  4 files changed, 54 insertions(+), 63 deletions(-)
> 
> diff --git a/arch/arm/mach-pxa/corgi.c b/arch/arm/mach-pxa/corgi.c
> index f2d73289230f..c9625dcae27c 100644
> --- a/arch/arm/mach-pxa/corgi.c
> +++ b/arch/arm/mach-pxa/corgi.c
> @@ -563,13 +563,20 @@ static void corgi_bl_kick_battery(void)
>       }
>  }
>  
> +static struct gpiod_lookup_table corgi_lcdcon_gpio_table = {
> +     .dev_id = "spi0.1",
> +     .table = {
> +             GPIO_LOOKUP("gpio-pxa", CORGI_GPIO_BACKLIGHT_CONT,
> +                         "BL_CONT", GPIO_ACTIVE_HIGH),
> +             { },
> +     },
> +};
> +
>  static struct corgi_lcd_platform_data corgi_lcdcon_info = {
>       .init_mode              = CORGI_LCD_MODE_VGA,
>       .max_intensity          = 0x2f,
>       .default_intensity      = 0x1f,
>       .limit_mask             = 0x0b,
> -     .gpio_backlight_cont    = CORGI_GPIO_BACKLIGHT_CONT,
> -     .gpio_backlight_on      = -1,
>       .kick_battery           = corgi_bl_kick_battery,
>  };
>  
> @@ -609,6 +616,7 @@ static struct spi_board_info corgi_spi_devices[] = {
>  static void __init corgi_init_spi(void)
>  {
>       pxa2xx_set_spi_info(1, &corgi_spi_info);
> +     gpiod_add_lookup_table(&corgi_lcdcon_gpio_table);
>       spi_register_board_info(ARRAY_AND_SIZE(corgi_spi_devices));
>  }
>  #else
> diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
> index a4fdc399d152..82e80a257c0f 100644
> --- a/arch/arm/mach-pxa/spitz.c
> +++ b/arch/arm/mach-pxa/spitz.c
> @@ -525,13 +525,33 @@ static void spitz_bl_kick_battery(void)
>       }
>  }
>  
> +static struct gpiod_lookup_table spitz_lcdcon_gpio_table = {
> +     .dev_id = "spi0.1",
> +     .table = {
> +             GPIO_LOOKUP("gpio-pxa", SPITZ_GPIO_BACKLIGHT_CONT,
> +                         "BL_CONT", GPIO_ACTIVE_LOW),
> +             GPIO_LOOKUP("gpio-pxa", SPITZ_GPIO_BACKLIGHT_ON,
> +                         "BL_ON", GPIO_ACTIVE_HIGH),
> +             { },
> +     },
> +};
> +
> +static struct gpiod_lookup_table akita_lcdcon_gpio_table = {
> +     .dev_id = "spi0.1",
> +     .table = {
> +             GPIO_LOOKUP("gpio-pxa", AKITA_GPIO_BACKLIGHT_CONT,
> +                         "BL_CONT", GPIO_ACTIVE_LOW),
> +             GPIO_LOOKUP("gpio-pxa", AKITA_GPIO_BACKLIGHT_ON,
> +                         "BL_ON", GPIO_ACTIVE_HIGH),
> +             { },
> +     },
> +};
> +
>  static struct corgi_lcd_platform_data spitz_lcdcon_info = {
>       .init_mode              = CORGI_LCD_MODE_VGA,
>       .max_intensity          = 0x2f,
>       .default_intensity      = 0x1f,
>       .limit_mask             = 0x0b,
> -     .gpio_backlight_cont    = SPITZ_GPIO_BACKLIGHT_CONT,
> -     .gpio_backlight_on      = SPITZ_GPIO_BACKLIGHT_ON,
>       .kick_battery           = spitz_bl_kick_battery,
>  };
>  
> @@ -574,12 +594,10 @@ static struct pxa2xx_spi_controller spitz_spi_info = {
>  
>  static void __init spitz_spi_init(void)
>  {
> -     struct corgi_lcd_platform_data *lcd_data = &spitz_lcdcon_info;
> -
> -     if (machine_is_akita()) {
> -             lcd_data->gpio_backlight_cont = AKITA_GPIO_BACKLIGHT_CONT;
> -             lcd_data->gpio_backlight_on = AKITA_GPIO_BACKLIGHT_ON;
> -     }
> +     if (machine_is_akita())
> +             gpiod_add_lookup_table(&akita_lcdcon_gpio_table);
> +     else
> +             gpiod_add_lookup_table(&spitz_lcdcon_gpio_table);
>  
>       pxa2xx_set_spi_info(2, &spitz_spi_info);
>       spi_register_board_info(ARRAY_AND_SIZE(spitz_spi_devices));
> diff --git a/drivers/video/backlight/corgi_lcd.c 
> b/drivers/video/backlight/corgi_lcd.c
> index 68f7592c5060..25ef0cbd7583 100644
> --- a/drivers/video/backlight/corgi_lcd.c
> +++ b/drivers/video/backlight/corgi_lcd.c
> @@ -15,7 +15,7 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/delay.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/fb.h>
>  #include <linux/lcd.h>
>  #include <linux/spi/spi.h>
> @@ -90,9 +90,8 @@ struct corgi_lcd {
>       int     mode;
>       char    buf[2];
>  
> -     int     gpio_backlight_on;
> -     int     gpio_backlight_cont;
> -     int     gpio_backlight_cont_inverted;
> +     struct gpio_desc *backlight_on;
> +     struct gpio_desc *backlight_cont;
>  
>       void (*kick_battery)(void);
>  };
> @@ -403,13 +402,13 @@ static int corgi_bl_set_intensity(struct corgi_lcd 
> *lcd, int intensity)
>       corgi_ssp_lcdtg_send(lcd, DUTYCTRL_ADRS, intensity);
>  
>       /* Bit 5 via GPIO_BACKLIGHT_CONT */
> -     cont = !!(intensity & 0x20) ^ lcd->gpio_backlight_cont_inverted;
> +     cont = !!(intensity & 0x20);
>  
> -     if (gpio_is_valid(lcd->gpio_backlight_cont))
> -             gpio_set_value_cansleep(lcd->gpio_backlight_cont, cont);
> +     if (lcd->backlight_cont)
> +             gpiod_set_value_cansleep(lcd->backlight_cont, cont);
>  
> -     if (gpio_is_valid(lcd->gpio_backlight_on))
> -             gpio_set_value_cansleep(lcd->gpio_backlight_on, intensity);
> +     if (lcd->backlight_on)
> +             gpiod_set_value_cansleep(lcd->backlight_on, intensity);
>  
>       if (lcd->kick_battery)
>               lcd->kick_battery();
> @@ -482,48 +481,17 @@ static int setup_gpio_backlight(struct corgi_lcd *lcd,
>                               struct corgi_lcd_platform_data *pdata)
>  {
>       struct spi_device *spi = lcd->spi_dev;
> -     int err;
> -
> -     lcd->gpio_backlight_on = -1;
> -     lcd->gpio_backlight_cont = -1;
> -
> -     if (gpio_is_valid(pdata->gpio_backlight_on)) {
> -             err = devm_gpio_request(&spi->dev, pdata->gpio_backlight_on,
> -                                     "BL_ON");
> -             if (err) {
> -                     dev_err(&spi->dev,
> -                             "failed to request GPIO%d for backlight_on\n",
> -                             pdata->gpio_backlight_on);
> -                     return err;
> -             }
> -
> -             lcd->gpio_backlight_on = pdata->gpio_backlight_on;
> -             gpio_direction_output(lcd->gpio_backlight_on, 0);
> -     }
>  
> -     if (gpio_is_valid(pdata->gpio_backlight_cont)) {
> -             err = devm_gpio_request(&spi->dev, pdata->gpio_backlight_cont,
> -                                     "BL_CONT");
> -             if (err) {
> -                     dev_err(&spi->dev,
> -                             "failed to request GPIO%d for backlight_cont\n",
> -                             pdata->gpio_backlight_cont);
> -                     return err;
> -             }
> -
> -             lcd->gpio_backlight_cont = pdata->gpio_backlight_cont;
> -
> -             /* spitz and akita use both GPIOs for backlight, and
> -              * have inverted polarity of GPIO_BACKLIGHT_CONT
> -              */
> -             if (gpio_is_valid(lcd->gpio_backlight_on)) {
> -                     lcd->gpio_backlight_cont_inverted = 1;
> -                     gpio_direction_output(lcd->gpio_backlight_cont, 1);
> -             } else {
> -                     lcd->gpio_backlight_cont_inverted = 0;
> -                     gpio_direction_output(lcd->gpio_backlight_cont, 0);
> -             }
> -     }
> +     lcd->backlight_on = devm_gpiod_get_optional(&spi->dev,
> +                                                 "BL_ON", GPIOD_OUT_LOW);
> +     if (IS_ERR(lcd->backlight_on))
> +             return PTR_ERR(lcd->backlight_on);
> +
> +     lcd->backlight_cont = devm_gpiod_get_optional(&spi->dev, "BL_CONT",
> +                                                   GPIOD_OUT_LOW);
> +     if (IS_ERR(lcd->backlight_cont))
> +             return PTR_ERR(lcd->backlight_cont);
> +
>       return 0;
>  }
>  
> diff --git a/include/linux/spi/corgi_lcd.h b/include/linux/spi/corgi_lcd.h
> index edf4beccdadb..0b857616919c 100644
> --- a/include/linux/spi/corgi_lcd.h
> +++ b/include/linux/spi/corgi_lcd.h
> @@ -11,9 +11,6 @@ struct corgi_lcd_platform_data {
>       int     default_intensity;
>       int     limit_mask;
>  
> -     int     gpio_backlight_on;      /* -1 if n/a */
> -     int     gpio_backlight_cont;    /* -1 if n/a */
> -
>       void (*notify)(int intensity);
>       void (*kick_battery)(void);
>  };
> -- 
> 2.23.0
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to