[PATCH] backlight: corgi: Convert to use GPIO descriptors

[PATCH] backlight: corgi: Convert to use GPIO descriptors

[Linus Walleij]

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.adami@gmail.com>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Signed-off-by: Linus Walleij <linus.walleij@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

Re: [PATCH] backlight: corgi: Convert to use GPIO descriptors

[Daniel Thompson]

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.adami@gmail.com>
> Cc: Robert Jarzmik <robert.jarzmik@free.fr>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Reviewed-by: Daniel Thompson <daniel.thompson@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

Re: [PATCH] backlight: corgi: Convert to use GPIO descriptors

[Robert Jarzmik]

Linus Walleij <linus.walleij@linaro.org> writes:

Hi Linus,

> @@ -525,13 +525,33 @@ static void spitz_bl_kick_battery(void)
>  	}
>  }
>  
> +static struct gpiod_lookup_table spitz_lcdcon_gpio_table = {
> +	.dev_id = "spi0.1",
How do you know the correct device name is "spi0.1" ?

Cheers.

--
Robert
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: corgi: Convert to use GPIO descriptors

[Linus Walleij]

On Sun, Dec 8, 2019 at 9:06 PM Robert Jarzmik <robert.jarzmik@free.fr> wrote:

> Linus Walleij <linus.walleij@linaro.org> writes:
> > @@ -525,13 +525,33 @@ static void spitz_bl_kick_battery(void)
> >       }
> >  }
> >
> > +static struct gpiod_lookup_table spitz_lcdcon_gpio_table = {
> > +     .dev_id = "spi0.1",
> How do you know the correct device name is "spi0.1" ?

With SPI devices it is always hard to know without access to the
actual hardware, so every patch is a request for testing...

I looked at arch/arm/mach-pxa/spitz.c and
it registers just one spi bus (AFAICT) with 3 chip
selects so that will be "spi0", and then
spi_register_board_info() is called with an array of 3
devices (spitz_spi_devices[]). Those are in order of
chip select so chip select 0, 1, 2. This is the second
device so chip select 1.

The code in drivers/spi/spi.c names the devices
using spi_dev_set_name() like this:
dev_set_name(&spi->dev, "%s.%u", dev_name(&spi->controller->dev),
                     spi->chip_select);

So it will theoretically "spi0.1"

Beware about bugs in the above interpreter because it is
just my brain.

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: corgi: Convert to use GPIO descriptors

[Robert Jarzmik]

Linus Walleij <linus.walleij@linaro.org> writes:

> On Sun, Dec 8, 2019 at 9:06 PM Robert Jarzmik <robert.jarzmik@free.fr> wrote:
>
>> Linus Walleij <linus.walleij@linaro.org> writes:
>> > @@ -525,13 +525,33 @@ static void spitz_bl_kick_battery(void)
>> >       }
>> >  }
>> >
>> > +static struct gpiod_lookup_table spitz_lcdcon_gpio_table = {
>> > +     .dev_id = "spi0.1",
>> How do you know the correct device name is "spi0.1" ?
>
> With SPI devices it is always hard to know without access to the
> actual hardware, so every patch is a request for testing...
>
> I looked at arch/arm/mach-pxa/spitz.c and
> it registers just one spi bus (AFAICT) with 3 chip
> selects so that will be "spi0", and then
> spi_register_board_info() is called with an array of 3
> devices (spitz_spi_devices[]). Those are in order of
> chip select so chip select 0, 1, 2. This is the second
> device so chip select 1.
>
> The code in drivers/spi/spi.c names the devices
> using spi_dev_set_name() like this:
> dev_set_name(&spi->dev, "%s.%u", dev_name(&spi->controller->dev),
>                      spi->chip_select);
>
> So it will theoretically "spi0.1"
>
> Beware about bugs in the above interpreter because it is
> just my brain.
Well, if nobody complains because of testing, why not, your explanation seems as
good as it could be.

Moreover my boards are actually packed for a house move, so I hope for the best
with all the patches for gpiod conversion, as I won't be able to test much for a
month.

If you would be so kind as to carry these changes through your tree instead of
the PXA one, please have my :
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>

Cheers.

-- 
Robert
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: corgi: Convert to use GPIO descriptors

[Linus Walleij]

On Fri, Dec 13, 2019 at 6:24 PM Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Linus Walleij <linus.walleij@linaro.org> writes:
> > On Sun, Dec 8, 2019 at 9:06 PM Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> >> Linus Walleij <linus.walleij@linaro.org> writes:

> > So it will theoretically "spi0.1"
> >
> > Beware about bugs in the above interpreter because it is
> > just my brain.
>
> Well, if nobody complains because of testing, why not, your explanation seems as
> good as it could be.

:D

> If you would be so kind as to carry these changes through your tree instead of
> the PXA one, please have my :
> Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>

I was hoping the backlight people would pick it up to the backlight tree.

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: corgi: Convert to use GPIO descriptors

[Lee Jones]

On Sat, 14 Dec 2019, Linus Walleij wrote:

> On Fri, Dec 13, 2019 at 6:24 PM Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> > Linus Walleij <linus.walleij@linaro.org> writes:
> > > On Sun, Dec 8, 2019 at 9:06 PM Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> > >> Linus Walleij <linus.walleij@linaro.org> writes:
> 
> > > So it will theoretically "spi0.1"
> > >
> > > Beware about bugs in the above interpreter because it is
> > > just my brain.
> >
> > Well, if nobody complains because of testing, why not, your explanation seems as
> > good as it could be.
> 
> :D
> 
> > If you would be so kind as to carry these changes through your tree instead of
> > the PXA one, please have my :
> > Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
> 
> I was hoping the backlight people would pick it up to the backlight tree.

Yep, that's the plan.

Big update today, bear with.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: corgi: Convert to use GPIO descriptors

[Lee Jones]

On Tue, 03 Dec 2019, 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.adami@gmail.com>
> Cc: Robert Jarzmik <robert.jarzmik@free.fr>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  arch/arm/mach-pxa/corgi.c           | 12 ++++-
>  arch/arm/mach-pxa/spitz.c           | 34 +++++++++++----

Does this have an Arm Ack yet?

>  drivers/video/backlight/corgi_lcd.c | 68 ++++++++---------------------
>  include/linux/spi/corgi_lcd.h       |  3 --

What about SPI?

>  4 files changed, 54 insertions(+), 63 deletions(-)

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] backlight: corgi: Convert to use GPIO descriptors

[Linus Walleij]

On Mon, Dec 16, 2019 at 5:27 PM Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 03 Dec 2019, 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.adami@gmail.com>
> > Cc: Robert Jarzmik <robert.jarzmik@free.fr>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> >  arch/arm/mach-pxa/corgi.c           | 12 ++++-
> >  arch/arm/mach-pxa/spitz.c           | 34 +++++++++++----
>
> Does this have an Arm Ack yet?

Yes Robert Jarzmik is the maintainer of arch/arm/mach-pxa,
or do you mean I should ask for the ARM SoC maintainers'
ACK explicitly (then I'd just resend, NP).

> >  drivers/video/backlight/corgi_lcd.c | 68 ++++++++---------------------
> >  include/linux/spi/corgi_lcd.h       |  3 --
>
> What about SPI?

That include file should technically be in the backlight
include path or platform_data (yeah someone should
fix...) but let's ask Mark explicitly if this is OK.

Mark: the hunk hitting the include/linux/spi/corgi_lcd.h
file looks like this:

@@ -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);
 };

OK with you? If you want I can resend the whole patch
as well so you can have a look.

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel