[PATCH 1/3 v5] gpio: pcf857x: Name instance after dev_name()

[PATCH 1/3 v5] gpio: pcf857x: Name instance after dev_name()

[Linus Walleij]

Put the label on this gpio_chip from the dev_name() instead of
the client name.

The client name will be pcf8574 etc for all instances even if
there are several chips on a system.

This manifests on the DaVinci DM6467 (non-devicetree) which
will contain 3 different pcf8574 devices that as a result cannot
be told apart because they are all named "pcf8574", affecting
the GPIO descriptor tables which need a unique label per chip.

By passing in .dev_name in the struct i2c_board_info we can
explicitly name each instance and use that to discern the chips
when using board files.

Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog ->v5:
- New patch to deal with the chip label
---
 drivers/gpio/gpio-pcf857x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index b7568ee33696..2271ec86e414 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -311,7 +311,7 @@ static int pcf857x_probe(struct i2c_client *client,
 	if (status < 0)
 		goto fail;
 
-	gpio->chip.label = client->name;
+	gpio->chip.label = dev_name(&client->dev);
 
 	gpio->client = client;
 	i2c_set_clientdata(client, gpio);
-- 
2.31.1

[PATCH 2/3 v5] ARM: davinci: dm644x: Convert LEDs to GPIO descriptor table

[Linus Walleij]

This converts the DaVinci DM644x LEDs to use GPIO
descriptor look-ups.

Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v4->v5:
- Name each I2C expander instance explicitly as "u2" etc.
- Tie the LEDs to the generated "i2c-u2" device name.
ChangeLog v3->v4:
- Rebase on v5.13-rc1
- Resend
- LED maintainers: please apply this patch, it is ACKed by a DaVinci
  maintainer
ChangeLog v2->v3:
- Rebase on v5.10-rc1
- Resend
ChangeLog v1->v2:
- Collect Bartosz' review tag
- Rebase on v5.9-rc1
- Resend
---
 arch/arm/mach-davinci/board-dm644x-evm.c | 49 ++++++++++++++++--------
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c b/arch/arm/mach-davinci/board-dm644x-evm.c
index cce3a621eb20..36d8eea7a342 100644
--- a/arch/arm/mach-davinci/board-dm644x-evm.c
+++ b/arch/arm/mach-davinci/board-dm644x-evm.c
@@ -319,17 +319,14 @@ static struct platform_device rtc_dev = {
 /* U2 -- LEDs */
 
 static struct gpio_led evm_leds[] = {
-	{ .name = "DS8", .active_low = 1,
-		.default_trigger = "heartbeat", },
-	{ .name = "DS7", .active_low = 1, },
-	{ .name = "DS6", .active_low = 1, },
-	{ .name = "DS5", .active_low = 1, },
-	{ .name = "DS4", .active_low = 1, },
-	{ .name = "DS3", .active_low = 1, },
-	{ .name = "DS2", .active_low = 1,
-		.default_trigger = "mmc0", },
-	{ .name = "DS1", .active_low = 1,
-		.default_trigger = "disk-activity", },
+	{ .name = "DS8", .default_trigger = "heartbeat", },
+	{ .name = "DS7", },
+	{ .name = "DS6", },
+	{ .name = "DS5", },
+	{ .name = "DS4", },
+	{ .name = "DS3", },
+	{ .name = "DS2", .default_trigger = "mmc0", },
+	{ .name = "DS1", .default_trigger = "disk-activity", },
 };
 
 static const struct gpio_led_platform_data evm_led_data = {
@@ -337,18 +334,35 @@ static const struct gpio_led_platform_data evm_led_data = {
 	.leds		= evm_leds,
 };
 
+static struct gpiod_lookup_table evm_leds_gpio_table = {
+	.dev_id = "leds-gpio.0",
+	.table = {
+		/*
+		 * These GPIOs are on a PCF8574 GPIO expander, which
+		 * is in turn named after the I2C device name. This is
+		 * device "u2" on I2C bus 1 with address 0x38.
+		 */
+		GPIO_LOOKUP_IDX("i2c-u2", 0, NULL, 0, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("i2c-u2", 1, NULL, 1, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("i2c-u2", 2, NULL, 2, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("i2c-u2", 3, NULL, 3, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("i2c-u2", 4, NULL, 4, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("i2c-u2", 5, NULL, 5, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("i2c-u2", 6, NULL, 6, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("i2c-u2", 7, NULL, 7, GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
 static struct platform_device *evm_led_dev;
 
 static int
 evm_led_setup(struct i2c_client *client, int gpio, unsigned ngpio, void *c)
 {
-	struct gpio_led *leds = evm_leds;
 	int status;
 
-	while (ngpio--) {
-		leds->gpio = gpio++;
-		leds++;
-	}
+	/* Add the lookup table */
+	gpiod_add_lookup_table(&evm_leds_gpio_table);
 
 	/* what an extremely annoying way to be forced to handle
 	 * device unregistration ...
@@ -639,14 +653,17 @@ static struct i2c_board_info __initdata i2c_info[] =  {
 	},
 	{
 		I2C_BOARD_INFO("pcf8574", 0x38),
+		.dev_name = "u2",
 		.platform_data	= &pcf_data_u2,
 	},
 	{
 		I2C_BOARD_INFO("pcf8574", 0x39),
+		.dev_name = "u18",
 		.platform_data	= &pcf_data_u18,
 	},
 	{
 		I2C_BOARD_INFO("pcf8574", 0x3a),
+		.dev_name = "u35",
 		.platform_data	= &pcf_data_u35,
 	},
 	{
-- 
2.31.1

[PATCH 3/3 v5] ARM: davinci: dm646x: Convert LEDs to GPIO descriptor table

[Linus Walleij]

This converts the DaVinci DM646x LEDs to use GPIO
descriptor look-ups.

Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v4->v5:
- Name each I2C expander instance explicitly as "u2" etc.
- Tie the LEDs to the generated "i2c-u2" device name.
ChangeLog v3->v4:
- Rebase on v5.13-rc1
- Resend
- LED maintainers: please apply this patch, it is ACKed by a DaVinci
  maintainer
ChangeLog v2->v3:
- Rebase on v5.10-rc1
- Resend
ChangeLog v1->v2:
- Collect Bartosz' review tag
- Rebase on v5.9-rc1
- Resend
---
 arch/arm/mach-davinci/board-dm646x-evm.c | 34 +++++++++++++++++-------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c
index ee91d81ebbfd..00c074fddce6 100644
--- a/arch/arm/mach-davinci/board-dm646x-evm.c
+++ b/arch/arm/mach-davinci/board-dm646x-evm.c
@@ -20,6 +20,7 @@
 #include <linux/init.h>
 #include <linux/leds.h>
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/platform_device.h>
 #include <linux/i2c.h>
 #include <linux/property.h>
@@ -202,10 +203,10 @@ static struct i2c_driver dm6467evm_cpld_driver = {
 /* LEDS */
 
 static struct gpio_led evm_leds[] = {
-	{ .name = "DS1", .active_low = 1, },
-	{ .name = "DS2", .active_low = 1, },
-	{ .name = "DS3", .active_low = 1, },
-	{ .name = "DS4", .active_low = 1, },
+	{ .name = "DS1" },
+	{ .name = "DS2" },
+	{ .name = "DS3" },
+	{ .name = "DS4" },
 };
 
 static const struct gpio_led_platform_data evm_led_data = {
@@ -213,18 +214,32 @@ static const struct gpio_led_platform_data evm_led_data = {
 	.leds     = evm_leds,
 };
 
+static struct gpiod_lookup_table evm_leds_gpio_table = {
+	.dev_id = "leds-gpio.0",
+	.table = {
+		/*
+		 * These GPIOs are on a PCF8574a GPIO expander, which
+		 * is in turn named after the I2C device name. This is
+		 * device "u2" on I2C bus 1 with address 0x38. These
+		 * leds are at offset 4, 5, 6, 7.
+		 */
+		GPIO_LOOKUP_IDX("i2c-u2", 4, NULL, 0, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("i2c-u2", 5, NULL, 1, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("i2c-u2", 6, NULL, 2, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("i2c-u2", 7, NULL, 3, GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
 static struct platform_device *evm_led_dev;
 
 static int evm_led_setup(struct i2c_client *client, int gpio,
 			unsigned int ngpio, void *c)
 {
-	struct gpio_led *leds = evm_leds;
 	int status;
 
-	while (ngpio--) {
-		leds->gpio = gpio++;
-		leds++;
-	}
+	/* Add the lookup table */
+	gpiod_add_lookup_table(&evm_leds_gpio_table);
 
 	evm_led_dev = platform_device_alloc("leds-gpio", 0);
 	platform_device_add_data(evm_led_dev, &evm_led_data,
@@ -438,6 +453,7 @@ static struct i2c_board_info __initdata i2c_info[] =  {
 	},
 	{
 		I2C_BOARD_INFO("pcf8574a", 0x38),
+		.dev_name = "u2",
 		.platform_data	= &pcf_data,
 	},
 	{
-- 
2.31.1

Re: [PATCH 1/3 v5] gpio: pcf857x: Name instance after dev_name()

[Bartosz Golaszewski]

On Tue, Jun 15, 2021 at 5:01 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Put the label on this gpio_chip from the dev_name() instead of
> the client name.
>
> The client name will be pcf8574 etc for all instances even if
> there are several chips on a system.
>
> This manifests on the DaVinci DM6467 (non-devicetree) which
> will contain 3 different pcf8574 devices that as a result cannot
> be told apart because they are all named "pcf8574", affecting
> the GPIO descriptor tables which need a unique label per chip.
>
> By passing in .dev_name in the struct i2c_board_info we can
> explicitly name each instance and use that to discern the chips
> when using board files.
>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog ->v5:
> - New patch to deal with the chip label
> ---
>  drivers/gpio/gpio-pcf857x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
> index b7568ee33696..2271ec86e414 100644
> --- a/drivers/gpio/gpio-pcf857x.c
> +++ b/drivers/gpio/gpio-pcf857x.c
> @@ -311,7 +311,7 @@ static int pcf857x_probe(struct i2c_client *client,
>         if (status < 0)
>                 goto fail;
>
> -       gpio->chip.label = client->name;
> +       gpio->chip.label = dev_name(&client->dev);
>
>         gpio->client = client;
>         i2c_set_clientdata(client, gpio);
> --
> 2.31.1
>

This makes sense but the i2c names are often not very descriptive. How
about adding a DEVID_AUTO/DEVID_NONE like mechanism to GPIO labels?
Nvmem has a thing like that precisely because labels can repeat.

Bart

Re: [PATCH 1/3 v5] gpio: pcf857x: Name instance after dev_name()

[Linus Walleij]

On Tue, Jun 22, 2021 at 2:58 PM Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:

> This makes sense but the i2c names are often not very descriptive. How
> about adding a DEVID_AUTO/DEVID_NONE like mechanism to GPIO labels?
> Nvmem has a thing like that precisely because labels can repeat.

Yeah :/ it feels like the subsystem should name the device properly
though. Like we're solving someone elses problem.

In this case the other patches provides .names in the I2C
board info so that the dev_name() ends up something
like "i2c-u2" and "i2c-u15". The u2 and u15 are common
names for components on a board so from an electronic point
of view that naming makes for good topology.

Yours,
Linus Walleij

Re: [PATCH 1/3 v5] gpio: pcf857x: Name instance after dev_name()

[Bartosz Golaszewski]

On Fri, Jun 25, 2021 at 1:52 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Jun 22, 2021 at 2:58 PM Bartosz Golaszewski
> <bgolaszewski@baylibre.com> wrote:
>
> > This makes sense but the i2c names are often not very descriptive. How
> > about adding a DEVID_AUTO/DEVID_NONE like mechanism to GPIO labels?
> > Nvmem has a thing like that precisely because labels can repeat.
>
> Yeah :/ it feels like the subsystem should name the device properly
> though. Like we're solving someone elses problem.
>
> In this case the other patches provides .names in the I2C
> board info so that the dev_name() ends up something
> like "i2c-u2" and "i2c-u15". The u2 and u15 are common
> names for components on a board so from an electronic point
> of view that naming makes for good topology.
>

Right. Alright, Sekhar can I take just this patch and you take the
ones for davinci?

Bart