Re: about pca955x led driver gpio management
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>> 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.
Re: about pca955x led driver gpio management
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 >> 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.
Re: about pca955x led driver gpio management
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 Jefferyin 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
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 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
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 Jefferyin 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
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 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
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
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
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
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
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, >From 5f8be223e8b302dbba2690560c1ea74b6e26c07f Mon Sep 17 00:00:00 2001 From: Andrea ScianDate: 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 --- 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 : >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 = >leds[offset]; - - if (led->type == PCA955X_TYPE_GPIO) - return 0; + struct pca955x_led *led =
Re: about pca955x led driver gpio management
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, >From 5f8be223e8b302dbba2690560c1ea74b6e26c07f Mon Sep 17 00:00:00 2001 From: Andrea Scian 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 --- 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 : >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 = >leds[offset]; - - if (led->type == PCA955X_TYPE_GPIO) - return 0; + struct pca955x_led *led = pca955x_gpio_get_led_index(pca955x, offset); -
Re: about pca955x led driver gpio management
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
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
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
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,