per_led_trigger/act_led_trigger not working with RaspberryPi3+

[jens-maus]

(I am honestly not sure if this ticket belongs here or to raspberrypi/kernel. However, as the firmware is the first to load I am posting it here. Please advise if otherwise.)

After having received my Pi3+ I updated my own distribution to utilize the new LAN75xx chip. I use the latest raspberrypi/firmware from the master branch and also updated to the latest 4.14.26 kernel from raspberrypi/kernel. Everything is working fine now and WiFI, Bluetooth and Ethernet is working again after the necessary kernel changes. However, the only thing that seems to remain different is the behavior of the onboard LEDs of the Pi3+.

With older Rpi models I was able to set dtparam=pwr_led_trigger=timer,act_led_trigger=mmc0 to change the PWR and ACT led triggers right at the firmware/bootloader level and see the LEDs immediately reacting on it. This is not the case with the Pi3+ anymore. In addition, after having booted my distribution all the /sys/class/leds/ledX links are missing as well as /sys/devices/platform/leds/ not containing any leds directory anymore. Thus, IMHO this suggests that there is still something different between the Pi3 and Pi3+ regarding the LED management.

As soon as I put the SD card from the Pi3+ into the older Pi3 the LED triggers seem to work again and /sys/class/leds/ledX exists again after boot up.

[pelwell]

I think you've missed something in your kernel or DTBs - your dtparam line works for me with a standard RPi kernel on a 3B+, and both /sys/devices/platform/leds/leds and /sys/class/leds contain the usual contents that works as expected.

Please run dtc -I fs /proc/device-tree > dt.txt and upload the output somewhere for me to see. If you have access to the vcdbg utility, also run sudo vcdbg log msg >& vcdbg.txt and again upload the output.

[jens-maus]

Thanks for your comments. Please find attached the two files you were requesting.

vcdbg.txt dt.txt

Hopefully you can spot something fishy in here because I really don't know why /sys/class/leds hasn't the usual content. I can really just swap the SD card to a plain non-plus Pi3 and the LEDs can be used accordingly.

[pelwell]

Those logs show me that the correct DT is loaded and that the dtparams are being applied correctly (note the triggers):

    leds {
        compatible = "gpio-leds";
        phandle = <0x75>;

        act {
            gpios = <0x10 0x1d 0x0>;
            label = "led0";
            phandle = <0x2e>;
            linux,default-trigger = "mmc0";
        };

        pwr {
            gpios = <0x17 0x2 0x1>;
            label = "led1";
            phandle = <0x2f>;
            linux,default-trigger = "timer";
        };
    };

Given that, the most likely reason for the lack of LEDS-support is that the kernel config setting is missing. Try this:

pi@raspberrypi:~ $ sudo modprobe configs
pi@raspberrypi:~ $ zcat /proc/config.gz | grep -E '^CONFIG_LEDS_'

The standard RPi kernels return:

CONFIG_LEDS_CLASS=y
CONFIG_LEDS_GPIO=y
CONFIG_LEDS_TRIGGERS=y
CONFIG_LEDS_TRIGGER_TIMER=y
CONFIG_LEDS_TRIGGER_ONESHOT=y
CONFIG_LEDS_TRIGGER_HEARTBEAT=y
CONFIG_LEDS_TRIGGER_BACKLIGHT=y
CONFIG_LEDS_TRIGGER_CPU=y
CONFIG_LEDS_TRIGGER_GPIO=y
CONFIG_LEDS_TRIGGER_DEFAULT_ON=y
CONFIG_LEDS_TRIGGER_TRANSIENT=m
CONFIG_LEDS_TRIGGER_CAMERA=m
CONFIG_LEDS_TRIGGER_INPUT=y
CONFIG_LEDS_TRIGGER_PANIC=y

[jens-maus]

This is unfortunately not the case:

# zcat /proc/config.gz | grep -E '^CONFIG_LEDS_'
CONFIG_LEDS_CLASS=y
CONFIG_LEDS_GPIO=y
CONFIG_LEDS_TRIGGERS=y
CONFIG_LEDS_TRIGGER_TIMER=y
CONFIG_LEDS_TRIGGER_ONESHOT=y
CONFIG_LEDS_TRIGGER_HEARTBEAT=y
CONFIG_LEDS_TRIGGER_BACKLIGHT=y
CONFIG_LEDS_TRIGGER_CPU=y
CONFIG_LEDS_TRIGGER_GPIO=y
CONFIG_LEDS_TRIGGER_DEFAULT_ON=y
CONFIG_LEDS_TRIGGER_TRANSIENT=m
CONFIG_LEDS_TRIGGER_CAMERA=m
CONFIG_LEDS_TRIGGER_INPUT=y
CONFIG_LEDS_TRIGGER_PANIC=y

As said, I can remove the SD card here from the Pi3+ where LED support in /sys is missing and put it into the older Pi3 and it boots up properly but there /sys/class/leds is properly populated to set the trigger to the defined values.

See here for the exakt kernel config I am using to reproduce the problem: https://github.com/jens-maus/RaspberryMatic/blob/master/buildroot-external/board/raspberrypi3/kernel_defconfig

And please note: I am using the latest HEAD of raspberrypi/firmware and raspberrypi/linux

[pelwell]

Your defconfig is missing two GPIO modules:

$ diff bcm2709_config jens_config  | grep GPIO
< CONFIG_GPIO_BCM_EXP=y
< CONFIG_GPIO_BCM_VIRT=y
> # CONFIG_GPIO_BCM_EXP is not set
> # CONFIG_GPIO_BCM_VIRT is not set

Pi 3 needs both of those drivers for the LEDs, whereas 3B+ only needs CONFIG_GPIO_BCM_EXP, so my theory is that failing to request one of the LED gpios is killing the LEDs subsystem. It also suggests that you aren't running exactly the same kernel on the Pi 3.

OK - with a tweaked bcm2709_defconfig that omits those two settings I also get no LEDs. Restoring CONFIG_GPIO_BCM_EXP is sufficient to make it work on the 3B+, but they are both small drivers so I would include both.

[jens-maus]

Thanks, that solves my issues. However, I still don't know why then it worked with the Pi3 while the same SD card didn't work in the Pi3+. No matter what, now it works, thanks! Learned my lesson.

Nevertheless, I tested it now, and here the PWR LED of the Pi3+ still behaves strange/different regarding setting triggers. With a plain Pi3 I am able to set the PWR, e.g. completely off by doing:

 echo none >/sys/class/leds/led1/trigger

With the Pi3+ this doesn't work (it stays on). However, setting the ACT LED (led0) off/on works as well as setting it to heartbeat. But also setting the PWR LED (led1) to heartbeat doesn't work with the Pi3+ while it perfectly works with the Pi3. Can you reproduce this?

NOTE: To me it seems the PWR LED is exactly behaving the other way around (e.g. none keeps it turned on, and setting it to mmc0 turns it off when there is SD card activity).

[pelwell]

Yes, the PWR LED is different on 3B+. Rather than running it as an input that senses under-voltage (UV) and drives the LED as a side effect - something modifying the trigger prevents(*) - on the 3B+ UV is detected in a different way and the PWR LED is just another LED. However, in order to get a visual indication that a UV situation is occurring, the firmware will toggle the LED. Unfortunately the current implementation sets the LED state each time we poll the UV state, wiping out the state set by the ARM.

It is a fairly simple change to remember the previous UV state and only update the LED should it change, with an extra refinement to sense the LED state when UV occurs and to restore it when the normal voltage returns, rather than explicitly switching the LED on. I'll consider this issue a request to make that change.

[jens-maus]

Thanks for the clarification. And it would be indeed good if you could implement that so that setting the PWR via sysfs works like with a plain Pi3.

[pelwell]

This change will be in the next firmware release.

[jens-maus]

Thanks, I just tested the latest firmware and the PWR led works now identical to a plain Pi3. Thus, closing this issue.