Led trigger not working on rpi3b+

[Paolo Pisati]

Impact:

User are reporting that the green led (the one next to the red power led) is not working on the RaspberryPi 3B+ board.

After debugging the issue, this is what i found:

1) during boot, all leds are initialized in a function loop, and if one of them fails to setup, the loop tears down all initialized instances:

...
[ 2.299216] leds-gpio: probe of soc:leds failed with error -2
...

in this particular case, it's not the green action led that fails to initialize, but it's red power led that fails and the loop tears down both of them but since the red (by default) is connected to Vcc, it stays lit (see point 3 below for more info on the wiring) so we have the impression that the one to fail is the green one.

This can easily tested by adding debug to drivers/leds/leds-gpio.c::gpio_leds_create() and drivers/leds/leds-gpio.c::gpio_led_probe(), or commenting out the red pwr_led block in arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts - the board will boot up, the above leds-gpio error will disappear and the green action led will work properly.

2) the pwr_led in the rpi3 b+ is not available direcly to the OS, instead only the Broadcom firmware can access its gpio, thus a new mechanism (the exgpio driver) that uses the bcm firmware as a middleman was developed: using a mailbox mechanism, messages are sent from the Linux kernel to the Broadcom firmware to query or set the status of the led, this way it's possible to plumb the Linux led subsystem to this gpio

3) the biasing of the two leds (power and action) is "opposite" and can be easily confirmed by looking at the board schematics[1]:

-D5 (the action led) uses the transistor Q5 in a switch configuration - when Q5 is not biased, it acts as an open switch and no current goes through the led - so by default the led is off

-D6 (the power led) uses Q4 in a sink configuration - when Q4 is off, current normally flows through the led, while when Q4 is biased, all current is sinked to GND via Q4 - the led by default on

Fix:

The fix is composed of 8 patches:

1f50a81 UBUNTU: [Config] GPIO_BCM_EXP=y
53bc3cc UBUNTU: SAUCE: dts: rpi3: remove hpd-gpios overwrite
a1252a5 UBUNTU: SAUCE: dts: cm3: revert hpd-gpios to gpio
0d136d8 UBUNTU: SAUCE: make pwr_led GPIO_ACTIVE_LOW
e2d3ba2 UBUNTU: SAUCE: make it build on bcm2709
245fc18 Revert "UBUNTU: SAUCE: dts: use the virtgpio driver"
be25728 bcm2835-gpio-exp: Copy/paste error adding base twice
4ccdf22 bcm2835-gpio-exp: Driver for GPIO expander via mailbox service

From the bottom:

-patch 0001 and 002 contain the "gpio via firmware" exgpio driver (and a fix for it) - those are the original patches that are present in the Github's RaspberryPi repository[2] rpi-4.9.y branch, and they were cherry-picked cleanly from it (minus same mechanical changes in Kconfig and Makefile surrounding context to make it apply).

-patch 0003 reverts a change that tried to pilot pwr led gpio using the default virtgpio driver

-patch 004 fixes the arch name - in 4.9 where the driver originated, the Raspberry arch was called "ARCH_BCM2835", while in Xenial 4.4 is "ARCH_BCM2709".

-patch 005 fixes the pwr led biasing level

-patch 006 and 007 reduce the "regression surface": when patch 001 was introduced in 4.9, the hdmi phy in the RaspberryPi3 and CM3 was moved to use the new driver, but since our hdmi driver is significantly different from the one in the rpy-4.9 tree, and since this bug deal only with leds and noone has opened one against the hvmi video output, to reduce the regression surface, i reverted these chunks in the new driver (using these two SAUCE patches) and kept using the mechanism we used so far in Xenial.

-patch 008 enable the new driver

By apply these patches, the power led correctly initialize during boot, and as a consequence the action led work too.

How to test:

Add this to config.txt:

dtparam=act_led_trigger=heartbeat
dtparam=pwr_led_trigger=heartbeat

and reboot - on a non-patched kernel, the power led (red) will be always-on, while the action led (green) will be off.
On a patched kernel, both leds will blink intermittently.

Regression:

It's new code, so there's always regression potential in it, but by reducing the scope of the original patch, the impact is limited to the led code, and only applies to the RaspberryPi3B+ board.

1: https://www.raspberrypi.org/documentation/hardware/raspberrypi/schematics/rpi_SCH_3bplus_1p0_reduced.pdf
2: https://github.com/raspberrypi/linux

[Launchpad Janitor (janitor)]

This bug was fixed in the package linux-raspi2 - 4.4.0-1103.111

---------------
linux-raspi2 (4.4.0-1103.111) xenial; urgency=medium

  * linux-raspi2: 4.4.0-1103.111 -proposed tracker (LP: #1811854)

  * Xenial update: 4.4.164 upstream stable release (LP: #1810947)
    - [CONFIG]: remove BCH_CONST_PARAMS=y

  * Led trigger not working on rpi3b+ (LP: #1808366)
    - bcm2835-gpio-exp: Driver for GPIO expander via mailbox service
    - bcm2835-gpio-exp: Copy/paste error adding base twice
    - Revert "UBUNTU: SAUCE: dts: use the virtgpio driver"
    - SAUCE: make it build on bcm2709
    - SAUCE: make pwr_led GPIO_ACTIVE_LOW
    - SAUCE: dts: cm3: revert hpd-gpios to gpio
    - SAUCE: dts: rpi3: remove hpd-gpios overwrite
    - [Config] GPIO_BCM_EXP=y

  [ Ubuntu: 4.4.0-142.168 ]

  * linux: 4.4.0-142.168 -proposed tracker (LP: #1811846)
  * Packaging resync (LP: #1786013)
    - [Packaging] update helper scripts
  * iptables connlimit allows more connections than the limit when using
    multiple CPUs (LP: #1811094)
    - netfilter: xt_connlimit: don't store address in the conn nodes
    - SAUCE: netfilter: xt_connlimit: remove the 'addr' parameter in add_hlist()
    - netfilter: nf_conncount: expose connection list interface
    - netfilter: nf_conncount: Fix garbage collection with zones
    - netfilter: nf_conncount: fix garbage collection confirm race
    - netfilter: nf_conncount: don't skip eviction when age is negative
  * CVE-2017-5715
    - SAUCE: x86/speculation: Cleanup IBPB runtime control handling
    - SAUCE: x86/speculation: Cleanup IBRS runtime control handling
    - SAUCE: x86/speculation: Use x86_spec_ctrl_base in entry/exit code
    - SAUCE: x86/speculation: Move RSB_CTXSW hunk
  * Xenial update: 4.4.167 upstream stable release (LP: #1811077)
    - media: em28xx: Fix use-after-free when disconnecting
    - Revert "wlcore: Add missing PM call for
      wlcore_cmd_wait_for_event_or_timeout()"
    - rapidio/rionet: do not free skb before reading its length
    - s390/qeth: fix length check in SNMP processing
    - usbnet: ipheth: fix potential recvmsg bug and recvmsg bug 2
    - kvm: mmu: Fix race in emulated page table writes
    - xtensa: enable coprocessors that are being flushed
    - xtensa: fix coprocessor context offset definitions
    - Btrfs: ensure path name is null terminated at btrfs_control_ioctl
    - ALSA: wss: Fix invalid snd_free_pages() at error path
    - ALSA: ac97: Fix incorrect bit shift at AC97-SPSA control write
    - ALSA: control: Fix race between adding and removing a user element
    - ALSA: sparc: Fix invalid snd_free_pages() at error path
    - ext2: fix potential use after free
    - dmaengine: at_hdmac: fix memory leak in at_dma_xlate()
    - dmaengine: at_hdmac: fix module unloading
    - btrfs: release metadata before running delayed refs
    - USB: usb-storage: Add new IDs to ums-realtek
    - usb: core: quirks: add RESET_RESUME quirk for Cherry G230 Stream series
    - misc: mic/scif: fix copy-paste error in scif_create_remote_lookup
    - Kbuild: suppress packed-not-aligned warning for default setting only
    - exec: avoid gcc-8 warning for get_task_comm
    - disable stringop truncation war...