Joel,
With the experiments that I have done, I can not express LEDs with PCA955X_TYPE_LED predominantly because LEDs won’t
retain states after the BMC reboot. I cooked a patch and tried but it does not work. I did an experiment where
I put the patch and then did a reboot and saw that the LEDs were [OFF] in the very early stage of probe itself.
>From a9fe9e956c624c15a455b88cc05262358519a541 Mon Sep 17 00:00:00 2001
From: Vishwanatha Subbanna <vishwa@xxxxxxxxxxxxxxxxxx>
Date: Fri, 23 Apr 2021 06:57:56 -0500
Subject: [PATCH 1/2] leds: pca955x: Add support for default-state
Signed-off-by: Vishwanatha Subbanna <vishwa@xxxxxxxxxxxxxxxxxx>
---
drivers/leds/leds-pca955x.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index bf7ead4..987415b 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -130,6 +130,7 @@ struct pca955x_led {
char name[32];
u32 type;
const char *default_trigger;
+ const char *default_state;
};
struct pca955x_platform_data {
@@ -408,6 +409,8 @@ static int pca955x_gpio_direction_output(struct gpio_chip *gc,
fwnode_property_read_u32(child, "type", &pdata->leds[reg].type);
fwnode_property_read_string(child, "linux,default-trigger",
&pdata->leds[reg].default_trigger);
+ fwnode_property_read_string(child, "default-state",
+ &pdata->leds[reg].default_state);
}
pdata->num_leds = chip->bits;
@@ -520,8 +523,13 @@ static int pca955x_probe(struct i2c_client *client,
if (err)
return err;
- /* Turn off LED */
- err = pca955x_led_set(&pca955x_led->led_cdev, LED_OFF);
+ /* If the default-state is "keep", don't change states */
+ if (strcmp(pdata->leds[i].default_state, "keep")) {
+ if (!strcmp(pdata->leds[i].default_state, "on"))
+ err = pca955x_led_set(&pca955x_led->led_cdev, LED_ON);
+ else
+ err = pca955x_led_set(&pca955x_led->led_cdev, LED_OFF);
+ }
if (err)
return err;
}
—
1.8.3.1
For `leds-gpio`, Andrew had put a patch, but I don’t see how that can be mapped to PCA955X. https://github.com/torvalds/linux/commit/f5808ac158f2b16b686a3d3c0879c5d6048aba14
Jacek,
Please could you help me here ?.. I need to express LEDs as PCA955X_TYPE_LED and also retain states post BMC reboot.
Thank you,
!! Vishwa !!
> On 25-Mar-2021, at 5:13 AM, Joel Stanley <joel@xxxxxxxxx> wrote:
>
> On Fri, 12 Mar 2021 at 07:05, vishwanatha subbanna
> <vishwa@xxxxxxxxxxxxxxxxxx> wrote:
>>
>>
>>
>> On 12-Mar-2021, at 6:00 AM, Joel Stanley <joel@xxxxxxxxx> wrote:
>>
>> On Fri, 12 Mar 2021 at 00:21, Milton Miller II <miltonm@xxxxxxxxxx> wrote:
>>
>>
>>
>>
>> -----"openbmc" <openbmc-bounces+miltonm=us.ibm.com@xxxxxxxxxxxxxxxx> wrote: -----
>>
>> To: Eddie James <eajames@xxxxxxxxxxxxx>
>> From: Joel Stanley
>> Sent by: "openbmc"
>> Date: 03/11/2021 06:09PM
>> Cc: OpenBMC Maillist <openbmc@xxxxxxxxxxxxxxxx>
>> 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@xxxxxxxxxxxxx>
>> wrote:
>>
>>
>> From: Vishwanatha Subbanna <vishwa@xxxxxxxxxxxxxxxxxx>
>>
>> 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.
>>
>>
>> Thanks for the rationale reminder.
>>
>> Are these led numbers important to userspace, or does the renumbering
>> affect device tree changes only?
>>
>>
>>
>> Here are my technical needs.
>> - I need these LEDs associated with names and this __must not__ change
>> - I need those LEDs represented as `/sys/class/leds/<$name>`
>>
>> What can I do :
>> - use `leds-gpio` like how it’s done today
>>
>> OR
>>
>> - Use “label” in PCA955X_TYPE_LED
>> - However, putting this label, it results in `/sys/class/leds/pca955x:<$label>`. As opposed to `/sys/class/leds/<$label>`.
>>
>> Is there a way where I can get `/sys/class/leds/<$label>` ?. I did not get this from the documentation. Seeing pca955x on 100 entries seems a noise
>
> The prefix has been present in the driver since it was introduced in
> 2008. Is there any reason we can't have userspace ignore the pca955x
> prefix?
>
> Cheers,
>
> Joel
