2011-06-14 09:00:51

by David Jander

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

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


2011-06-14 09:01:34

by David Jander

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

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 <[email protected]>
---
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

2011-06-14 09:01:31

by David Jander

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

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 <[email protected]>
---
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

2011-06-14 09:01:32

by David Jander

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

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 <[email protected]>
---
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

2011-06-14 09:00:53

by David Jander

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

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

Signed-off-by: David Jander <[email protected]>
---
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

2011-06-14 09:01:09

by David Jander

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

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 <[email protected]>
---
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

2011-06-16 19:45:42

by Grant Likely

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

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 <[email protected]>

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
>

2011-06-16 19:45:44

by Grant Likely

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

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 <[email protected]>

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
>

2011-06-16 19:52:47

by Grant Likely

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

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 <[email protected]>

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
>