This patch series
- removes the irq_demux_work
- Uses devm_request_threaded_irq
- Call the user handler iff gpio_to_irq is done.
v1 --> v2
Split v1 to 3 patches
v2 --> v3
Remove the unnecessary dts patches.
v3 --> v4
Remove gpio->irq (in patch 2)
Note: these patches were made after applying [1].
[1] - [PATCH v5] gpio: pcf857x: Add OF support - https://lkml.org/lkml/2013/8/27/70
George Cherian (3):
gpio: pcf857x: change to devm_request_threaded_irq
gpio: pcf857x: remove the irq_demux_work and gpio->irq
gpio: pcf857x: call the gpio user handler iff gpio_to_irq is done
drivers/gpio/gpio-pcf857x.c | 53 ++++++++++++++++++++++-----------------------
1 file changed, 26 insertions(+), 27 deletions(-)
--
1.8.1.4
Remove the request_irq and use devm_request_threaded_irq
also cleanup free_irq. devm_* takes care of that.
Signed-off-by: George Cherian <[email protected]>
---
drivers/gpio/gpio-pcf857x.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 947cff4..4d0d28c 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -191,6 +191,25 @@ static int pcf857x_to_irq(struct gpio_chip *chip, unsigned offset)
return irq_create_mapping(gpio->irq_domain, offset);
}
+static irqreturn_t pcf857x_irq(int irq, void *data)
+{
+ struct pcf857x *gpio = data;
+ unsigned long change, i, status, flags;
+
+ status = gpio->read(gpio->client);
+
+ spin_lock_irqsave(&gpio->slock, flags);
+
+ change = gpio->status ^ status;
+ for_each_set_bit(i, &change, gpio->chip.ngpio)
+ generic_handle_irq(irq_find_mapping(gpio->irq_domain, i));
+ gpio->status = status;
+
+ spin_unlock_irqrestore(&gpio->slock, flags);
+
+ return IRQ_HANDLED;
+}
+
static void pcf857x_irq_demux_work(struct work_struct *work)
{
struct pcf857x *gpio = container_of(work,
@@ -241,8 +260,6 @@ static void pcf857x_irq_domain_cleanup(struct pcf857x *gpio)
if (gpio->irq_domain)
irq_domain_remove(gpio->irq_domain);
- if (gpio->irq)
- free_irq(gpio->irq, gpio);
}
static int pcf857x_irq_domain_init(struct pcf857x *gpio,
@@ -258,8 +275,11 @@ static int pcf857x_irq_domain_init(struct pcf857x *gpio,
goto fail;
/* enable real irq */
- status = request_irq(client->irq, pcf857x_irq_demux, 0,
- dev_name(&client->dev), gpio);
+ status = devm_request_threaded_irq(&client->dev, client->irq,
+ NULL, pcf857x_irq, IRQF_ONESHOT |
+ IRQF_TRIGGER_FALLING,
+ dev_name(&client->dev), gpio);
+
if (status)
goto fail;
--
1.8.1.4
Now that we are using devm_request_threaded_irq no need for
irq_demux_work and gpio->irq. Remove all its references.
Signed-off-by: George Cherian <[email protected]>
---
drivers/gpio/gpio-pcf857x.c | 37 -------------------------------------
1 file changed, 37 deletions(-)
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 4d0d28c..ef52ee3 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -30,7 +30,6 @@
#include <linux/of_device.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
-#include <linux/workqueue.h>
static const struct i2c_device_id pcf857x_id[] = {
@@ -89,12 +88,10 @@ struct pcf857x {
struct gpio_chip chip;
struct i2c_client *client;
struct mutex lock; /* protect 'out' */
- struct work_struct work; /* irq demux work */
struct irq_domain *irq_domain; /* for irq demux */
spinlock_t slock; /* protect irq demux */
unsigned out; /* software latch */
unsigned status; /* current status */
- int irq; /* real irq number */
int (*write)(struct i2c_client *client, unsigned data);
int (*read)(struct i2c_client *client);
@@ -210,38 +207,6 @@ static irqreturn_t pcf857x_irq(int irq, void *data)
return IRQ_HANDLED;
}
-static void pcf857x_irq_demux_work(struct work_struct *work)
-{
- struct pcf857x *gpio = container_of(work,
- struct pcf857x,
- work);
- unsigned long change, i, status, flags;
-
- status = gpio->read(gpio->client);
-
- spin_lock_irqsave(&gpio->slock, flags);
-
- change = gpio->status ^ status;
- for_each_set_bit(i, &change, gpio->chip.ngpio)
- generic_handle_irq(irq_find_mapping(gpio->irq_domain, i));
- gpio->status = status;
-
- spin_unlock_irqrestore(&gpio->slock, flags);
-}
-
-static irqreturn_t pcf857x_irq_demux(int irq, void *data)
-{
- struct pcf857x *gpio = data;
-
- /*
- * pcf857x can't read/write data here,
- * since i2c data access might go to sleep.
- */
- schedule_work(&gpio->work);
-
- return IRQ_HANDLED;
-}
-
static int pcf857x_irq_domain_map(struct irq_domain *domain, unsigned int virq,
irq_hw_number_t hw)
{
@@ -284,9 +249,7 @@ static int pcf857x_irq_domain_init(struct pcf857x *gpio,
goto fail;
/* enable gpio_to_irq() */
- INIT_WORK(&gpio->work, pcf857x_irq_demux_work);
gpio->chip.to_irq = pcf857x_to_irq;
- gpio->irq = client->irq;
return 0;
--
1.8.1.4
For pcf857x driver if the initial state is not set properly (proper
n_latch is not passed), we get bad irq prints on console.
We get this only for the first interrupt and doesnot repeat for further
interrupts unles and until there are other gpio pins which are not flipping
continously.
following prints are seen on console.
[ 40.983924] irq 0, desc: ce004080, depth: 1, count: 0, unhandled: 0
[ 40.990511] ->handle_irq(): c00aa538, handle_bad_irq+0x0/0x260
[ 40.996768] ->irq_data.chip(): c080b6ec, no_irq_chip+0x0/0x60
[ 41.002842] ->action(): (null)
[ 41.006242] IRQ_NOPROBE set
[ 41.009465] IRQ_NOREQUEST set
Signed-off-by: George Cherian <[email protected]>
---
drivers/gpio/gpio-pcf857x.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index ef52ee3..4887b8c 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -92,6 +92,7 @@ struct pcf857x {
spinlock_t slock; /* protect irq demux */
unsigned out; /* software latch */
unsigned status; /* current status */
+ unsigned irq_mapped; /* mapped gpio irqs */
int (*write)(struct i2c_client *client, unsigned data);
int (*read)(struct i2c_client *client);
@@ -184,8 +185,13 @@ static void pcf857x_set(struct gpio_chip *chip, unsigned offset, int value)
static int pcf857x_to_irq(struct gpio_chip *chip, unsigned offset)
{
struct pcf857x *gpio = container_of(chip, struct pcf857x, chip);
+ int ret;
- return irq_create_mapping(gpio->irq_domain, offset);
+ ret = irq_create_mapping(gpio->irq_domain, offset);
+ if (ret > 0)
+ gpio->irq_mapped |= (1 << offset);
+
+ return ret;
}
static irqreturn_t pcf857x_irq(int irq, void *data)
@@ -197,7 +203,12 @@ static irqreturn_t pcf857x_irq(int irq, void *data)
spin_lock_irqsave(&gpio->slock, flags);
- change = gpio->status ^ status;
+ /*
+ * call the interrupt handler iff gpio is used as
+ * interrupt source, just to avoid bad irqs
+ */
+
+ change = ((gpio->status ^ status) & gpio->irq_mapped);
for_each_set_bit(i, &change, gpio->chip.ngpio)
generic_handle_irq(irq_find_mapping(gpio->irq_domain, i));
gpio->status = status;
@@ -210,9 +221,14 @@ static irqreturn_t pcf857x_irq(int irq, void *data)
static int pcf857x_irq_domain_map(struct irq_domain *domain, unsigned int virq,
irq_hw_number_t hw)
{
+ struct pcf857x *gpio = domain->host_data;
+
irq_set_chip_and_handler(virq,
&dummy_irq_chip,
handle_level_irq);
+ set_irq_flags(virq, IRQF_VALID);
+ gpio->irq_mapped |= (1 << hw);
+
return 0;
}
@@ -235,7 +251,7 @@ static int pcf857x_irq_domain_init(struct pcf857x *gpio,
gpio->irq_domain = irq_domain_add_linear(client->dev.of_node,
gpio->chip.ngpio,
&pcf857x_irq_domain_ops,
- NULL);
+ gpio);
if (!gpio->irq_domain)
goto fail;
--
1.8.1.4
Hi
> This patch series
> - removes the irq_demux_work
> - Uses devm_request_threaded_irq
> - Call the user handler iff gpio_to_irq is done.
>
> v1 --> v2
> Split v1 to 3 patches
> v2 --> v3
> Remove the unnecessary dts patches.
> v3 --> v4
> Remove gpio->irq (in patch 2)
>
> Note: these patches were made after applying [1].
> [1] - [PATCH v5] gpio: pcf857x: Add OF support - https://lkml.org/lkml/2013/8/27/70
>
> George Cherian (3):
> gpio: pcf857x: change to devm_request_threaded_irq
> gpio: pcf857x: remove the irq_demux_work and gpio->irq
> gpio: pcf857x: call the gpio user handler iff gpio_to_irq is done
>
> drivers/gpio/gpio-pcf857x.c | 53 ++++++++++++++++++++++-----------------------
> 1 file changed, 26 insertions(+), 27 deletions(-)
For all patches
Acked-by: Kuninori Morimoto <[email protected]>
Best regards
---
Kuninori Morimoto
On Wed, Sep 4, 2013 at 9:03 AM, George Cherian <[email protected]> wrote:
> This patch series
> - removes the irq_demux_work
> - Uses devm_request_threaded_irq
> - Call the user handler iff gpio_to_irq is done.
>
> v1 --> v2
> Split v1 to 3 patches
> v2 --> v3
> Remove the unnecessary dts patches.
> v3 --> v4
> Remove gpio->irq (in patch 2)
Applied all three patches with Kuninoro's ACK.
However your last iteration was still labeled "v3" in the subject
so I was a bit confused, but I took all those sent on sep 4.
Please check the result in my GPIO tree or linux-next and
make sure it works.
> Note: these patches were made after applying [1].
> [1] - [PATCH v5] gpio: pcf857x: Add OF support - https://lkml.org/lkml/2013/8/27/70
Well that patch is not finished, but by rebasing patch 2
with patch -p1 < patch2.patch it applied anyway.
Now Laurent has to finalize his DT bindings on top
of your patches instead.
Yours,
Linus Walleij
Hi Linus,
On Tuesday 17 September 2013 14:51:48 Linus Walleij wrote:
> On Wed, Sep 4, 2013 at 9:03 AM, George Cherian wrote:
> > This patch series
> >
> > - removes the irq_demux_work
> > - Uses devm_request_threaded_irq
> > - Call the user handler iff gpio_to_irq is done.
> >
> > v1 --> v2
> >
> > Split v1 to 3 patches
> >
> > v2 --> v3
> >
> > Remove the unnecessary dts patches.
> >
> > v3 --> v4
> >
> > Remove gpio->irq (in patch 2)
>
> Applied all three patches with Kuninoro's ACK.
>
> However your last iteration was still labeled "v3" in the subject
> so I was a bit confused, but I took all those sent on sep 4.
>
> Please check the result in my GPIO tree or linux-next and
> make sure it works.
>
> > Note: these patches were made after applying [1].
> > [1] - [PATCH v5] gpio: pcf857x: Add OF support -
> > https://lkml.org/lkml/2013/8/27/70
>
> Well that patch is not finished, but by rebasing patch 2 with
> patch -p1 < patch2.patch it applied anyway.
>
> Now Laurent has to finalize his DT bindings on top of your patches instead.
I will do so. At first sight I don't see any conflict issue.
--
Regards,
Laurent Pinchart