Re: lp5523/lp5xx-common : Keyboard and RGB LEDs Not Working on Nokia N900

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;
                };
        };




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux