gpio: driver for the RPi3 GPIO expander

Baruch Siach <[email protected]> hat am 11. Januar 2018 um 20:44 geschrieben:

Add latest firmware property tags from the latest Raspberry Pi downstream kernel. This is needed for the GPIO tags, so we can control the GPIO multiplexor lines.

Signed-off-by Baruch Siach <[email protected]>

Quoted 7 lines

Baruch Siach <[email protected]> hat am 11. Januar 2018 um 20:44 geschrieben:

Add latest firmware property tags from the latest Raspberry Pi downstream kernel. This is needed for the GPIO tags, so we can control the GPIO multiplexor lines.

Quoted 1 line

Signed-off-by Baruch Siach <[email protected]>

Acked-by Stefan Wahren <[email protected]>

Maybe this is obvious, but patch #3 depends on this change so no cherry-picking allowed.

Quoted 10 lines · Stefan Wahren

Quoted 7 lines

Baruch Siach <[email protected]> hat am 11. Januar 2018 um 20:44 geschrieben:

Add latest firmware property tags from the latest Raspberry Pi downstream kernel. This is needed for the GPIO tags, so we can control the GPIO multiplexor lines.

Signed-off-by Baruch Siach <[email protected]>

Acked-by Stefan Wahren <[email protected]>

Quoted 1 line

Maybe this is obvious, but patch #3 depends on this change so no cherry-picking allowed.

I will save this for after the merge window as it feels now.

But as far as I have the right ACKs I can put it in the GPIO tree.

Baruch: can you resend the series with all ACKs and push me to apply it all in the GPIO tree if you're fine with this approach?

Hi Linus,

Quoted 15 lines · Linus Walleij

Quoted 10 lines · Stefan Wahren

Quoted 7 lines

Baruch Siach <[email protected]> hat am 11. Januar 2018 um 20:44 geschrieben:

Add latest firmware property tags from the latest Raspberry Pi downstream kernel. This is needed for the GPIO tags, so we can control the GPIO multiplexor lines.

Signed-off-by Baruch Siach <[email protected]>

Acked-by Stefan Wahren <[email protected]>

Maybe this is obvious, but patch #3 depends on this change so no cherry-picking allowed.

I will save this for after the merge window as it feels now.

But as far as I have the right ACKs I can put it in the GPIO tree.

Quoted 2 lines

Baruch: can you resend the series with all ACKs and push me to apply it all in the GPIO tree if you're fine with this approach?

I plan to send another revision of this series later today to address Stefan's comments on this one. Once Stefan and/or Eric ack the entire series, I'm fine with you taking it through the GPIO tree if it is OK with the BCM2835 maintainers.

baruch

Hi Baruch,

Am 16.01.2018 um 11:49 schrieb Baruch Siach:

Quoted 25 lines

Hi Linus,

Quoted 10 lines · Linus Walleij

Quoted 9 lines · Stefan Wahren

Quoted 7 lines

Baruch Siach <[email protected]> hat am 11. Januar 2018 um 20:44 geschrieben:

Add latest firmware property tags from the latest Raspberry Pi downstream kernel. This is needed for the GPIO tags, so we can control the GPIO multiplexor lines.

Signed-off-by Baruch Siach <[email protected]>

Acked-by Stefan Wahren <[email protected]>

Quoted 7 lines

Quoted 1 line

Maybe this is obvious, but patch #3 depends on this change so no cherry-picking allowed.

I will save this for after the merge window as it feels now.

But as far as I have the right ACKs I can put it in the GPIO tree.

Baruch: can you resend the series with all ACKs and push me to apply it all in the GPIO tree if you're fine with this approach?

I plan to send another revision of this series later today to address Stefan's comments on this one. Once Stefan and/or Eric ack the entire series, I'm fine with you taking it through the GPIO tree if it is OK with the BCM2835 maintainers.

Quoted 2 lines

baruch

we also need an ACK from the devicetree maintainers. So please send a copy of the binding patch V3 to Rob Herring and Marc Rutland.

Baruch Siach <[email protected]> hat am 11. Januar 2018 um 20:44 geschrieben:

The Raspberry Pi 3 GPIO expander is controlled by the VC4 firmware over I2C. The firmware mailbox interface allows the ARM core to control the GPIO lines.

Signed-off-by Baruch Siach <[email protected]>

---
v2:
* Rename compatible string to raspberrypi,firmware-gpio

---
 .../bindings/gpio/raspberrypi,firmware-gpio.txt    | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt

Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt new file +9 -0

new file mode 100644
index 000000000000..8faf269cf541
@@ -0,0 +1,24 @@
+Raspberry Pi GPIO expander
+
+The Raspberry Pi 3 GPIO expander is controlled by the VC4 firmware. The
+firmware exposes a mailbox interface that allows the ARM core to control the
+GPIO lines on the expander.
+
+Required properties:
+
+- compatible : Should be "raspberrypi,firmware-gpio"

Baruch Siach <[email protected]> hat am 11. Januar 2018 um 20:44 geschrieben:

From: Dave Stevenson <[email protected]>

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.

Signed-off-by Dave Stevenson <[email protected]>

Signed-off-by Baruch Siach <[email protected]>

---
v2:
* Rename driver to gpio-raspberrypi-exp
* Populate the gpiochip parent device pointer
* Use macro for the mailbox base GPIO number
* Drop linux/gpio.h and GPIOF_DIR_*
* Check and print firmware error value
* Use devm_gpiochip_add_data(); drop .remove
* A few more minor tweaks

---
 drivers/gpio/Kconfig                |   9 ++
 drivers/gpio/Makefile               |   1 +
 drivers/gpio/gpio-raspberrypi-exp.c | 258 ++++++++++++++++++++++++++++++++++++
 3 files changed, 268 insertions(+)
 create mode 100644 drivers/gpio/gpio-raspberrypi-exp.c

drivers/gpio/Kconfig +0 -0

index d6a8e851ad13..8df901c75bef 100644

drivers/gpio/Kconfig +4 -0

+	help
+	  Turn on GPIO support for the expander on Raspberry Pi 3 boards, 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)

drivers/gpio/Makefile +1 -0

index 4bc24febb889..71021b0e71da 100644
@@ -33,6 +33,7 @@ obj-$(CONFIG_GPIO_ARIZONA)	+= gpio-arizona.o
 obj-$(CONFIG_GPIO_ATH79)	+= gpio-ath79.o
 obj-$(CONFIG_GPIO_ASPEED)	+= gpio-aspeed.o
 obj-$(CONFIG_GPIO_AXP209)	+= gpio-axp209.o
+obj-$(CONFIG_GPIO_RASPBERRYPI_EXP)	+= gpio-raspberrypi-exp.o
 obj-$(CONFIG_GPIO_BCM_KONA)	+= gpio-bcm-kona.o
 obj-$(CONFIG_GPIO_BD9571MWV)	+= gpio-bd9571mwv.o
 obj-$(CONFIG_GPIO_BRCMSTB)	+= gpio-brcmstb.o

drivers/gpio/gpio-raspberrypi-exp.c new file +0 -0

new file mode 100644
index 000000000000..95b1957a69fa

drivers/gpio/gpio-raspberrypi-exp.c new file +45 -0

+#include <soc/bcm2835/raspberrypi-firmware.h>
+
+#define MODULE_NAME "raspberrypi-exp-gpio"
+#define NUM_GPIO 8
+
+#define RPI_EXP_GPIO_BASE	128
+
+#define RPI_EXP_GPIO_DIR_IN	0
+#define RPI_EXP_GPIO_DIR_OUT	1
+
+struct rpi_exp_gpio {
+	struct gpio_chip gc;
+	struct rpi_firmware *fw;
+};
+
+/* VC4 firmware mailbox interface data structures */
+
+struct gpio_set_config {
+	u32 gpio;
+	u32 direction;
+	u32 polarity;
+	u32 term_en;
+	u32 term_pull_up;
+	u32 state;
+};
+
+struct gpio_get_config {
+	u32 gpio;
+	u32 direction;
+	u32 polarity;
+	u32 term_en;
+	u32 term_pull_up;
+};
+
+struct gpio_get_set_state {
+	u32 gpio;
+	u32 state;
+};
+
+static int rpi_exp_gpio_get_polarity(struct gpio_chip *gc, unsigned int off)
+{
+	struct rpi_exp_gpio *gpio;
+	struct gpio_get_config get;
+	int ret;
+

drivers/gpio/gpio-raspberrypi-exp.c new file +6 -0

+		return ret ? ret : -EIO;
+	}
+	return get.polarity;
+}
+
+static int rpi_exp_gpio_dir_in(struct gpio_chip *gc, unsigned int off)

drivers/gpio/gpio-raspberrypi-exp.c new file +6 -0

+		return ret ? ret : -EIO;
+	}
+	return 0;
+}
+
+static int rpi_exp_gpio_dir_out(struct gpio_chip *gc, unsigned int off, int val)

drivers/gpio/gpio-raspberrypi-exp.c new file +111 -0

+					/* 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 || set_out.gpio != 0) {
+		dev_err(gc->parent,
+			"Failed to set GPIO %u to output (%d %x)\n", off, ret,
+			set_out.gpio);
+		return ret ? ret : -EIO;
+	}
+	return 0;
+}
+
+static int rpi_exp_gpio_get_direction(struct gpio_chip *gc, unsigned int off)
+{
+	struct rpi_exp_gpio *gpio;
+	struct gpio_get_config get;
+	int ret;
+
+	gpio = gpiochip_get_data(gc);
+
+	get.gpio = off + RPI_EXP_GPIO_BASE;	/* GPIO to update */
+
+	ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_CONFIG,
+				    &get, sizeof(get));
+	if (ret || get.gpio != 0) {
+		dev_err(gc->parent,
+			"Failed to get GPIO %u config (%d %x)\n", off, ret,
+			get.gpio);
+		return ret ? ret : -EIO;
+	}
+	return !get.direction;
+}
+
+static int rpi_exp_gpio_get(struct gpio_chip *gc, unsigned int off)
+{
+	struct rpi_exp_gpio *gpio;
+	struct gpio_get_set_state get;
+	int ret;
+
+	gpio = gpiochip_get_data(gc);
+
+	get.gpio = off + RPI_EXP_GPIO_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 || get.gpio != 0) {
+		dev_err(gc->parent,
+			"Failed to get GPIO %u state (%d %x)\n", off, ret,
+			get.gpio);
+		return ret ? ret : -EIO;
+	}
+	return !!get.state;
+}
+
+static void rpi_exp_gpio_set(struct gpio_chip *gc, unsigned int off, int val)
+{
+	struct rpi_exp_gpio *gpio;
+	struct gpio_get_set_state set;
+	int ret;
+
+	gpio = gpiochip_get_data(gc);
+
+	set.gpio = off + RPI_EXP_GPIO_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 || set.gpio != 0)
+		dev_err(gc->parent,
+			"Failed to set GPIO %u state (%d %x)\n", off, ret,
+			set.gpio);
+}
+
+static int rpi_exp_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 rpi_exp_gpio *rpi_gpio;
+
+	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;
+
+	rpi_gpio = devm_kzalloc(dev, sizeof(*rpi_gpio), GFP_KERNEL);
+	if (!rpi_gpio)
+		return -ENOMEM;
+
+	rpi_gpio->fw = fw;
+	rpi_gpio->gc.parent = dev;
+	rpi_gpio->gc.label = MODULE_NAME;
+	rpi_gpio->gc.owner = THIS_MODULE;
+	rpi_gpio->gc.of_node = np;
+	rpi_gpio->gc.base = -1;
+	rpi_gpio->gc.ngpio = NUM_GPIO;
+
+	rpi_gpio->gc.direction_input = rpi_exp_gpio_dir_in;
+	rpi_gpio->gc.direction_output = rpi_exp_gpio_dir_out;

drivers/gpio/gpio-raspberrypi-exp.c new file +4 -0

+
+	return 0;
+}
+

Hi Baruch,

Quoted 36 lines

Baruch Siach <[email protected]> hat am 11. Januar 2018 um 20:44 geschrieben:

From: Dave Stevenson <[email protected]>

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.

Signed-off-by Dave Stevenson <[email protected]>

Signed-off-by Baruch Siach <[email protected]>

---
v2:
* Rename driver to gpio-raspberrypi-exp
* Populate the gpiochip parent device pointer
* Use macro for the mailbox base GPIO number
* Drop linux/gpio.h and GPIOF_DIR_*
* Check and print firmware error value
* Use devm_gpiochip_add_data(); drop .remove
* A few more minor tweaks

---
 drivers/gpio/Kconfig                |   9 ++
 drivers/gpio/Makefile               |   1 +
 drivers/gpio/gpio-raspberrypi-exp.c | 258 ++++++++++++++++++++++++++++++++++++
 3 files changed, 268 insertions(+)
 create mode 100644 drivers/gpio/gpio-raspberrypi-exp.c

drivers/gpio/Kconfig +0 -0

index d6a8e851ad13..8df901c75bef 100644
@@ -128,6 +128,15 @@ config GPIO_AXP209
      help
        Say yes to enable GPIO support for the AXP209 PMIC
 

drivers/gpio/Kconfig config GPIO_AXP209 · line 132

Quoted 2 lines

drivers/gpio/Kconfig +2 -0

+config GPIO_RASPBERRYPI_EXP
+     bool "RaspberryPi 3 Exp GPIO"

How about "RaspberryPi 3 GPIO Expander" ?

Why not tristate? On arm64 it's preferred to have a kernel module.

drivers/gpio/Kconfig config GPIO_AXP209 · line 135

Quoted 3 lines

drivers/gpio/Kconfig +3 -0

+     default RASPBERRYPI_FIRMWARE
+     depends on OF_GPIO && RASPBERRYPI_FIRMWARE && \
+             (ARCH_BCM2835 || COMPILE_TEST)

Since this is default on RASPBERRYPI_FIRMWARE, we could remove it from the dependencies.

Quoted 25 lines

drivers/gpio/Kconfig +4 -0

+     help
+       Turn on GPIO support for the expander on Raspberry Pi 3 boards, 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)

drivers/gpio/Makefile +1 -0

index 4bc24febb889..71021b0e71da 100644
@@ -33,6 +33,7 @@ obj-$(CONFIG_GPIO_ARIZONA)  += gpio-arizona.o
 obj-$(CONFIG_GPIO_ATH79)     += gpio-ath79.o
 obj-$(CONFIG_GPIO_ASPEED)    += gpio-aspeed.o
 obj-$(CONFIG_GPIO_AXP209)    += gpio-axp209.o
+obj-$(CONFIG_GPIO_RASPBERRYPI_EXP)   += gpio-raspberrypi-exp.o
 obj-$(CONFIG_GPIO_BCM_KONA)  += gpio-bcm-kona.o
 obj-$(CONFIG_GPIO_BD9571MWV) += gpio-bd9571mwv.o
 obj-$(CONFIG_GPIO_BRCMSTB)   += gpio-brcmstb.o

drivers/gpio/gpio-raspberrypi-exp.c new file +0 -0

new file mode 100644
index 000000000000..95b1957a69fa

drivers/gpio/gpio-raspberrypi-exp.c line 8

Quoted 8 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +8 -0

@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Raspberry Pi 3 expander GPIO driver
+ *
+ *  Uses the firmware mailbox service to communicate with the
+ *  GPIO expander on the VPU.
+ *
+ *  Copyright (C) 2017 Raspberry Pi Trading Ltd.

2018?

drivers/gpio/gpio-raspberrypi-exp.c line 15

Quoted 7 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +7 -0

+ */
+
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>

I can't see any DMA handling, so please remove it.

Quoted 45 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +45 -0

+#include <soc/bcm2835/raspberrypi-firmware.h>
+
+#define MODULE_NAME "raspberrypi-exp-gpio"
+#define NUM_GPIO 8
+
+#define RPI_EXP_GPIO_BASE    128
+
+#define RPI_EXP_GPIO_DIR_IN  0
+#define RPI_EXP_GPIO_DIR_OUT 1
+
+struct rpi_exp_gpio {
+     struct gpio_chip gc;
+     struct rpi_firmware *fw;
+};
+
+/* VC4 firmware mailbox interface data structures */
+
+struct gpio_set_config {
+     u32 gpio;
+     u32 direction;
+     u32 polarity;
+     u32 term_en;
+     u32 term_pull_up;
+     u32 state;
+};
+
+struct gpio_get_config {
+     u32 gpio;
+     u32 direction;
+     u32 polarity;
+     u32 term_en;
+     u32 term_pull_up;
+};
+
+struct gpio_get_set_state {
+     u32 gpio;
+     u32 state;
+};
+
+static int rpi_exp_gpio_get_polarity(struct gpio_chip *gc, unsigned int off)
+{
+     struct rpi_exp_gpio *gpio;
+     struct gpio_get_config get;
+     int ret;
+

drivers/gpio/gpio-raspberrypi-exp.c line 70

Quoted 10 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +10 -0

+     gpio = gpiochip_get_data(gc);
+
+     get.gpio = off + RPI_EXP_GPIO_BASE;     /* GPIO to update */
+
+     ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_CONFIG,
+                                 &get, sizeof(get));
+     if (ret || get.gpio != 0) {
+             dev_err(gc->parent,
+                     "Failed to get GPIO %u config (%d %x)\n", off, ret,
+                     get.gpio);

Can't we change this to fit in 2 lines?

Quoted 6 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +6 -0

+             return ret ? ret : -EIO;
+     }
+     return get.polarity;
+}
+
+static int rpi_exp_gpio_dir_in(struct gpio_chip *gc, unsigned int off)

drivers/gpio/gpio-raspberrypi-exp.c line 86

Quoted 10 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +10 -0

+{
+     struct rpi_exp_gpio *gpio;
+     struct gpio_set_config set_in;
+     int ret;
+
+     gpio = gpiochip_get_data(gc);
+
+     set_in.gpio = off + RPI_EXP_GPIO_BASE;  /* GPIO to update */
+     set_in.direction = RPI_EXP_GPIO_DIR_IN;
+     set_in.polarity = rpi_exp_gpio_get_polarity(gc, off);

This could fail, so please check the return code.

drivers/gpio/gpio-raspberrypi-exp.c line 97

Quoted 11 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +11 -0

+                                     /* 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 || set_in.gpio != 0) {
+             dev_err(gc->parent,
+                     "Failed to set GPIO %u to input (%d %x)\n",
+                     off, ret, set_in.gpio);

see above

Quoted 6 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +6 -0

+             return ret ? ret : -EIO;
+     }
+     return 0;
+}
+
+static int rpi_exp_gpio_dir_out(struct gpio_chip *gc, unsigned int off, int val)

drivers/gpio/gpio-raspberrypi-exp.c line 113

Quoted 10 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +10 -0

+{
+     struct rpi_exp_gpio *gpio;
+     struct gpio_set_config set_out;
+     int ret;
+
+     gpio = gpiochip_get_data(gc);
+
+     set_out.gpio = off + RPI_EXP_GPIO_BASE; /* GPIO to update */
+     set_out.direction = RPI_EXP_GPIO_DIR_OUT;
+     set_out.polarity = rpi_exp_gpio_get_polarity(gc, off);

This could also fail.

Quoted 111 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +111 -0

+                                     /* 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 || set_out.gpio != 0) {
+             dev_err(gc->parent,
+                     "Failed to set GPIO %u to output (%d %x)\n", off, ret,
+                     set_out.gpio);
+             return ret ? ret : -EIO;
+     }
+     return 0;
+}
+
+static int rpi_exp_gpio_get_direction(struct gpio_chip *gc, unsigned int off)
+{
+     struct rpi_exp_gpio *gpio;
+     struct gpio_get_config get;
+     int ret;
+
+     gpio = gpiochip_get_data(gc);
+
+     get.gpio = off + RPI_EXP_GPIO_BASE;     /* GPIO to update */
+
+     ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_GET_GPIO_CONFIG,
+                                 &get, sizeof(get));
+     if (ret || get.gpio != 0) {
+             dev_err(gc->parent,
+                     "Failed to get GPIO %u config (%d %x)\n", off, ret,
+                     get.gpio);
+             return ret ? ret : -EIO;
+     }
+     return !get.direction;
+}
+
+static int rpi_exp_gpio_get(struct gpio_chip *gc, unsigned int off)
+{
+     struct rpi_exp_gpio *gpio;
+     struct gpio_get_set_state get;
+     int ret;
+
+     gpio = gpiochip_get_data(gc);
+
+     get.gpio = off + RPI_EXP_GPIO_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 || get.gpio != 0) {
+             dev_err(gc->parent,
+                     "Failed to get GPIO %u state (%d %x)\n", off, ret,
+                     get.gpio);
+             return ret ? ret : -EIO;
+     }
+     return !!get.state;
+}
+
+static void rpi_exp_gpio_set(struct gpio_chip *gc, unsigned int off, int val)
+{
+     struct rpi_exp_gpio *gpio;
+     struct gpio_get_set_state set;
+     int ret;
+
+     gpio = gpiochip_get_data(gc);
+
+     set.gpio = off + RPI_EXP_GPIO_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 || set.gpio != 0)
+             dev_err(gc->parent,
+                     "Failed to set GPIO %u state (%d %x)\n", off, ret,
+                     set.gpio);
+}
+
+static int rpi_exp_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 rpi_exp_gpio *rpi_gpio;
+
+     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;
+
+     rpi_gpio = devm_kzalloc(dev, sizeof(*rpi_gpio), GFP_KERNEL);
+     if (!rpi_gpio)
+             return -ENOMEM;
+
+     rpi_gpio->fw = fw;
+     rpi_gpio->gc.parent = dev;
+     rpi_gpio->gc.label = MODULE_NAME;
+     rpi_gpio->gc.owner = THIS_MODULE;
+     rpi_gpio->gc.of_node = np;
+     rpi_gpio->gc.base = -1;
+     rpi_gpio->gc.ngpio = NUM_GPIO;
+
+     rpi_gpio->gc.direction_input = rpi_exp_gpio_dir_in;
+     rpi_gpio->gc.direction_output = rpi_exp_gpio_dir_out;

drivers/gpio/gpio-raspberrypi-exp.c line 234

Quoted 10 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +10 -0

+     rpi_gpio->gc.get_direction = rpi_exp_gpio_get_direction;
+     rpi_gpio->gc.get = rpi_exp_gpio_get;
+     rpi_gpio->gc.set = rpi_exp_gpio_set;
+     rpi_gpio->gc.can_sleep = true;
+
+     err = devm_gpiochip_add_data(dev, &rpi_gpio->gc, rpi_gpio);
+     if (err)
+             return err;
+
+     platform_set_drvdata(pdev, rpi_gpio);

Since we use devm_gpiochip_add_data, we can drop this.

Quoted 4 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +4 -0

+
+     return 0;
+}
+

drivers/gpio/gpio-raspberrypi-exp.c line 248

Quoted 10 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +10 -0

+static const struct of_device_id rpi_exp_gpio_ids[] = {
+     { .compatible = "raspberrypi,firmware-gpio" },
+     { }
+};
+MODULE_DEVICE_TABLE(of, rpi_exp_gpio_ids);
+
+static struct platform_driver rpi_exp_gpio_driver = {
+     .driver = {
+             .name           = MODULE_NAME,
+             .owner          = THIS_MODULE,

Please drop this, too.

Thanks
Stefan

_______________________________________________
linux-rpi-kernel mailing list
linux-rpi-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel

drivers/gpio/Kconfig config GPIO_AXP209 · line 135

Quoted 3 lines

drivers/gpio/Kconfig +3 -0

+	default RASPBERRYPI_FIRMWARE
+	depends on OF_GPIO && RASPBERRYPI_FIRMWARE && \
+		(ARCH_BCM2835 || COMPILE_TEST)

Since this is default on RASPBERRYPI_FIRMWARE, we could remove it from the dependencies.

Quoted 4 lines

drivers/gpio/Kconfig +4 -0

+	help
+	  Turn on GPIO support for the expander on Raspberry Pi 3 boards, using
+	  the firmware mailbox to communicate with VideoCore on BCM283x chips.
+

Quoted 5 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +2 -0

@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*

drivers/gpio/gpio-raspberrypi-exp.c line 8

Quoted 6 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +6 -0

+ *  Raspberry Pi 3 expander GPIO driver
+ *
+ *  Uses the firmware mailbox service to communicate with the
+ *  GPIO expander on the VPU.
+ *
+ *  Copyright (C) 2017 Raspberry Pi Trading Ltd.

2018?

drivers/gpio/gpio-raspberrypi-exp.c line 248

Quoted 4 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +4 -0

+static struct platform_driver rpi_exp_gpio_driver = {
+	.driver	= {
+		.name		= MODULE_NAME,
+		.owner		= THIS_MODULE,

Please drop this, too.

Baruch Siach <[email protected]> hat am 14. Januar 2018 um 07:08 geschrieben:

Hi Stefan,

Thanks for reviewing. Please find below a few questions.

Quoted 5 lines · Stefan Wahren

drivers/gpio/Kconfig config GPIO_AXP209 · line 135

Quoted 3 lines

drivers/gpio/Kconfig +3 -0

+	default RASPBERRYPI_FIRMWARE
+	depends on OF_GPIO && RASPBERRYPI_FIRMWARE && \
+		(ARCH_BCM2835 || COMPILE_TEST)

Since this is default on RASPBERRYPI_FIRMWARE, we could remove it from the dependencies.

This driver does not work when RASPBERRYPI_FIRMWARE is not enabled. So the driver should not be selectable, regardless of its default enable/disable state.

Quoted 5 lines

Quoted 5 lines

drivers/gpio/Kconfig +4 -0

+	help
+	  Turn on GPIO support for the expander on Raspberry Pi 3 boards, using
+	  the firmware mailbox to communicate with VideoCore on BCM283x chips.
+

[...]

Quoted 5 lines

Quoted 5 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +2 -0

@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*

Quoted 8 lines

drivers/gpio/gpio-raspberrypi-exp.c line 8

Quoted 6 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +6 -0

+ *  Raspberry Pi 3 expander GPIO driver
+ *
+ *  Uses the firmware mailbox service to communicate with the
+ *  GPIO expander on the VPU.
+ *
+ *  Copyright (C) 2017 Raspberry Pi Trading Ltd.

2018?

Why? Raspberry Pi Trading Ltd added no code to this driver in 2018.

[...]

Quoted 6 lines

drivers/gpio/gpio-raspberrypi-exp.c line 248

Quoted 4 lines

drivers/gpio/gpio-raspberrypi-exp.c new file +4 -0

+static struct platform_driver rpi_exp_gpio_driver = {
+	.driver	= {
+		.name		= MODULE_NAME,
+		.owner		= THIS_MODULE,

Please drop this, too.

Why? Recent GPIO drivers include this line.

I have seen no commits removing
.owner from GPIO drivers in mainline or in current development tree.

baruch

Signature · 8 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Baruch Siach <[email protected]> hat am 11. Januar 2018 um 20:44 geschrieben:

Add a description of the RPi3 GPIO expander that the VC4 firmware controls.

Signed-off-by Baruch Siach <[email protected]>

---
v2:
* Move GPIO expander node out of the soc container
* Rename compatible string
* Add gpio-line-names property

---
 arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

arch/arm/boot/dts/bcm2837-rpi-3-b.dts +0 -0

index b44b3b5af00d..24555e8a43ec 100644
@@ -23,6 +23,16 @@
 			gpios = <&gpio 47 0>;

arch/arm/boot/dts/bcm2837-rpi-3-b.dts +2 -0

+		status = "okay";
+	};
 };
 
 /* uart0 communicates with the BT module */

Signature · 1 line

2.15.1