ACK: [PATCH 0/8] [SRU] [X/raspi2] Fix led triggers on Rpi3b+

Fri Jan 11 16:21:54 UTC 2019

On 1/11/19 3:17 PM, Paolo Pisati wrote:
> BugLink: http://bugs.launchpad.net/bugs/1808366
>
> 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
>
> Dave Stevenson (2):
>   bcm2835-gpio-exp: Driver for GPIO expander via mailbox service
>   bcm2835-gpio-exp: Copy/paste error adding base twice
>
> Paolo Pisati (6):
>   Revert "UBUNTU: SAUCE: dts: use the virtgpio driver"
>   UBUNTU: SAUCE: make it build on bcm2709
>   UBUNTU: SAUCE: make pwr_led GPIO_ACTIVE_LOW
>   UBUNTU: SAUCE: dts: cm3: revert hpd-gpios to gpio
>   UBUNTU: SAUCE: dts: rpi3: remove hpd-gpios overwrite
>   UBUNTU: [Config] GPIO_BCM_EXP=y
>
>  arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts |   6 +-
>  arch/arm/boot/dts/bcm2710-rpi-3-b.dts      |  18 ++
>  arch/arm/configs/bcm2709_defconfig         |   1 +
>  debian.raspi2/config/config.common.ubuntu  |   1 +
>  drivers/gpio/Kconfig                       |   7 +
>  drivers/gpio/Makefile                      |   1 +
>  drivers/gpio/gpio-bcm-exp.c                | 254 +++++++++++++++++++++++++++++
>  include/soc/bcm2835/raspberrypi-firmware.h |   4 +
>  8 files changed, 289 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpio/gpio-bcm-exp.c
>
Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>