LED controller (LP5523) fails ASUS Lyra (MAP-AC2200) on 22.03.0-rc1 and r19605-203ffc4ca7

[tboege]

in 22.03.0-rc1

Running v 21.02.3 the led's works fine. But on both 22.03.0-rc1 and snapshot from today, 20220705, 19605-203ffc4ca7 the initialization of lp5523 failes with
[ 1.969494] lp5523x: probe of 0-0032 failed with error -22

So I cannot control the LED's on the newer versions, and is is just "Random Breathing"

[chunkeey]

The MR33 was in a similar pinch with the lp5562: 85a42fa
(Same reason here.)

The MAP-AC2200 device-tree file will need an update. Can you please check if the following patch:
https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=2a46f222f9f76e2a31b6e5407163d14f5b751e53
https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=124a0e69a6348eaa3ed7494fab8f3f58c862fbef
helps?

[tboege]

I have applied the patch on master and uploaded - I can confirm, that the patch fixes the problem

This time, the led is not "breathing" - but a low-intensive led is on after startup.

The driver is found and initialized (it seems):

root@OpenWrt:~# logread |grep 5523
Sat May  7 15:23:59 2022 kern.info kernel: [    2.356456] lp5523x 0-0032: lp5523 Programmable led chip found

And I can control the leds:

root@OpenWrt:/# ls -l /sys/class/leds/

lrwxrwxrwx    1 root     root             0 May  7 15:30 ath10k-phy0 -> ../../devices/platform/soc/40000000.pci/pci0000:00/0000:00:00.0/0000:01:00.0/leds/ath10k-phy0
lrwxrwxrwx    1 root     root             0 Jan  1  1970 blue-0 -> ../../devices/platform/soc/78b7000.i2c/i2c-0/0-0032/leds/blue-0
lrwxrwxrwx    1 root     root             0 Jan  1  1970 blue-1 -> ../../devices/platform/soc/78b7000.i2c/i2c-0/0-0032/leds/blue-1
lrwxrwxrwx    1 root     root             0 Jan  1  1970 blue-2 -> ../../devices/platform/soc/78b7000.i2c/i2c-0/0-0032/leds/blue-2
lrwxrwxrwx    1 root     root             0 Jan  1  1970 green-0 -> ../../devices/platform/soc/78b7000.i2c/i2c-0/0-0032/leds/green-0
lrwxrwxrwx    1 root     root             0 Jan  1  1970 green-1 -> ../../devices/platform/soc/78b7000.i2c/i2c-0/0-0032/leds/green-1
lrwxrwxrwx    1 root     root             0 Jan  1  1970 green-2 -> ../../devices/platform/soc/78b7000.i2c/i2c-0/0-0032/leds/green-2
lrwxrwxrwx    1 root     root             0 Jan  1  1970 red-0 -> ../../devices/platform/soc/78b7000.i2c/i2c-0/0-0032/leds/red-0
lrwxrwxrwx    1 root     root             0 Jan  1  1970 red-1 -> ../../devices/platform/soc/78b7000.i2c/i2c-0/0-0032/leds/red-1
lrwxrwxrwx    1 root     root             0 Jan  1  1970 red-2 -> ../../devices/platform/soc/78b7000.i2c/i2c-0/0-0032/leds/red-2

I am not sure, that "ath10k-phy0" is an acutal led. Maybe it is only visible on the pcb - og not mounted at all.

root@OpenWrt:~# cat /etc/openwrt_*
root@OpenWrt:/sys/class/leds#
root@OpenWrt:/sys/class/leds# cd
root@OpenWrt:~# cat /etc/openwrt_*
DISTRIB_ID='OpenWrt'
DISTRIB_RELEASE='SNAPSHOT'
DISTRIB_REVISION='r19615-580723e86a'
DISTRIB_TARGET='ipq40xx/generic'
DISTRIB_ARCH='arm_cortex-a7_neon-vfpv4'
DISTRIB_DESCRIPTION='OpenWrt SNAPSHOT r19615-580723e86a'
DISTRIB_TAINTS='no-all'
r19615-580723e86a

Should I just close this issue - or should I wait until the patch is merged into master?

[chunkeey]

I have applied the patch on master and uploaded - I can confirm, that the patch fixes the problem

This time, the led is not "breathing" - but a low-intensive led is on after startup.

This is good to know. Does the device behave as it did with 21.2 there, or feels something "off" with regards to the LEDs?

• i.e.: are the leds all in the correct order, or are some swapped?
• is it possible to switch off the LED completely by writing 0 to the brightness through the sysfs interface?

(If possible, OpenWrt tries to stick with what the vendor had in mind with the LED and what the color could mean.)

For the future it would be great to know which of the 9 channels is part of which individual RGB-LED. I suspect that there are three independent RGB-LEDs on this device. If that's the case, the binding should be extended to have the newfangled LED_RGB too which bundles them together as RGB-LEDs.

I am not sure, that "ath10k-phy0" is an actual led. Maybe it is only visible on the pcb - og not mounted at all.

This LED is a dd-wrt/openwrt enhancement for ath10k.

Both ath10k-ct and ath10k will register a LED for the pcie chip variants, even if no leds is present.
You can ignore that led, it's not there for most routers. (The use-case for this are the dedicated ath10k (m)pcie cards
that you can buy for laptops/PCs. There the LED-pin might be useful since it could be connected.)

Should I just close this issue - or should I wait until the patch is merged into master?

Just leave it open. It's best to reference the fix with the closing comment.

BTW: If you want, you can provide a Reported-By: / Tested-By: Tag for the patch. For this tags your name and a valid e-mail is reguired. If you want to stay anonymous, that's OK too. (The patch will in both cases contain a BugLink reference to the issue.)

[pkgadd]

I won't be near my map-ac2200 for at least 3, probably 4-5, weeks and can't test it at the moment, but there seems to be only a single LED (array?):

this is an extremely bright LED, using the top case as diffusor to create a round/ honey-comb ambient light.
On kernel <= v5.4, there were two sysfs nodes per colour, a low(er) intensity one and a high(er) intensity one (and yes, the brightness could be tuned freely within those brackets).

It's been almost three years since I ran the OEM firmware (and only for two days before flashing OpenWrt), so I don't really remember the colour codes - but if I remember correctly, the defaults showed a steady LED colour (was it green?) after powering up, with different colours based on the mesh status (I only ever got a single device, so I haven't tested those parts either).

[if necessary, I can arrange to get my map-ac2200 sent over to me, to test further things - but that would take a couple of days at least].

[tboege]

• i.e.: are the leds all in the correct order, or are some swapped?

I dont think there leds has any order defined. As I understand it, there are 3 blue led's somehow coupled together. So it is impossible to see, if you turn on blue-0,blue-1 or blue-2. When using them together, you can increase the brightness way more than would be possible with a single led

• is it possible to switch off the LED completely by writing 0 to the brightness through the sysfs interface?

Yes, for all 9 leds I can select on/off (0/255) or any value between.

(If possible, OpenWrt tries to stick with what the vendor had in mind with the LED and what the color could mean.)

The vendor has 5 led colors defined:

1. Light Cyan: Everything is good
2. Red: Loss connection to the primary Lyra
   3 Orange Loss connection to the internet
3. Solid white: Lyra is ready for setup. It will start blinking when setting up with the app
4. Random Breathing: Lyra is booting up or appying settings.

When the device boots in OpenWrt with the patch, it blinks, and when boot is finished, it shows the steady blue-0 with brightness 255
So to mimic the vendors behavior, the color should be light cyan - but I dont find it important.

BTW: If you want, you can provide a Reported-By: / Tested-By: Tag for the patch.

I will try to figure out how to create such a tag, but I would like some more detailed help.

[pkgadd]

• i.e.: are the leds all in the correct order, or are some swapped?

I dont think there leds has any order defined. As I understand it, there are 3 blue led's somehow coupled together.

If I remember the details correctly, the different per-colour LEDs slightly affect how wide the light focus goes (smaller diameter to full diameter, but those differences are indeed rather small).

BTW: If you want, you can provide a Reported-By: / Tested-By: Tag for the patch.

I will try to figure out how to create such a tag, but I would like some more detailed help.

This is simply providing a (text-) tag:

Reported-By: Joe User <mymail@example.com>
Tested-By: Joe User <mymail@example.com>

It's meant to attribute the efforts to report/ test the changes to a user (and maybe provide a means to come back for further testing, should any issues arise with the change later on).

[tboege]

If I remember the details correctly, the different per-colour LEDs slightly affect how wide the light focus goes (smaller diameter to full diameter, but those differences are indeed rather small).

Did som testing - the differneces are smaller than I can detect.

I'm afraid I am complete noob regarding providing tags. Is it something I should do on github? https://git.openwrt.org?.
Acutally, I didnt even know how to apply the patch with some git-magic, so I created a testfile with the patch and patched the source with
patch target/linux/ipq40xx/files/arch/arm/boot/dts/qcom-ipq4019-map-ac2200.dts <patch.txt

[pkgadd]

I'm afraid I am complete noob regarding providing tags. Is it something I should do on github?

No, it's rather free-form, you just add the (adapted) two lines I mentioned above in a response to this issue report. chunkeey can then add them (copy'n'paste) to the commit message when applying the patch to OpenWrt.

The intention behind it is you just 'officially' providing the tags (name/ mail address) to be used in the commit message, providing an 'audit trail' in the version control system/ git history. This is important for marking the author of a commit (From/ Signed-off-by), while Reported-by/ Tested-by are more optional and mostly meant to attribute the efforts going into reporting/ testing to a person.

See https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff_plain;h=98bb26f9f762408e42bd8a906f0eb01c41ada10a for a random (recent) example of how this might end up in the git history.

Technically these tags are free-form, just part of the text of the commit message, but these well-known tags can be parsed by higher level tools.

So yes, you simply state

Reported-By: Joe User <mymail@example.com>
Tested-By: Joe User <mymail@example.com>

as a response (or you don't, if you don't want those to become these part of the git history, as -other than From/ Signed-off-by of the author/ committer- Reported-by/ Tested-by are not mandatory and 'just' encouraged).

[tboege]

Reported-By: Thomas Bøge <thomas@boegenielsen.dk>
Tested-By: Thomas Bøge <thomas@boegenielsen.dk>

[chunkeey]

https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=a3b2d6d329fe3fc89adf659a1718e2b5c60d4d87

Updated the patch with migration scripts (to update the previous color:chan$i -> color-$i)

There is only a single extremely bright RGB-LED.
The RGB-color channels (i.e.: blue-0, blue-1 and blue-2)
are running in parallel to increase the current delivery
beyond what a single PWM-output on the LED controller
could do.

From what I can tell based on the description+picture this should be accurate (i.e. 9 channels for one single RGB-LED),
or is there something missing?

Bonding several channels together is something new. I think upstream hasn't thought about that when they were designing the RGB/MULTILED support?! That said, I noticed that the led-cur + max-cur for each individual LED looked very high and "odd".
(For reference: https://www.kernel.org/doc/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml).

=> ASUS is maxing out what the output stage of the led-controller could deliver.

[chunkeey]

(queued for 22.03 too, but not yet pushed
)

[tboege]

I can confirm, that the LED's now work as expected, so I close this issue. Thank you to everyone who contributed to the fix.