| Message ID | 1547216273-19433-1-git-send-email-paolo.pisati@canonical.com |
|---|---|
| Headers | show |
| Series | Fix led triggers on Rpi3b+ | expand |
On 11.01.19 15:17, 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 > Since this is limited to the raspi2 flavour and by that should be testable and even if failing, loosing the status leds should be not critical (as long as the do not blow up physically). So Acked-by: Stefan Bader <stefan.bader@canonical.com>
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@canonical.com>
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 > Applied to xenial/raspi2 branch, fixing the https protocol of the BugLink's. Thanks, Kleber