Hi
On Tue, Apr 13, 2021 at 07:02:26PM +0200, Sicelo wrote:
> Hi
>
> For some time now, the keyboard LEDs on Nokia N900 have not been
> working. In dmesg is the error:
>
> lp5523x probe of 2-0032 failed with error -22
>
> and consequently they do not get populated under /sys/class/leds.
>
> I have finally had time to look into this, and found that the N900 dts
> falls short of the current lp55xx binding specs. I have updated it and
> will be submitting a separate patch. However, this was not enough to
I have since realized that I should have included the adjusted dts in
the initial email. Please find it attached in this follow-up email and
sincere apologies for this ommission.
> make them work, and a new error appeared:
>
> [11363.247375] lp5523x 2-0032: GPIO lookup for consumer enable
> [11363.254394] lp5523x 2-0032: using device tree for GPIO lookup
> [11363.254455] of_get_named_gpiod_flags: can't parse 'enable-gpios' property of node '/ocp@68000000/i2c@48072000/lp5523@32[0]'
> [11363.254547] of_get_named_gpiod_flags: parsed 'enable-gpio' property of node '/ocp@68000000/i2c@48072000/lp5523@32[0]' - status (0)
> [11363.254638] gpio gpiochip1: Persistence not supported for GPIO 9
> [11363.254669] gpio-41 (enable): no flags found for enable
> [11363.295959] lp5523x 2-0032: device detection err: -121
> [11363.303710] lp5523x: probe of 2-0032 failed with error -121
>
>
> This is because the chip does not get enabled even though dts contains:
>
> enable-gpios = <&gpio2 9 GPIO_ACTIVE_HIGH>; /* 41 */
>
> which is correct for this board.
>
>
> I came up with this patch (against 5.12-rc6), which makes it work again.
>
>
> diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
> index 81de1346bf5d..9af84fc335b3 100644
> --- a/drivers/leds/leds-lp55xx-common.c
> +++ b/drivers/leds/leds-lp55xx-common.c
> @@ -694,7 +694,7 @@ struct lp55xx_platform_data *lp55xx_of_populate_pdata(struct device *dev,
> of_property_read_u8(np, "clock-mode", &pdata->clock_mode);
>
> pdata->enable_gpiod = devm_gpiod_get_optional(dev, "enable",
> - GPIOD_ASIS);
> + GPIOD_OUT_HIGH);
> if (IS_ERR(pdata->enable_gpiod))
> return ERR_CAST(pdata->enable_gpiod);
>
>
> However, I am not sure if this is a proper way to fix the issue, or if
> it is a dirty hack. It looks like all the chips handled by this driver
> (lp5521, lp5523, lp5562, and lp8501) get enabled by pulling their ENable
> pin high, so I suppose it is fine, but would like to get some feedback
> before submitting a cleaned up patch. It is also possible that this
> should be defined via dts elsewhere, although at this point I am not
> sure how.
>
> Looking forward to your help.
---
diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 32335d4ce478..d40c3d2c4914 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -8,6 +8,7 @@
#include "omap34xx.dtsi"
#include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
/*
* Default secure signed bootloader (Nokia X-Loader) does not enable L3 firewall
@@ -630,63 +631,92 @@ indicator {
};
lp5523: lp5523@32 {
+ #address-cells = <1>;
+ #size-cells = <0>;
compatible = "national,lp5523";
reg = <0x32>;
clock-mode = /bits/ 8 <0>; /* LP55XX_CLOCK_AUTO */
- enable-gpio = <&gpio2 9 GPIO_ACTIVE_HIGH>; /* 41 */
+ enable-gpios = <&gpio2 9 GPIO_ACTIVE_HIGH>; /* 41 */
- chan0 {
+ led@0 {
+ reg = <0>;
chan-name = "lp5523:kb1";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_KBD_BACKLIGHT;
};
- chan1 {
+ led@1 {
+ reg = <1>;
chan-name = "lp5523:kb2";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_KBD_BACKLIGHT;
};
- chan2 {
+ led@2 {
+ reg = <2>;
chan-name = "lp5523:kb3";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_KBD_BACKLIGHT;
};
- chan3 {
+ led@3 {
+ reg = <3>;
chan-name = "lp5523:kb4";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_KBD_BACKLIGHT;
};
- chan4 {
+ led@4 {
+ reg = <4>;
chan-name = "lp5523:b";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ color = <LED_COLOR_ID_BLUE>;
+ function = LED_FUNCTION_STATUS;
};
- chan5 {
+ led@5 {
+ reg = <5>;
chan-name = "lp5523:g";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ color = <LED_COLOR_ID_GREEN>;
+ function = LED_FUNCTION_STATUS;
};
- chan6 {
+ led@6 {
+ reg = <6>;
chan-name = "lp5523:r";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ color = <LED_COLOR_ID_RED>;
+ function = LED_FUNCTION_STATUS;
};
- chan7 {
+ led@7 {
+ reg = <7>;
chan-name = "lp5523:kb5";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_KBD_BACKLIGHT;
};
- chan8 {
+ led@8 {
+ reg = <8>;
chan-name = "lp5523:kb6";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ color = <LED_COLOR_ID_WHITE>;
+ function = LED_FUNCTION_KBD_BACKLIGHT;
};
};
