Re: Leds-gpio discarding the entries in /sys/class/leds : Linux 5.4.38

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

On 6/22/20 12:54 PM, Jacek Anaszewski wrote:
> Hi Vishwanatha,
> 
> On 6/22/20 8:58 AM, Vishwanatha Subbanna wrote:
>> Thank you very much Jacek.
>>
>>> On 22-Jun-2020, at 3:12 AM, Jacek Anaszewski 
>>> <jacek.anaszewski@gmail.com> wrote:
>>>
>>> Hi Vishwanatha,
>>>
>>> On 6/20/20 7:25 PM, Vishwanatha Subbanna wrote:
>>>> Hi Jacek,
>>>> Thank you very much for the quick response. Greatly appreciate that.
>>>
>>> You're welcome.
>>>
>>>>> On 20-Jun-2020, at 3:27 AM, Jacek Anaszewski 
>>>>> <jacek.anaszewski@gmail.com> wrote:
>>>>>
>>>>> Hi Vishwanatha,
>>>>>
>>>>> Please refer to 
>>>>> Documentation/devicetree/bindings/leds/leds-pca955x.txt.
>>>>>
>>>>> At first glance I don't get why you have gpio-leds node, which is for
>>>>> leds-gpio driver.
>>>> Not sure I understood it right.. But if you are asking me why I have 
>>>> "leds {"  and “gpio-leds” in there, then it is to get the entries in 
>>>> /sys/class/leds.
>>>> The GPIOs from PCA9552 are connected to LED. Also, that is how we 
>>>> have had in the past, and that worked.
>>>> Example: 
>>>> https://github.com/openbmc/linux/blob/dev-5.4/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts#L115 
>>>>
>>>
>>> Thanks. Yeah, that looks OK, I had to take closer look at the driver.
>>>
>>>> The problem I am running into is for : 
>>>> https://github.com/openbmc/linux/blob/dev-5.4/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts 
>>>>
>>>>>
>>>>> On 6/19/20 3:34 PM, Vishwanatha Subbanna wrote:
>>>>>> Hello,
>>>>>> I am Vishwanath, working with IBM and looking for your help on one 
>>>>>> of the issues that I am running into. Would really appreciate help 
>>>>>> on this. I run Linux 5.4.38
>>>>>> I have 2 number of PCA9552 chips, one on the Planar and other on 
>>>>>> the card that is optionally pluggable. The optional card must be 
>>>>>> plugged prior to booting and is not hot pluggable. In my 
>>>>>> experiment, I am running *without* the optional card plugged in.
>>>>>> In the device tree, I have a "leds {" section that looks like 
>>>>>> below for the PCA9552 that is on the planar and everything works 
>>>>>> fine and I can see /sys/class/leds/fan0
>>>>>>   leds {
>>>>>>          compatible = "gpio-leds”;
>>>>>>          fan0 {
>>>>>>              retain-state-shutdown;
>>>>>>              default-state = "keep";
>>>>>>              gpios = <&pca0 0 GPIO_ACTIVE_LOW>;
>>>>>>          };
>>>>>> };
>>>>>> &i2c7 {
>>>>>>      status = "okay”;
>>>>>>      pca0: pca9552@61 {
>>>>>>         compatible = "nxp,pca9552";
>>>>>>          reg = <0x61>;
>>>>>>          #address-cells = <1>;
>>>>>>          #size-cells = <0>;
>>>>>>          gpio-controller;
>>>>>>          #gpio-cells = <2>;
>>>>>>          gpio@0 {
>>>>>>              reg = <0>;
>>>>>>              type = <PCA955X_TYPE_GPIO>;
>>>>>>           };
>>>>>>      };
>>>>>> };
>>>>>> Similarly, if I update the device tree entry for PCA9552 for the 
>>>>>> card that is optionally pluggable, then I don’t see any leds 
>>>>>> entries in /sys/class/leds.
>>>>>
>>>>> Please share your DT node after the update.
>>>>>
>>>> Pasting the GPIO_0 entry only here for brevity.
>>>> leds {
>>>>          compatible = "gpio-leds”;
>>>>          fan0 {
>>>>              retain-state-shutdown;
>>>>              default-state = "keep";
>>>>              gpios = <&pca0 0 GPIO_ACTIVE_LOW>;
>>>>          };
>>>>          nvmeslot0 {
>>>>              retain-state-shutdown;
>>>>              default-state = "keep";
>>>>              gpios = <&pca1 0 GPIO_ACTIVE_LOW>;
>>>>          };
>>>> };
>>>> &i2c7 {
>>>>      status = "okay”;
>>>>      pca0: pca9552@61 {
>>>>         compatible = "nxp,pca9552";
>>>>          reg = <0x61>;
>>>>          #address-cells = <1>;
>>>>          #size-cells = <0>;
>>>>          gpio-controller;
>>>>          #gpio-cells = <2>;
>>>>          gpio@0 {
>>>>              reg = <0>;
>>>>              type = <PCA955X_TYPE_GPIO>;
>>>>           };
>>>>      };
>>>> };
>>>> &i2c13
>>>> {
>>>>      pca1: pca9552@60 {
>>>>         compatible = "nxp,pca9552";
>>>>         reg = <0x60>;
>>>>         #address-cells = <1>;
>>>>         #size-cells = <0>;
>>>>         gpio-controller;
>>>>         #gpio-cells = <2>;
>>>>         gpio@0 {
>>>>             reg = <0>;
>>>>             type = <PCA955X_TYPE_GPIO>;
>>>>         };
>>>>      };
>>>> };
>>>> Thanks
>>>> !! Vishwa !!
>>>>>> I don’t even see “fan0” that is on the PCA9552 on planar also. I 
>>>>>> was expecting that I should see “/sys/class/leds/fan0”.
>>>>>> However, I could see all the entries in “/proc/device-tree/leds”.
>>>>>> Data from the failure.
>>>>>> [    7.895757] leds-pca955x 7-0061: leds-pca955x: Using pca9552 
>>>>>> 16-bit LED driver at slave address 0x61
>>>>>> [    7.907659] leds-pca955x 7-0061: gpios 168...183
>>>
>>> It is weird that you don't see "fan0" LED since this gpio seems to have
>>> been properly registered according to this log.
>>>
>>
>>
>> This is exactly what I don’t understand. I would expect “fan0” to 
>> appear in /sys/class/leds. Is there any reason why this might not be 
>> appearing ?..
> 
> OK, now the reason is clear to me. If leds-gpio driver fails to register
> any of the LEDs found in DT node it returns with an error from the
> probe(), which results in unregistering any of the LEDs registered in
> the previous iteration steps.
> 
> Look at the function gpio_leds_create() in
> drivers/leds/leds-gpio.c.
> 
> Probably it is devm_fwnode_get_gpiod_from_child() that fails
> while parsing nvmeslot0 node.

IOW you have to have two separate dts files for the arrangement
with the card and without it.

-- 
Best regards,
Jacek Anaszewski