[PATCH 0/3] pcf857x: Add OF support

[PATCH 0/3] pcf857x: Add OF support

[Laurent Pinchart]

Hello,

Here's a small series of patches that add OF support to the pcf857x driver.
The first two patches prepare the driver for the third one that introduces DT
bindings and parses the device tree node.

Laurent Pinchart (3):
  gpio: pcf857x: Sort headers alphabetically
  gpio: pcf857x: Remove pdata argument to pcf857x_irq_domain_init()
  gpio: pcf857x: Add OF support

 .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 69 +++++++++++++++++++++
 drivers/gpio/gpio-pcf857x.c                        | 70 +++++++++++++++++-----
 2 files changed, 123 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt

-- 
Regards,

Laurent Pinchart

[PATCH 1/3] gpio: pcf857x: Sort headers alphabetically

[Laurent Pinchart]

This makes checking for duplicates when adding a new #include easier.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpio/gpio-pcf857x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index e8faf53..ea9f3580 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -18,15 +18,15 @@
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
-#include <linux/kernel.h>
-#include <linux/slab.h>
 #include <linux/gpio.h>
 #include <linux/i2c.h>
 #include <linux/i2c/pcf857x.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
 
-- 
1.8.1.5

[PATCH 2/3] gpio: pcf857x: Remove pdata argument to pcf857x_irq_domain_init()

[Laurent Pinchart]

The argument is not used, remove it. No board registers a pcf857x device
with an IRQ without specifying platform data, IRQ domain registration
behaviour is thus not affected by this change.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpio/gpio-pcf857x.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index ea9f3580..070e81f 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -223,7 +223,6 @@ static void pcf857x_irq_domain_cleanup(struct pcf857x *gpio)
 }
 
 static int pcf857x_irq_domain_init(struct pcf857x *gpio,
-				   struct pcf857x_platform_data *pdata,
 				   struct i2c_client *client)
 {
 	int status;
@@ -286,8 +285,8 @@ static int pcf857x_probe(struct i2c_client *client,
 	gpio->chip.ngpio		= id->driver_data;
 
 	/* enable gpio_to_irq() if platform has settings */
-	if (pdata && client->irq) {
-		status = pcf857x_irq_domain_init(gpio, pdata, client);
+	if (client->irq) {
+		status = pcf857x_irq_domain_init(gpio, client);
 		if (status < 0) {
 			dev_err(&client->dev, "irq_domain init failed\n");
 			goto fail;
@@ -388,7 +387,7 @@ fail:
 	dev_dbg(&client->dev, "probe error %d for '%s'\n",
 			status, client->name);
 
-	if (pdata && client->irq)
+	if (client->irq)
 		pcf857x_irq_domain_cleanup(gpio);
 
 	return status;
@@ -411,7 +410,7 @@ static int pcf857x_remove(struct i2c_client *client)
 		}
 	}
 
-	if (pdata && client->irq)
+	if (client->irq)
 		pcf857x_irq_domain_cleanup(gpio);
 
 	status = gpiochip_remove(&gpio->chip);
-- 
1.8.1.5

[PATCH 3/3] gpio: pcf857x: Add OF support

[Laurent Pinchart]

Add DT bindings for the pcf857x-compatible chips and parse the device
tree node in the driver.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 69 ++++++++++++++++++++++
 drivers/gpio/gpio-pcf857x.c                        | 57 +++++++++++++++---
 2 files changed, 117 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
new file mode 100644
index 0000000..575b05e
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
@@ -0,0 +1,69 @@
+* PCF857x-compatible I/O expanders
+
+The PCF857x-compatible chips have "quasi-bidirectional" I/O pins that can be
+driven high by a pull-up current source or driven low to ground. This combines
+the direction and output level into a single bit per pin, which can't be read
+back. We can't actually know at initialization time whether a pin is configured
+(a) as output and driving the signal low/high, or (b) as input and reporting a
+low/high value, without knowing the last value written since the chip came out
+of reset (if any). The only reliable solution for setting up pin direction is
+thus to do it explicitly.
+
+Required Properties:
+
+  - compatible: should be one of the following.
+    - "maxim,max7328": For the Maxim MAX7378
+    - "maxim,max7329": For the Maxim MAX7329
+    - "nxp,pca8574": For the NXP PCA8574
+    - "nxp,pca8575": For the NXP PCA8575
+    - "nxp,pca9670": For the NXP PCA9670
+    - "nxp,pca9671": For the NXP PCA9671
+    - "nxp,pca9672": For the NXP PCA9672
+    - "nxp,pca9673": For the NXP PCA9673
+    - "nxp,pca9674": For the NXP PCA9674
+    - "nxp,pca9675": For the NXP PCA9675
+    - "nxp,pcf8574": For the NXP PCF8574
+    - "nxp,pcf8574a": For the NXP PCF8574A
+    - "nxp,pcf8575": For the NXP PCF8575
+    - "ti,tca9554": For the TI TCA9554
+
+  - reg: I2C slave address.
+
+  - gpio-controller: Marks the device node as a gpio controller.
+  - #gpio-cells: Should be 2. The first cell is the GPIO number and the second
+    cell specifies GPIO flags, as defined in <dt-bindings/gpio/gpio.h>. Only the
+    GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
+
+Optional Properties:
+
+  - pins-initial-state: Bitmask that specifies the initial state of each pin.
+  When a bit is set to zero, the corresponding pin will be initialized to the
+  input (pulled-up) state. When the  bit is set to one, the pin will be
+  initialized the the low-level output state. If the property is not specified
+  all pins will be initialized to the input state.
+
+  The I/O expander can detect input state changes, and thus optionally act as
+  an interrupt controller. When interrupts support is desired all the following
+  properties must be set. For more information please see the interrupt
+  controller device tree bindings documentation available at
+  Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
+
+  - interrupt-controller: Identifies the node as an interrupt controller.
+  - #interrupt-cells: Number of cells to encode an interrupt source, shall be 2.
+  - interrupt-parent: phandle of the parent interrupt controller.
+  - interrupts: Interrupt specifier for the controllers interrupt.
+
+
+Please refer to gpio.txt in this directory for details of the common GPIO
+bindings used by client devices.
+
+Example: PCF8575 I/O expander node
+
+	pcf8575: gpio@20 {
+		compatible = "nxp,pcf8575";
+		reg = <0x20>;
+		interrupt-parent = <&irqpin2>;
+		interrupts = <3 0>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+	};
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 070e81f..50a90f1 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -26,6 +26,8 @@
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
@@ -50,6 +52,27 @@ static const struct i2c_device_id pcf857x_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pcf857x_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id pcf857x_of_table[] = {
+	{ .compatible = "nxp,pcf8574", .data = (void *)8 },
+	{ .compatible = "nxp,pcf8574a", .data = (void *)8 },
+	{ .compatible = "nxp,pca8574", .data = (void *)8 },
+	{ .compatible = "nxp,pca9670", .data = (void *)8 },
+	{ .compatible = "nxp,pca9672", .data = (void *)8 },
+	{ .compatible = "nxp,pca9674", .data = (void *)8 },
+	{ .compatible = "nxp,pcf8575", .data = (void *)16 },
+	{ .compatible = "nxp,pca8575", .data = (void *)16 },
+	{ .compatible = "nxp,pca9671", .data = (void *)16 },
+	{ .compatible = "nxp,pca9673", .data = (void *)16 },
+	{ .compatible = "nxp,pca9675", .data = (void *)16 },
+	{ .compatible = "maxim,max7328", .data = (void *)8 },
+	{ .compatible = "maxim,max7329", .data = (void *)8 },
+	{ .compatible = "ti,tca9554", .data = (void *)8 },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pcf857x_of_table);
+#endif
+
 /*
  * The pcf857x, pca857x, and pca967x chips only expose one read and one
  * write register.  Writing a "one" bit (to match the reset state) lets
@@ -257,14 +280,29 @@ fail:
 static int pcf857x_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
-	struct pcf857x_platform_data	*pdata;
+	struct pcf857x_platform_data	*pdata = client->dev.platform_data;
+	struct device_node		*np = client->dev.of_node;
 	struct pcf857x			*gpio;
+	unsigned int			n_latch = 0;
+	unsigned int			ngpio;
 	int				status;
 
-	pdata = client->dev.platform_data;
-	if (!pdata) {
+#ifdef CONFIG_OF
+	if (np) {
+		const struct of_device_id *of_id;
+
+		of_id = of_match_device(pcf857x_of_table, &client->dev);
+		ngpio = (unsigned int)of_id->data;
+	} else
+#endif
+		ngpio = id->driver_data;
+
+	if (pdata)
+		n_latch = pdata->n_latch;
+	else if (IS_ENABLED(CONFIG_OF) && np)
+		of_property_read_u32(np, "pins-initial-state", &n_latch);
+	else
 		dev_dbg(&client->dev, "no platform data\n");
-	}
 
 	/* Allocate, initialize, and register this gpio_chip. */
 	gpio = devm_kzalloc(&client->dev, sizeof(*gpio), GFP_KERNEL);
@@ -282,7 +320,7 @@ static int pcf857x_probe(struct i2c_client *client,
 	gpio->chip.set			= pcf857x_set;
 	gpio->chip.direction_input	= pcf857x_input;
 	gpio->chip.direction_output	= pcf857x_output;
-	gpio->chip.ngpio		= id->driver_data;
+	gpio->chip.ngpio		= ngpio;
 
 	/* enable gpio_to_irq() if platform has settings */
 	if (client->irq) {
@@ -357,11 +395,11 @@ static int pcf857x_probe(struct i2c_client *client,
 	 * may cause transient glitching since it can't know the last value
 	 * written (some pins may need to be driven low).
 	 *
-	 * Using pdata->n_latch avoids that trouble.  When left initialized
-	 * to zero, our software copy of the "latch" then matches the chip's
-	 * all-ones reset state.  Otherwise it flags pins to be driven low.
+	 * Using n_latch avoids that trouble.  When left initialized to zero,
+	 * our software copy of the "latch" then matches the chip's all-ones
+	 * reset state.  Otherwise it flags pins to be driven low.
 	 */
-	gpio->out = pdata ? ~pdata->n_latch : ~0;
+	gpio->out = ~n_latch;
 	gpio->status = gpio->out;
 
 	status = gpiochip_add(&gpio->chip);
@@ -423,6 +461,7 @@ static struct i2c_driver pcf857x_driver = {
 	.driver = {
 		.name	= "pcf857x",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(pcf857x_of_table),
 	},
 	.probe	= pcf857x_probe,
 	.remove	= pcf857x_remove,
-- 
1.8.1.5

Re: [3/3] gpio: pcf857x: Add OF support

[Laurent Pinchart]

Hello,

I'd like to get this patch set in v3.12. If there's no objection, Linus, could 
you please take it in your tree ?

On Monday 29 July 2013 20:39:05 Laurent Pinchart wrote:
> Add DT bindings for the pcf857x-compatible chips and parse the device
> tree node in the driver.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> ---
> .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 69 ++++++++++++++++++++
> drivers/gpio/gpio-pcf857x.c                        | 57 +++++++++++++++---
> 2 files changed, 117 insertions(+), 9 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt new file mode
> 100644
> index 0000000..575b05e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> @@ -0,0 +1,69 @@
> +* PCF857x-compatible I/O expanders
> +
> +The PCF857x-compatible chips have "quasi-bidirectional" I/O pins that can
> be +driven high by a pull-up current source or driven low to ground. This
> combines +the direction and output level into a single bit per pin, which
> can't be read +back. We can't actually know at initialization time whether
> a pin is configured +(a) as output and driving the signal low/high, or (b)
> as input and reporting a +low/high value, without knowing the last value
> written since the chip came out +of reset (if any). The only reliable
> solution for setting up pin direction is +thus to do it explicitly.
> +
> +Required Properties:
> +
> +  - compatible: should be one of the following.
> +    - "maxim,max7328": For the Maxim MAX7378
> +    - "maxim,max7329": For the Maxim MAX7329
> +    - "nxp,pca8574": For the NXP PCA8574
> +    - "nxp,pca8575": For the NXP PCA8575
> +    - "nxp,pca9670": For the NXP PCA9670
> +    - "nxp,pca9671": For the NXP PCA9671
> +    - "nxp,pca9672": For the NXP PCA9672
> +    - "nxp,pca9673": For the NXP PCA9673
> +    - "nxp,pca9674": For the NXP PCA9674
> +    - "nxp,pca9675": For the NXP PCA9675
> +    - "nxp,pcf8574": For the NXP PCF8574
> +    - "nxp,pcf8574a": For the NXP PCF8574A
> +    - "nxp,pcf8575": For the NXP PCF8575
> +    - "ti,tca9554": For the TI TCA9554
> +
> +  - reg: I2C slave address.
> +
> +  - gpio-controller: Marks the device node as a gpio controller.
> +  - #gpio-cells: Should be 2. The first cell is the GPIO number and the
> second +    cell specifies GPIO flags, as defined in
> <dt-bindings/gpio/gpio.h>. Only the +    GPIO_ACTIVE_HIGH and
> GPIO_ACTIVE_LOW flags are supported.
> +
> +Optional Properties:
> +
> +  - pins-initial-state: Bitmask that specifies the initial state of each
> pin. +  When a bit is set to zero, the corresponding pin will be
> initialized to the +  input (pulled-up) state. When the  bit is set to one,
> the pin will be +  initialized the the low-level output state. If the
> property is not specified +  all pins will be initialized to the input
> state.
> +
> +  The I/O expander can detect input state changes, and thus optionally act
> as +  an interrupt controller. When interrupts support is desired all the
> following +  properties must be set. For more information please see the
> interrupt +  controller device tree bindings documentation available at
> +  Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
> +
> +  - interrupt-controller: Identifies the node as an interrupt controller.
> +  - #interrupt-cells: Number of cells to encode an interrupt source, shall
> be 2. +  - interrupt-parent: phandle of the parent interrupt controller. + 
> - interrupts: Interrupt specifier for the controllers interrupt. +
> +
> +Please refer to gpio.txt in this directory for details of the common GPIO
> +bindings used by client devices.
> +
> +Example: PCF8575 I/O expander node
> +
> +	pcf8575: gpio@20 {
> +		compatible = "nxp,pcf8575";
> +		reg = <0x20>;
> +		interrupt-parent = <&irqpin2>;
> +		interrupts = <3 0>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +	};
> diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
> index 070e81f..50a90f1 100644
> --- a/drivers/gpio/gpio-pcf857x.c
> +++ b/drivers/gpio/gpio-pcf857x.c
> @@ -26,6 +26,8 @@
>  #include <linux/irqdomain.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
>  #include <linux/workqueue.h>
> @@ -50,6 +52,27 @@ static const struct i2c_device_id pcf857x_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, pcf857x_id);
> 
> +#ifdef CONFIG_OF
> +static const struct of_device_id pcf857x_of_table[] = {
> +	{ .compatible = "nxp,pcf8574", .data = (void *)8 },
> +	{ .compatible = "nxp,pcf8574a", .data = (void *)8 },
> +	{ .compatible = "nxp,pca8574", .data = (void *)8 },
> +	{ .compatible = "nxp,pca9670", .data = (void *)8 },
> +	{ .compatible = "nxp,pca9672", .data = (void *)8 },
> +	{ .compatible = "nxp,pca9674", .data = (void *)8 },
> +	{ .compatible = "nxp,pcf8575", .data = (void *)16 },
> +	{ .compatible = "nxp,pca8575", .data = (void *)16 },
> +	{ .compatible = "nxp,pca9671", .data = (void *)16 },
> +	{ .compatible = "nxp,pca9673", .data = (void *)16 },
> +	{ .compatible = "nxp,pca9675", .data = (void *)16 },
> +	{ .compatible = "maxim,max7328", .data = (void *)8 },
> +	{ .compatible = "maxim,max7329", .data = (void *)8 },
> +	{ .compatible = "ti,tca9554", .data = (void *)8 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, pcf857x_of_table);
> +#endif
> +
>  /*
>   * The pcf857x, pca857x, and pca967x chips only expose one read and one
>   * write register.  Writing a "one" bit (to match the reset state) lets
> @@ -257,14 +280,29 @@ fail:
>  static int pcf857x_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
> -	struct pcf857x_platform_data	*pdata;
> +	struct pcf857x_platform_data	*pdata = client->dev.platform_data;
> +	struct device_node		*np = client->dev.of_node;
>  	struct pcf857x			*gpio;
> +	unsigned int			n_latch = 0;
> +	unsigned int			ngpio;
>  	int				status;
> 
> -	pdata = client->dev.platform_data;
> -	if (!pdata) {
> +#ifdef CONFIG_OF
> +	if (np) {
> +		const struct of_device_id *of_id;
> +
> +		of_id = of_match_device(pcf857x_of_table, &client->dev);
> +		ngpio = (unsigned int)of_id->data;
> +	} else
> +#endif
> +		ngpio = id->driver_data;
> +
> +	if (pdata)
> +		n_latch = pdata->n_latch;
> +	else if (IS_ENABLED(CONFIG_OF) && np)
> +		of_property_read_u32(np, "pins-initial-state", &n_latch);
> +	else
>  		dev_dbg(&client->dev, "no platform data\n");
> -	}
> 
>  	/* Allocate, initialize, and register this gpio_chip. */
>  	gpio = devm_kzalloc(&client->dev, sizeof(*gpio), GFP_KERNEL);
> @@ -282,7 +320,7 @@ static int pcf857x_probe(struct i2c_client *client,
>  	gpio->chip.set			= pcf857x_set;
>  	gpio->chip.direction_input	= pcf857x_input;
>  	gpio->chip.direction_output	= pcf857x_output;
> -	gpio->chip.ngpio		= id->driver_data;
> +	gpio->chip.ngpio		= ngpio;
> 
>  	/* enable gpio_to_irq() if platform has settings */
>  	if (client->irq) {
> @@ -357,11 +395,11 @@ static int pcf857x_probe(struct i2c_client *client,
>  	 * may cause transient glitching since it can't know the last value
>  	 * written (some pins may need to be driven low).
>  	 *
> -	 * Using pdata->n_latch avoids that trouble.  When left initialized
> -	 * to zero, our software copy of the "latch" then matches the chip's
> -	 * all-ones reset state.  Otherwise it flags pins to be driven low.
> +	 * Using n_latch avoids that trouble.  When left initialized to zero,
> +	 * our software copy of the "latch" then matches the chip's all-ones
> +	 * reset state.  Otherwise it flags pins to be driven low.
>  	 */
> -	gpio->out = pdata ? ~pdata->n_latch : ~0;
> +	gpio->out = ~n_latch;
>  	gpio->status = gpio->out;
> 
>  	status = gpiochip_add(&gpio->chip);
> @@ -423,6 +461,7 @@ static struct i2c_driver pcf857x_driver = {
>  	.driver = {
>  		.name	= "pcf857x",
>  		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(pcf857x_of_table),
>  	},
>  	.probe	= pcf857x_probe,
>  	.remove	= pcf857x_remove,
-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] gpio: pcf857x: Add OF support

[Linus Walleij]

On Mon, Jul 29, 2013 at 8:39 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:

> Add DT bindings for the pcf857x-compatible chips and parse the device
> tree node in the driver.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 69 ++++++++++++++++++++++

Do you think you can get one of the DT bindings mainatiners to ACK
this part?

I am told we need to get more strict on binding review.

It will be subject to timeout if it takes to long and you made a reasonable
effort to get their consent though.

> +  - interrupt-controller: Identifies the node as an interrupt controller.
> +  - #interrupt-cells: Number of cells to encode an interrupt source, shall be 2.
> +  - interrupt-parent: phandle of the parent interrupt controller.
> +  - interrupts: Interrupt specifier for the controllers interrupt.

So this is another one of of those GPIO controller that are
also interrupt controllers.

So it will be subject to the same dilemma as the OMAP
expander that has caused so much stir :-(

Can you provide your input in the discussion regarding my
patch with the subject:
"RFC: interrupt consistency check for OF GPIO IRQs"

This patch will affect this driver also, in a very direct way, by just
stealing and setting as input any GPIO line used for an IRQ.

> +Please refer to gpio.txt in this directory for details of the common GPIO
> +bindings used by client devices.
> +
> +Example: PCF8575 I/O expander node
> +
> +       pcf8575: gpio@20 {
> +               compatible = "nxp,pcf8575";
> +               reg = <0x20>;
> +               interrupt-parent = <&irqpin2>;
> +               interrupts = <3 0>;
> +               interrupt-controller;
> +               #interrupt-cells = <2>;
> +       };

Not gpio-controller?

Apart from that it looks good, please help out with the
gpio-controller vs interrupt-controller dilemma if you can
because it's getting really frustrating...

Yours,
Linus Walleij

Re: [PATCH 3/3] gpio: pcf857x: Add OF support

[Laurent Pinchart]

Hi Linus,

On Friday 16 August 2013 17:00:29 Linus Walleij wrote:
> On Mon, Jul 29, 2013 at 8:39 PM, Laurent Pinchart wrote:
> > Add DT bindings for the pcf857x-compatible chips and parse the device
> > tree node in the driver.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 69 +++++++++++++++++
>
> Do you think you can get one of the DT bindings mainatiners to ACK this
> part?

They're already CC'ed. I suppose vacations played a role in the lack of reply. 
I'd like to avoid having to switch to stalker mode ;-)

> I am told we need to get more strict on binding review.
> 
> It will be subject to timeout if it takes to long and you made a reasonable
> effort to get their consent though.
> 
> > +  - interrupt-controller: Identifies the node as an interrupt controller.
> > +  - #interrupt-cells: Number of cells to encode an interrupt source,
> > shall be 2.
> > +  - interrupt-parent: phandle of the parent interrupt controller.
> > +  - interrupts: Interrupt specifier for the controllers interrupt.
>
> So this is another one of of those GPIO controller that are also interrupt
> controllers.
> 
> So it will be subject to the same dilemma as the OMAP expander that has
> caused so much stir :-(
> 
> Can you provide your input in the discussion regarding my patch with the
> subject:
> "RFC: interrupt consistency check for OF GPIO IRQs"

Done.

> This patch will affect this driver also, in a very direct way, by just
> stealing and setting as input any GPIO line used for an IRQ.
> 
> > +Please refer to gpio.txt in this directory for details of the common GPIO
> > +bindings used by client devices.
> > +
> > +Example: PCF8575 I/O expander node
> > +
> > +       pcf8575: gpio@20 {
> > +               compatible = "nxp,pcf8575";
> > +               reg = <0x20>;
> > +               interrupt-parent = <&irqpin2>;
> > +               interrupts = <3 0>;
> > +               interrupt-controller;
> > +               #interrupt-cells = <2>;
> > +       };
> 
> Not gpio-controller?

I've fixed that in my tree already, it seems I forgot to repost. I will do so 
now.

> Apart from that it looks good, please help out with the gpio-controller vs
> interrupt-controller dilemma if you can because it's getting really
> frustrating...

-- 
Regards,

Laurent Pinchart