[PATCH v5 0/5] GPIO: pca953x.c: Fix IRQ support and OF device-tree bindings

[PATCH v5 0/5] GPIO: pca953x.c: Fix IRQ support and OF device-tree bindings

[David Jander]

This patch series fixes IRQ support and cleans up OF device-tree support for
the PCA953X gpio driver.
This is version 5 of the patch series.

Changes in this version:
 - new: GPIO: pca953x.c: Remove dynamic platform data pointer
 - Reverted completely removing of device-tree binding properties, and
   deprecated them instead issueing WARN() if they are used.

David Jander (5):
  GPIO: pca953x.c: Fix IRQ support.
  GPIO: pca953x.c: Remove dynamic platform data pointer
  GPIO: pca953x.c: Deprecate meaningless device-tree bindings
  GPIO: pca953x.c: Interrupt pin is active-low
  GPIO: pca953x.c: Fix warning of enabled interrupts in handler

 drivers/gpio/pca953x.c |  103 ++++++++++++++++++++++--------------------------
 1 files changed, 47 insertions(+), 56 deletions(-)

-- 
1.7.4.1

[PATCH v5 1/5] GPIO: pca953x.c: Fix IRQ support.

[David Jander]

It seems that in the normal case, IRQ_NOREQUEST needs to be explicitly
cleared, otherwise claiming the interrupt fails.
In the case of sparse interrupts, the descriptor needs to be allocated
first.

Signed-off-by: David Jander <david@protonic.nl>
---
 drivers/gpio/pca953x.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/pca953x.c b/drivers/gpio/pca953x.c
index 0451d7a..5f6940a 100644
--- a/drivers/gpio/pca953x.c
+++ b/drivers/gpio/pca953x.c
@@ -474,12 +474,16 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 		 * this purpose.
 		 */
 		chip->irq_stat &= chip->reg_direction;
-		chip->irq_base = pdata->irq_base;
 		mutex_init(&chip->irq_lock);
 
+		chip->irq_base = irq_alloc_descs(-1, pdata->irq_base, chip->gpio_chip.ngpio, -1);
+		if (chip->irq_base < 0)
+			goto out_failed;
+
 		for (lvl = 0; lvl < chip->gpio_chip.ngpio; lvl++) {
 			int irq = lvl + chip->irq_base;
 
+			irq_clear_status_flags(irq, IRQ_NOREQUEST);
 			irq_set_chip_data(irq, chip);
 			irq_set_chip_and_handler(irq, &pca953x_irq_chip,
 						 handle_simple_irq);
@@ -514,8 +518,10 @@ out_failed:
 
 static void pca953x_irq_teardown(struct pca953x_chip *chip)
 {
-	if (chip->irq_base != -1)
+	if (chip->irq_base != -1) {
+		irq_free_descs(chip->irq_base, chip->gpio_chip.ngpio);
 		free_irq(chip->client->irq, chip);
+	}
 }
 #else /* CONFIG_GPIO_PCA953X_IRQ */
 static int pca953x_irq_setup(struct pca953x_chip *chip,
-- 
1.7.4.1

Re: [PATCH v5 1/5] GPIO: pca953x.c: Fix IRQ support.

[Grant Likely]

On Tue, Jun 14, 2011 at 11:00:54AM +0200, David Jander wrote:
> It seems that in the normal case, IRQ_NOREQUEST needs to be explicitly
> cleared, otherwise claiming the interrupt fails.
> In the case of sparse interrupts, the descriptor needs to be allocated
> first.
> 
> Signed-off-by: David Jander <david@protonic.nl>

Merged, thanks.

g.

> ---
>  drivers/gpio/pca953x.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/pca953x.c b/drivers/gpio/pca953x.c
> index 0451d7a..5f6940a 100644
> --- a/drivers/gpio/pca953x.c
> +++ b/drivers/gpio/pca953x.c
> @@ -474,12 +474,16 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
>  		 * this purpose.
>  		 */
>  		chip->irq_stat &= chip->reg_direction;
> -		chip->irq_base = pdata->irq_base;
>  		mutex_init(&chip->irq_lock);
>  
> +		chip->irq_base = irq_alloc_descs(-1, pdata->irq_base, chip->gpio_chip.ngpio, -1);
> +		if (chip->irq_base < 0)
> +			goto out_failed;
> +
>  		for (lvl = 0; lvl < chip->gpio_chip.ngpio; lvl++) {
>  			int irq = lvl + chip->irq_base;
>  
> +			irq_clear_status_flags(irq, IRQ_NOREQUEST);
>  			irq_set_chip_data(irq, chip);
>  			irq_set_chip_and_handler(irq, &pca953x_irq_chip,
>  						 handle_simple_irq);
> @@ -514,8 +518,10 @@ out_failed:
>  
>  static void pca953x_irq_teardown(struct pca953x_chip *chip)
>  {
> -	if (chip->irq_base != -1)
> +	if (chip->irq_base != -1) {
> +		irq_free_descs(chip->irq_base, chip->gpio_chip.ngpio);
>  		free_irq(chip->client->irq, chip);
> +	}
>  }
>  #else /* CONFIG_GPIO_PCA953X_IRQ */
>  static int pca953x_irq_setup(struct pca953x_chip *chip,
> -- 
> 1.7.4.1
> 

[PATCH v5 2/5] GPIO: pca953x.c: Remove dynamic platform data pointer

[David Jander]

In the case that we obtain device-tree data to fill in platform_data, the new
platform data struct was dynamically allocated, but the pointer to it was not
used everywhere it should. It seems easier to fix this issue by removing the
dynamic allocation altogether since its data is only used during driver
probing.

Signed-off-by: David Jander <david@protonic.nl>
---
 drivers/gpio/pca953x.c |   77 +++++++++++++++++------------------------------
 1 files changed, 28 insertions(+), 49 deletions(-)

diff --git a/drivers/gpio/pca953x.c b/drivers/gpio/pca953x.c
index 5f6940a..db69d9e 100644
--- a/drivers/gpio/pca953x.c
+++ b/drivers/gpio/pca953x.c
@@ -85,7 +85,6 @@ struct pca953x_chip {
 #endif
 
 	struct i2c_client *client;
-	struct pca953x_platform_data *dyn_pdata;
 	struct gpio_chip gpio_chip;
 	const char *const *names;
 	int	chip_type;
@@ -446,13 +445,13 @@ static irqreturn_t pca953x_irq_handler(int irq, void *devid)
 }
 
 static int pca953x_irq_setup(struct pca953x_chip *chip,
-			     const struct i2c_device_id *id)
+			     const struct i2c_device_id *id,
+			     int irq_base)
 {
 	struct i2c_client *client = chip->client;
-	struct pca953x_platform_data *pdata = client->dev.platform_data;
 	int ret, offset = 0;
 
-	if (pdata->irq_base != -1
+	if (irq_base != -1
 			&& (id->driver_data & PCA_INT)) {
 		int lvl;
 
@@ -476,7 +475,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 		chip->irq_stat &= chip->reg_direction;
 		mutex_init(&chip->irq_lock);
 
-		chip->irq_base = irq_alloc_descs(-1, pdata->irq_base, chip->gpio_chip.ngpio, -1);
+		chip->irq_base = irq_alloc_descs(-1, irq_base, chip->gpio_chip.ngpio, -1);
 		if (chip->irq_base < 0)
 			goto out_failed;
 
@@ -525,12 +524,12 @@ static void pca953x_irq_teardown(struct pca953x_chip *chip)
 }
 #else /* CONFIG_GPIO_PCA953X_IRQ */
 static int pca953x_irq_setup(struct pca953x_chip *chip,
-			     const struct i2c_device_id *id)
+			     const struct i2c_device_id *id,
+			     int irq_base)
 {
 	struct i2c_client *client = chip->client;
-	struct pca953x_platform_data *pdata = client->dev.platform_data;
 
-	if (pdata->irq_base != -1 && (id->driver_data & PCA_INT))
+	if (irq_base != -1 && (id->driver_data & PCA_INT))
 		dev_warn(&client->dev, "interrupt support not compiled in\n");
 
 	return 0;
@@ -548,45 +547,35 @@ static void pca953x_irq_teardown(struct pca953x_chip *chip)
 /*
  * Translate OpenFirmware node properties into platform_data
  */
-static struct pca953x_platform_data *
-pca953x_get_alt_pdata(struct i2c_client *client)
+void
+pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, int *invert)
 {
-	struct pca953x_platform_data *pdata;
 	struct device_node *node;
 	const __be32 *val;
 	int size;
 
 	node = client->dev.of_node;
 	if (node == NULL)
-		return NULL;
+		return;
 
-	pdata = kzalloc(sizeof(struct pca953x_platform_data), GFP_KERNEL);
-	if (pdata == NULL) {
-		dev_err(&client->dev, "Unable to allocate platform_data\n");
-		return NULL;
-	}
-
-	pdata->gpio_base = -1;
+	*gpio_base = -1;
 	val = of_get_property(node, "linux,gpio-base", &size);
 	if (val) {
 		if (size != sizeof(*val))
 			dev_warn(&client->dev, "%s: wrong linux,gpio-base\n",
 				 node->full_name);
 		else
-			pdata->gpio_base = be32_to_cpup(val);
+			*gpio_base = be32_to_cpup(val);
 	}
 
 	val = of_get_property(node, "polarity", NULL);
 	if (val)
-		pdata->invert = *val;
-
-	return pdata;
+		*invert = *val;
 }
 #else
-static struct pca953x_platform_data *
-pca953x_get_alt_pdata(struct i2c_client *client)
+void
+pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, int *invert)
 {
-	return NULL;
 }
 #endif
 
@@ -648,6 +637,7 @@ static int __devinit pca953x_probe(struct i2c_client *client,
 {
 	struct pca953x_platform_data *pdata;
 	struct pca953x_chip *chip;
+	int irq_base=-1, invert=0;
 	int ret = 0;
 
 	chip = kzalloc(sizeof(struct pca953x_chip), GFP_KERNEL);
@@ -655,26 +645,17 @@ static int __devinit pca953x_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	pdata = client->dev.platform_data;
-	if (pdata == NULL) {
-		pdata = pca953x_get_alt_pdata(client);
-		/*
-		 * Unlike normal platform_data, this is allocated
-		 * dynamically and must be freed in the driver
-		 */
-		chip->dyn_pdata = pdata;
-	}
-
-	if (pdata == NULL) {
-		dev_dbg(&client->dev, "no platform data\n");
-		ret = -EINVAL;
-		goto out_failed;
+	if (pdata) {
+		irq_base = pdata->irq_base;
+		chip->gpio_start = pdata->gpio_base;
+		invert = pdata->invert;
+		chip->names = pdata->names;
+	} else {
+		pca953x_get_alt_pdata(client, &chip->gpio_start, &invert);
 	}
 
 	chip->client = client;
 
-	chip->gpio_start = pdata->gpio_base;
-
-	chip->names = pdata->names;
 	chip->chip_type = id->driver_data & (PCA953X_TYPE | PCA957X_TYPE);
 
 	mutex_init(&chip->i2c_lock);
@@ -685,13 +666,13 @@ static int __devinit pca953x_probe(struct i2c_client *client,
 	pca953x_setup_gpio(chip, id->driver_data & PCA_GPIO_MASK);
 
 	if (chip->chip_type == PCA953X_TYPE)
-		device_pca953x_init(chip, pdata->invert);
+		device_pca953x_init(chip, invert);
 	else if (chip->chip_type == PCA957X_TYPE)
-		device_pca957x_init(chip, pdata->invert);
+		device_pca957x_init(chip, invert);
 	else
 		goto out_failed;
 
-	ret = pca953x_irq_setup(chip, id);
+	ret = pca953x_irq_setup(chip, id, irq_base);
 	if (ret)
 		goto out_failed;
 
@@ -699,7 +680,7 @@ static int __devinit pca953x_probe(struct i2c_client *client,
 	if (ret)
 		goto out_failed_irq;
 
-	if (pdata->setup) {
+	if (pdata && pdata->setup) {
 		ret = pdata->setup(client, chip->gpio_chip.base,
 				chip->gpio_chip.ngpio, pdata->context);
 		if (ret < 0)
@@ -712,7 +693,6 @@ static int __devinit pca953x_probe(struct i2c_client *client,
 out_failed_irq:
 	pca953x_irq_teardown(chip);
 out_failed:
-	kfree(chip->dyn_pdata);
 	kfree(chip);
 	return ret;
 }
@@ -723,7 +703,7 @@ static int pca953x_remove(struct i2c_client *client)
 	struct pca953x_chip *chip = i2c_get_clientdata(client);
 	int ret = 0;
 
-	if (pdata->teardown) {
+	if (pdata && pdata->teardown) {
 		ret = pdata->teardown(client, chip->gpio_chip.base,
 				chip->gpio_chip.ngpio, pdata->context);
 		if (ret < 0) {
@@ -741,7 +721,6 @@ static int pca953x_remove(struct i2c_client *client)
 	}
 
 	pca953x_irq_teardown(chip);
-	kfree(chip->dyn_pdata);
 	kfree(chip);
 	return 0;
 }
-- 
1.7.4.1

Re: [PATCH v5 2/5] GPIO: pca953x.c: Remove dynamic platform data pointer

[Grant Likely]

On Tue, Jun 14, 2011 at 11:00:55AM +0200, David Jander wrote:
> In the case that we obtain device-tree data to fill in platform_data, the new
> platform data struct was dynamically allocated, but the pointer to it was not
> used everywhere it should. It seems easier to fix this issue by removing the
> dynamic allocation altogether since its data is only used during driver
> probing.
> 
> Signed-off-by: David Jander <david@protonic.nl>

Merged, thanks.

g.

> ---
>  drivers/gpio/pca953x.c |   77 +++++++++++++++++------------------------------
>  1 files changed, 28 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/gpio/pca953x.c b/drivers/gpio/pca953x.c
> index 5f6940a..db69d9e 100644
> --- a/drivers/gpio/pca953x.c
> +++ b/drivers/gpio/pca953x.c
> @@ -85,7 +85,6 @@ struct pca953x_chip {
>  #endif
>  
>  	struct i2c_client *client;
> -	struct pca953x_platform_data *dyn_pdata;
>  	struct gpio_chip gpio_chip;
>  	const char *const *names;
>  	int	chip_type;
> @@ -446,13 +445,13 @@ static irqreturn_t pca953x_irq_handler(int irq, void *devid)
>  }
>  
>  static int pca953x_irq_setup(struct pca953x_chip *chip,
> -			     const struct i2c_device_id *id)
> +			     const struct i2c_device_id *id,
> +			     int irq_base)
>  {
>  	struct i2c_client *client = chip->client;
> -	struct pca953x_platform_data *pdata = client->dev.platform_data;
>  	int ret, offset = 0;
>  
> -	if (pdata->irq_base != -1
> +	if (irq_base != -1
>  			&& (id->driver_data & PCA_INT)) {
>  		int lvl;
>  
> @@ -476,7 +475,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
>  		chip->irq_stat &= chip->reg_direction;
>  		mutex_init(&chip->irq_lock);
>  
> -		chip->irq_base = irq_alloc_descs(-1, pdata->irq_base, chip->gpio_chip.ngpio, -1);
> +		chip->irq_base = irq_alloc_descs(-1, irq_base, chip->gpio_chip.ngpio, -1);
>  		if (chip->irq_base < 0)
>  			goto out_failed;
>  
> @@ -525,12 +524,12 @@ static void pca953x_irq_teardown(struct pca953x_chip *chip)
>  }
>  #else /* CONFIG_GPIO_PCA953X_IRQ */
>  static int pca953x_irq_setup(struct pca953x_chip *chip,
> -			     const struct i2c_device_id *id)
> +			     const struct i2c_device_id *id,
> +			     int irq_base)
>  {
>  	struct i2c_client *client = chip->client;
> -	struct pca953x_platform_data *pdata = client->dev.platform_data;
>  
> -	if (pdata->irq_base != -1 && (id->driver_data & PCA_INT))
> +	if (irq_base != -1 && (id->driver_data & PCA_INT))
>  		dev_warn(&client->dev, "interrupt support not compiled in\n");
>  
>  	return 0;
> @@ -548,45 +547,35 @@ static void pca953x_irq_teardown(struct pca953x_chip *chip)
>  /*
>   * Translate OpenFirmware node properties into platform_data
>   */
> -static struct pca953x_platform_data *
> -pca953x_get_alt_pdata(struct i2c_client *client)
> +void
> +pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, int *invert)
>  {
> -	struct pca953x_platform_data *pdata;
>  	struct device_node *node;
>  	const __be32 *val;
>  	int size;
>  
>  	node = client->dev.of_node;
>  	if (node == NULL)
> -		return NULL;
> +		return;
>  
> -	pdata = kzalloc(sizeof(struct pca953x_platform_data), GFP_KERNEL);
> -	if (pdata == NULL) {
> -		dev_err(&client->dev, "Unable to allocate platform_data\n");
> -		return NULL;
> -	}
> -
> -	pdata->gpio_base = -1;
> +	*gpio_base = -1;
>  	val = of_get_property(node, "linux,gpio-base", &size);
>  	if (val) {
>  		if (size != sizeof(*val))
>  			dev_warn(&client->dev, "%s: wrong linux,gpio-base\n",
>  				 node->full_name);
>  		else
> -			pdata->gpio_base = be32_to_cpup(val);
> +			*gpio_base = be32_to_cpup(val);
>  	}
>  
>  	val = of_get_property(node, "polarity", NULL);
>  	if (val)
> -		pdata->invert = *val;
> -
> -	return pdata;
> +		*invert = *val;
>  }
>  #else
> -static struct pca953x_platform_data *
> -pca953x_get_alt_pdata(struct i2c_client *client)
> +void
> +pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, int *invert)
>  {
> -	return NULL;
>  }
>  #endif
>  
> @@ -648,6 +637,7 @@ static int __devinit pca953x_probe(struct i2c_client *client,
>  {
>  	struct pca953x_platform_data *pdata;
>  	struct pca953x_chip *chip;
> +	int irq_base=-1, invert=0;
>  	int ret = 0;
>  
>  	chip = kzalloc(sizeof(struct pca953x_chip), GFP_KERNEL);
> @@ -655,26 +645,17 @@ static int __devinit pca953x_probe(struct i2c_client *client,
>  		return -ENOMEM;
>  
>  	pdata = client->dev.platform_data;
> -	if (pdata == NULL) {
> -		pdata = pca953x_get_alt_pdata(client);
> -		/*
> -		 * Unlike normal platform_data, this is allocated
> -		 * dynamically and must be freed in the driver
> -		 */
> -		chip->dyn_pdata = pdata;
> -	}
> -
> -	if (pdata == NULL) {
> -		dev_dbg(&client->dev, "no platform data\n");
> -		ret = -EINVAL;
> -		goto out_failed;
> +	if (pdata) {
> +		irq_base = pdata->irq_base;
> +		chip->gpio_start = pdata->gpio_base;
> +		invert = pdata->invert;
> +		chip->names = pdata->names;
> +	} else {
> +		pca953x_get_alt_pdata(client, &chip->gpio_start, &invert);
>  	}
>  
>  	chip->client = client;
>  
> -	chip->gpio_start = pdata->gpio_base;
> -
> -	chip->names = pdata->names;
>  	chip->chip_type = id->driver_data & (PCA953X_TYPE | PCA957X_TYPE);
>  
>  	mutex_init(&chip->i2c_lock);
> @@ -685,13 +666,13 @@ static int __devinit pca953x_probe(struct i2c_client *client,
>  	pca953x_setup_gpio(chip, id->driver_data & PCA_GPIO_MASK);
>  
>  	if (chip->chip_type == PCA953X_TYPE)
> -		device_pca953x_init(chip, pdata->invert);
> +		device_pca953x_init(chip, invert);
>  	else if (chip->chip_type == PCA957X_TYPE)
> -		device_pca957x_init(chip, pdata->invert);
> +		device_pca957x_init(chip, invert);
>  	else
>  		goto out_failed;
>  
> -	ret = pca953x_irq_setup(chip, id);
> +	ret = pca953x_irq_setup(chip, id, irq_base);
>  	if (ret)
>  		goto out_failed;
>  
> @@ -699,7 +680,7 @@ static int __devinit pca953x_probe(struct i2c_client *client,
>  	if (ret)
>  		goto out_failed_irq;
>  
> -	if (pdata->setup) {
> +	if (pdata && pdata->setup) {
>  		ret = pdata->setup(client, chip->gpio_chip.base,
>  				chip->gpio_chip.ngpio, pdata->context);
>  		if (ret < 0)
> @@ -712,7 +693,6 @@ static int __devinit pca953x_probe(struct i2c_client *client,
>  out_failed_irq:
>  	pca953x_irq_teardown(chip);
>  out_failed:
> -	kfree(chip->dyn_pdata);
>  	kfree(chip);
>  	return ret;
>  }
> @@ -723,7 +703,7 @@ static int pca953x_remove(struct i2c_client *client)
>  	struct pca953x_chip *chip = i2c_get_clientdata(client);
>  	int ret = 0;
>  
> -	if (pdata->teardown) {
> +	if (pdata && pdata->teardown) {
>  		ret = pdata->teardown(client, chip->gpio_chip.base,
>  				chip->gpio_chip.ngpio, pdata->context);
>  		if (ret < 0) {
> @@ -741,7 +721,6 @@ static int pca953x_remove(struct i2c_client *client)
>  	}
>  
>  	pca953x_irq_teardown(chip);
> -	kfree(chip->dyn_pdata);
>  	kfree(chip);
>  	return 0;
>  }
> -- 
> 1.7.4.1
> 

[PATCH v5 3/5] GPIO: pca953x.c: Deprecate meaningless device-tree bindings

[David Jander]

The property 'polarity' is handled by the GPIO core, and the 'gpio-base'
should be assigned automatically. It is meaningless in the device-tree,
since GPIO's are identified by the "chip-name"/offset pair.
This way, the whole pca953x_get_alt_pdata() can hopefully soon go away.
We still need to check whether we really want GPIO-interrupt functionality
by simply looking if the I2C node has an interrupts property defined, since
this property is not used for anything else.

Signed-off-by: David Jander <david@protonic.nl>
---
 drivers/gpio/pca953x.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/pca953x.c b/drivers/gpio/pca953x.c
index db69d9e..c28fc98 100644
--- a/drivers/gpio/pca953x.c
+++ b/drivers/gpio/pca953x.c
@@ -21,7 +21,6 @@
 #include <linux/slab.h>
 #ifdef CONFIG_OF_GPIO
 #include <linux/of_platform.h>
-#include <linux/of_gpio.h>
 #endif
 
 #define PCA953X_INPUT		0
@@ -546,6 +545,7 @@ static void pca953x_irq_teardown(struct pca953x_chip *chip)
 #ifdef CONFIG_OF_GPIO
 /*
  * Translate OpenFirmware node properties into platform_data
+ * WARNING: This is DEPRECATED and will be removed eventually!
  */
 void
 pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, int *invert)
@@ -560,6 +560,7 @@ pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, int *invert)
 
 	*gpio_base = -1;
 	val = of_get_property(node, "linux,gpio-base", &size);
+	WARN(val, "%s: device-tree property 'linux,gpio-base' is deprecated!", __func__);
 	if (val) {
 		if (size != sizeof(*val))
 			dev_warn(&client->dev, "%s: wrong linux,gpio-base\n",
@@ -569,6 +570,7 @@ pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, int *invert)
 	}
 
 	val = of_get_property(node, "polarity", NULL);
+	WARN(val, "%s: device-tree property 'polarity' is deprecated!", __func__);
 	if (val)
 		*invert = *val;
 }
@@ -637,7 +639,7 @@ static int __devinit pca953x_probe(struct i2c_client *client,
 {
 	struct pca953x_platform_data *pdata;
 	struct pca953x_chip *chip;
-	int irq_base=-1, invert=0;
+	int irq_base=0, invert=0;
 	int ret = 0;
 
 	chip = kzalloc(sizeof(struct pca953x_chip), GFP_KERNEL);
@@ -652,6 +654,11 @@ static int __devinit pca953x_probe(struct i2c_client *client,
 		chip->names = pdata->names;
 	} else {
 		pca953x_get_alt_pdata(client, &chip->gpio_start, &invert);
+#ifdef CONFIG_OF_GPIO
+		/* If I2C node has no interrupts property, disable GPIO interrupts */
+		if (of_find_property(client->dev.of_node, "interrupts", NULL) == NULL)
+			irq_base = -1;
+#endif
 	}
 
 	chip->client = client;
-- 
1.7.4.1

Re: [PATCH v5 3/5] GPIO: pca953x.c: Deprecate meaningless device-tree bindings

[Grant Likely]

On Tue, Jun 14, 2011 at 11:00:56AM +0200, David Jander wrote:
> The property 'polarity' is handled by the GPIO core, and the 'gpio-base'
> should be assigned automatically. It is meaningless in the device-tree,
> since GPIO's are identified by the "chip-name"/offset pair.
> This way, the whole pca953x_get_alt_pdata() can hopefully soon go away.
> We still need to check whether we really want GPIO-interrupt functionality
> by simply looking if the I2C node has an interrupts property defined, since
> this property is not used for anything else.
> 
> Signed-off-by: David Jander <david@protonic.nl>

Merged, thanks.

g.

> ---
>  drivers/gpio/pca953x.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/pca953x.c b/drivers/gpio/pca953x.c
> index db69d9e..c28fc98 100644
> --- a/drivers/gpio/pca953x.c
> +++ b/drivers/gpio/pca953x.c
> @@ -21,7 +21,6 @@
>  #include <linux/slab.h>
>  #ifdef CONFIG_OF_GPIO
>  #include <linux/of_platform.h>
> -#include <linux/of_gpio.h>
>  #endif
>  
>  #define PCA953X_INPUT		0
> @@ -546,6 +545,7 @@ static void pca953x_irq_teardown(struct pca953x_chip *chip)
>  #ifdef CONFIG_OF_GPIO
>  /*
>   * Translate OpenFirmware node properties into platform_data
> + * WARNING: This is DEPRECATED and will be removed eventually!
>   */
>  void
>  pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, int *invert)
> @@ -560,6 +560,7 @@ pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, int *invert)
>  
>  	*gpio_base = -1;
>  	val = of_get_property(node, "linux,gpio-base", &size);
> +	WARN(val, "%s: device-tree property 'linux,gpio-base' is deprecated!", __func__);
>  	if (val) {
>  		if (size != sizeof(*val))
>  			dev_warn(&client->dev, "%s: wrong linux,gpio-base\n",
> @@ -569,6 +570,7 @@ pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, int *invert)
>  	}
>  
>  	val = of_get_property(node, "polarity", NULL);
> +	WARN(val, "%s: device-tree property 'polarity' is deprecated!", __func__);
>  	if (val)
>  		*invert = *val;
>  }
> @@ -637,7 +639,7 @@ static int __devinit pca953x_probe(struct i2c_client *client,
>  {
>  	struct pca953x_platform_data *pdata;
>  	struct pca953x_chip *chip;
> -	int irq_base=-1, invert=0;
> +	int irq_base=0, invert=0;
>  	int ret = 0;
>  
>  	chip = kzalloc(sizeof(struct pca953x_chip), GFP_KERNEL);
> @@ -652,6 +654,11 @@ static int __devinit pca953x_probe(struct i2c_client *client,
>  		chip->names = pdata->names;
>  	} else {
>  		pca953x_get_alt_pdata(client, &chip->gpio_start, &invert);
> +#ifdef CONFIG_OF_GPIO
> +		/* If I2C node has no interrupts property, disable GPIO interrupts */
> +		if (of_find_property(client->dev.of_node, "interrupts", NULL) == NULL)
> +			irq_base = -1;
> +#endif
>  	}
>  
>  	chip->client = client;
> -- 
> 1.7.4.1
> 

[PATCH v5 4/5] GPIO: pca953x.c: Interrupt pin is active-low

[David Jander]

The interrupt pin of the PCA953x is active low, and on the rising edge
no interrupt should be produced.

Signed-off-by: David Jander <david@protonic.nl>
---
 drivers/gpio/pca953x.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/pca953x.c b/drivers/gpio/pca953x.c
index c28fc98..9bdbc16 100644
--- a/drivers/gpio/pca953x.c
+++ b/drivers/gpio/pca953x.c
@@ -495,8 +495,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 		ret = request_threaded_irq(client->irq,
 					   NULL,
 					   pca953x_irq_handler,
-					   IRQF_TRIGGER_RISING |
-					   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 					   dev_name(&client->dev), chip);
 		if (ret) {
 			dev_err(&client->dev, "failed to request irq %d\n",
-- 
1.7.4.1

[PATCH v5 5/5] GPIO: pca953x.c: Fix warning of enabled interrupts in handler

[David Jander]

When using nested threaded irqs, use handle_nested_irq(). This function
does not call the chip handler, so no handler is set.

Signed-off-by: David Jander <david@protonic.nl>
---
 drivers/gpio/pca953x.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/pca953x.c b/drivers/gpio/pca953x.c
index 9bdbc16..2eceb2c 100644
--- a/drivers/gpio/pca953x.c
+++ b/drivers/gpio/pca953x.c
@@ -435,7 +435,7 @@ static irqreturn_t pca953x_irq_handler(int irq, void *devid)
 
 	do {
 		level = __ffs(pending);
-		generic_handle_irq(level + chip->irq_base);
+		handle_nested_irq(level + chip->irq_base);
 
 		pending &= ~(1 << level);
 	} while (pending);
@@ -483,8 +483,8 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 
 			irq_clear_status_flags(irq, IRQ_NOREQUEST);
 			irq_set_chip_data(irq, chip);
-			irq_set_chip_and_handler(irq, &pca953x_irq_chip,
-						 handle_simple_irq);
+			irq_set_chip(irq, &pca953x_irq_chip);
+			irq_set_nested_thread(irq, true);
 #ifdef CONFIG_ARM
 			set_irq_flags(irq, IRQF_VALID);
 #else
-- 
1.7.4.1