[PATCH v2 0/4] gpio: driver for the RPi3 GPIO expander

[PATCH v2 0/4] gpio: driver for the RPi3 GPIO expander

[Baruch Siach]

The Raspberry Pi 3 has a GPIO expander that controls, among others, the 
activity LED, and the camera connector GPIOs. The GPIO expander on an I2C bus 
that is not directly controlled from the ARM core. The VC4 firmware controls 
the I2C bus, and allows the ARM core to set/get GPIO settings over its mailbox 
interface.

This series adds support for the RPi3 expander.

The driver is ported from the downstream kernel at 
https://github.com/raspberrypi/linux/, branch rpi-4.9.y.

Changes in v2:

  * Driver renamed to gpio-raspberrypi-exp
  * Compatible string renamed to "raspberrypi,firmware-gpio"
  * Firmware error check
  * Smaller tweaks listed in individual patches

Baruch Siach (3):
  ARM: bcm2835: sync firmware properties with downstream
  dt-bindings: gpio: add raspberry pi GPIO expander binding
  ARM: dts: bcm2837-rpi-3-b: add GPIO expander

Dave Stevenson (1):
  gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox
    service

 .../bindings/gpio/raspberrypi,firmware-gpio.txt    |  24 ++
 arch/arm/boot/dts/bcm2837-rpi-3-b.dts              |  10 +
 drivers/gpio/Kconfig                               |   9 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-raspberrypi-exp.c                | 258 +++++++++++++++++++++
 include/soc/bcm2835/raspberrypi-firmware.h         |  18 ++
 6 files changed, 320 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
 create mode 100644 drivers/gpio/gpio-raspberrypi-exp.c

-- 
2.15.1

[PATCH v2 0/4] gpio: driver for the RPi3 GPIO expander

[Baruch Siach]

The Raspberry Pi 3 has a GPIO expander that controls, among others, the 
activity LED, and the camera connector GPIOs. The GPIO expander on an I2C bus 
that is not directly controlled from the ARM core. The VC4 firmware controls 
the I2C bus, and allows the ARM core to set/get GPIO settings over its mailbox 
interface.

This series adds support for the RPi3 expander.

The driver is ported from the downstream kernel at 
https://github.com/raspberrypi/linux/, branch rpi-4.9.y.

Changes in v2:

  * Driver renamed to gpio-raspberrypi-exp
  * Compatible string renamed to "raspberrypi,firmware-gpio"
  * Firmware error check
  * Smaller tweaks listed in individual patches

Baruch Siach (3):
  ARM: bcm2835: sync firmware properties with downstream
  dt-bindings: gpio: add raspberry pi GPIO expander binding
  ARM: dts: bcm2837-rpi-3-b: add GPIO expander

Dave Stevenson (1):
  gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox
    service

 .../bindings/gpio/raspberrypi,firmware-gpio.txt    |  24 ++
 arch/arm/boot/dts/bcm2837-rpi-3-b.dts              |  10 +
 drivers/gpio/Kconfig                               |   9 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-raspberrypi-exp.c                | 258 +++++++++++++++++++++
 include/soc/bcm2835/raspberrypi-firmware.h         |  18 ++
 6 files changed, 320 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
 create mode 100644 drivers/gpio/gpio-raspberrypi-exp.c

-- 
2.15.1

[PATCH v2 1/4] ARM: bcm2835: sync firmware properties with downstream

[Baruch Siach]

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 <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 include/soc/bcm2835/raspberrypi-firmware.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index cb979ad90401..50df5b28d2c9 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -63,6 +63,7 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_GET_MIN_VOLTAGE =                        0x00030008,
 	RPI_FIRMWARE_GET_TURBO =                              0x00030009,
 	RPI_FIRMWARE_GET_MAX_TEMPERATURE =                    0x0003000a,
+	RPI_FIRMWARE_GET_STC =                                0x0003000b,
 	RPI_FIRMWARE_ALLOCATE_MEMORY =                        0x0003000c,
 	RPI_FIRMWARE_LOCK_MEMORY =                            0x0003000d,
 	RPI_FIRMWARE_UNLOCK_MEMORY =                          0x0003000e,
@@ -72,12 +73,22 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_SET_ENABLE_QPU =                         0x00030012,
 	RPI_FIRMWARE_GET_DISPMANX_RESOURCE_MEM_HANDLE =       0x00030014,
 	RPI_FIRMWARE_GET_EDID_BLOCK =                         0x00030020,
+	RPI_FIRMWARE_GET_CUSTOMER_OTP =                       0x00030021,
 	RPI_FIRMWARE_GET_DOMAIN_STATE =                       0x00030030,
 	RPI_FIRMWARE_SET_CLOCK_STATE =                        0x00038001,
 	RPI_FIRMWARE_SET_CLOCK_RATE =                         0x00038002,
 	RPI_FIRMWARE_SET_VOLTAGE =                            0x00038003,
 	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,
+	RPI_FIRMWARE_GET_PERIPH_REG =                         0x00030045,
+	RPI_FIRMWARE_SET_PERIPH_REG =                         0x00038045,
+
 
 	/* Dispmanx TAGS */
 	RPI_FIRMWARE_FRAMEBUFFER_ALLOCATE =                   0x00040001,
@@ -91,6 +102,8 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_FRAMEBUFFER_GET_VIRTUAL_OFFSET =         0x00040009,
 	RPI_FIRMWARE_FRAMEBUFFER_GET_OVERSCAN =               0x0004000a,
 	RPI_FIRMWARE_FRAMEBUFFER_GET_PALETTE =                0x0004000b,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_TOUCHBUF =               0x0004000f,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_GPIOVIRTBUF =            0x00040010,
 	RPI_FIRMWARE_FRAMEBUFFER_RELEASE =                    0x00048001,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_PHYSICAL_WIDTH_HEIGHT = 0x00044003,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_VIRTUAL_WIDTH_HEIGHT =  0x00044004,
@@ -100,6 +113,7 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_VIRTUAL_OFFSET =        0x00044009,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_OVERSCAN =              0x0004400a,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_PALETTE =               0x0004400b,
+	RPI_FIRMWARE_FRAMEBUFFER_TEST_VSYNC =                 0x0004400e,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_PHYSICAL_WIDTH_HEIGHT =  0x00048003,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_VIRTUAL_WIDTH_HEIGHT =   0x00048004,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_DEPTH =                  0x00048005,
@@ -108,6 +122,10 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_FRAMEBUFFER_SET_VIRTUAL_OFFSET =         0x00048009,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_OVERSCAN =               0x0004800a,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_PALETTE =                0x0004800b,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_TOUCHBUF =               0x0004801f,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_GPIOVIRTBUF =            0x00048020,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_VSYNC =                  0x0004800e,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_BACKLIGHT =              0x0004800f,
 
 	RPI_FIRMWARE_VCHIQ_INIT =                             0x00048010,
 
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/4] ARM: bcm2835: sync firmware properties with downstream

[Baruch Siach]

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 <baruch@tkos.co.il>
---
 include/soc/bcm2835/raspberrypi-firmware.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index cb979ad90401..50df5b28d2c9 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -63,6 +63,7 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_GET_MIN_VOLTAGE =                        0x00030008,
 	RPI_FIRMWARE_GET_TURBO =                              0x00030009,
 	RPI_FIRMWARE_GET_MAX_TEMPERATURE =                    0x0003000a,
+	RPI_FIRMWARE_GET_STC =                                0x0003000b,
 	RPI_FIRMWARE_ALLOCATE_MEMORY =                        0x0003000c,
 	RPI_FIRMWARE_LOCK_MEMORY =                            0x0003000d,
 	RPI_FIRMWARE_UNLOCK_MEMORY =                          0x0003000e,
@@ -72,12 +73,22 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_SET_ENABLE_QPU =                         0x00030012,
 	RPI_FIRMWARE_GET_DISPMANX_RESOURCE_MEM_HANDLE =       0x00030014,
 	RPI_FIRMWARE_GET_EDID_BLOCK =                         0x00030020,
+	RPI_FIRMWARE_GET_CUSTOMER_OTP =                       0x00030021,
 	RPI_FIRMWARE_GET_DOMAIN_STATE =                       0x00030030,
 	RPI_FIRMWARE_SET_CLOCK_STATE =                        0x00038001,
 	RPI_FIRMWARE_SET_CLOCK_RATE =                         0x00038002,
 	RPI_FIRMWARE_SET_VOLTAGE =                            0x00038003,
 	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,
+	RPI_FIRMWARE_GET_PERIPH_REG =                         0x00030045,
+	RPI_FIRMWARE_SET_PERIPH_REG =                         0x00038045,
+
 
 	/* Dispmanx TAGS */
 	RPI_FIRMWARE_FRAMEBUFFER_ALLOCATE =                   0x00040001,
@@ -91,6 +102,8 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_FRAMEBUFFER_GET_VIRTUAL_OFFSET =         0x00040009,
 	RPI_FIRMWARE_FRAMEBUFFER_GET_OVERSCAN =               0x0004000a,
 	RPI_FIRMWARE_FRAMEBUFFER_GET_PALETTE =                0x0004000b,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_TOUCHBUF =               0x0004000f,
+	RPI_FIRMWARE_FRAMEBUFFER_GET_GPIOVIRTBUF =            0x00040010,
 	RPI_FIRMWARE_FRAMEBUFFER_RELEASE =                    0x00048001,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_PHYSICAL_WIDTH_HEIGHT = 0x00044003,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_VIRTUAL_WIDTH_HEIGHT =  0x00044004,
@@ -100,6 +113,7 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_VIRTUAL_OFFSET =        0x00044009,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_OVERSCAN =              0x0004400a,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_PALETTE =               0x0004400b,
+	RPI_FIRMWARE_FRAMEBUFFER_TEST_VSYNC =                 0x0004400e,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_PHYSICAL_WIDTH_HEIGHT =  0x00048003,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_VIRTUAL_WIDTH_HEIGHT =   0x00048004,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_DEPTH =                  0x00048005,
@@ -108,6 +122,10 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_FRAMEBUFFER_SET_VIRTUAL_OFFSET =         0x00048009,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_OVERSCAN =               0x0004800a,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_PALETTE =                0x0004800b,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_TOUCHBUF =               0x0004801f,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_GPIOVIRTBUF =            0x00048020,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_VSYNC =                  0x0004800e,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_BACKLIGHT =              0x0004800f,
 
 	RPI_FIRMWARE_VCHIQ_INIT =                             0x00048010,
 
-- 
2.15.1

[PATCH v2 2/4] dt-bindings: gpio: add raspberry pi GPIO expander binding

[Baruch Siach]

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 <baruch@tkos.co.il>
---
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

diff --git a/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt b/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
new file mode 100644
index 000000000000..8faf269cf541
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
@@ -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"
+- gpio-controller : Marks the device node as a gpio controller
+- #gpio-cells : Should be two.  The first cell is the pin number, and
+  the second cell is used to specify the gpio polarity:
+  0 = active high
+  1 = active low
+- firmware : Reference to the RPi firmware device node
+
+Example:
+
+expgpio: expgpio {
+	compatible = "raspberrypi,firmware-gpio";
+	gpio-controller;
+	#gpio-cells = <2>;
+	firmware = <&firmware>;
+};
-- 
2.15.1

[PATCH v2 2/4] dt-bindings: gpio: add raspberry pi GPIO expander binding

[Baruch Siach]

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 <baruch@tkos.co.il>
---
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

diff --git a/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt b/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
new file mode 100644
index 000000000000..8faf269cf541
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
@@ -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"
+- gpio-controller : Marks the device node as a gpio controller
+- #gpio-cells : Should be two.  The first cell is the pin number, and
+  the second cell is used to specify the gpio polarity:
+  0 = active high
+  1 = active low
+- firmware : Reference to the RPi firmware device node
+
+Example:
+
+expgpio: expgpio {
+	compatible = "raspberrypi,firmware-gpio";
+	gpio-controller;
+	#gpio-cells = <2>;
+	firmware = <&firmware>;
+};
-- 
2.15.1

[PATCH v2 3/4] gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service

[Baruch Siach]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

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 <dave.stevenson@raspberrypi.org>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
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

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6a8e851ad13..8df901c75bef 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -128,6 +128,15 @@ config GPIO_AXP209
 	help
 	  Say yes to enable GPIO support for the AXP209 PMIC
 
+config GPIO_RASPBERRYPI_EXP
+	bool "RaspberryPi 3 Exp GPIO"
+	default RASPBERRYPI_FIRMWARE
+	depends on OF_GPIO && RASPBERRYPI_FIRMWARE && \
+		(ARCH_BCM2835 || COMPILE_TEST)
+	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)
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4bc24febb889..71021b0e71da 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -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
diff --git a/drivers/gpio/gpio-raspberrypi-exp.c b/drivers/gpio/gpio-raspberrypi-exp.c
new file mode 100644
index 000000000000..95b1957a69fa
--- /dev/null
+++ b/drivers/gpio/gpio-raspberrypi-exp.c
@@ -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.
+ */
+
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#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;
+
+	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.polarity;
+}
+
+static int rpi_exp_gpio_dir_in(struct gpio_chip *gc, unsigned int off)
+{
+	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);
+					/* 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);
+		return ret ? ret : -EIO;
+	}
+	return 0;
+}
+
+static int rpi_exp_gpio_dir_out(struct gpio_chip *gc, unsigned int off, int val)
+{
+	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);
+					/* 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;
+	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);
+
+	return 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,
+		.of_match_table	= of_match_ptr(rpi_exp_gpio_ids),
+	},
+	.probe	= rpi_exp_gpio_probe,
+};
+module_platform_driver(rpi_exp_gpio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Dave Stevenson <dave.stevenson@raspberrypi.org>");
+MODULE_DESCRIPTION("Raspberry Pi 3 expander GPIO driver");
+MODULE_ALIAS("platform:rpi-exp-gpio");
-- 
2.15.1

[PATCH v2 3/4] gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service

[Baruch Siach]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

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 <dave.stevenson@raspberrypi.org>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
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

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6a8e851ad13..8df901c75bef 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -128,6 +128,15 @@ config GPIO_AXP209
 	help
 	  Say yes to enable GPIO support for the AXP209 PMIC
 
+config GPIO_RASPBERRYPI_EXP
+	bool "RaspberryPi 3 Exp GPIO"
+	default RASPBERRYPI_FIRMWARE
+	depends on OF_GPIO && RASPBERRYPI_FIRMWARE && \
+		(ARCH_BCM2835 || COMPILE_TEST)
+	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)
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4bc24febb889..71021b0e71da 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -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
diff --git a/drivers/gpio/gpio-raspberrypi-exp.c b/drivers/gpio/gpio-raspberrypi-exp.c
new file mode 100644
index 000000000000..95b1957a69fa
--- /dev/null
+++ b/drivers/gpio/gpio-raspberrypi-exp.c
@@ -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.
+ */
+
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#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;
+
+	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.polarity;
+}
+
+static int rpi_exp_gpio_dir_in(struct gpio_chip *gc, unsigned int off)
+{
+	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);
+					/* 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);
+		return ret ? ret : -EIO;
+	}
+	return 0;
+}
+
+static int rpi_exp_gpio_dir_out(struct gpio_chip *gc, unsigned int off, int val)
+{
+	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);
+					/* 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;
+	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);
+
+	return 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,
+		.of_match_table	= of_match_ptr(rpi_exp_gpio_ids),
+	},
+	.probe	= rpi_exp_gpio_probe,
+};
+module_platform_driver(rpi_exp_gpio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Dave Stevenson <dave.stevenson@raspberrypi.org>");
+MODULE_DESCRIPTION("Raspberry Pi 3 expander GPIO driver");
+MODULE_ALIAS("platform:rpi-exp-gpio");
-- 
2.15.1

[PATCH v2 4/4] ARM: dts: bcm2837-rpi-3-b: add GPIO expander

[Baruch Siach]

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

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
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(+)

diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
index b44b3b5af00d..24555e8a43ec 100644
--- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
+++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
@@ -23,6 +23,16 @@
 			gpios = <&gpio 47 0>;
 		};
 	};
+
+	expgpio: gpio-expander {
+		compatible = "raspberrypi,firmware-gpio";
+		gpio-controller;
+		#gpio-cells = <2>;
+		firmware = <&firmware>;
+		gpio-line-names = "BT_ON", "WL_ON", "STATUS_LED", "LAN_RUN",
+			"HPD_N", "CAM_GPIO0", "CAM_GPIO1", "PWR_LOW_N";
+		status = "okay";
+	};
 };
 
 /* uart0 communicates with the BT module */
-- 
2.15.1

[PATCH v2 4/4] ARM: dts: bcm2837-rpi-3-b: add GPIO expander

[Baruch Siach]

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

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
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(+)

diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
index b44b3b5af00d..24555e8a43ec 100644
--- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
+++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
@@ -23,6 +23,16 @@
 			gpios = <&gpio 47 0>;
 		};
 	};
+
+	expgpio: gpio-expander {
+		compatible = "raspberrypi,firmware-gpio";
+		gpio-controller;
+		#gpio-cells = <2>;
+		firmware = <&firmware>;
+		gpio-line-names = "BT_ON", "WL_ON", "STATUS_LED", "LAN_RUN",
+			"HPD_N", "CAM_GPIO0", "CAM_GPIO1", "PWR_LOW_N";
+		status = "okay";
+	};
 };
 
 /* uart0 communicates with the BT module */
-- 
2.15.1

Re: [PATCH v2 1/4] ARM: bcm2835: sync firmware properties with downstream

[Stefan Wahren]

> Baruch Siach <baruch@tkos.co.il> 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 <baruch@tkos.co.il>

Acked-by: Stefan Wahren <stefan.wahren@i2se.com>

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

[PATCH v2 1/4] ARM: bcm2835: sync firmware properties with downstream

[Stefan Wahren]

> Baruch Siach <baruch@tkos.co.il> 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 <baruch@tkos.co.il>

Acked-by: Stefan Wahren <stefan.wahren@i2se.com>

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

Re: [PATCH v2 2/4] dt-bindings: gpio: add raspberry pi GPIO expander binding

[Stefan Wahren]

Hi Baruch,

> Baruch Siach <baruch@tkos.co.il> 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 <baruch@tkos.co.il>
> ---
> 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
> 
> diff --git a/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt b/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
> new file mode 100644
> index 000000000000..8faf269cf541
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
> @@ -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"
> +- gpio-controller : Marks the device node as a gpio controller
> +- #gpio-cells : Should be two.  The first cell is the pin number, and
> +  the second cell is used to specify the gpio polarity:
> +  0 = active high
> +  1 = active low
> +- firmware : Reference to the RPi firmware device node
> +
> +Example:
> +
> +expgpio: expgpio {

just a nit, please change this to

expgpio: gpio-expander {

just like in patch #4


Thanks Stefan

[PATCH v2 2/4] dt-bindings: gpio: add raspberry pi GPIO expander binding

[Stefan Wahren]

Hi Baruch,

> Baruch Siach <baruch@tkos.co.il> 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 <baruch@tkos.co.il>
> ---
> 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
> 
> diff --git a/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt b/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
> new file mode 100644
> index 000000000000..8faf269cf541
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
> @@ -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"
> +- gpio-controller : Marks the device node as a gpio controller
> +- #gpio-cells : Should be two.  The first cell is the pin number, and
> +  the second cell is used to specify the gpio polarity:
> +  0 = active high
> +  1 = active low
> +- firmware : Reference to the RPi firmware device node
> +
> +Example:
> +
> +expgpio: expgpio {

just a nit, please change this to

expgpio: gpio-expander {

just like in patch #4


Thanks Stefan

Re: [PATCH v2 3/4] gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service

[Stefan Wahren]

Hi Baruch,

> Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> hat am 11. Januar 2018 um 20:44 geschrieben:
> 
> 
> From: Dave Stevenson <dave.stevenson-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
> 
> 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 <dave.stevenson-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
> Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
> ---
> 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
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index d6a8e851ad13..8df901c75bef 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -128,6 +128,15 @@ config GPIO_AXP209
>  	help
>  	  Say yes to enable GPIO support for the AXP209 PMIC
>  
> +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.


> +	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.

> +	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)
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 4bc24febb889..71021b0e71da 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -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
> diff --git a/drivers/gpio/gpio-raspberrypi-exp.c b/drivers/gpio/gpio-raspberrypi-exp.c
> new file mode 100644
> index 000000000000..95b1957a69fa
> --- /dev/null
> +++ b/drivers/gpio/gpio-raspberrypi-exp.c
> @@ -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?

> + */
> +
> +#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.

> +#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;
> +
> +	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?

> +		return ret ? ret : -EIO;
> +	}
> +	return get.polarity;
> +}
> +
> +static int rpi_exp_gpio_dir_in(struct gpio_chip *gc, unsigned int off)
> +{
> +	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.

> +					/* 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

> +		return ret ? ret : -EIO;
> +	}
> +	return 0;
> +}
> +
> +static int rpi_exp_gpio_dir_out(struct gpio_chip *gc, unsigned int off, int val)
> +{
> +	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.

> +					/* 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;
> +	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.

> +
> +	return 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

[PATCH v2 3/4] gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service

[Stefan Wahren]

Hi Baruch,

> Baruch Siach <baruch@tkos.co.il> hat am 11. Januar 2018 um 20:44 geschrieben:
> 
> 
> From: Dave Stevenson <dave.stevenson@raspberrypi.org>
> 
> 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 <dave.stevenson@raspberrypi.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> 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
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index d6a8e851ad13..8df901c75bef 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -128,6 +128,15 @@ config GPIO_AXP209
>  	help
>  	  Say yes to enable GPIO support for the AXP209 PMIC
>  
> +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.


> +	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.

> +	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)
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 4bc24febb889..71021b0e71da 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -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
> diff --git a/drivers/gpio/gpio-raspberrypi-exp.c b/drivers/gpio/gpio-raspberrypi-exp.c
> new file mode 100644
> index 000000000000..95b1957a69fa
> --- /dev/null
> +++ b/drivers/gpio/gpio-raspberrypi-exp.c
> @@ -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?

> + */
> +
> +#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.

> +#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;
> +
> +	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?

> +		return ret ? ret : -EIO;
> +	}
> +	return get.polarity;
> +}
> +
> +static int rpi_exp_gpio_dir_in(struct gpio_chip *gc, unsigned int off)
> +{
> +	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.

> +					/* 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

> +		return ret ? ret : -EIO;
> +	}
> +	return 0;
> +}
> +
> +static int rpi_exp_gpio_dir_out(struct gpio_chip *gc, unsigned int off, int val)
> +{
> +	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.

> +					/* 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;
> +	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.

> +
> +	return 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

Re: [PATCH v2 4/4] ARM: dts: bcm2837-rpi-3-b: add GPIO expander

[Stefan Wahren]

Hi Baruch,

> Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> 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 <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
> ---
> 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(+)
> 
> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> index b44b3b5af00d..24555e8a43ec 100644
> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> @@ -23,6 +23,16 @@
>  			gpios = <&gpio 47 0>;
>  		};
>  	};
> +
> +	expgpio: gpio-expander {
> +		compatible = "raspberrypi,firmware-gpio";
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +		firmware = <&firmware>;
> +		gpio-line-names = "BT_ON", "WL_ON", "STATUS_LED", "LAN_RUN",
> +			"HPD_N", "CAM_GPIO0", "CAM_GPIO1", "PWR_LOW_N";

please one GPIO name per line like in the other files. This makes it easier to review changes and add comments.

Stefan

> +		status = "okay";
> +	};
>  };
>  
>  /* uart0 communicates with the BT module */
> -- 
> 2.15.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/4] ARM: dts: bcm2837-rpi-3-b: add GPIO expander

[Stefan Wahren]

Hi Baruch,

> Baruch Siach <baruch@tkos.co.il> 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 <baruch@tkos.co.il>
> ---
> 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(+)
> 
> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> index b44b3b5af00d..24555e8a43ec 100644
> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> @@ -23,6 +23,16 @@
>  			gpios = <&gpio 47 0>;
>  		};
>  	};
> +
> +	expgpio: gpio-expander {
> +		compatible = "raspberrypi,firmware-gpio";
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +		firmware = <&firmware>;
> +		gpio-line-names = "BT_ON", "WL_ON", "STATUS_LED", "LAN_RUN",
> +			"HPD_N", "CAM_GPIO0", "CAM_GPIO1", "PWR_LOW_N";

please one GPIO name per line like in the other files. This makes it easier to review changes and add comments.

Stefan

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

Re: [PATCH v2 3/4] gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service

[Peter Robinson]

On Sat, Jan 13, 2018 at 10:33 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> Hi Baruch,
>
>> Baruch Siach <baruch@tkos.co.il> hat am 11. Januar 2018 um 20:44 geschrieben:
>>
>>
>> From: Dave Stevenson <dave.stevenson@raspberrypi.org>
>>
>> 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 <dave.stevenson@raspberrypi.org>
>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> ---
>> 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
>>
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index d6a8e851ad13..8df901c75bef 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -128,6 +128,15 @@ config GPIO_AXP209
>>       help
>>         Say yes to enable GPIO support for the AXP209 PMIC
>>
>> +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.

And on all generic distros, like Fedora, that cater for generic
kernels with support for 100s of devices where most things are modular
to keep the kernel as small as possible.

>> +     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.
>
>> +     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)
>> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
>> index 4bc24febb889..71021b0e71da 100644
>> --- a/drivers/gpio/Makefile
>> +++ b/drivers/gpio/Makefile
>> @@ -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
>> diff --git a/drivers/gpio/gpio-raspberrypi-exp.c b/drivers/gpio/gpio-raspberrypi-exp.c
>> new file mode 100644
>> index 000000000000..95b1957a69fa
>> --- /dev/null
>> +++ b/drivers/gpio/gpio-raspberrypi-exp.c
>> @@ -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?
>
>> + */
>> +
>> +#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.
>
>> +#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;
>> +
>> +     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?
>
>> +             return ret ? ret : -EIO;
>> +     }
>> +     return get.polarity;
>> +}
>> +
>> +static int rpi_exp_gpio_dir_in(struct gpio_chip *gc, unsigned int off)
>> +{
>> +     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.
>
>> +                                     /* 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
>
>> +             return ret ? ret : -EIO;
>> +     }
>> +     return 0;
>> +}
>> +
>> +static int rpi_exp_gpio_dir_out(struct gpio_chip *gc, unsigned int off, int val)
>> +{
>> +     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.
>
>> +                                     /* 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;
>> +     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.
>
>> +
>> +     return 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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel

[PATCH v2 3/4] gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service

[Peter Robinson]

On Sat, Jan 13, 2018 at 10:33 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> Hi Baruch,
>
>> Baruch Siach <baruch@tkos.co.il> hat am 11. Januar 2018 um 20:44 geschrieben:
>>
>>
>> From: Dave Stevenson <dave.stevenson@raspberrypi.org>
>>
>> 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 <dave.stevenson@raspberrypi.org>
>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> ---
>> 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
>>
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index d6a8e851ad13..8df901c75bef 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -128,6 +128,15 @@ config GPIO_AXP209
>>       help
>>         Say yes to enable GPIO support for the AXP209 PMIC
>>
>> +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.

And on all generic distros, like Fedora, that cater for generic
kernels with support for 100s of devices where most things are modular
to keep the kernel as small as possible.

>> +     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.
>
>> +     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)
>> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
>> index 4bc24febb889..71021b0e71da 100644
>> --- a/drivers/gpio/Makefile
>> +++ b/drivers/gpio/Makefile
>> @@ -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
>> diff --git a/drivers/gpio/gpio-raspberrypi-exp.c b/drivers/gpio/gpio-raspberrypi-exp.c
>> new file mode 100644
>> index 000000000000..95b1957a69fa
>> --- /dev/null
>> +++ b/drivers/gpio/gpio-raspberrypi-exp.c
>> @@ -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?
>
>> + */
>> +
>> +#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.
>
>> +#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;
>> +
>> +     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?
>
>> +             return ret ? ret : -EIO;
>> +     }
>> +     return get.polarity;
>> +}
>> +
>> +static int rpi_exp_gpio_dir_in(struct gpio_chip *gc, unsigned int off)
>> +{
>> +     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.
>
>> +                                     /* 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
>
>> +             return ret ? ret : -EIO;
>> +     }
>> +     return 0;
>> +}
>> +
>> +static int rpi_exp_gpio_dir_out(struct gpio_chip *gc, unsigned int off, int val)
>> +{
>> +     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.
>
>> +                                     /* 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;
>> +     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.
>
>> +
>> +     return 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

Re: [PATCH v2 3/4] gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service

[Baruch Siach]

Hi Stefan,

Thanks for reviewing. Please find below a few questions.

On Sat, Jan 13, 2018 at 11:33:15AM +0100, Stefan Wahren wrote: 
> > +	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.

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

[...]

> > --- /dev/null
> > +++ b/drivers/gpio/gpio-raspberrypi-exp.c
> > @@ -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?

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

[...]

> > +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

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

[PATCH v2 3/4] gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service

[Baruch Siach]

Hi Stefan,

Thanks for reviewing. Please find below a few questions.

On Sat, Jan 13, 2018 at 11:33:15AM +0100, Stefan Wahren wrote: 
> > +	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.

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

[...]

> > --- /dev/null
> > +++ b/drivers/gpio/gpio-raspberrypi-exp.c
> > @@ -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?

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

[...]

> > +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

-- 
     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 -

Re: [PATCH v2 3/4] gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service

[Stefan Wahren]

Hi Baruch,

> Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> hat am 14. Januar 2018 um 07:08 geschrieben:
> 
> 
> Hi Stefan,
> 
> Thanks for reviewing. Please find below a few questions.
> 
> On Sat, Jan 13, 2018 at 11:33:15AM +0100, Stefan Wahren wrote: 
> > > +	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.

I know. My idea was to increase build test coverage. Nevertheless the more common style would be:

depends on ARCH_BCM2835 || COMPILE_TEST
depends on OF_GPIO && RASPBERRYPI_FIRMWARE

> 
> > > +	help
> > > +	  Turn on GPIO support for the expander on Raspberry Pi 3 boards, using
> > > +	  the firmware mailbox to communicate with VideoCore on BCM283x chips.
> > > +
> 
> [...]
> 
> > > --- /dev/null
> > > +++ b/drivers/gpio/gpio-raspberrypi-exp.c
> > > @@ -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?
> 
> Why? Raspberry Pi Trading Ltd added no code to this driver in 2018.

Sure. Don't you want to add your copyright?

> 
> [...]
> 
> > > +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 don't know which driver your are referring to, but platform driver doesn't need this.

Please grep for all platform_driver in gpio and you won't see any setting of ownership. I'm not speaking about the gpiochip.

Btw my replies to #2 and #4 got blocked, should i try to resend it to you.

Thanks
Stefan

> I have seen no commits removing 
> .owner from GPIO drivers in mainline or in current development tree.
> 
> baruch
> 
> -- 
>      http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
>    - baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.2.679.5364, http://www.tkos.co.il -
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/4] gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service

[Stefan Wahren]

Hi Baruch,

> Baruch Siach <baruch@tkos.co.il> hat am 14. Januar 2018 um 07:08 geschrieben:
> 
> 
> Hi Stefan,
> 
> Thanks for reviewing. Please find below a few questions.
> 
> On Sat, Jan 13, 2018 at 11:33:15AM +0100, Stefan Wahren wrote: 
> > > +	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.

I know. My idea was to increase build test coverage. Nevertheless the more common style would be:

depends on ARCH_BCM2835 || COMPILE_TEST
depends on OF_GPIO && RASPBERRYPI_FIRMWARE

> 
> > > +	help
> > > +	  Turn on GPIO support for the expander on Raspberry Pi 3 boards, using
> > > +	  the firmware mailbox to communicate with VideoCore on BCM283x chips.
> > > +
> 
> [...]
> 
> > > --- /dev/null
> > > +++ b/drivers/gpio/gpio-raspberrypi-exp.c
> > > @@ -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?
> 
> Why? Raspberry Pi Trading Ltd added no code to this driver in 2018.

Sure. Don't you want to add your copyright?

> 
> [...]
> 
> > > +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 don't know which driver your are referring to, but platform driver doesn't need this.

Please grep for all platform_driver in gpio and you won't see any setting of ownership. I'm not speaking about the gpiochip.

Btw my replies to #2 and #4 got blocked, should i try to resend it to you.

Thanks
Stefan

> I have seen no commits removing 
> .owner from GPIO drivers in mainline or in current development tree.
> 
> baruch
> 
> -- 
>      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

Re: [PATCH v2 1/4] ARM: bcm2835: sync firmware properties with downstream

[Linus Walleij]

On Sat, Jan 13, 2018 at 10:57 AM, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
>> Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> 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 <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
>
> Acked-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>
> 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?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] ARM: bcm2835: sync firmware properties with downstream

[Baruch Siach]

Hi Linus,

On Tue, Jan 16, 2018 at 10:57:32AM +0100, Linus Walleij wrote:
> On Sat, Jan 13, 2018 at 10:57 AM, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
> >> Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> 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 <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
> >
> > Acked-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> >
> > 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.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.52.368.4656, http://www.tkos.co.il -
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] ARM: bcm2835: sync firmware properties with downstream

[Stefan Wahren]

Hi Baruch,

Am 16.01.2018 um 11:49 schrieb Baruch Siach:
> Hi Linus,
>
> On Tue, Jan 16, 2018 at 10:57:32AM +0100, Linus Walleij wrote:
>> On Sat, Jan 13, 2018 at 10:57 AM, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:
>>>> Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> 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 <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
>>> Acked-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>>>
>>> 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.
>
> 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.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] ARM: bcm2835: sync firmware properties with downstream

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1955 bytes --]

Stefan Wahren <stefan.wahren@i2se.com> writes:

> Hi Baruch,
>
> Am 16.01.2018 um 11:49 schrieb Baruch Siach:
>> Hi Linus,
>>
>> On Tue, Jan 16, 2018 at 10:57:32AM +0100, Linus Walleij wrote:
>>> On Sat, Jan 13, 2018 at 10:57 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>>>>> Baruch Siach <baruch@tkos.co.il> 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 <baruch@tkos.co.il>
>>>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>>
>>>> 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.
>>
>> 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.

I'll be happy to see patch 1-3 go through the GPIO tree, and we'd take
#4 in the 2835 tree.  The only risk I see is if we need patch 1 for some
other change in this cycle -- seems unlikely, but if Linus applied the
series starting from a rc1 tag and merged it in, then we'll also be able
to merge that patch over to any other branch we need without duplicating
it in git history.

Once you get this resent with the DT folks Cced, hopefully it'll be
pretty quick.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[PATCH v2 1/4] ARM: bcm2835: sync firmware properties with downstream

[Eric Anholt]

Stefan Wahren <stefan.wahren@i2se.com> writes:

> Hi Baruch,
>
> Am 16.01.2018 um 11:49 schrieb Baruch Siach:
>> Hi Linus,
>>
>> On Tue, Jan 16, 2018 at 10:57:32AM +0100, Linus Walleij wrote:
>>> On Sat, Jan 13, 2018 at 10:57 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>>>>> Baruch Siach <baruch@tkos.co.il> 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 <baruch@tkos.co.il>
>>>> Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>>
>>>> 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.
>>
>> 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.

I'll be happy to see patch 1-3 go through the GPIO tree, and we'd take
#4 in the 2835 tree.  The only risk I see is if we need patch 1 for some
other change in this cycle -- seems unlikely, but if Linus applied the
series starting from a rc1 tag and merged it in, then we'll also be able
to merge that patch over to any other branch we need without duplicating
it in git history.

Once you get this resent with the DT folks Cced, hopefully it'll be
pretty quick.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180118/b094e1b4/attachment.sig>