[PATCH] gpio: Enable pcf857x GPIO expander for Device Tree

[PATCH] gpio: Enable pcf857x GPIO expander for Device Tree

[Archit Taneja]

Add code to parse the GPIO expander Device Tree node and extract platform data
out of it, and populate the struct 'pcf857x_platform_data' maintained by the
driver. This enables devices to reference the gpio expander from Device Tree.

Add DT binding info in Documentation.

CC: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Archit Taneja <archit@ti.com>
---
 .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 44 +++++++++++++++++
 drivers/gpio/gpio-pcf857x.c                        | 57 ++++++++++++++++++++--
 2 files changed, 97 insertions(+), 4 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..0556512
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
@@ -0,0 +1,44 @@
+PCF857x I2C based GPIO controller bindings
+
+Required properties:
+- compatible:
+  - "nxp,pca9670" for NXP PCA9670 8 bit I/O expander
+  - "nxp,pca9672" for NXP PCA9672 8 bit I/O expander with interrupt
+  - "nxp,pca9674" for NXP PCA9672 8 bit I/O expander with interrupt
+  - "nxp,pca8574" for NXP PCA8574 8 bit I/O expander with interrupt
+  - "nxp,pca8575" for NXP PCA8575 16 bit I/O expander with interrupt
+  - "nxp,pca9671" for NXP PCA9671 16 bit I/O expander
+  - "nxp,pca9673" for NXP PCA9673 16 bit I/O expander with interrupt
+  - "nxp,pca9675" for NXP PCA9675 16 bit I/O expander with interrupt
+  - "ti,pcf8574"  for TI PCF8574 8 bit I/O expander with interrupt
+  - "ti,pcf8574a" for TI PCF8574A 8 bit I/O expander with interrupt
+  - "ti,pcf8575"  for TI PCF8575 16 bit I/O expander with interrupt
+  - "ti,tca9554"  for TI TCA9554 8 bit I/O expander with interrupt
+  - "maxim,max7328" for MAXIM MAX7328 8 bit I/O expander with interrupt
+  - "maxim,max7329" for MAXIM MAX7329 8 bit I/O expander with interrupt
+- gpio-controller : Marks the device node as a GPIO controller.
+- #gpio-cells : Should be two.
+  - first cell is the pin number.
+  - second cell is unused.
+- interrupt-controller: Mark the device node as an interrupt controller.
+- #interrupt-cells : Should be two.
+  - first cell is the GPIO number.
+  - second cell is unused.
+- reg: I2C address of the chip.
+
+Device speific properties:
+- n_latch:		optional bit-inverse of initial register value; if
+			you leave this initialized to zero the driver will act
+			like the chip was just reset.
+
+Example:
+
+pcf8575: pcf8575 {
+	compatible = "ti,pcf8575";
+	gpio-controller;
+	#gpio-cells = <2>;
+	interrupt-controller;
+	#interrupt-cells = <2>;
+	reg = <0x20>;
+	n_latch = <0x0>;
+};
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index e8faf53..3435790 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -21,6 +21,7 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/gpio.h>
+#include <linux/of.h>
 #include <linux/i2c.h>
 #include <linux/i2c/pcf857x.h>
 #include <linux/interrupt.h>
@@ -255,17 +256,42 @@ fail:
 
 /*-------------------------------------------------------------------------*/
 
+static struct pcf857x_platform_data *of_gpio_pcf857x(struct device *dev)
+{
+	struct pcf857x_platform_data *pdata;
+	int r;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	pdata->gpio_base = -1;
+
+	r = of_property_read_u32(dev->of_node, "n_latch", &pdata->n_latch);
+	if (r) {
+		dev_dbg(dev, "couldn't find n-latch, use default\n");
+		pdata->n_latch = 0;
+	}
+
+	return pdata;
+}
+
 static int pcf857x_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct pcf857x_platform_data	*pdata;
+	struct device_node		*node;
 	struct pcf857x			*gpio;
 	int				status;
 
 	pdata = client->dev.platform_data;
-	if (!pdata) {
+	node = client->dev.of_node;
+
+	if (!pdata && node)
+		pdata = of_gpio_pcf857x(&client->dev);
+
+	if (!pdata)
 		dev_dbg(&client->dev, "no platform data\n");
-	}
 
 	/* Allocate, initialize, and register this gpio_chip. */
 	gpio = devm_kzalloc(&client->dev, sizeof(*gpio), GFP_KERNEL);
@@ -420,10 +446,33 @@ static int pcf857x_remove(struct i2c_client *client)
 	return status;
 }
 
+static const struct of_device_id pcf857x_dt_ids[] = {
+	{ .compatible = "nxp,pca9670", },
+	{ .compatible = "nxp,pca9672", },
+	{ .compatible = "nxp,pca9674", },
+	{ .compatible = "nxp,pca8574", },
+	{ .compatible = "nxp,pca8575", },
+	{ .compatible = "nxp,pca9671", },
+	{ .compatible = "nxp,pca9673", },
+	{ .compatible = "nxp,pca9675", },
+
+	{ .compatible = "ti,pcf8574", },
+	{ .compatible = "ti,pcf8574a", },
+	{ .compatible = "ti,pcf8575", },
+	{ .compatible = "ti,tca9554", },
+
+	{ .compatible = "maxim,max7328", },
+	{ .compatible = "maxim,max7329", },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, pcf857x_dt_ids);
+
 static struct i2c_driver pcf857x_driver = {
 	.driver = {
-		.name	= "pcf857x",
-		.owner	= THIS_MODULE,
+		.name		= "pcf857x",
+		.owner		= THIS_MODULE,
+		.of_match_table	= pcf857x_dt_ids,
 	},
 	.probe	= pcf857x_probe,
 	.remove	= pcf857x_remove,
-- 
1.8.1.2

Re: [PATCH] gpio: Enable pcf857x GPIO expander for Device Tree

[Archit Taneja]

Hi Linus,

On Thursday 06 June 2013 07:35 PM, Archit Taneja wrote:
> Add code to parse the GPIO expander Device Tree node and extract platform data
> out of it, and populate the struct 'pcf857x_platform_data' maintained by the
> driver. This enables devices to reference the gpio expander from Device Tree.
>
> Add DT binding info in Documentation.

Any comments on this patch?

Thanks,
Archit

>
> CC: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
>   .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 44 +++++++++++++++++
>   drivers/gpio/gpio-pcf857x.c                        | 57 ++++++++++++++++++++--
>   2 files changed, 97 insertions(+), 4 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..0556512
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> @@ -0,0 +1,44 @@
> +PCF857x I2C based GPIO controller bindings
> +
> +Required properties:
> +- compatible:
> +  - "nxp,pca9670" for NXP PCA9670 8 bit I/O expander
> +  - "nxp,pca9672" for NXP PCA9672 8 bit I/O expander with interrupt
> +  - "nxp,pca9674" for NXP PCA9672 8 bit I/O expander with interrupt
> +  - "nxp,pca8574" for NXP PCA8574 8 bit I/O expander with interrupt
> +  - "nxp,pca8575" for NXP PCA8575 16 bit I/O expander with interrupt
> +  - "nxp,pca9671" for NXP PCA9671 16 bit I/O expander
> +  - "nxp,pca9673" for NXP PCA9673 16 bit I/O expander with interrupt
> +  - "nxp,pca9675" for NXP PCA9675 16 bit I/O expander with interrupt
> +  - "ti,pcf8574"  for TI PCF8574 8 bit I/O expander with interrupt
> +  - "ti,pcf8574a" for TI PCF8574A 8 bit I/O expander with interrupt
> +  - "ti,pcf8575"  for TI PCF8575 16 bit I/O expander with interrupt
> +  - "ti,tca9554"  for TI TCA9554 8 bit I/O expander with interrupt
> +  - "maxim,max7328" for MAXIM MAX7328 8 bit I/O expander with interrupt
> +  - "maxim,max7329" for MAXIM MAX7329 8 bit I/O expander with interrupt
> +- gpio-controller : Marks the device node as a GPIO controller.
> +- #gpio-cells : Should be two.
> +  - first cell is the pin number.
> +  - second cell is unused.
> +- interrupt-controller: Mark the device node as an interrupt controller.
> +- #interrupt-cells : Should be two.
> +  - first cell is the GPIO number.
> +  - second cell is unused.
> +- reg: I2C address of the chip.
> +
> +Device speific properties:
> +- n_latch:		optional bit-inverse of initial register value; if
> +			you leave this initialized to zero the driver will act
> +			like the chip was just reset.
> +
> +Example:
> +
> +pcf8575: pcf8575 {
> +	compatible = "ti,pcf8575";
> +	gpio-controller;
> +	#gpio-cells = <2>;
> +	interrupt-controller;
> +	#interrupt-cells = <2>;
> +	reg = <0x20>;
> +	n_latch = <0x0>;
> +};
> diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
> index e8faf53..3435790 100644
> --- a/drivers/gpio/gpio-pcf857x.c
> +++ b/drivers/gpio/gpio-pcf857x.c
> @@ -21,6 +21,7 @@
>   #include <linux/kernel.h>
>   #include <linux/slab.h>
>   #include <linux/gpio.h>
> +#include <linux/of.h>
>   #include <linux/i2c.h>
>   #include <linux/i2c/pcf857x.h>
>   #include <linux/interrupt.h>
> @@ -255,17 +256,42 @@ fail:
>
>   /*-------------------------------------------------------------------------*/
>
> +static struct pcf857x_platform_data *of_gpio_pcf857x(struct device *dev)
> +{
> +	struct pcf857x_platform_data *pdata;
> +	int r;
> +
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return NULL;
> +
> +	pdata->gpio_base = -1;
> +
> +	r = of_property_read_u32(dev->of_node, "n_latch", &pdata->n_latch);
> +	if (r) {
> +		dev_dbg(dev, "couldn't find n-latch, use default\n");
> +		pdata->n_latch = 0;
> +	}
> +
> +	return pdata;
> +}
> +
>   static int pcf857x_probe(struct i2c_client *client,
>   			 const struct i2c_device_id *id)
>   {
>   	struct pcf857x_platform_data	*pdata;
> +	struct device_node		*node;
>   	struct pcf857x			*gpio;
>   	int				status;
>
>   	pdata = client->dev.platform_data;
> -	if (!pdata) {
> +	node = client->dev.of_node;
> +
> +	if (!pdata && node)
> +		pdata = of_gpio_pcf857x(&client->dev);
> +
> +	if (!pdata)
>   		dev_dbg(&client->dev, "no platform data\n");
> -	}
>
>   	/* Allocate, initialize, and register this gpio_chip. */
>   	gpio = devm_kzalloc(&client->dev, sizeof(*gpio), GFP_KERNEL);
> @@ -420,10 +446,33 @@ static int pcf857x_remove(struct i2c_client *client)
>   	return status;
>   }
>
> +static const struct of_device_id pcf857x_dt_ids[] = {
> +	{ .compatible = "nxp,pca9670", },
> +	{ .compatible = "nxp,pca9672", },
> +	{ .compatible = "nxp,pca9674", },
> +	{ .compatible = "nxp,pca8574", },
> +	{ .compatible = "nxp,pca8575", },
> +	{ .compatible = "nxp,pca9671", },
> +	{ .compatible = "nxp,pca9673", },
> +	{ .compatible = "nxp,pca9675", },
> +
> +	{ .compatible = "ti,pcf8574", },
> +	{ .compatible = "ti,pcf8574a", },
> +	{ .compatible = "ti,pcf8575", },
> +	{ .compatible = "ti,tca9554", },
> +
> +	{ .compatible = "maxim,max7328", },
> +	{ .compatible = "maxim,max7329", },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, pcf857x_dt_ids);
> +
>   static struct i2c_driver pcf857x_driver = {
>   	.driver = {
> -		.name	= "pcf857x",
> -		.owner	= THIS_MODULE,
> +		.name		= "pcf857x",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= pcf857x_dt_ids,
>   	},
>   	.probe	= pcf857x_probe,
>   	.remove	= pcf857x_remove,
>

Re: [PATCH] gpio: Enable pcf857x GPIO expander for Device Tree

[Linus Walleij]

On Thu, Jun 6, 2013 at 4:05 PM, Archit Taneja <archit@ti.com> wrote:

> Add code to parse the GPIO expander Device Tree node and extract platform data
> out of it, and populate the struct 'pcf857x_platform_data' maintained by the
> driver. This enables devices to reference the gpio expander from Device Tree.
>
> Add DT binding info in Documentation.
>
> CC: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Archit Taneja <archit@ti.com>

(...)
> +++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> @@ -0,0 +1,44 @@
> +PCF857x I2C based GPIO controller bindings
> +
> +Required properties:
> +- compatible:
> +  - "nxp,pca9670" for NXP PCA9670 8 bit I/O expander
> +  - "nxp,pca9672" for NXP PCA9672 8 bit I/O expander with interrupt
> +  - "nxp,pca9674" for NXP PCA9672 8 bit I/O expander with interrupt
> +  - "nxp,pca8574" for NXP PCA8574 8 bit I/O expander with interrupt
> +  - "nxp,pca8575" for NXP PCA8575 16 bit I/O expander with interrupt
> +  - "nxp,pca9671" for NXP PCA9671 16 bit I/O expander
> +  - "nxp,pca9673" for NXP PCA9673 16 bit I/O expander with interrupt
> +  - "nxp,pca9675" for NXP PCA9675 16 bit I/O expander with interrupt
> +  - "ti,pcf8574"  for TI PCF8574 8 bit I/O expander with interrupt
> +  - "ti,pcf8574a" for TI PCF8574A 8 bit I/O expander with interrupt
> +  - "ti,pcf8575"  for TI PCF8575 16 bit I/O expander with interrupt
> +  - "ti,tca9554"  for TI TCA9554 8 bit I/O expander with interrupt
> +  - "maxim,max7328" for MAXIM MAX7328 8 bit I/O expander with interrupt
> +  - "maxim,max7329" for MAXIM MAX7329 8 bit I/O expander with interrupt
> +- gpio-controller : Marks the device node as a GPIO controller.
> +- #gpio-cells : Should be two.
> +  - first cell is the pin number.
> +  - second cell is unused.

I guess you're adding this because the generic GPIO bindings use it and
of_gpio_simple_xlate() depends on this two-cell layout.

Make a reference to the generic GPIO bindings and note that the
second cell is *NOT* unused, as it is used in the GPIOlib!

> +- interrupt-controller: Mark the device node as an interrupt controller.
> +- #interrupt-cells : Should be two.
> +  - first cell is the GPIO number.

Surely it is the IRQ number and not the GPIO number.
The fact that the IRQ originates in a GPIO controller does not
matter.

> +  - second cell is unused.

So why do you add it? Usually this is used for trigger flags.
Are you planning to add this later, i.e. does the chip support this,
and if it doesn't then get rid of this flag.

> +- reg: I2C address of the chip.
> +
> +Device speific properties:
> +- n_latch:             optional bit-inverse of initial register value; if
> +                       you leave this initialized to zero the driver will act
> +                       like the chip was just reset.

Explain what happens if you do *not* leave it as zero and what the
bits mean in that case.

Yours,
Linus Walleij

Re: [PATCH] gpio: Enable pcf857x GPIO expander for Device Tree

[Archit Taneja]

Hi,

On Monday 17 June 2013 02:35 PM, Linus Walleij wrote:
> On Thu, Jun 6, 2013 at 4:05 PM, Archit Taneja <archit@ti.com> wrote:
>
>> Add code to parse the GPIO expander Device Tree node and extract platform data
>> out of it, and populate the struct 'pcf857x_platform_data' maintained by the
>> driver. This enables devices to reference the gpio expander from Device Tree.
>>
>> Add DT binding info in Documentation.
>>
>> CC: Grant Likely <grant.likely@secretlab.ca>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>
> (...)
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
>> @@ -0,0 +1,44 @@
>> +PCF857x I2C based GPIO controller bindings
>> +
>> +Required properties:
>> +- compatible:
>> +  - "nxp,pca9670" for NXP PCA9670 8 bit I/O expander
>> +  - "nxp,pca9672" for NXP PCA9672 8 bit I/O expander with interrupt
>> +  - "nxp,pca9674" for NXP PCA9672 8 bit I/O expander with interrupt
>> +  - "nxp,pca8574" for NXP PCA8574 8 bit I/O expander with interrupt
>> +  - "nxp,pca8575" for NXP PCA8575 16 bit I/O expander with interrupt
>> +  - "nxp,pca9671" for NXP PCA9671 16 bit I/O expander
>> +  - "nxp,pca9673" for NXP PCA9673 16 bit I/O expander with interrupt
>> +  - "nxp,pca9675" for NXP PCA9675 16 bit I/O expander with interrupt
>> +  - "ti,pcf8574"  for TI PCF8574 8 bit I/O expander with interrupt
>> +  - "ti,pcf8574a" for TI PCF8574A 8 bit I/O expander with interrupt
>> +  - "ti,pcf8575"  for TI PCF8575 16 bit I/O expander with interrupt
>> +  - "ti,tca9554"  for TI TCA9554 8 bit I/O expander with interrupt
>> +  - "maxim,max7328" for MAXIM MAX7328 8 bit I/O expander with interrupt
>> +  - "maxim,max7329" for MAXIM MAX7329 8 bit I/O expander with interrupt
>> +- gpio-controller : Marks the device node as a GPIO controller.
>> +- #gpio-cells : Should be two.
>> +  - first cell is the pin number.
>> +  - second cell is unused.
>
> I guess you're adding this because the generic GPIO bindings use it and
> of_gpio_simple_xlate() depends on this two-cell layout.

Thanks for the review. I'm new to this and clearly lacking some 
knowledge here.

>
> Make a reference to the generic GPIO bindings and note that the
> second cell is *NOT* unused, as it is used in the GPIOlib!

Right, my mistake. Just a query, there is an example in gpio.txt in the 
gpio bindings documentation which sets #gpio-cells as 1. Is this is a 
wrong example, or are 1 cell gpio controllers valid?

>
>> +- interrupt-controller: Mark the device node as an interrupt controller.
>> +- #interrupt-cells : Should be two.
>> +  - first cell is the GPIO number.
>
> Surely it is the IRQ number and not the GPIO number.
> The fact that the IRQ originates in a GPIO controller does not
> matter.

Okay, I took gpio-omap.txt as reference(in other words, copy-pasted from 
there), I guess 'first cell is the GPIO number' means that a slave 
having it's interrupt line connected to an omap gpio bank has to mention 
the gpio number in the first cell.

About this chip, a change in any of it's GPIOs configured as inputs will 
generate an interrupt, then it's up to the driver to figure out which 
GPIOs changed and handle their corresponding irqs. So shouldn't a device 
connected to the chip describe the gpio number within the pcf857x chip 
as it's first cell?

I've made a hypothetical example of a pcf8575 chip, which has it's 
interrupt line connected to an omap gpio, and pcf8575's 7th gpio is 
connected to 'pcf_slave'. The pcf_slave's driver requests for an 
interrupt. Is this the correct way to describe this? :

pcf: pcf8575@23 {
	compatible = "ti,pcf8575";
	reg = <0x23>;
	gpio-controller;
	#gpio-cells = <2>;
	#interrupt-controller;
	#interrupt-cells = <1>;
	interrupt-parent = <&gpio2>;	/* an omap gpio bank */
	interrupts = <2 8>;		/* gpio line 34, low triggered*/
};

pcf_slave: slave {
	...
	...
	#interrupt-parent = <&pcf>;
	interrupts = <7>;	/* connected to 7th IO pin of pcf857x*/	
};

>
>> +  - second cell is unused.
>
> So why do you add it? Usually this is used for trigger flags.
> Are you planning to add this later, i.e. does the chip support this,
> and if it doesn't then get rid of this flag.

I haven't used the chip for interrupts, but going through the driver and 
it's platform_data struct for board files, I don't see any trigger 
information needed. I'll remove it.

>
>> +- reg: I2C address of the chip.
>> +
>> +Device speific properties:
>> +- n_latch:             optional bit-inverse of initial register value; if
>> +                       you leave this initialized to zero the driver will act
>> +                       like the chip was just reset.
>
> Explain what happens if you do *not* leave it as zero and what the
> bits mean in that case.

I'll do that. Apologies for the trivial issues.

Thanks,
Archit

Re: [PATCH] gpio: Enable pcf857x GPIO expander for Device Tree

[Linus Walleij]

On Tue, Jun 18, 2013 at 12:57 PM, Stijn Devriendt <highguy@gmail.com> wrote:
> On Thu, Jun 6, 2013 at 4:05 PM, Archit Taneja <archit@ti.com> wrote:

>> +static struct pcf857x_platform_data *of_gpio_pcf857x(struct device *dev)
>> +{
>> +       struct pcf857x_platform_data *pdata;
>> +       int r;
>> +
>> +       pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>
>
> This memory is never freed, is it?

Why should it, given it's allocated with devres?

Yours,
Linus Walleij

Re: [PATCH] gpio: Enable pcf857x GPIO expander for Device Tree

[Linus Walleij]

On Mon, Jun 17, 2013 at 1:46 PM, Archit Taneja <archit@ti.com> wrote:
> On Monday 17 June 2013 02:35 PM, Linus Walleij wrote:

>Just a query, there is an example in gpio.txt in the gpio
> bindings documentation which sets #gpio-cells as 1. Is this is a wrong
> example, or are 1 cell gpio controllers valid?

I don't think so. Try it and see if it works!

(If you want it, you may have to go in and fix drivers/gpio/gpiolib-of.c.)

> About this chip, a change in any of it's GPIOs configured as inputs will
> generate an interrupt, then it's up to the driver to figure out which GPIOs
> changed and handle their corresponding irqs. So shouldn't a device connected
> to the chip describe the gpio number within the pcf857x chip as it's first
> cell?

I guess so...

> I've made a hypothetical example of a pcf8575 chip, which has it's interrupt
> line connected to an omap gpio, and pcf8575's 7th gpio is connected to
> 'pcf_slave'. The pcf_slave's driver requests for an interrupt. Is this the
> correct way to describe this? :
>
> pcf: pcf8575@23 {
>         compatible = "ti,pcf8575";
>         reg = <0x23>;
>         gpio-controller;
>         #gpio-cells = <2>;
>         #interrupt-controller;
>         #interrupt-cells = <1>;
>         interrupt-parent = <&gpio2>;    /* an omap gpio bank */
>         interrupts = <2 8>;             /* gpio line 34, low triggered*/
> };
>
> pcf_slave: slave {
>         ...
>         ...
>         #interrupt-parent = <&pcf>;
>         interrupts = <7>;       /* connected to 7th IO pin of pcf857x*/
> };

There are two paths for dereferencing GPIOs and IRQs.

Simple approach:

give your slave a gpios = <...>;

and in the driver use gpio_to_irq() to dereference an IRQ number
from the GPIO number you get. The IRQdomain etc in the
GPIO driver will take care of the rest.

How to code up a driver so that it can use irqs directly from a GPIO
controller without referring to the GPIO line it is tied into is currently
quite unclear. Atleast to me. It's been discussed for the OMAP
case so search the archives...

Yours,
Linus Walleij

[PATCH v2] gpio: Enable pcf857x GPIO expander for Device Tree

[Archit Taneja]

Add code to parse the GPIO expander Device Tree node and extract platform data
out of it, and populate the struct 'pcf857x_platform_data' maintained by the
driver. This enables devices to reference the gpio expander from Device Tree.

Add DT binding info in Documentation.

Signed-off-by: Archit Taneja <archit@ti.com>
---
- v2 posted after quite a while, sorry for the delay!

Changes in v2:
- second gpeio-cell description corrected in Documentation
- interrupt controller description removed for now, will be added in a follow
  up patch
- n_latch description updated

 .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 41 ++++++++++++++++
 drivers/gpio/gpio-pcf857x.c                        | 57 ++++++++++++++++++++--
 2 files changed, 94 insertions(+), 4 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..504d114
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
@@ -0,0 +1,41 @@
+PCF857x I2C based GPIO controller bindings
+
+Required properties:
+- compatible:
+  - "nxp,pca9670" for NXP PCA9670 8 bit I/O expander
+  - "nxp,pca9672" for NXP PCA9672 8 bit I/O expander with interrupt
+  - "nxp,pca9674" for NXP PCA9672 8 bit I/O expander with interrupt
+  - "nxp,pca8574" for NXP PCA8574 8 bit I/O expander with interrupt
+  - "nxp,pca8575" for NXP PCA8575 16 bit I/O expander with interrupt
+  - "nxp,pca9671" for NXP PCA9671 16 bit I/O expander
+  - "nxp,pca9673" for NXP PCA9673 16 bit I/O expander with interrupt
+  - "nxp,pca9675" for NXP PCA9675 16 bit I/O expander with interrupt
+  - "ti,pcf8574"  for TI PCF8574 8 bit I/O expander with interrupt
+  - "ti,pcf8574a" for TI PCF8574A 8 bit I/O expander with interrupt
+  - "ti,pcf8575"  for TI PCF8575 16 bit I/O expander with interrupt
+  - "ti,tca9554"  for TI TCA9554 8 bit I/O expander with interrupt
+  - "maxim,max7328" for MAXIM MAX7328 8 bit I/O expander with interrupt
+  - "maxim,max7329" for MAXIM MAX7329 8 bit I/O expander with interrupt
+- gpio-controller : Marks the device node as a GPIO controller.
+- #gpio-cells : Should be two.
+  - first cell is the pin number.
+  - second cell is used to specify optional parameters (currently unused, but
+    needed by gpiolib).
+- reg: I2C address of the chip.
+
+Device speific properties:
+- n_latch:		optional bit-inverse of initial register value; if
+			you leave this initialized to zero the driver will act
+			like the chip was just reset. If it is a non-zero
+			value, the driver will program the inverted value of
+			n_latch as the initial state of the gpios.
+
+Example:
+
+pcf8575: pcf8575 {
+	compatible = "ti,pcf8575";
+	gpio-controller;
+	#gpio-cells = <2>;
+	reg = <0x20>;
+	n_latch = <0x0>;
+};
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index e8faf53..3435790 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -21,6 +21,7 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/gpio.h>
+#include <linux/of.h>
 #include <linux/i2c.h>
 #include <linux/i2c/pcf857x.h>
 #include <linux/interrupt.h>
@@ -255,17 +256,42 @@ fail:
 
 /*-------------------------------------------------------------------------*/
 
+static struct pcf857x_platform_data *of_gpio_pcf857x(struct device *dev)
+{
+	struct pcf857x_platform_data *pdata;
+	int r;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	pdata->gpio_base = -1;
+
+	r = of_property_read_u32(dev->of_node, "n_latch", &pdata->n_latch);
+	if (r) {
+		dev_dbg(dev, "couldn't find n-latch, use default\n");
+		pdata->n_latch = 0;
+	}
+
+	return pdata;
+}
+
 static int pcf857x_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct pcf857x_platform_data	*pdata;
+	struct device_node		*node;
 	struct pcf857x			*gpio;
 	int				status;
 
 	pdata = client->dev.platform_data;
-	if (!pdata) {
+	node = client->dev.of_node;
+
+	if (!pdata && node)
+		pdata = of_gpio_pcf857x(&client->dev);
+
+	if (!pdata)
 		dev_dbg(&client->dev, "no platform data\n");
-	}
 
 	/* Allocate, initialize, and register this gpio_chip. */
 	gpio = devm_kzalloc(&client->dev, sizeof(*gpio), GFP_KERNEL);
@@ -420,10 +446,33 @@ static int pcf857x_remove(struct i2c_client *client)
 	return status;
 }
 
+static const struct of_device_id pcf857x_dt_ids[] = {
+	{ .compatible = "nxp,pca9670", },
+	{ .compatible = "nxp,pca9672", },
+	{ .compatible = "nxp,pca9674", },
+	{ .compatible = "nxp,pca8574", },
+	{ .compatible = "nxp,pca8575", },
+	{ .compatible = "nxp,pca9671", },
+	{ .compatible = "nxp,pca9673", },
+	{ .compatible = "nxp,pca9675", },
+
+	{ .compatible = "ti,pcf8574", },
+	{ .compatible = "ti,pcf8574a", },
+	{ .compatible = "ti,pcf8575", },
+	{ .compatible = "ti,tca9554", },
+
+	{ .compatible = "maxim,max7328", },
+	{ .compatible = "maxim,max7329", },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, pcf857x_dt_ids);
+
 static struct i2c_driver pcf857x_driver = {
 	.driver = {
-		.name	= "pcf857x",
-		.owner	= THIS_MODULE,
+		.name		= "pcf857x",
+		.owner		= THIS_MODULE,
+		.of_match_table	= pcf857x_dt_ids,
 	},
 	.probe	= pcf857x_probe,
 	.remove	= pcf857x_remove,
-- 
1.8.1.2

Re: [PATCH v2] gpio: Enable pcf857x GPIO expander for Device Tree

[Linus Walleij]

On Mon, Aug 26, 2013 at 8:21 AM, Archit Taneja <archit@ti.com> wrote:

> Add code to parse the GPIO expander Device Tree node and extract platform data
> out of it, and populate the struct 'pcf857x_platform_data' maintained by the
> driver. This enables devices to reference the gpio expander from Device Tree.
>
> Add DT binding info in Documentation.
>
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
> - v2 posted after quite a while, sorry for the delay!
>
> Changes in v2:
> - second gpeio-cell description corrected in Documentation
> - interrupt controller description removed for now, will be added in a follow
>   up patch
> - n_latch description updated

The devicetree portions of this patch needs to be reviewed by
devicetree@vger.kernel.org, so please repost including that
list.

Yours,
Linus Walleij

[RESEND PATCH v2] gpio: Enable pcf857x GPIO expander for Device Tree

[Archit Taneja]

Add code to parse the GPIO expander Device Tree node and extract platform data
out of it, and populate the struct 'pcf857x_platform_data' maintained by the
driver. This enables devices to reference the gpio expander from Device Tree.

Add DT binding info in Documentation.

Signed-off-by: Archit Taneja <archit@ti.com>
---
- v2 posted after quite a while, sorry for the delay!

Changes in v2:
- second gpeio-cell description corrected in Documentation
- interrupt controller description removed for now, will be added in a follow
  up patch
- n_latch description updated

 .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 41 ++++++++++++++++
 drivers/gpio/gpio-pcf857x.c                        | 57 ++++++++++++++++++++--
 2 files changed, 94 insertions(+), 4 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..504d114
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
@@ -0,0 +1,41 @@
+PCF857x I2C based GPIO controller bindings
+
+Required properties:
+- compatible:
+  - "nxp,pca9670" for NXP PCA9670 8 bit I/O expander
+  - "nxp,pca9672" for NXP PCA9672 8 bit I/O expander with interrupt
+  - "nxp,pca9674" for NXP PCA9672 8 bit I/O expander with interrupt
+  - "nxp,pca8574" for NXP PCA8574 8 bit I/O expander with interrupt
+  - "nxp,pca8575" for NXP PCA8575 16 bit I/O expander with interrupt
+  - "nxp,pca9671" for NXP PCA9671 16 bit I/O expander
+  - "nxp,pca9673" for NXP PCA9673 16 bit I/O expander with interrupt
+  - "nxp,pca9675" for NXP PCA9675 16 bit I/O expander with interrupt
+  - "ti,pcf8574"  for TI PCF8574 8 bit I/O expander with interrupt
+  - "ti,pcf8574a" for TI PCF8574A 8 bit I/O expander with interrupt
+  - "ti,pcf8575"  for TI PCF8575 16 bit I/O expander with interrupt
+  - "ti,tca9554"  for TI TCA9554 8 bit I/O expander with interrupt
+  - "maxim,max7328" for MAXIM MAX7328 8 bit I/O expander with interrupt
+  - "maxim,max7329" for MAXIM MAX7329 8 bit I/O expander with interrupt
+- gpio-controller : Marks the device node as a GPIO controller.
+- #gpio-cells : Should be two.
+  - first cell is the pin number.
+  - second cell is used to specify optional parameters (currently unused, but
+    needed by gpiolib).
+- reg: I2C address of the chip.
+
+Device speific properties:
+- n_latch:		optional bit-inverse of initial register value; if
+			you leave this initialized to zero the driver will act
+			like the chip was just reset. If it is a non-zero
+			value, the driver will program the inverted value of
+			n_latch as the initial state of the gpios.
+
+Example:
+
+pcf8575: pcf8575 {
+	compatible = "ti,pcf8575";
+	gpio-controller;
+	#gpio-cells = <2>;
+	reg = <0x20>;
+	n_latch = <0x0>;
+};
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index e8faf53..3435790 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -21,6 +21,7 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/gpio.h>
+#include <linux/of.h>
 #include <linux/i2c.h>
 #include <linux/i2c/pcf857x.h>
 #include <linux/interrupt.h>
@@ -255,17 +256,42 @@ fail:
 
 /*-------------------------------------------------------------------------*/
 
+static struct pcf857x_platform_data *of_gpio_pcf857x(struct device *dev)
+{
+	struct pcf857x_platform_data *pdata;
+	int r;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	pdata->gpio_base = -1;
+
+	r = of_property_read_u32(dev->of_node, "n_latch", &pdata->n_latch);
+	if (r) {
+		dev_dbg(dev, "couldn't find n-latch, use default\n");
+		pdata->n_latch = 0;
+	}
+
+	return pdata;
+}
+
 static int pcf857x_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct pcf857x_platform_data	*pdata;
+	struct device_node		*node;
 	struct pcf857x			*gpio;
 	int				status;
 
 	pdata = client->dev.platform_data;
-	if (!pdata) {
+	node = client->dev.of_node;
+
+	if (!pdata && node)
+		pdata = of_gpio_pcf857x(&client->dev);
+
+	if (!pdata)
 		dev_dbg(&client->dev, "no platform data\n");
-	}
 
 	/* Allocate, initialize, and register this gpio_chip. */
 	gpio = devm_kzalloc(&client->dev, sizeof(*gpio), GFP_KERNEL);
@@ -420,10 +446,33 @@ static int pcf857x_remove(struct i2c_client *client)
 	return status;
 }
 
+static const struct of_device_id pcf857x_dt_ids[] = {
+	{ .compatible = "nxp,pca9670", },
+	{ .compatible = "nxp,pca9672", },
+	{ .compatible = "nxp,pca9674", },
+	{ .compatible = "nxp,pca8574", },
+	{ .compatible = "nxp,pca8575", },
+	{ .compatible = "nxp,pca9671", },
+	{ .compatible = "nxp,pca9673", },
+	{ .compatible = "nxp,pca9675", },
+
+	{ .compatible = "ti,pcf8574", },
+	{ .compatible = "ti,pcf8574a", },
+	{ .compatible = "ti,pcf8575", },
+	{ .compatible = "ti,tca9554", },
+
+	{ .compatible = "maxim,max7328", },
+	{ .compatible = "maxim,max7329", },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, pcf857x_dt_ids);
+
 static struct i2c_driver pcf857x_driver = {
 	.driver = {
-		.name	= "pcf857x",
-		.owner	= THIS_MODULE,
+		.name		= "pcf857x",
+		.owner		= THIS_MODULE,
+		.of_match_table	= pcf857x_dt_ids,
 	},
 	.probe	= pcf857x_probe,
 	.remove	= pcf857x_remove,
-- 
1.8.1.2

Re: [PATCH v2] gpio: Enable pcf857x GPIO expander for Device Tree

[Laurent Pinchart]

On Tuesday 27 August 2013 05:20:45 Linus Walleij wrote:
> On Mon, Aug 26, 2013 at 8:21 AM, Archit Taneja <archit@ti.com> wrote:
> > Add code to parse the GPIO expander Device Tree node and extract platform
> > data out of it, and populate the struct 'pcf857x_platform_data'
> > maintained by the driver. This enables devices to reference the gpio
> > expander from Device Tree.
> > 
> > Add DT binding info in Documentation.
> > 
> > Signed-off-by: Archit Taneja <archit@ti.com>
> > ---
> > - v2 posted after quite a while, sorry for the delay!
> > 
> > Changes in v2:
> > - second gpeio-cell description corrected in Documentation
> > - interrupt controller description removed for now, will be added in a
> >   follow up patch
> > - n_latch description updated

And it seems that the patch conflicts with

https://lkml.org/lkml/2013/8/26/248

:-/

If we end up merging this one, I'd like to at least rename the DT n-latch 
property to pin-initial-states as in my patch and update the documentation, as 
I find it more explicit.

> The devicetree portions of this patch needs to be reviewed by
> devicetree@vger.kernel.org, so please repost including that
> list.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] gpio: Enable pcf857x GPIO expander for Device Tree

[Archit Taneja]

On Tuesday 27 August 2013 12:17 PM, Laurent Pinchart wrote:
> On Tuesday 27 August 2013 05:20:45 Linus Walleij wrote:
>> On Mon, Aug 26, 2013 at 8:21 AM, Archit Taneja <archit@ti.com> wrote:
>>> Add code to parse the GPIO expander Device Tree node and extract platform
>>> data out of it, and populate the struct 'pcf857x_platform_data'
>>> maintained by the driver. This enables devices to reference the gpio
>>> expander from Device Tree.
>>>
>>> Add DT binding info in Documentation.
>>>
>>> Signed-off-by: Archit Taneja <archit@ti.com>
>>> ---
>>> - v2 posted after quite a while, sorry for the delay!
>>>
>>> Changes in v2:
>>> - second gpeio-cell description corrected in Documentation
>>> - interrupt controller description removed for now, will be added in a
>>>    follow up patch
>>> - n_latch description updated
>
> And it seems that the patch conflicts with
>
> https://lkml.org/lkml/2013/8/26/248
>
> :-/

:/

>
> If we end up merging this one, I'd like to at least rename the DT n-latch
> property to pin-initial-states as in my patch and update the documentation, as
> I find it more explicit.

I think your patch is cleaner and more descriptive. The only thing which 
I am not clear about in your patch is the extra work done in getting 
ngpio, that should be available in i2c_device_id pointer argument in 
probe(id points to the array pcf857x_id already available in the driver 
with the number of gpios for each chip variant).

Archit

Re: [PATCH v2] gpio: Enable pcf857x GPIO expander for Device Tree

[Laurent Pinchart]

Hi Archit,

On Tuesday 27 August 2013 12:30:18 Archit Taneja wrote:
> On Tuesday 27 August 2013 12:17 PM, Laurent Pinchart wrote:
> > On Tuesday 27 August 2013 05:20:45 Linus Walleij wrote:
> >> On Mon, Aug 26, 2013 at 8:21 AM, Archit Taneja <archit@ti.com> wrote:
> >>> Add code to parse the GPIO expander Device Tree node and extract
> >>> platform
> >>> data out of it, and populate the struct 'pcf857x_platform_data'
> >>> maintained by the driver. This enables devices to reference the gpio
> >>> expander from Device Tree.
> >>> 
> >>> Add DT binding info in Documentation.
> >>> 
> >>> Signed-off-by: Archit Taneja <archit@ti.com>
> >>> ---
> >>> - v2 posted after quite a while, sorry for the delay!
> >>> 
> >>> Changes in v2:
> >>> - second gpeio-cell description corrected in Documentation
> >>> - interrupt controller description removed for now, will be added in a
> >>> 
> >>>    follow up patch
> >>> 
> >>> - n_latch description updated
> > 
> > And it seems that the patch conflicts with
> > 
> > https://lkml.org/lkml/2013/8/26/248
> > 
> > :-/
>
> :/
>
> > If we end up merging this one, I'd like to at least rename the DT n-latch
> > property to pin-initial-states as in my patch and update the
> > documentation, as I find it more explicit.
> 
> I think your patch is cleaner and more descriptive.

Thank you.

> The only thing which I am not clear about in your patch is the extra work
> done in getting ngpio, that should be available in i2c_device_id pointer
> argument in probe(id points to the array pcf857x_id already available in the
> driver with the number of gpios for each chip variant).

You're right. I'll fix that and repost.

-- 
Regards,

Laurent Pinchart