about pca955x led driver gpio management

about pca955x led driver gpio management

[Andrea Scian - DAVE Embedded Systems]

Dear all,

I'm working on an iMX6 based board with a PCA9555 which is used both to 
drive LEDs and manage some GPIOs.

My current kernel is quite old (4.1.15) but I've found Cédric patches on 
mainline and backported to this old revision.

I'm facing an issue with it, because it seems that it fails when it's 
used in a mixed (led/gpios) environment.

E.g.: let's say that I have one led connected to LED0 output and one 
GPIO connected at LED1 output.

I define it as

                         pca9551: pca9551@60 {
                                 compatible = "nxp,pca9551";
                                 reg = <0x60>;
                                 #address-cells = <1>;
                                 #size-cells = <0>;
                                 #gpio-cells = <1>;

                                 led@0 {
                                         label = "led0";
                                         reg = <0>;
                                         linux,default-trigger = "none";
                                 };
                                 gpio@1 {
                                         label = "gpio1";
                                         reg = <1>;
                                         type = <2>; /* GPIO */
                                 };
                 };

At boot it's probed as

root@sbc-lynx:~# dmesg | grep pca
[    5.315425] leds-pca955x 5-0060: leds-pca955x: Using pca9551 8-bit 
LED driver at slave address 0x60
[    5.350349] leds-pca955x 5-0060: gpios 511...511

But I cannot access it:

root@sbc-lynx:~# echo 511 > /sys/class/gpio/export
-sh: echo: write error: Device or resource busy

Because for pca955x_gpio_request_pin() this is at offset 0 (in fact is 
the first gpio registered of this gpio_chip) but it's the index 1 inside 
pca955x->leds[]

Am I missing something? (maybe I made a mistake in my backport and/or 
I'm missing some patches about the GPIO subsystems).

If I'm right I think I can send a patch to fix this (I'm thinking about 
having an array of GPIO index to map offset -> pca955x->leds[] index or 
just register all pins as GPIOs and then just report the busy state)

WDYT?

Kind Regards,

-- 


Andrea SCIAN



*DAVE Embedded Systems*

via Talponedo 29/A 33080 Porcia (PN) - Italy
Telephone: +39.0434.921215
Telefax: +39.0434.1994030
web: www.dave.eu <http://www.dave.eu>

get in touch:
<https://twitter.com/DAVE_Embedded> 
<https://www.linkedin.com/company/dave-embedded-systems?trk=nav_account_sub_nav_company_admin> 
<http://wiki.dave.eu/index.php/Main_Page>

Re: about pca955x led driver gpio management

[Cédric Le Goater]

On 10/17/2017 09:36 AM, Andrea Scian - DAVE Embedded Systems wrote:
> Dear all,
> 
> I'm working on an iMX6 based board with a PCA9555 which is used both to drive LEDs and manage some GPIOs.

The PCA9555 chip and the PCA955[0-3] chips have different control 
registers. You need a different led driver for it.

C.  

> My current kernel is quite old (4.1.15) but I've found Cédric patches on mainline and backported to this old revision.
> 
> I'm facing an issue with it, because it seems that it fails when it's used in a mixed (led/gpios) environment.
> 
> E.g.: let's say that I have one led connected to LED0 output and one GPIO connected at LED1 output.
> 
> I define it as
> 
>                         pca9551: pca9551@60 {
>                                 compatible = "nxp,pca9551";
>                                 reg = <0x60>;
>                                 #address-cells = <1>;
>                                 #size-cells = <0>;
>                                 #gpio-cells = <1>;
> 
>                                 led@0 {
>                                         label = "led0";
>                                         reg = <0>;
>                                         linux,default-trigger = "none";
>                                 };
>                                 gpio@1 {
>                                         label = "gpio1";
>                                         reg = <1>;
>                                         type = <2>; /* GPIO */
>                                 };
>                 };
> 
> At boot it's probed as
> 
> root@sbc-lynx:~# dmesg | grep pca
> [    5.315425] leds-pca955x 5-0060: leds-pca955x: Using pca9551 8-bit LED driver at slave address 0x60
> [    5.350349] leds-pca955x 5-0060: gpios 511...511
> 
> But I cannot access it:
> 
> root@sbc-lynx:~# echo 511 > /sys/class/gpio/export
> -sh: echo: write error: Device or resource busy
> 
> Because for pca955x_gpio_request_pin() this is at offset 0 (in fact is the first gpio registered of this gpio_chip) but it's the index 1 inside pca955x->leds[]
> 
> Am I missing something? (maybe I made a mistake in my backport and/or I'm missing some patches about the GPIO subsystems).
> 
> If I'm right I think I can send a patch to fix this (I'm thinking about having an array of GPIO index to map offset -> pca955x->leds[] index or just register all pins as GPIOs and then just report the busy state)
> 
> WDYT?
> 
> Kind Regards,
> 

Re: about pca955x led driver gpio management

[Andrea Scian - DAVE Embedded Systems]

Il 17/10/2017 10:18, Cédric Le Goater ha scritto:
> On 10/17/2017 09:36 AM, Andrea Scian - DAVE Embedded Systems wrote:
>> Dear all,
>>
>> I'm working on an iMX6 based board with a PCA9555 which is used both to drive LEDs and manage some GPIOs.
> The PCA9555 chip and the PCA955[0-3] chips have different control
> registers. You need a different led driver for it.

My typo sorry, as you can see in the device tree below, I'm using pca9551

>> My current kernel is quite old (4.1.15) but I've found Cédric patches on mainline and backported to this old revision.
>>
>> I'm facing an issue with it, because it seems that it fails when it's used in a mixed (led/gpios) environment.
>>
>> E.g.: let's say that I have one led connected to LED0 output and one GPIO connected at LED1 output.
>>
>> I define it as
>>
>>                          pca9551: pca9551@60 {
>>                                  compatible = "nxp,pca9551";
>>                                  reg = <0x60>;
>>                                  #address-cells = <1>;
>>                                  #size-cells = <0>;
>>                                  #gpio-cells = <1>;
>>
>>                                  led@0 {
>>                                          label = "led0";
>>                                          reg = <0>;
>>                                          linux,default-trigger = "none";
>>                                  };
>>                                  gpio@1 {
>>                                          label = "gpio1";
>>                                          reg = <1>;
>>                                          type = <2>; /* GPIO */
>>                                  };
>>                  };
>>
>> At boot it's probed as
>>
>> root@sbc-lynx:~# dmesg | grep pca
>> [    5.315425] leds-pca955x 5-0060: leds-pca955x: Using pca9551 8-bit LED driver at slave address 0x60
>> [    5.350349] leds-pca955x 5-0060: gpios 511...511
>>
>> But I cannot access it:
>>
>> root@sbc-lynx:~# echo 511 > /sys/class/gpio/export
>> -sh: echo: write error: Device or resource busy
>>
>> Because for pca955x_gpio_request_pin() this is at offset 0 (in fact is the first gpio registered of this gpio_chip) but it's the index 1 inside pca955x->leds[]
>>
>> Am I missing something? (maybe I made a mistake in my backport and/or I'm missing some patches about the GPIO subsystems).
>>
>> If I'm right I think I can send a patch to fix this (I'm thinking about having an array of GPIO index to map offset -> pca955x->leds[] index or just register all pins as GPIOs and then just report the busy state)
>>
>> WDYT?
>>
>> Kind Regards,
>>

Re: about pca955x led driver gpio management

[Cédric Le Goater]

On 10/17/2017 10:20 AM, Andrea Scian - DAVE Embedded Systems wrote:
> 
> Il 17/10/2017 10:18, Cédric Le Goater ha scritto:
>> On 10/17/2017 09:36 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>> Dear all,
>>>
>>> I'm working on an iMX6 based board with a PCA9555 which is used both to drive LEDs and manage some GPIOs.
>> The PCA9555 chip and the PCA955[0-3] chips have different control
>> registers. You need a different led driver for it.
> 
> My typo sorry, as you can see in the device tree below, I'm using pca9551

ok.

You might want to take a look at how we mixed gpios and leds on the witherspoon
system using pca9552 chips. we added a gpio-leds binding.

https://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts

Cheers,

C. 

> 
>>> My current kernel is quite old (4.1.15) but I've found Cédric patches on mainline and backported to this old revision.
>>>
>>> I'm facing an issue with it, because it seems that it fails when it's used in a mixed (led/gpios) environment.
>>>
>>> E.g.: let's say that I have one led connected to LED0 output and one GPIO connected at LED1 output.
>>>
>>> I define it as
>>>
>>>                          pca9551: pca9551@60 {
>>>                                  compatible = "nxp,pca9551";
>>>                                  reg = <0x60>;
>>>                                  #address-cells = <1>;
>>>                                  #size-cells = <0>;
>>>                                  #gpio-cells = <1>;
>>>
>>>                                  led@0 {
>>>                                          label = "led0";
>>>                                          reg = <0>;
>>>                                          linux,default-trigger = "none";
>>>                                  };
>>>                                  gpio@1 {
>>>                                          label = "gpio1";
>>>                                          reg = <1>;
>>>                                          type = <2>; /* GPIO */
>>>                                  };
>>>                  };
>>>
>>> At boot it's probed as
>>>
>>> root@sbc-lynx:~# dmesg | grep pca
>>> [    5.315425] leds-pca955x 5-0060: leds-pca955x: Using pca9551 8-bit LED driver at slave address 0x60
>>> [    5.350349] leds-pca955x 5-0060: gpios 511...511
>>>
>>> But I cannot access it:
>>>
>>> root@sbc-lynx:~# echo 511 > /sys/class/gpio/export
>>> -sh: echo: write error: Device or resource busy
>>>
>>> Because for pca955x_gpio_request_pin() this is at offset 0 (in fact is the first gpio registered of this gpio_chip) but it's the index 1 inside pca955x->leds[]
>>>
>>> Am I missing something? (maybe I made a mistake in my backport and/or I'm missing some patches about the GPIO subsystems).
>>>
>>> If I'm right I think I can send a patch to fix this (I'm thinking about having an array of GPIO index to map offset -> pca955x->leds[] index or just register all pins as GPIOs and then just report the busy state)
>>>
>>> WDYT?
>>>
>>> Kind Regards,
>>>
> 

Re: about pca955x led driver gpio management

[Andrea Scian - DAVE Embedded Systems]

[-- Attachment #1: Type: text/plain, Size: 3591 bytes --]


> On 10/17/2017 10:20 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>
>> Il 17/10/2017 10:18, Cédric Le Goater ha scritto:
>>> On 10/17/2017 09:36 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>>> Dear all,
>>>>
>>>> I'm working on an iMX6 based board with a PCA9555 which is used both to drive LEDs and manage some GPIOs.
>>> The PCA9555 chip and the PCA955[0-3] chips have different control
>>> registers. You need a different led driver for it.
>>
>> My typo sorry, as you can see in the device tree below, I'm using pca9551
> 
> ok.
> 
> You might want to take a look at how we mixed gpios and leds on the witherspoon
> system using pca9552 chips. we added a gpio-leds binding.
> 
> https://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts

understood: you configure all pins of PCA955x as GPIOs and the map the 
one you need as led with gpio-leds binding.

However to me this is a kind of workaround or, at least, there's nothing 
about this limitation into the devicetree binding (in fact, IMHO, the 
device tree binding example will just fail)

I wrote the attached patch which should fix the issue and allow a more 
generic approach. WDYT?

(in case it looks good, I'll send the patch in the correct way)


Kind Regards,

Andrea

>>
>>>> My current kernel is quite old (4.1.15) but I've found Cédric patches on mainline and backported to this old revision.
>>>>
>>>> I'm facing an issue with it, because it seems that it fails when it's used in a mixed (led/gpios) environment.
>>>>
>>>> E.g.: let's say that I have one led connected to LED0 output and one GPIO connected at LED1 output.
>>>>
>>>> I define it as
>>>>
>>>>                           pca9551: pca9551@60 {
>>>>                                   compatible = "nxp,pca9551";
>>>>                                   reg = <0x60>;
>>>>                                   #address-cells = <1>;
>>>>                                   #size-cells = <0>;
>>>>                                   #gpio-cells = <1>;
>>>>
>>>>                                   led@0 {
>>>>                                           label = "led0";
>>>>                                           reg = <0>;
>>>>                                           linux,default-trigger = "none";
>>>>                                   };
>>>>                                   gpio@1 {
>>>>                                           label = "gpio1";
>>>>                                           reg = <1>;
>>>>                                           type = <2>; /* GPIO */
>>>>                                   };
>>>>                   };
>>>>
>>>> At boot it's probed as
>>>>
>>>> root@sbc-lynx:~# dmesg | grep pca
>>>> [    5.315425] leds-pca955x 5-0060: leds-pca955x: Using pca9551 8-bit LED driver at slave address 0x60
>>>> [    5.350349] leds-pca955x 5-0060: gpios 511...511
>>>>
>>>> But I cannot access it:
>>>>
>>>> root@sbc-lynx:~# echo 511 > /sys/class/gpio/export
>>>> -sh: echo: write error: Device or resource busy
>>>>
>>>> Because for pca955x_gpio_request_pin() this is at offset 0 (in fact is the first gpio registered of this gpio_chip) but it's the index 1 inside pca955x->leds[]
>>>>
>>>> Am I missing something? (maybe I made a mistake in my backport and/or I'm missing some patches about the GPIO subsystems).
>>>>
>>>> If I'm right I think I can send a patch to fix this (I'm thinking about having an array of GPIO index to map offset -> pca955x->leds[] index or just register all pins as GPIOs and then just report the busy state)
>>>>
>>>> WDYT?
>>>>
>>>> Kind Regards,
>>>>
>>
> 

[-- Attachment #2: 0001-leds-pca955x-map-gpio-offset-to-led-index-correctly.patch --]
[-- Type: text/plain, Size: 3421 bytes --]

>From 5f8be223e8b302dbba2690560c1ea74b6e26c07f Mon Sep 17 00:00:00 2001
From: Andrea Scian <andrea.scian@dave.eu>
Date: Tue, 17 Oct 2017 11:09:11 +0200
Subject: [PATCH] leds: pca955x: map gpio offset to led index correctly

We need to map gpio offset to led index of this device otherwise we
cannot mix led and gpios into device tree definition

Signed-off-by: Andrea Scian <andrea.scian@dave.eu>
---
 drivers/leds/leds-pca955x.c |   38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index cdd7a4c..27942aa 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -122,6 +122,7 @@ struct pca955x {
 	struct i2c_client	*client;
 #ifdef CONFIG_LEDS_PCA955X_GPIO
 	struct gpio_chip gpio;
+	int *gpio_to_index;
 #endif
 };
 
@@ -313,25 +314,36 @@ static int pca955x_read_input(struct i2c_client *client, int n, u8 *val)
 	}
 	*val = (u8)ret;
 	return 0;
+}
 
+/*
+ * Map from gpio offset to led index.
+ * Return NULL in case of unmapped GPIO (even if it should never happens)
+ */
+static struct pca955x_led *
+pca955x_gpio_get_led_index(struct pca955x *pca955x, unsigned int offset)
+{
+	int index = pca955x->gpio_to_index[offset];
+
+	return index < 0 ? NULL : &pca955x->leds[index];
 }
 
 static int pca955x_gpio_request_pin(struct gpio_chip *gc, unsigned int offset)
 {
 	struct pca955x *pca955x = to_pca(gc);
-	struct pca955x_led *led = &pca955x->leds[offset];
-
-	if (led->type == PCA955X_TYPE_GPIO)
-		return 0;
+	struct pca955x_led *led = pca955x_gpio_get_led_index(pca955x, offset);
 
-	return -EBUSY;
+	return led == NULL ? -EBUSY : 0;
 }
 
 static int pca955x_set_value(struct gpio_chip *gc, unsigned int offset,
 			     int val)
 {
 	struct pca955x *pca955x = to_pca(gc);
-	struct pca955x_led *led = &pca955x->leds[offset];
+	struct pca955x_led *led = pca955x_gpio_get_led_index(pca955x, offset);
+
+	if (led == NULL)
+		return -EINVAL;
 
 	if (val)
 		return pca955x_led_set(&led->led_cdev, LED_FULL);
@@ -348,10 +360,14 @@ static void pca955x_gpio_set_value(struct gpio_chip *gc, unsigned int offset,
 static int pca955x_gpio_get_value(struct gpio_chip *gc, unsigned int offset)
 {
 	struct pca955x *pca955x = to_pca(gc);
-	struct pca955x_led *led = &pca955x->leds[offset];
+	struct pca955x_led *led = pca955x_gpio_get_led_index(pca955x, offset);
 	u8 reg = 0;
 
 	/* There is nothing we can do about errors */
+	if (led == NULL)
+		return 0;
+
+	/* There is nothing we can do about errors */
 	pca955x_read_input(pca955x->client, led->led_num / 8, &reg);
 
 	return !!(reg & (1 << (led->led_num % 8)));
@@ -497,6 +513,11 @@ static int pca955x_probe(struct i2c_client *client,
 	if (!pca955x->leds)
 		return -ENOMEM;
 
+	pca955x->gpio_to_index = devm_kzalloc(&client->dev,
+			sizeof(int) * chip->bits, GFP_KERNEL);
+	if (!pca955x->gpio_to_index)
+		return -ENOMEM;
+
 	i2c_set_clientdata(client, pca955x);
 
 	mutex_init(&pca955x->lock);
@@ -508,12 +529,13 @@ static int pca955x_probe(struct i2c_client *client,
 		pca955x_led->led_num = i;
 		pca955x_led->pca955x = pca955x;
 		pca955x_led->type = pdata->leds[i].type;
+		pca955x->gpio_to_index[i] = -ENOENT;
 
 		switch (pca955x_led->type) {
 		case PCA955X_TYPE_NONE:
 			break;
 		case PCA955X_TYPE_GPIO:
-			ngpios++;
+			pca955x->gpio_to_index[ngpios++] = i;
 			break;
 		case PCA955X_TYPE_LED:
 			/*
-- 
1.7.9.5

Re: about pca955x led driver gpio management

[Jacek Anaszewski]

Hi Cedric,

Andrea seems to come across a real issue. We might proceed with the
contribution of his patch without your review and possibly
Tested-by, but it would be nevertheless beneficial if we had them.

Best regards,
Jacek Anaszewski

On 10/17/2017 11:16 AM, Andrea Scian - DAVE Embedded Systems wrote:
> 
>> On 10/17/2017 10:20 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>>
>>> Il 17/10/2017 10:18, Cédric Le Goater ha scritto:
>>>> On 10/17/2017 09:36 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>>>> Dear all,
>>>>>
>>>>> I'm working on an iMX6 based board with a PCA9555 which is used
>>>>> both to drive LEDs and manage some GPIOs.
>>>> The PCA9555 chip and the PCA955[0-3] chips have different control
>>>> registers. You need a different led driver for it.
>>>
>>> My typo sorry, as you can see in the device tree below, I'm using
>>> pca9551
>>
>> ok.
>>
>> You might want to take a look at how we mixed gpios and leds on the
>> witherspoon
>> system using pca9552 chips. we added a gpio-leds binding.
>>
>> https://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>>
> 
> understood: you configure all pins of PCA955x as GPIOs and the map the
> one you need as led with gpio-leds binding.
> 
> However to me this is a kind of workaround or, at least, there's nothing
> about this limitation into the devicetree binding (in fact, IMHO, the
> device tree binding example will just fail)
> 
> I wrote the attached patch which should fix the issue and allow a more
> generic approach. WDYT?
> 
> (in case it looks good, I'll send the patch in the correct way)
> 
> 
> Kind Regards,
> 
> Andrea
> 
>>>
>>>>> My current kernel is quite old (4.1.15) but I've found Cédric
>>>>> patches on mainline and backported to this old revision.
>>>>>
>>>>> I'm facing an issue with it, because it seems that it fails when
>>>>> it's used in a mixed (led/gpios) environment.
>>>>>
>>>>> E.g.: let's say that I have one led connected to LED0 output and
>>>>> one GPIO connected at LED1 output.
>>>>>
>>>>> I define it as
>>>>>
>>>>>                           pca9551: pca9551@60 {
>>>>>                                   compatible = "nxp,pca9551";
>>>>>                                   reg = <0x60>;
>>>>>                                   #address-cells = <1>;
>>>>>                                   #size-cells = <0>;
>>>>>                                   #gpio-cells = <1>;
>>>>>
>>>>>                                   led@0 {
>>>>>                                           label = "led0";
>>>>>                                           reg = <0>;
>>>>>                                           linux,default-trigger =
>>>>> "none";
>>>>>                                   };
>>>>>                                   gpio@1 {
>>>>>                                           label = "gpio1";
>>>>>                                           reg = <1>;
>>>>>                                           type = <2>; /* GPIO */
>>>>>                                   };
>>>>>                   };
>>>>>
>>>>> At boot it's probed as
>>>>>
>>>>> root@sbc-lynx:~# dmesg | grep pca
>>>>> [    5.315425] leds-pca955x 5-0060: leds-pca955x: Using pca9551
>>>>> 8-bit LED driver at slave address 0x60
>>>>> [    5.350349] leds-pca955x 5-0060: gpios 511...511
>>>>>
>>>>> But I cannot access it:
>>>>>
>>>>> root@sbc-lynx:~# echo 511 > /sys/class/gpio/export
>>>>> -sh: echo: write error: Device or resource busy
>>>>>
>>>>> Because for pca955x_gpio_request_pin() this is at offset 0 (in fact
>>>>> is the first gpio registered of this gpio_chip) but it's the index
>>>>> 1 inside pca955x->leds[]
>>>>>
>>>>> Am I missing something? (maybe I made a mistake in my backport
>>>>> and/or I'm missing some patches about the GPIO subsystems).
>>>>>
>>>>> If I'm right I think I can send a patch to fix this (I'm thinking
>>>>> about having an array of GPIO index to map offset ->
>>>>> pca955x->leds[] index or just register all pins as GPIOs and then
>>>>> just report the busy state)
>>>>>
>>>>> WDYT?
>>>>>
>>>>> Kind Regards,
>>>>>
>>>
>>

Re: about pca955x led driver gpio management

[Cédric Le Goater]

On 10/17/2017 11:16 AM, Andrea Scian - DAVE Embedded Systems wrote:
> 
>> On 10/17/2017 10:20 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>>
>>> Il 17/10/2017 10:18, Cédric Le Goater ha scritto:
>>>> On 10/17/2017 09:36 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>>>> Dear all,
>>>>>
>>>>> I'm working on an iMX6 based board with a PCA9555 which is used both to drive LEDs and manage some GPIOs.
>>>> The PCA9555 chip and the PCA955[0-3] chips have different control
>>>> registers. You need a different led driver for it.
>>>
>>> My typo sorry, as you can see in the device tree below, I'm using pca9551
>>
>> ok.
>>
>> You might want to take a look at how we mixed gpios and leds on the witherspoon
>> system using pca9552 chips. we added a gpio-leds binding.
>>
>> https://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> 
> understood: you configure all pins of PCA955x as GPIOs and the map the one you need as led with gpio-leds binding.

Sorry I got distracted.

> However to me this is a kind of workaround or, at least, there's nothing about this limitation into the devicetree binding (in fact, IMHO, the device tree binding example will just fail)

euh ? what do you mean. There is a real system using this device tree. 
I think we would know about it if it didn't work. Please explain

> I wrote the attached patch which should fix the issue and allow a more generic approach. WDYT?
> 
> (in case it looks good, I'll send the patch in the correct way)

Please send the patch to discuss, add Andrew Jeffery <andrew@aj.id.au> 
in cc:

Thanks,

C.


> 
> Kind Regards,
> 
> Andrea
> 
>>>
>>>>> My current kernel is quite old (4.1.15) but I've found Cédric patches on mainline and backported to this old revision.
>>>>>
>>>>> I'm facing an issue with it, because it seems that it fails when it's used in a mixed (led/gpios) environment.
>>>>>
>>>>> E.g.: let's say that I have one led connected to LED0 output and one GPIO connected at LED1 output.
>>>>>
>>>>> I define it as
>>>>>
>>>>>                           pca9551: pca9551@60 {
>>>>>                                   compatible = "nxp,pca9551";
>>>>>                                   reg = <0x60>;
>>>>>                                   #address-cells = <1>;
>>>>>                                   #size-cells = <0>;
>>>>>                                   #gpio-cells = <1>;
>>>>>
>>>>>                                   led@0 {
>>>>>                                           label = "led0";
>>>>>                                           reg = <0>;
>>>>>                                           linux,default-trigger = "none";
>>>>>                                   };
>>>>>                                   gpio@1 {
>>>>>                                           label = "gpio1";
>>>>>                                           reg = <1>;
>>>>>                                           type = <2>; /* GPIO */
>>>>>                                   };
>>>>>                   };
>>>>>
>>>>> At boot it's probed as
>>>>>
>>>>> root@sbc-lynx:~# dmesg | grep pca
>>>>> [    5.315425] leds-pca955x 5-0060: leds-pca955x: Using pca9551 8-bit LED driver at slave address 0x60
>>>>> [    5.350349] leds-pca955x 5-0060: gpios 511...511
>>>>>
>>>>> But I cannot access it:
>>>>>
>>>>> root@sbc-lynx:~# echo 511 > /sys/class/gpio/export
>>>>> -sh: echo: write error: Device or resource busy
>>>>>
>>>>> Because for pca955x_gpio_request_pin() this is at offset 0 (in fact is the first gpio registered of this gpio_chip) but it's the index 1 inside pca955x->leds[]
>>>>>
>>>>> Am I missing something? (maybe I made a mistake in my backport and/or I'm missing some patches about the GPIO subsystems).
>>>>>
>>>>> If I'm right I think I can send a patch to fix this (I'm thinking about having an array of GPIO index to map offset -> pca955x->leds[] index or just register all pins as GPIOs and then just report the busy state)
>>>>>
>>>>> WDYT?
>>>>>
>>>>> Kind Regards,
>>>>>
>>>
>>

Re: about pca955x led driver gpio management

[Andrea Scian - DAVE Embedded Systems]

Il 24/10/2017 07:51, Cédric Le Goater ha scritto:
> On 10/17/2017 11:16 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>
>>> On 10/17/2017 10:20 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>>>
>>>> Il 17/10/2017 10:18, Cédric Le Goater ha scritto:
>>>>> On 10/17/2017 09:36 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>>>>> Dear all,
>>>>>>
>>>>>> I'm working on an iMX6 based board with a PCA9555 which is used both to drive LEDs and manage some GPIOs.
>>>>> The PCA9555 chip and the PCA955[0-3] chips have different control
>>>>> registers. You need a different led driver for it.
>>>>
>>>> My typo sorry, as you can see in the device tree below, I'm using pca9551
>>>
>>> ok.
>>>
>>> You might want to take a look at how we mixed gpios and leds on the witherspoon
>>> system using pca9552 chips. we added a gpio-leds binding.
>>>
>>> https://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>>
>> understood: you configure all pins of PCA955x as GPIOs and the map the one you need as led with gpio-leds binding.
> 
> Sorry I got distracted.
> 
>> However to me this is a kind of workaround or, at least, there's nothing about this limitation into the devicetree binding (in fact, IMHO, the device tree binding example will just fail)
> 
> euh ? what do you mean. There is a real system using this device tree.
> I think we would know about it if it didn't work. Please explain
> 

I mean that, for what I understand, your configuration is working 
because all pin of 9551 are configured as GPIOs (and the binded back as led)
In my configuration I have some pins configured as led and some as 
GPIOs, and this is not working.

>> I wrote the attached patch which should fix the issue and allow a more generic approach. WDYT?
>>
>> (in case it looks good, I'll send the patch in the correct way)
> 
> Please send the patch to discuss, add Andrew Jeffery <andrew@aj.id.au>
> in cc:
> 

Sorry for the delay but I'm busy in Prague with OSSummit and ELCE. I'll 
send a proper patch and more comments, if needed, by the begin of next week.

(BTW, if some of you are here, drop me a line and have a beer together! 
;-) )

Re: about pca955x led driver gpio management

[Cédric Le Goater]

On 10/25/2017 12:36 AM, Andrea Scian - DAVE Embedded Systems wrote:
> 
> Il 24/10/2017 07:51, Cédric Le Goater ha scritto:
>> On 10/17/2017 11:16 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>>
>>>> On 10/17/2017 10:20 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>>>>
>>>>> Il 17/10/2017 10:18, Cédric Le Goater ha scritto:
>>>>>> On 10/17/2017 09:36 AM, Andrea Scian - DAVE Embedded Systems wrote:
>>>>>>> Dear all,
>>>>>>>
>>>>>>> I'm working on an iMX6 based board with a PCA9555 which is used both to drive LEDs and manage some GPIOs.
>>>>>> The PCA9555 chip and the PCA955[0-3] chips have different control
>>>>>> registers. You need a different led driver for it.
>>>>>
>>>>> My typo sorry, as you can see in the device tree below, I'm using pca9551
>>>>
>>>> ok.
>>>>
>>>> You might want to take a look at how we mixed gpios and leds on the witherspoon
>>>> system using pca9552 chips. we added a gpio-leds binding.
>>>>
>>>> https://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>>>
>>> understood: you configure all pins of PCA955x as GPIOs and the map the one you need as led with gpio-leds binding.
>>
>> Sorry I got distracted.
>>
>>> However to me this is a kind of workaround or, at least, there's nothing about this limitation into the devicetree binding (in fact, IMHO, the device tree binding example will just fail)
>>
>> euh ? what do you mean. There is a real system using this device tree.
>> I think we would know about it if it didn't work. Please explain
>>
> 
> I mean that, for what I understand, your configuration is working because all pin of 9551 are configured as GPIOs (and the binded back as led)
> In my configuration I have some pins configured as led and some as GPIOs, and this is not working.

ok. That might the case. 

We have declared all pins as GPIOs on pca0 then used on top :  

	gpio-keys-polled {
		compatible = "gpio-keys-polled";
		...
	}

and

	leds {
		compatible = "gpio-leds";
		...
	}

which works perfecty fine.


>>> I wrote the attached patch which should fix the issue and allow a more generic approach. WDYT?
>>>
>>> (in case it looks good, I'll send the patch in the correct way)
>>
>> Please send the patch to discuss, add Andrew Jeffery <andrew@aj.id.au>
>> in cc:
>>
> 
> Sorry for the delay but I'm busy in Prague with OSSummit and ELCE. 
> I 'll send a proper patch 
> and more comments, if needed, by the begin of next week.

ok thanks, I have not seen anything wrong with it. I will see if we can 
get it tested on our systems.

> (BTW, if some of you are here, drop me a line and have a beer together! ;-) )

Nah, not this time. Enjoy :)

Cheers,

C.