RE: [PATCH linux dev-5.10 06/35] ARM: dts: aspeed: rainier: Add leds that are off PCA9552

["Milton Miller II" <miltonm@us.ibm.com>]

-----"openbmc" <openbmc-bounces+miltonm=us.ibm.com@lists.ozlabs.org> wrote: -----

>To: Eddie James <eajames@linux.ibm.com>
>From: Joel Stanley 
>Sent by: "openbmc" 
>Date: 03/11/2021 06:09PM
>Cc: OpenBMC Maillist <openbmc@lists.ozlabs.org>
>Subject: [EXTERNAL] Re: [PATCH linux dev-5.10 06/35] ARM: dts:
>aspeed: rainier: Add leds that are off PCA9552
>
>On Mon, 8 Mar 2021 at 22:54, Eddie James <eajames@linux.ibm.com>
>wrote:
>>
>> From: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>
>>
>> These LEDs are on the fans and are connected via a
>> pca9551 i2c expander
>
>This change doesn't make sense. The pca9551 is an i2c LED expander,
>so
>we don't need to expose the pins as GPIOs and then attach a gpio-leds
>driver to them. We should instead simply configure the pca955x driver
>to drive the LEDs as LEDs.

I'll refresh your memory on why we have been doing this in our 
devie trees and then let you consider if this is desired or not.

The led system insistes on creating a compact map (no holes) (as
does the reset subsystem).

However, this means the relative led number for a pin changes 
as the prior pins change from gpio to led configuration.

For example if pins 2 and 7 are leds, they become leds 0 and 1.  
Changing pin 5 to also be an led means that pin 7 is now led 2 
not led 1 on the led subsystem.

The workaround we have done for existing systems has been to use 
gpio leds for pca family devices.

milton

>
>>
>> Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>
>> ---
>>  arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 41
>++++++++++++++++++++
>>  1 file changed, 41 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>> index 6684485a2db0..514a14d3f914 100644
>> --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>> +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
>> @@ -263,6 +263,47 @@ fan5-presence {
>>                         linux,code = <11>;
>>                 };
>>         };
>> +
>> +       leds {
>> +               compatible = "gpio-leds";
>> +
>> +               fan0 {
>> +                       retain-state-shutdown;
>> +                       default-state = "keep";
>> +                       gpios = <&pca0 0 GPIO_ACTIVE_LOW>;
>> +               };
>> +
>> +               fan1 {
>> +                       retain-state-shutdown;
>> +                       default-state = "keep";
>> +                       gpios = <&pca0 1 GPIO_ACTIVE_LOW>;
>> +               };
>> +
>> +               fan2 {
>> +                       retain-state-shutdown;
>> +                       default-state = "keep";
>> +                       gpios = <&pca0 2 GPIO_ACTIVE_LOW>;
>> +               };
>> +
>> +               fan3 {
>> +                       retain-state-shutdown;
>> +                       default-state = "keep";
>> +                       gpios = <&pca0 3 GPIO_ACTIVE_LOW>;
>> +               };
>> +
>> +               fan4 {
>> +                       retain-state-shutdown;
>> +                       default-state = "keep";
>> +                       gpios = <&pca0 4 GPIO_ACTIVE_LOW>;
>> +               };
>> +
>> +               fan5 {
>> +                       retain-state-shutdown;
>> +                       default-state = "keep";
>> +                       gpios = <&pca0 5 GPIO_ACTIVE_LOW>;
>> +               };
>> +       };
>> +
>>  };
>>
>>  &ehci1 {
>> --
>> 2.27.0
>>
>
>