[PATCH v2 0/3] cleanup of gpio

linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v2 0/3] cleanup of gpio_pcf857x.c
@ 2013-08-30 10:48 George Cherian
  2013-08-30 10:48 ` [PATCH v2 1/3] gpio: pcf857x: change to devm_request_threaded_irq George Cherian
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: George Cherian @ 2013-08-30 10:48 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, linux-kernel, linux-omap, kuninori.morimoto.gx,
	n.a.balandin, George Cherian

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

George Cherian (3):
  gpio: pcf857x: change to  devm_request_threaded_irq
  gpio: pcf857x: remove the irq_demux_work
  gpio: pcf857x: call the gpio user  handler  iff gpio_to_irq is done

 arch/arm/boot/dts/dra7-evm.dts |  6 ++---
 drivers/gpio/gpio-pcf857x.c    | 51 +++++++++++++++++++++---------------------
 2 files changed, 29 insertions(+), 28 deletions(-)

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/3] gpio: pcf857x: change to  devm_request_threaded_irq
  2013-08-30 10:48 [PATCH v2 0/3] cleanup of gpio_pcf857x.c George Cherian
@ 2013-08-30 10:48 ` George Cherian
  2013-09-02  1:20   ` Kuninori Morimoto
  2013-08-30 10:48 ` [PATCH v2 2/3] gpio: pcf857x: remove the irq_demux_work George Cherian
  2013-08-30 10:48 ` [PATCH v2 3/3] gpio: pcf857x: call the gpio user handler iff gpio_to_irq is done George Cherian
  2 siblings, 1 reply; 6+ messages in thread
From: George Cherian @ 2013-08-30 10:48 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, linux-kernel, linux-omap, kuninori.morimoto.gx,
	n.a.balandin, George Cherian

Remove the request_irq and use devm_request_threaded_irq
also cleanup free_irq. devm_* takes care of that.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 arch/arm/boot/dts/dra7-evm.dts |  2 +-
 drivers/gpio/gpio-pcf857x.c    | 28 ++++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 8b0738a..39b44bc 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -65,7 +65,7 @@
 		n_latch = <0x1408>;
 		gpio-controller;
 		#gpio-cells = <2>;
-		interrupt-parent = <&pcf_20>;
+		interrupt-parent = <&gpio20>;
 		interrupts = <14 2>;
 		interrupt-controller;
 		#interrupt-cells = <2>;
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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/3] gpio: pcf857x: change to  devm_request_threaded_irq
  2013-08-30 10:48 ` [PATCH v2 1/3] gpio: pcf857x: change to devm_request_threaded_irq George Cherian
@ 2013-09-02  1:20   ` Kuninori Morimoto
  2013-09-02  2:02     ` George Cherian
  0 siblings, 1 reply; 6+ messages in thread
From: Kuninori Morimoto @ 2013-09-02  1:20 UTC (permalink / raw)
  To: George Cherian
  Cc: linus.walleij, linux-gpio, linux-kernel, linux-omap, n.a.balandin


Hi

> Remove the request_irq and use devm_request_threaded_irq
> also cleanup free_irq. devm_* takes care of that.
> 
> Signed-off-by: George Cherian <george.cherian@ti.com>
> ---
>  arch/arm/boot/dts/dra7-evm.dts |  2 +-
>  drivers/gpio/gpio-pcf857x.c    | 28 ++++++++++++++++++++++++----
>  2 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
> index 8b0738a..39b44bc 100644
> --- a/arch/arm/boot/dts/dra7-evm.dts
> +++ b/arch/arm/boot/dts/dra7-evm.dts
> @@ -65,7 +65,7 @@
>  		n_latch = <0x1408>;
>  		gpio-controller;
>  		#gpio-cells = <2>;
> -		interrupt-parent = <&pcf_20>;
> +		interrupt-parent = <&gpio20>;
>  		interrupts = <14 2>;
>  		interrupt-controller;
>  		#interrupt-cells = <2>;

I'm not sure detail, 
but does above "exchange interrupt-parent" and "using devm_request_threaded_irq()"
have any relationship ?
Separate patch seems nice ?

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/3] gpio: pcf857x: change to  devm_request_threaded_irq
  2013-09-02  1:20   ` Kuninori Morimoto
@ 2013-09-02  2:02     ` George Cherian
  0 siblings, 0 replies; 6+ messages in thread
From: George Cherian @ 2013-09-02  2:02 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: linus.walleij, linux-gpio, linux-kernel, linux-omap, n.a.balandin

On 9/2/2013 6:50 AM, Kuninori Morimoto wrote:
> Hi
>
>> Remove the request_irq and use devm_request_threaded_irq
>> also cleanup free_irq. devm_* takes care of that.
>>
>> Signed-off-by: George Cherian <george.cherian@ti.com>
>> ---
>>   arch/arm/boot/dts/dra7-evm.dts |  2 +-
>>   drivers/gpio/gpio-pcf857x.c    | 28 ++++++++++++++++++++++++----
>>   2 files changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
>> index 8b0738a..39b44bc 100644
>> --- a/arch/arm/boot/dts/dra7-evm.dts
>> +++ b/arch/arm/boot/dts/dra7-evm.dts
>> @@ -65,7 +65,7 @@
>>   		n_latch = <0x1408>;
>>   		gpio-controller;
>>   		#gpio-cells = <2>;
>> -		interrupt-parent = <&pcf_20>;
>> +		interrupt-parent = <&gpio20>;
>>   		interrupts = <14 2>;
>>   		interrupt-controller;
>>   		#interrupt-cells = <2>;
> I'm not sure detail,
> but does above "exchange interrupt-parent" and "using devm_request_threaded_irq()"
> have any relationship ?
Exactly. will resend this change alone.
> Separate patch seems nice ?

okay
> Best regards
> ---
> Kuninori Morimoto


-- 
-George


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] gpio: pcf857x: remove the irq_demux_work
  2013-08-30 10:48 [PATCH v2 0/3] cleanup of gpio_pcf857x.c George Cherian
  2013-08-30 10:48 ` [PATCH v2 1/3] gpio: pcf857x: change to devm_request_threaded_irq George Cherian
@ 2013-08-30 10:48 ` George Cherian
  2013-08-30 10:48 ` [PATCH v2 3/3] gpio: pcf857x: call the gpio user handler iff gpio_to_irq is done George Cherian
  2 siblings, 0 replies; 6+ messages in thread
From: George Cherian @ 2013-08-30 10:48 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, linux-kernel, linux-omap, kuninori.morimoto.gx,
	n.a.balandin, George Cherian

Now that we are using devm_request_threaded_irq no need for
irq_demux_work. Remove all its references.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/gpio/gpio-pcf857x.c | 35 -----------------------------------
 1 file changed, 35 deletions(-)

diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 4d0d28c..4890e97 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,7 +88,6 @@ 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 */
@@ -210,38 +208,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,7 +250,6 @@ 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;
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] gpio: pcf857x: call the gpio user  handler  iff gpio_to_irq is done
  2013-08-30 10:48 [PATCH v2 0/3] cleanup of gpio_pcf857x.c George Cherian
  2013-08-30 10:48 ` [PATCH v2 1/3] gpio: pcf857x: change to devm_request_threaded_irq George Cherian
  2013-08-30 10:48 ` [PATCH v2 2/3] gpio: pcf857x: remove the irq_demux_work George Cherian
@ 2013-08-30 10:48 ` George Cherian
  2 siblings, 0 replies; 6+ messages in thread
From: George Cherian @ 2013-08-30 10:48 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, linux-kernel, linux-omap, kuninori.morimoto.gx,
	n.a.balandin, George Cherian

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 <george.cherian@ti.com>
---
 arch/arm/boot/dts/dra7-evm.dts |  4 ++--
 drivers/gpio/gpio-pcf857x.c    | 22 +++++++++++++++++++---
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 39b44bc..00fb0df 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -50,7 +50,7 @@
 	gpio20: pcf8575@20 {
 		compatible = "ti,pcf8575";
 		reg = <0x20>;
-		n_latch = <0x4000>;
+		n_latch = <0x0000>;
 		gpio-controller;
 		#gpio-cells = <2>;
 		interrupt-parent = <&gpio6>;
@@ -62,7 +62,7 @@
 	gpio21: pcf8575@21 {
 		compatible = "ti,pcf8575";
 		reg = <0x21>;
-		n_latch = <0x1408>;
+		/*n_latch = <0x1408>;*/
 		gpio-controller;
 		#gpio-cells = <2>;
 		interrupt-parent = <&gpio20>;
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 4890e97..578d93f 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -93,6 +93,7 @@ struct pcf857x {
 	unsigned		out;		/* software latch */
 	unsigned		status;		/* current status */
 	int			irq;		/* real irq number */
+	unsigned		irq_mapped;	/* mapped gpio irqs */
 
 	int (*write)(struct i2c_client *client, unsigned data);
 	int (*read)(struct i2c_client *client);
@@ -185,8 +186,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)
@@ -198,7 +204,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;
@@ -211,9 +222,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;
 }
 
@@ -236,7 +252,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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-09-02  2:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-30 10:48 [PATCH v2 0/3] cleanup of gpio_pcf857x.c George Cherian
2013-08-30 10:48 ` [PATCH v2 1/3] gpio: pcf857x: change to devm_request_threaded_irq George Cherian
2013-09-02  1:20   ` Kuninori Morimoto
2013-09-02  2:02     ` George Cherian
2013-08-30 10:48 ` [PATCH v2 2/3] gpio: pcf857x: remove the irq_demux_work George Cherian
2013-08-30 10:48 ` [PATCH v2 3/3] gpio: pcf857x: call the gpio user handler iff gpio_to_irq is done George Cherian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).