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

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Paolo Pisati-5
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

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/8] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service

Paolo Pisati-5
From: Dave Stevenson <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1808366

Pi3 and Compute Module 3 have a GPIO expander that the
VPU communicates with.
There is a mailbox service that now allows control of this
expander, so add a kernel driver that can make use of it.

Pwr_led node added to device-tree for Pi3.

Signed-off-by: Dave Stevenson <[hidden email]>
(cherry picked from commit 5d1da107bc619c1cc091ee5301f85cd89a6d92fb
https://github.com/raspberrypi/linux rpi-4.9.y)
Signed-off-by: Paolo Pisati <[hidden email]>
---
 arch/arm/boot/dts/bcm2710-rpi-3-b.dts      |  22 +++
 arch/arm/boot/dts/bcm2710-rpi-cm3.dts      |  10 +-
 arch/arm/configs/bcm2709_defconfig         |   1 +
 drivers/gpio/Kconfig                       |   7 +
 drivers/gpio/Makefile                      |   1 +
 drivers/gpio/gpio-bcm-exp.c                | 256 +++++++++++++++++++++++++++++
 include/soc/bcm2835/raspberrypi-firmware.h |   4 +
 7 files changed, 300 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpio/gpio-bcm-exp.c

diff --git a/arch/arm/boot/dts/bcm2710-rpi-3-b.dts b/arch/arm/boot/dts/bcm2710-rpi-3-b.dts
index 98352b5..193e08e 100644
--- a/arch/arm/boot/dts/bcm2710-rpi-3-b.dts
+++ b/arch/arm/boot/dts/bcm2710-rpi-3-b.dts
@@ -92,6 +92,14 @@
  firmware = <&firmware>;
  status = "okay";
  };
+
+ expgpio: expgpio {
+ compatible = "brcm,bcm2835-expgpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ firmware = <&firmware>;
+ status = "okay";
+ };
 };
 
 &fb {
@@ -164,6 +172,16 @@
  linux,default-trigger = "mmc0";
  gpios = <&virtgpio 0 0>;
  };
+
+ pwr_led: pwr {
+ label = "led1";
+ linux,default-trigger = "input";
+ gpios = <&expgpio 7 GPIO_ACTIVE_LOW>;
+ };
+};
+
+&hdmi {
+ hpd-gpios = <&expgpio 4 GPIO_ACTIVE_LOW>;
 };
 
 &audio {
@@ -196,6 +214,10 @@
  act_led_activelow = <&act_led>,"gpios:8";
  act_led_trigger = <&act_led>,"linux,default-trigger";
 
+ pwr_led_gpio = <&pwr_led>,"gpios:4";
+ pwr_led_activelow = <&pwr_led>,"gpios:8";
+ pwr_led_trigger = <&pwr_led>,"linux,default-trigger";
+
  audio = <&audio>,"status";
  watchdog = <&watchdog>,"status";
  random = <&random>,"status";
diff --git a/arch/arm/boot/dts/bcm2710-rpi-cm3.dts b/arch/arm/boot/dts/bcm2710-rpi-cm3.dts
index 7ceb16b..40f6066 100644
--- a/arch/arm/boot/dts/bcm2710-rpi-cm3.dts
+++ b/arch/arm/boot/dts/bcm2710-rpi-cm3.dts
@@ -66,6 +66,14 @@
  firmware = <&firmware>;
  status = "okay";
  };
+
+ expgpio: expgpio {
+ compatible = "brcm,bcm2835-expgpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ firmware = <&firmware>;
+ status = "okay";
+ };
 };
 
 &fb {
@@ -129,7 +137,7 @@
 };
 
 &hdmi {
- hpd-gpios = <&gpio 46 GPIO_ACTIVE_LOW>;
+ hpd-gpios = <&expgpio 0 GPIO_ACTIVE_LOW>;
 };
 
 &audio {
diff --git a/arch/arm/configs/bcm2709_defconfig b/arch/arm/configs/bcm2709_defconfig
index 783d2c2..6e7f1e6 100644
--- a/arch/arm/configs/bcm2709_defconfig
+++ b/arch/arm/configs/bcm2709_defconfig
@@ -624,6 +624,7 @@ CONFIG_PPS=m
 CONFIG_PPS_CLIENT_LDISC=m
 CONFIG_PPS_CLIENT_GPIO=m
 CONFIG_GPIO_SYSFS=y
+CONFIG_GPIO_BCM_EXP=y
 CONFIG_GPIO_BCM_VIRT=y
 CONFIG_GPIO_ARIZONA=m
 CONFIG_GPIO_STMPE=y
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 45e6729..f2eaf8b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -127,6 +127,13 @@ config GPIO_AMDPT
   driver for GPIO functionality on Promontory IOHub
   Require ACPI ASL code to enumerate as a platform device.
 
+config GPIO_BCM_EXP
+ bool "Broadcom Exp GPIO"
+ depends on OF_GPIO && RASPBERRYPI_FIRMWARE && (ARCH_BCM2835 || COMPILE_TEST)
+ help
+  Turn on GPIO support for Broadcom chips using the firmware mailbox
+  to communicate with VideoCore on BCM283x chips.
+
 config GPIO_BCM_KONA
  bool "Broadcom Kona GPIO"
  depends on OF_GPIO && (ARCH_BCM_MOBILE || COMPILE_TEST)
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index b2ccc9f..1270487 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_GPIO_AMD8111) += gpio-amd8111.o
 obj-$(CONFIG_GPIO_AMDPT) += gpio-amdpt.o
 obj-$(CONFIG_GPIO_ARIZONA) += gpio-arizona.o
 obj-$(CONFIG_ATH79) += gpio-ath79.o
+obj-$(CONFIG_GPIO_BCM_EXP) += gpio-bcm-exp.o
 obj-$(CONFIG_GPIO_BCM_KONA) += gpio-bcm-kona.o
 obj-$(CONFIG_GPIO_BCM_VIRT) += gpio-bcm-virt.o
 obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o
diff --git a/drivers/gpio/gpio-bcm-exp.c b/drivers/gpio/gpio-bcm-exp.c
new file mode 100644
index 0000000..681a914
--- /dev/null
+++ b/drivers/gpio/gpio-bcm-exp.c
@@ -0,0 +1,256 @@
+/*
+ *  Broadcom expander GPIO driver
+ *
+ *  Uses the firmware mailbox service to communicate with the
+ *  GPIO expander on the VPU.
+ *
+ *  Copyright (C) 2017 Raspberry Pi Trading Ltd.
+ *
+ *  Author: Dave Stevenson <[hidden email]>
+ *  Based on gpio-bcm-virt.c by Dom Cobley <[hidden email]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
+#define MODULE_NAME "brcmexp-gpio"
+#define NUM_GPIO 8
+
+struct brcmexp_gpio {
+ struct gpio_chip gc;
+ struct device *dev;
+ struct rpi_firmware *fw;
+};
+
+struct gpio_set_config {
+ u32 gpio, direction, polarity, term_en, term_pull_up, state;
+};
+
+struct gpio_get_config {
+ u32 gpio, direction, polarity, term_en, term_pull_up;
+};
+
+struct gpio_get_set_state {
+ u32 gpio, state;
+};
+
+static int brcmexp_gpio_get_polarity(struct gpio_chip *gc, unsigned int off)
+{
+ struct brcmexp_gpio *gpio;
+ struct gpio_get_config get;
+ int ret;
+
+ gpio = container_of(gc, struct brcmexp_gpio, gc);
+
+ get.gpio = off + gpio->gc.base; /* GPIO to update */
+
+ ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_CONFIG,
+    &get, sizeof(get));
+ if (ret) {
+ dev_err(gpio->dev,
+ "Failed to get GPIO %u config (%d)\n", off, ret);
+ return ret;
+ }
+ return get.polarity;
+}
+
+static int brcmexp_gpio_dir_in(struct gpio_chip *gc, unsigned int off)
+{
+ struct brcmexp_gpio *gpio;
+ struct gpio_set_config set_in;
+ int ret;
+
+ gpio = container_of(gc, struct brcmexp_gpio, gc);
+
+ set_in.gpio = off + gpio->gc.base; /* GPIO to update */
+ set_in.direction = 0; /* Input */
+ set_in.polarity = brcmexp_gpio_get_polarity(gc, off);
+ /* Retain existing setting */
+ set_in.term_en = 0; /* termination disabled */
+ set_in.term_pull_up = 0; /* n/a as termination disabled */
+ set_in.state = 0; /* n/a as configured as an input */
+
+ ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_SET_GPIO_CONFIG,
+    &set_in, sizeof(set_in));
+ if (ret) {
+ dev_err(gpio->dev,
+ "Failed to set GPIO %u to input (%d)\n",
+ off, ret);
+ return ret;
+ }
+ return 0;
+}
+
+static int brcmexp_gpio_dir_out(struct gpio_chip *gc, unsigned int off, int val)
+{
+ struct brcmexp_gpio *gpio;
+ struct gpio_set_config set_out;
+ int ret;
+
+ gpio = container_of(gc, struct brcmexp_gpio, gc);
+
+ set_out.gpio = off + gpio->gc.base; /* GPIO to update */
+ set_out.direction = 1; /* Output */
+ set_out.polarity = brcmexp_gpio_get_polarity(gc, off);
+ /* Retain existing setting */
+ set_out.term_en = 0; /* n/a as an output */
+ set_out.term_pull_up = 0; /* n/a as termination disabled */
+ set_out.state = val; /* Output state */
+
+ ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_SET_GPIO_CONFIG,
+    &set_out, sizeof(set_out));
+ if (ret) {
+ dev_err(gpio->dev,
+ "Failed to set GPIO %u to output (%d)\n", off, ret);
+ return ret;
+ }
+ return 0;
+}
+
+static int brcmexp_gpio_get_direction(struct gpio_chip *gc, unsigned int off)
+{
+ struct brcmexp_gpio *gpio;
+ struct gpio_get_config get;
+ int ret;
+
+ gpio = container_of(gc, struct brcmexp_gpio, gc);
+
+ get.gpio = off + gpio->gc.base; /* GPIO to update */
+
+ ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_CONFIG,
+    &get, sizeof(get));
+ if (ret) {
+ dev_err(gpio->dev,
+ "Failed to get GPIO %u config (%d)\n", off, ret);
+ return ret;
+ }
+ return get.direction ? GPIOF_DIR_OUT : GPIOF_DIR_IN;
+}
+
+static int brcmexp_gpio_get(struct gpio_chip *gc, unsigned int off)
+{
+ struct brcmexp_gpio *gpio;
+ struct gpio_get_set_state get;
+ int ret;
+
+ gpio = container_of(gc, struct brcmexp_gpio, gc);
+
+ get.gpio = off + gpio->gc.base; /* GPIO to update */
+ get.state = 0; /* storage for returned value */
+
+ ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_STATE,
+ &get, sizeof(get));
+ if (ret) {
+ dev_err(gpio->dev,
+ "Failed to get GPIO %u state (%d)\n", off, ret);
+ return ret;
+ }
+ return !!get.state;
+}
+
+static void brcmexp_gpio_set(struct gpio_chip *gc, unsigned int off, int val)
+{
+ struct brcmexp_gpio *gpio;
+ struct gpio_get_set_state set;
+ int ret;
+
+ gpio = container_of(gc, struct brcmexp_gpio, gc);
+
+ off += gpio->gc.base;
+
+ set.gpio = off + gpio->gc.base; /* GPIO to update */
+ set.state = val; /* Output state */
+
+ ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_SET_GPIO_STATE,
+ &set, sizeof(set));
+ if (ret)
+ dev_err(gpio->dev,
+ "Failed to set GPIO %u state (%d)\n", off, ret);
+}
+
+static int brcmexp_gpio_probe(struct platform_device *pdev)
+{
+ int err = 0;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct device_node *fw_node;
+ struct rpi_firmware *fw;
+ struct brcmexp_gpio *ucb;
+
+ fw_node = of_parse_phandle(np, "firmware", 0);
+ if (!fw_node) {
+ dev_err(dev, "Missing firmware node\n");
+ return -ENOENT;
+ }
+
+ fw = rpi_firmware_get(fw_node);
+ if (!fw)
+ return -EPROBE_DEFER;
+
+ ucb = devm_kzalloc(dev, sizeof(*ucb), GFP_KERNEL);
+ if (!ucb)
+ return -EINVAL;
+
+ ucb->fw = fw;
+ ucb->dev = dev;
+ ucb->gc.label = MODULE_NAME;
+ ucb->gc.owner = THIS_MODULE;
+ ucb->gc.of_node = np;
+ ucb->gc.base = 128;
+ ucb->gc.ngpio = NUM_GPIO;
+
+ ucb->gc.direction_input = brcmexp_gpio_dir_in;
+ ucb->gc.direction_output = brcmexp_gpio_dir_out;
+ ucb->gc.get_direction = brcmexp_gpio_get_direction;
+ ucb->gc.get = brcmexp_gpio_get;
+ ucb->gc.set = brcmexp_gpio_set;
+ ucb->gc.can_sleep = true;
+
+ err = gpiochip_add(&ucb->gc);
+ if (err)
+ return err;
+
+ platform_set_drvdata(pdev, ucb);
+
+ return 0;
+}
+
+static int brcmexp_gpio_remove(struct platform_device *pdev)
+{
+ struct brcmexp_gpio *ucb = platform_get_drvdata(pdev);
+
+ gpiochip_remove(&ucb->gc);
+
+ return 0;
+}
+
+static const struct of_device_id __maybe_unused brcmexp_gpio_ids[] = {
+ { .compatible = "brcm,bcm2835-expgpio" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, brcmexp_gpio_ids);
+
+static struct platform_driver brcmexp_gpio_driver = {
+ .driver = {
+ .name = MODULE_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(brcmexp_gpio_ids),
+ },
+ .probe = brcmexp_gpio_probe,
+ .remove = brcmexp_gpio_remove,
+};
+module_platform_driver(brcmexp_gpio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Dave Stevenson <[hidden email]>");
+MODULE_DESCRIPTION("brcm-exp GPIO driver");
+MODULE_ALIAS("platform:brcmexp-gpio");
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index 2859db0..56b3f0f 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -83,7 +83,11 @@ enum rpi_firmware_property_tag {
  RPI_FIRMWARE_SET_TURBO =                              0x00038009,
  RPI_FIRMWARE_SET_CUSTOMER_OTP =                       0x00038021,
  RPI_FIRMWARE_SET_DOMAIN_STATE =                       0x00038030,
+ RPI_FIRMWARE_GET_GPIO_STATE =                         0x00030041,
+ RPI_FIRMWARE_SET_GPIO_STATE =                         0x00038041,
  RPI_FIRMWARE_SET_SDHOST_CLOCK =                       0x00038042,
+ RPI_FIRMWARE_GET_GPIO_CONFIG =                        0x00030043,
+ RPI_FIRMWARE_SET_GPIO_CONFIG =                        0x00038043,
 
  /* Dispmanx TAGS */
  RPI_FIRMWARE_FRAMEBUFFER_ALLOCATE =                   0x00040001,
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/8] bcm2835-gpio-exp: Copy/paste error adding base twice

Paolo Pisati-5
In reply to this post by Paolo Pisati-5
From: Dave Stevenson <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1808366

brcmexp_gpio_set was adding gpio->gc.base to the offset
twice, so passing an invalid number to the mailbox service.
The firmware treated it modulo-8 anyway, but was logging an
assert every time.

Signed-off-by: Dave Stevenson <[hidden email]>
(cherry picked from commit 8308f3f93b7360bda893c18801e22d62c1d61378
https://github.com/raspberrypi/linux rpi-4.9.y)
Signed-off-by: Paolo Pisati <[hidden email]>
---
 drivers/gpio/gpio-bcm-exp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpio/gpio-bcm-exp.c b/drivers/gpio/gpio-bcm-exp.c
index 681a914..d68adaf 100644
--- a/drivers/gpio/gpio-bcm-exp.c
+++ b/drivers/gpio/gpio-bcm-exp.c
@@ -165,8 +165,6 @@ static void brcmexp_gpio_set(struct gpio_chip *gc, unsigned int off, int val)
 
  gpio = container_of(gc, struct brcmexp_gpio, gc);
 
- off += gpio->gc.base;
-
  set.gpio = off + gpio->gc.base; /* GPIO to update */
  set.state = val; /* Output state */
 
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/8] Revert "UBUNTU: SAUCE: dts: use the virtgpio driver"

Paolo Pisati-5
In reply to this post by Paolo Pisati-5
BugLink: http://bugs.launchpad.net/bugs/1808366

This reverts commit 0d970c30138874b61db1e89b6846e7830cdf638d.

Signed-off-by: Paolo Pisati <[hidden email]>
---
 arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts b/arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts
index f34f6b4..a4b400a 100644
--- a/arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts
+++ b/arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts
@@ -85,8 +85,8 @@
 };
 
 &soc {
- virtgpio: virtgpio {
- compatible = "brcm,bcm2835-virtgpio";
+ expgpio: expgpio {
+ compatible = "brcm,bcm2835-expgpio";
  gpio-controller;
  #gpio-cells = <2>;
  firmware = <&firmware>;
@@ -163,7 +163,7 @@
  pwr_led: pwr {
  label = "led1";
  linux,default-trigger = "default-on";
- gpios = <&virtgpio 2 0>;
+ gpios = <&expgpio 2 0>;
  };
 };
 
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 4/8] UBUNTU: SAUCE: make it build on bcm2709

Paolo Pisati-5
In reply to this post by Paolo Pisati-5
BugLink: http://bugs.launchpad.net/bugs/1808366

Signed-off-by: Paolo Pisati <[hidden email]>
---
 drivers/gpio/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f2eaf8b..1eb9032 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -129,7 +129,7 @@ config GPIO_AMDPT
 
 config GPIO_BCM_EXP
  bool "Broadcom Exp GPIO"
- depends on OF_GPIO && RASPBERRYPI_FIRMWARE && (ARCH_BCM2835 || COMPILE_TEST)
+ depends on OF_GPIO && RASPBERRYPI_FIRMWARE && (ARCH_BCM2709 || COMPILE_TEST)
  help
   Turn on GPIO support for Broadcom chips using the firmware mailbox
   to communicate with VideoCore on BCM283x chips.
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 5/8] UBUNTU: SAUCE: make pwr_led GPIO_ACTIVE_LOW

Paolo Pisati-5
In reply to this post by Paolo Pisati-5
BugLink: http://bugs.launchpad.net/bugs/1808366

Signed-off-by: Paolo Pisati <[hidden email]>
---
 arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts b/arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts
index a4b400a..9b21140 100644
--- a/arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts
+++ b/arch/arm/boot/dts/bcm2710-rpi-3-b-plus.dts
@@ -163,7 +163,7 @@
  pwr_led: pwr {
  label = "led1";
  linux,default-trigger = "default-on";
- gpios = <&expgpio 2 0>;
+ gpios = <&expgpio 2 1>;
  };
 };
 
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 6/8] UBUNTU: SAUCE: dts: cm3: revert hpd-gpios to gpio

Paolo Pisati-5
In reply to this post by Paolo Pisati-5
BugLink: http://bugs.launchpad.net/bugs/1808366

Signed-off-by: Paolo Pisati <[hidden email]>
---
 arch/arm/boot/dts/bcm2710-rpi-cm3.dts | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/arch/arm/boot/dts/bcm2710-rpi-cm3.dts b/arch/arm/boot/dts/bcm2710-rpi-cm3.dts
index 40f6066..7ceb16b 100644
--- a/arch/arm/boot/dts/bcm2710-rpi-cm3.dts
+++ b/arch/arm/boot/dts/bcm2710-rpi-cm3.dts
@@ -66,14 +66,6 @@
  firmware = <&firmware>;
  status = "okay";
  };
-
- expgpio: expgpio {
- compatible = "brcm,bcm2835-expgpio";
- gpio-controller;
- #gpio-cells = <2>;
- firmware = <&firmware>;
- status = "okay";
- };
 };
 
 &fb {
@@ -137,7 +129,7 @@
 };
 
 &hdmi {
- hpd-gpios = <&expgpio 0 GPIO_ACTIVE_LOW>;
+ hpd-gpios = <&gpio 46 GPIO_ACTIVE_LOW>;
 };
 
 &audio {
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 7/8] UBUNTU: SAUCE: dts: rpi3: remove hpd-gpios overwrite

Paolo Pisati-5
In reply to this post by Paolo Pisati-5
BugLink: http://bugs.launchpad.net/bugs/1808366

Signed-off-by: Paolo Pisati <[hidden email]>
---
 arch/arm/boot/dts/bcm2710-rpi-3-b.dts | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/arm/boot/dts/bcm2710-rpi-3-b.dts b/arch/arm/boot/dts/bcm2710-rpi-3-b.dts
index 193e08e..7abeee6 100644
--- a/arch/arm/boot/dts/bcm2710-rpi-3-b.dts
+++ b/arch/arm/boot/dts/bcm2710-rpi-3-b.dts
@@ -180,10 +180,6 @@
  };
 };
 
-&hdmi {
- hpd-gpios = <&expgpio 4 GPIO_ACTIVE_LOW>;
-};
-
 &audio {
  pinctrl-names = "default";
  pinctrl-0 = <&audio_pins>;
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 8/8] UBUNTU: [Config] GPIO_BCM_EXP=y

Paolo Pisati-5
In reply to this post by Paolo Pisati-5
BugLink: http://bugs.launchpad.net/bugs/1808366

Signed-off-by: Paolo Pisati <[hidden email]>
---
 debian.raspi2/config/config.common.ubuntu | 1 +
 1 file changed, 1 insertion(+)

diff --git a/debian.raspi2/config/config.common.ubuntu b/debian.raspi2/config/config.common.ubuntu
index 215e2bb..05f4da9 100644
--- a/debian.raspi2/config/config.common.ubuntu
+++ b/debian.raspi2/config/config.common.ubuntu
@@ -1650,6 +1650,7 @@ CONFIG_GPIO_ADP5520=m
 CONFIG_GPIO_ADP5588=m
 CONFIG_GPIO_ALTERA=m
 CONFIG_GPIO_ARIZONA=m
+CONFIG_GPIO_BCM_EXP=y
 CONFIG_GPIO_BCM_VIRT=y
 CONFIG_GPIO_CRYSTAL_COVE=m
 CONFIG_GPIO_DA9052=m
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Stefan Bader-2
In reply to this post by Paolo Pisati-5
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 <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Kleber Souza
In reply to this post by Paolo Pisati-5
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 <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/8] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service

Kleber Souza
In reply to this post by Paolo Pisati-5
On 1/11/19 3:17 PM, Paolo Pisati wrote:
> From: Dave Stevenson <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1808366

We are now using https for the BugLink's, that can be fixed when
applying though.


Kleber

>
> Pi3 and Compute Module 3 have a GPIO expander that the
> VPU communicates with.
> There is a mailbox service that now allows control of this
> expander, so add a kernel driver that can make use of it.
>
> Pwr_led node added to device-tree for Pi3.
>
> Signed-off-by: Dave Stevenson <[hidden email]>
> (cherry picked from commit 5d1da107bc619c1cc091ee5301f85cd89a6d92fb
> https://github.com/raspberrypi/linux rpi-4.9.y)
> Signed-off-by: Paolo Pisati <[hidden email]>
> ---
>  arch/arm/boot/dts/bcm2710-rpi-3-b.dts      |  22 +++
>  arch/arm/boot/dts/bcm2710-rpi-cm3.dts      |  10 +-
>  arch/arm/configs/bcm2709_defconfig         |   1 +
>  drivers/gpio/Kconfig                       |   7 +
>  drivers/gpio/Makefile                      |   1 +
>  drivers/gpio/gpio-bcm-exp.c                | 256 +++++++++++++++++++++++++++++
>  include/soc/bcm2835/raspberrypi-firmware.h |   4 +
>  7 files changed, 300 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpio/gpio-bcm-exp.c
>
> diff --git a/arch/arm/boot/dts/bcm2710-rpi-3-b.dts b/arch/arm/boot/dts/bcm2710-rpi-3-b.dts
> index 98352b5..193e08e 100644
> --- a/arch/arm/boot/dts/bcm2710-rpi-3-b.dts
> +++ b/arch/arm/boot/dts/bcm2710-rpi-3-b.dts
> @@ -92,6 +92,14 @@
>   firmware = <&firmware>;
>   status = "okay";
>   };
> +
> + expgpio: expgpio {
> + compatible = "brcm,bcm2835-expgpio";
> + gpio-controller;
> + #gpio-cells = <2>;
> + firmware = <&firmware>;
> + status = "okay";
> + };
>  };
>  
>  &fb {
> @@ -164,6 +172,16 @@
>   linux,default-trigger = "mmc0";
>   gpios = <&virtgpio 0 0>;
>   };
> +
> + pwr_led: pwr {
> + label = "led1";
> + linux,default-trigger = "input";
> + gpios = <&expgpio 7 GPIO_ACTIVE_LOW>;
> + };
> +};
> +
> +&hdmi {
> + hpd-gpios = <&expgpio 4 GPIO_ACTIVE_LOW>;
>  };
>  
>  &audio {
> @@ -196,6 +214,10 @@
>   act_led_activelow = <&act_led>,"gpios:8";
>   act_led_trigger = <&act_led>,"linux,default-trigger";
>  
> + pwr_led_gpio = <&pwr_led>,"gpios:4";
> + pwr_led_activelow = <&pwr_led>,"gpios:8";
> + pwr_led_trigger = <&pwr_led>,"linux,default-trigger";
> +
>   audio = <&audio>,"status";
>   watchdog = <&watchdog>,"status";
>   random = <&random>,"status";
> diff --git a/arch/arm/boot/dts/bcm2710-rpi-cm3.dts b/arch/arm/boot/dts/bcm2710-rpi-cm3.dts
> index 7ceb16b..40f6066 100644
> --- a/arch/arm/boot/dts/bcm2710-rpi-cm3.dts
> +++ b/arch/arm/boot/dts/bcm2710-rpi-cm3.dts
> @@ -66,6 +66,14 @@
>   firmware = <&firmware>;
>   status = "okay";
>   };
> +
> + expgpio: expgpio {
> + compatible = "brcm,bcm2835-expgpio";
> + gpio-controller;
> + #gpio-cells = <2>;
> + firmware = <&firmware>;
> + status = "okay";
> + };
>  };
>  
>  &fb {
> @@ -129,7 +137,7 @@
>  };
>  
>  &hdmi {
> - hpd-gpios = <&gpio 46 GPIO_ACTIVE_LOW>;
> + hpd-gpios = <&expgpio 0 GPIO_ACTIVE_LOW>;
>  };
>  
>  &audio {
> diff --git a/arch/arm/configs/bcm2709_defconfig b/arch/arm/configs/bcm2709_defconfig
> index 783d2c2..6e7f1e6 100644
> --- a/arch/arm/configs/bcm2709_defconfig
> +++ b/arch/arm/configs/bcm2709_defconfig
> @@ -624,6 +624,7 @@ CONFIG_PPS=m
>  CONFIG_PPS_CLIENT_LDISC=m
>  CONFIG_PPS_CLIENT_GPIO=m
>  CONFIG_GPIO_SYSFS=y
> +CONFIG_GPIO_BCM_EXP=y
>  CONFIG_GPIO_BCM_VIRT=y
>  CONFIG_GPIO_ARIZONA=m
>  CONFIG_GPIO_STMPE=y
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 45e6729..f2eaf8b 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -127,6 +127,13 @@ config GPIO_AMDPT
>    driver for GPIO functionality on Promontory IOHub
>    Require ACPI ASL code to enumerate as a platform device.
>  
> +config GPIO_BCM_EXP
> + bool "Broadcom Exp GPIO"
> + depends on OF_GPIO && RASPBERRYPI_FIRMWARE && (ARCH_BCM2835 || COMPILE_TEST)
> + help
> +  Turn on GPIO support for Broadcom chips using the firmware mailbox
> +  to communicate with VideoCore on BCM283x chips.
> +
>  config GPIO_BCM_KONA
>   bool "Broadcom Kona GPIO"
>   depends on OF_GPIO && (ARCH_BCM_MOBILE || COMPILE_TEST)
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index b2ccc9f..1270487 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_GPIO_AMD8111) += gpio-amd8111.o
>  obj-$(CONFIG_GPIO_AMDPT) += gpio-amdpt.o
>  obj-$(CONFIG_GPIO_ARIZONA) += gpio-arizona.o
>  obj-$(CONFIG_ATH79) += gpio-ath79.o
> +obj-$(CONFIG_GPIO_BCM_EXP) += gpio-bcm-exp.o
>  obj-$(CONFIG_GPIO_BCM_KONA) += gpio-bcm-kona.o
>  obj-$(CONFIG_GPIO_BCM_VIRT) += gpio-bcm-virt.o
>  obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o
> diff --git a/drivers/gpio/gpio-bcm-exp.c b/drivers/gpio/gpio-bcm-exp.c
> new file mode 100644
> index 0000000..681a914
> --- /dev/null
> +++ b/drivers/gpio/gpio-bcm-exp.c
> @@ -0,0 +1,256 @@
> +/*
> + *  Broadcom expander GPIO driver
> + *
> + *  Uses the firmware mailbox service to communicate with the
> + *  GPIO expander on the VPU.
> + *
> + *  Copyright (C) 2017 Raspberry Pi Trading Ltd.
> + *
> + *  Author: Dave Stevenson <[hidden email]>
> + *  Based on gpio-bcm-virt.c by Dom Cobley <[hidden email]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
> +#include <soc/bcm2835/raspberrypi-firmware.h>
> +
> +#define MODULE_NAME "brcmexp-gpio"
> +#define NUM_GPIO 8
> +
> +struct brcmexp_gpio {
> + struct gpio_chip gc;
> + struct device *dev;
> + struct rpi_firmware *fw;
> +};
> +
> +struct gpio_set_config {
> + u32 gpio, direction, polarity, term_en, term_pull_up, state;
> +};
> +
> +struct gpio_get_config {
> + u32 gpio, direction, polarity, term_en, term_pull_up;
> +};
> +
> +struct gpio_get_set_state {
> + u32 gpio, state;
> +};
> +
> +static int brcmexp_gpio_get_polarity(struct gpio_chip *gc, unsigned int off)
> +{
> + struct brcmexp_gpio *gpio;
> + struct gpio_get_config get;
> + int ret;
> +
> + gpio = container_of(gc, struct brcmexp_gpio, gc);
> +
> + get.gpio = off + gpio->gc.base; /* GPIO to update */
> +
> + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_CONFIG,
> +    &get, sizeof(get));
> + if (ret) {
> + dev_err(gpio->dev,
> + "Failed to get GPIO %u config (%d)\n", off, ret);
> + return ret;
> + }
> + return get.polarity;
> +}
> +
> +static int brcmexp_gpio_dir_in(struct gpio_chip *gc, unsigned int off)
> +{
> + struct brcmexp_gpio *gpio;
> + struct gpio_set_config set_in;
> + int ret;
> +
> + gpio = container_of(gc, struct brcmexp_gpio, gc);
> +
> + set_in.gpio = off + gpio->gc.base; /* GPIO to update */
> + set_in.direction = 0; /* Input */
> + set_in.polarity = brcmexp_gpio_get_polarity(gc, off);
> + /* Retain existing setting */
> + set_in.term_en = 0; /* termination disabled */
> + set_in.term_pull_up = 0; /* n/a as termination disabled */
> + set_in.state = 0; /* n/a as configured as an input */
> +
> + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_SET_GPIO_CONFIG,
> +    &set_in, sizeof(set_in));
> + if (ret) {
> + dev_err(gpio->dev,
> + "Failed to set GPIO %u to input (%d)\n",
> + off, ret);
> + return ret;
> + }
> + return 0;
> +}
> +
> +static int brcmexp_gpio_dir_out(struct gpio_chip *gc, unsigned int off, int val)
> +{
> + struct brcmexp_gpio *gpio;
> + struct gpio_set_config set_out;
> + int ret;
> +
> + gpio = container_of(gc, struct brcmexp_gpio, gc);
> +
> + set_out.gpio = off + gpio->gc.base; /* GPIO to update */
> + set_out.direction = 1; /* Output */
> + set_out.polarity = brcmexp_gpio_get_polarity(gc, off);
> + /* Retain existing setting */
> + set_out.term_en = 0; /* n/a as an output */
> + set_out.term_pull_up = 0; /* n/a as termination disabled */
> + set_out.state = val; /* Output state */
> +
> + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_SET_GPIO_CONFIG,
> +    &set_out, sizeof(set_out));
> + if (ret) {
> + dev_err(gpio->dev,
> + "Failed to set GPIO %u to output (%d)\n", off, ret);
> + return ret;
> + }
> + return 0;
> +}
> +
> +static int brcmexp_gpio_get_direction(struct gpio_chip *gc, unsigned int off)
> +{
> + struct brcmexp_gpio *gpio;
> + struct gpio_get_config get;
> + int ret;
> +
> + gpio = container_of(gc, struct brcmexp_gpio, gc);
> +
> + get.gpio = off + gpio->gc.base; /* GPIO to update */
> +
> + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_CONFIG,
> +    &get, sizeof(get));
> + if (ret) {
> + dev_err(gpio->dev,
> + "Failed to get GPIO %u config (%d)\n", off, ret);
> + return ret;
> + }
> + return get.direction ? GPIOF_DIR_OUT : GPIOF_DIR_IN;
> +}
> +
> +static int brcmexp_gpio_get(struct gpio_chip *gc, unsigned int off)
> +{
> + struct brcmexp_gpio *gpio;
> + struct gpio_get_set_state get;
> + int ret;
> +
> + gpio = container_of(gc, struct brcmexp_gpio, gc);
> +
> + get.gpio = off + gpio->gc.base; /* GPIO to update */
> + get.state = 0; /* storage for returned value */
> +
> + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_STATE,
> + &get, sizeof(get));
> + if (ret) {
> + dev_err(gpio->dev,
> + "Failed to get GPIO %u state (%d)\n", off, ret);
> + return ret;
> + }
> + return !!get.state;
> +}
> +
> +static void brcmexp_gpio_set(struct gpio_chip *gc, unsigned int off, int val)
> +{
> + struct brcmexp_gpio *gpio;
> + struct gpio_get_set_state set;
> + int ret;
> +
> + gpio = container_of(gc, struct brcmexp_gpio, gc);
> +
> + off += gpio->gc.base;
> +
> + set.gpio = off + gpio->gc.base; /* GPIO to update */
> + set.state = val; /* Output state */
> +
> + ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_SET_GPIO_STATE,
> + &set, sizeof(set));
> + if (ret)
> + dev_err(gpio->dev,
> + "Failed to set GPIO %u state (%d)\n", off, ret);
> +}
> +
> +static int brcmexp_gpio_probe(struct platform_device *pdev)
> +{
> + int err = 0;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct device_node *fw_node;
> + struct rpi_firmware *fw;
> + struct brcmexp_gpio *ucb;
> +
> + fw_node = of_parse_phandle(np, "firmware", 0);
> + if (!fw_node) {
> + dev_err(dev, "Missing firmware node\n");
> + return -ENOENT;
> + }
> +
> + fw = rpi_firmware_get(fw_node);
> + if (!fw)
> + return -EPROBE_DEFER;
> +
> + ucb = devm_kzalloc(dev, sizeof(*ucb), GFP_KERNEL);
> + if (!ucb)
> + return -EINVAL;
> +
> + ucb->fw = fw;
> + ucb->dev = dev;
> + ucb->gc.label = MODULE_NAME;
> + ucb->gc.owner = THIS_MODULE;
> + ucb->gc.of_node = np;
> + ucb->gc.base = 128;
> + ucb->gc.ngpio = NUM_GPIO;
> +
> + ucb->gc.direction_input = brcmexp_gpio_dir_in;
> + ucb->gc.direction_output = brcmexp_gpio_dir_out;
> + ucb->gc.get_direction = brcmexp_gpio_get_direction;
> + ucb->gc.get = brcmexp_gpio_get;
> + ucb->gc.set = brcmexp_gpio_set;
> + ucb->gc.can_sleep = true;
> +
> + err = gpiochip_add(&ucb->gc);
> + if (err)
> + return err;
> +
> + platform_set_drvdata(pdev, ucb);
> +
> + return 0;
> +}
> +
> +static int brcmexp_gpio_remove(struct platform_device *pdev)
> +{
> + struct brcmexp_gpio *ucb = platform_get_drvdata(pdev);
> +
> + gpiochip_remove(&ucb->gc);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id __maybe_unused brcmexp_gpio_ids[] = {
> + { .compatible = "brcm,bcm2835-expgpio" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, brcmexp_gpio_ids);
> +
> +static struct platform_driver brcmexp_gpio_driver = {
> + .driver = {
> + .name = MODULE_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(brcmexp_gpio_ids),
> + },
> + .probe = brcmexp_gpio_probe,
> + .remove = brcmexp_gpio_remove,
> +};
> +module_platform_driver(brcmexp_gpio_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Dave Stevenson <[hidden email]>");
> +MODULE_DESCRIPTION("brcm-exp GPIO driver");
> +MODULE_ALIAS("platform:brcmexp-gpio");
> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
> index 2859db0..56b3f0f 100644
> --- a/include/soc/bcm2835/raspberrypi-firmware.h
> +++ b/include/soc/bcm2835/raspberrypi-firmware.h
> @@ -83,7 +83,11 @@ enum rpi_firmware_property_tag {
>   RPI_FIRMWARE_SET_TURBO =                              0x00038009,
>   RPI_FIRMWARE_SET_CUSTOMER_OTP =                       0x00038021,
>   RPI_FIRMWARE_SET_DOMAIN_STATE =                       0x00038030,
> + RPI_FIRMWARE_GET_GPIO_STATE =                         0x00030041,
> + RPI_FIRMWARE_SET_GPIO_STATE =                         0x00038041,
>   RPI_FIRMWARE_SET_SDHOST_CLOCK =                       0x00038042,
> + RPI_FIRMWARE_GET_GPIO_CONFIG =                        0x00030043,
> + RPI_FIRMWARE_SET_GPIO_CONFIG =                        0x00038043,
>  
>   /* Dispmanx TAGS */
>   RPI_FIRMWARE_FRAMEBUFFER_ALLOCATE =                   0x00040001,



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Kleber Souza
In reply to this post by Paolo Pisati-5
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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team