[PATCH 0/7] twl4030: finish threaded IRQ conversion

[PATCH 0/7] twl4030: finish threaded IRQ conversion

[Felipe Balbi]

Hi,

the following patches where boot-tested on beagle xM
and everything seems fine. MMC root filesystem still
mounts, /proc/interrupts looks like ps aux | grep irq
shows our threads, etc.

please give it a thorough review as there are places
where I was confused of what to do.

Later patches could (should?) come making use of
Sparse IRQ numbers (by allocating irq_descs dynamically)
and I'm not sure I like that chained handler too much.

If I understood correctly the chained handler is simply
avoiding a request_threaded_irq() on e.g. twl4030-gpio.c
driver. Not sure if it's that nice. Anyway, here we have
the final conversion.

Felipe Balbi (7):
  mfd: twl4030-irq: remove trailing whitespaces
  mfd: twl4030-irq: implement bus_lock/bus_sync_unlock methods
  mfd: twl4030-irq: drop the kthread
  mfd: twl4030-irq: drop mask_work
  mfd: twl4030-irq: drop edge_work
  mfd: twl4030-irq: set irq nested flag
  rtc: twl: move to threaded irq

 drivers/mfd/twl4030-irq.c |  342 +++++++++++++++++----------------------------
 drivers/rtc/rtc-twl.c     |   10 +--
 2 files changed, 132 insertions(+), 220 deletions(-)

-- 
1.7.6

[PATCH 1/7] mfd: twl4030-irq: remove trailing whitespaces

[Felipe Balbi]

trivial patch, no functional changes.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/mfd/twl4030-irq.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index 8a7ee31..7be97cb 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -620,7 +620,7 @@ static int twl4030_sih_set_type(struct irq_data *data, unsigned trigger)
 
 static struct irq_chip twl4030_sih_irq_chip = {
 	.name		= "twl4030",
-	.irq_mask      	= twl4030_sih_mask,
+	.irq_mask	= twl4030_sih_mask,
 	.irq_unmask	= twl4030_sih_unmask,
 	.irq_set_type	= twl4030_sih_set_type,
 };
-- 
1.7.6

[PATCH 2/7] mfd: twl4030-irq: implement bus_lock/bus_sync_unlock methods

[Felipe Balbi]

for doing that, drop the locking and change that
to a mutex.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/mfd/twl4030-irq.c |   37 +++++++++++++++++++++----------------
 1 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index 7be97cb..291364c 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -460,8 +460,6 @@ static inline void activate_irq(int irq)
 
 /*----------------------------------------------------------------------*/
 
-static DEFINE_SPINLOCK(sih_agent_lock);
-
 static struct workqueue_struct *wq;
 
 struct sih_agent {
@@ -474,6 +472,8 @@ struct sih_agent {
 
 	u32			edge_change;
 	struct work_struct	edge_work;
+
+	struct mutex		irq_lock;
 };
 
 static void twl4030_sih_do_mask(struct work_struct *work)
@@ -489,7 +489,6 @@ static void twl4030_sih_do_mask(struct work_struct *work)
 	agent = container_of(work, struct sih_agent, mask_work);
 
 	/* see what work we have */
-	spin_lock_irq(&sih_agent_lock);
 	if (agent->imr_change_pending) {
 		sih = agent->sih;
 		/* byte[0] gets overwritten as we write ... */
@@ -497,7 +496,6 @@ static void twl4030_sih_do_mask(struct work_struct *work)
 		agent->imr_change_pending = false;
 	} else
 		sih = NULL;
-	spin_unlock_irq(&sih_agent_lock);
 	if (!sih)
 		return;
 
@@ -520,11 +518,9 @@ static void twl4030_sih_do_edge(struct work_struct *work)
 	agent = container_of(work, struct sih_agent, edge_work);
 
 	/* see what work we have */
-	spin_lock_irq(&sih_agent_lock);
 	edge_change = agent->edge_change;
 	agent->edge_change = 0;
 	sih = edge_change ? agent->sih : NULL;
-	spin_unlock_irq(&sih_agent_lock);
 	if (!sih)
 		return;
 
@@ -580,49 +576,57 @@ static void twl4030_sih_do_edge(struct work_struct *work)
 static void twl4030_sih_mask(struct irq_data *data)
 {
 	struct sih_agent *sih = irq_data_get_irq_chip_data(data);
-	unsigned long flags;
 
-	spin_lock_irqsave(&sih_agent_lock, flags);
 	sih->imr |= BIT(data->irq - sih->irq_base);
 	sih->imr_change_pending = true;
 	queue_work(wq, &sih->mask_work);
-	spin_unlock_irqrestore(&sih_agent_lock, flags);
 }
 
 static void twl4030_sih_unmask(struct irq_data *data)
 {
 	struct sih_agent *sih = irq_data_get_irq_chip_data(data);
-	unsigned long flags;
 
-	spin_lock_irqsave(&sih_agent_lock, flags);
 	sih->imr &= ~BIT(data->irq - sih->irq_base);
 	sih->imr_change_pending = true;
 	queue_work(wq, &sih->mask_work);
-	spin_unlock_irqrestore(&sih_agent_lock, flags);
 }
 
 static int twl4030_sih_set_type(struct irq_data *data, unsigned trigger)
 {
 	struct sih_agent *sih = irq_data_get_irq_chip_data(data);
-	unsigned long flags;
 
 	if (trigger & ~(IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
 		return -EINVAL;
 
-	spin_lock_irqsave(&sih_agent_lock, flags);
 	if (irqd_get_trigger_type(data) != trigger) {
 		sih->edge_change |= BIT(data->irq - sih->irq_base);
 		queue_work(wq, &sih->edge_work);
 	}
-	spin_unlock_irqrestore(&sih_agent_lock, flags);
+
 	return 0;
 }
 
+static void twl4030_sih_bus_lock(struct irq_data *data)
+{
+	struct sih_agent *sih = irq_data_get_irq_chip_data(data);
+
+	mutex_lock(&sih->irq_lock);
+}
+
+static void twl4030_sih_bus_sync_unlock(struct irq_data *data)
+{
+	struct sih_agent *sih = irq_data_get_irq_chip_data(data);
+
+	mutex_unlock(&sih->irq_lock);
+}
+
 static struct irq_chip twl4030_sih_irq_chip = {
 	.name		= "twl4030",
 	.irq_mask	= twl4030_sih_mask,
 	.irq_unmask	= twl4030_sih_unmask,
 	.irq_set_type	= twl4030_sih_set_type,
+	.irq_bus_lock	= twl4030_sih_bus_lock,
+	.irq_bus_sync_unlock = twl4030_sih_bus_sync_unlock,
 };
 
 /*----------------------------------------------------------------------*/
@@ -718,15 +722,16 @@ int twl4030_sih_setup(int module)
 	agent->irq_base = irq_base;
 	agent->sih = sih;
 	agent->imr = ~0;
+	mutex_init(&agent->irq_lock);
 	INIT_WORK(&agent->mask_work, twl4030_sih_do_mask);
 	INIT_WORK(&agent->edge_work, twl4030_sih_do_edge);
 
 	for (i = 0; i < sih->bits; i++) {
 		irq = irq_base + i;
 
+		irq_set_chip_data(irq, agent);
 		irq_set_chip_and_handler(irq, &twl4030_sih_irq_chip,
 					 handle_edge_irq);
-		irq_set_chip_data(irq, agent);
 		activate_irq(irq);
 	}
 
-- 
1.7.6

[PATCH 3/7] mfd: twl4030-irq: drop the kthread

[Felipe Balbi]

... and use threaded IRQ infrastructure. Later
patches will come dropping both workqueues and
setting the nested thread flag.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/mfd/twl4030-irq.c |   94 ++++++++++-----------------------------------
 1 files changed, 21 insertions(+), 73 deletions(-)

diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index 291364c..e1e0944 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -30,7 +30,6 @@
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
-#include <linux/kthread.h>
 #include <linux/slab.h>
 
 #include <linux/i2c/twl.h>
@@ -278,59 +277,6 @@ static const struct sih sih_modules_twl5031[8] = {
 
 static unsigned twl4030_irq_base;
 
-static struct completion irq_event;
-
-/*
- * This thread processes interrupts reported by the Primary Interrupt Handler.
- */
-static int twl4030_irq_thread(void *data)
-{
-	long irq = (long)data;
-	static unsigned i2c_errors;
-	static const unsigned max_i2c_errors = 100;
-
-
-	current->flags |= PF_NOFREEZE;
-
-	while (!kthread_should_stop()) {
-		int ret;
-		int module_irq;
-		u8 pih_isr;
-
-		/* Wait for IRQ, then read PIH irq status (also blocking) */
-		wait_for_completion_interruptible(&irq_event);
-
-		ret = twl_i2c_read_u8(TWL4030_MODULE_PIH, &pih_isr,
-					  REG_PIH_ISR_P1);
-		if (ret) {
-			pr_warning("twl4030: I2C error %d reading PIH ISR\n",
-					ret);
-			if (++i2c_errors >= max_i2c_errors) {
-				printk(KERN_ERR "Maximum I2C error count"
-						" exceeded.  Terminating %s.\n",
-						__func__);
-				break;
-			}
-			complete(&irq_event);
-			continue;
-		}
-
-		/* these handlers deal with the relevant SIH irq status */
-		local_irq_disable();
-		for (module_irq = twl4030_irq_base;
-				pih_isr;
-				pih_isr >>= 1, module_irq++) {
-			if (pih_isr & 0x1)
-				generic_handle_irq(module_irq);
-		}
-		local_irq_enable();
-
-		enable_irq(irq);
-	}
-
-	return 0;
-}
-
 /*
  * handle_twl4030_pih() is the desc->handle method for the twl4030 interrupt.
  * This is a chained interrupt, so there is no desc->action method for it.
@@ -342,9 +288,25 @@ static int twl4030_irq_thread(void *data)
  */
 static irqreturn_t handle_twl4030_pih(int irq, void *devid)
 {
-	/* Acknowledge, clear *AND* mask the interrupt... */
-	disable_irq_nosync(irq);
-	complete(devid);
+	int		module_irq;
+	irqreturn_t	ret;
+	u8		pih_isr;
+
+	ret = twl_i2c_read_u8(TWL4030_MODULE_PIH, &pih_isr,
+			REG_PIH_ISR_P1);
+	if (ret) {
+		pr_warning("twl4030: I2C error %d reading PIH ISR\n", ret);
+		return IRQ_NONE;
+	}
+
+	/* these handlers deal with the relevant SIH irq status */
+	for (module_irq = twl4030_irq_base;
+			pih_isr;
+			pih_isr >>= 1, module_irq++) {
+		if (pih_isr & 0x1)
+			generic_handle_irq(module_irq);
+	}
+
 	return IRQ_HANDLED;
 }
 /*----------------------------------------------------------------------*/
@@ -763,7 +725,6 @@ int twl4030_init_irq(int irq_num, unsigned irq_base, unsigned irq_end)
 
 	int			status;
 	int			i;
-	struct task_struct	*task;
 
 	/*
 	 * Mask and clear all TWL4030 interrupts since initially we do
@@ -806,27 +767,14 @@ int twl4030_init_irq(int irq_num, unsigned irq_base, unsigned irq_end)
 	}
 
 	/* install an irq handler to demultiplex the TWL4030 interrupt */
-
-
-	init_completion(&irq_event);
-
-	status = request_irq(irq_num, handle_twl4030_pih, IRQF_DISABLED,
-				"TWL4030-PIH", &irq_event);
+	status = request_threaded_irq(irq_num, NULL, handle_twl4030_pih,
+			IRQF_DISABLED, "TWL4030-PIH", NULL);
 	if (status < 0) {
 		pr_err("twl4030: could not claim irq%d: %d\n", irq_num, status);
 		goto fail_rqirq;
 	}
 
-	task = kthread_run(twl4030_irq_thread, (void *)(long)irq_num,
-								"twl4030-irq");
-	if (IS_ERR(task)) {
-		pr_err("twl4030: could not create irq %d thread!\n", irq_num);
-		status = PTR_ERR(task);
-		goto fail_kthread;
-	}
 	return status;
-fail_kthread:
-	free_irq(irq_num, &irq_event);
 fail_rqirq:
 	/* clean up twl4030_sih_setup */
 fail:
-- 
1.7.6

[regression] Re: [PATCH 3/7] mfd: twl4030-irq: drop the kthread

[Felipe Contreras]

On Thu, Jun 30, 2011 at 12:51 PM, Felipe Balbi <balbi@ti.com> wrote:
> ... and use threaded IRQ infrastructure. Later
> patches will come dropping both workqueues and
> setting the nested thread flag.

There's something wrong. When I boot my Nokia N900 with this, as soon
as I press a key from the keyboard the whole device hangs. I don't see
anything on the console framebuffer, and with a jig I can't boot from
MMC, so there isn't much more I can provide at the moment.

Cheers.

-- 
Felipe Contreras

Re: [regression] Re: [PATCH 3/7] mfd: twl4030-irq: drop the kthread

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 724 bytes --]

On Tue, Dec 06, 2011 at 05:28:58PM +0200, Felipe Contreras wrote:
> On Thu, Jun 30, 2011 at 12:51 PM, Felipe Balbi <balbi@ti.com> wrote:
> > ... and use threaded IRQ infrastructure. Later
> > patches will come dropping both workqueues and
> > setting the nested thread flag.
> 
> There's something wrong. When I boot my Nokia N900 with this, as soon
> as I press a key from the keyboard the whole device hangs. I don't see
> anything on the console framebuffer, and with a jig I can't boot from
> MMC, so there isn't much more I can provide at the moment.

Funny, this booted fine on my beagle and MMC card uses a twl GPIO for
card detection. Maybe it's something fairly obvious that I missed :-(

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [regression] Re: [PATCH 3/7] mfd: twl4030-irq: drop the kthread

[Felipe Contreras]

On Wed, Dec 7, 2011 at 8:51 AM, Felipe Balbi <balbi@ti.com> wrote:
> On Tue, Dec 06, 2011 at 05:28:58PM +0200, Felipe Contreras wrote:
>> On Thu, Jun 30, 2011 at 12:51 PM, Felipe Balbi <balbi@ti.com> wrote:
>> > ... and use threaded IRQ infrastructure. Later
>> > patches will come dropping both workqueues and
>> > setting the nested thread flag.
>>
>> There's something wrong. When I boot my Nokia N900 with this, as soon
>> as I press a key from the keyboard the whole device hangs. I don't see
>> anything on the console framebuffer, and with a jig I can't boot from
>> MMC, so there isn't much more I can provide at the moment.
>
> Funny, this booted fine on my beagle and MMC card uses a twl GPIO for
> card detection. Maybe it's something fairly obvious that I missed :-(

Did you try typing anything? That's where the problem happened on my side.

I'll try to flash a system to NAND, since I can't seem to just get the
MMC to work when the cover is open and the device is connected to the
jig to see if I can see something from serial console.

You don't have any idea about what to try?

-- 
Felipe Contreras

Re: [regression] Re: [PATCH 3/7] mfd: twl4030-irq: drop the kthread

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1556 bytes --]

On Wed, 7 Dec 2011 13:04:03 +0200 Felipe Contreras
<felipe.contreras@gmail.com> wrote:

> On Wed, Dec 7, 2011 at 8:51 AM, Felipe Balbi <balbi@ti.com> wrote:
> > On Tue, Dec 06, 2011 at 05:28:58PM +0200, Felipe Contreras wrote:
> >> On Thu, Jun 30, 2011 at 12:51 PM, Felipe Balbi <balbi@ti.com> wrote:
> >> > ... and use threaded IRQ infrastructure. Later
> >> > patches will come dropping both workqueues and
> >> > setting the nested thread flag.
> >>
> >> There's something wrong. When I boot my Nokia N900 with this, as soon
> >> as I press a key from the keyboard the whole device hangs. I don't see
> >> anything on the console framebuffer, and with a jig I can't boot from
> >> MMC, so there isn't much more I can provide at the moment.
> >
> > Funny, this booted fine on my beagle and MMC card uses a twl GPIO for
> > card detection. Maybe it's something fairly obvious that I missed :-(
> 
> Did you try typing anything? That's where the problem happened on my side.
> 
> I'll try to flash a system to NAND, since I can't seem to just get the
> MMC to work when the cover is open and the device is connected to the
> jig to see if I can see something from serial console.
> 
> You don't have any idea about what to try?
> 

 Try the patches I posted to linux-kernel on Nov 27:

   Subject: [PATCH 0/4] fixes for twl4030-irq in mainline

and following

   http://lkml.org/lkml/2011/11/26/92

I think I hit exactly the same bug and you and (eventually) fix it (that was
a fun learning experience).

NeilBrown


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [regression] Re: [PATCH 3/7] mfd: twl4030-irq: drop the kthread

[Felipe Contreras]

On Wed, Dec 7, 2011 at 1:30 PM, NeilBrown <neilb@suse.de> wrote:
>  Try the patches I posted to linux-kernel on Nov 27:
>
>   Subject: [PATCH 0/4] fixes for twl4030-irq in mainline
>
> and following
>
>   http://lkml.org/lkml/2011/11/26/92
>
> I think I hit exactly the same bug and you and (eventually) fix it (that was
> a fun learning experience).

Yeap, those patches fix the issue :)

Cheers.

-- 
Felipe Contreras

Re: [regression] Re: [PATCH 3/7] mfd: twl4030-irq: drop the kthread

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

On Wed, Dec 07, 2011 at 01:04:03PM +0200, Felipe Contreras wrote:
> On Wed, Dec 7, 2011 at 8:51 AM, Felipe Balbi <balbi@ti.com> wrote:
> > On Tue, Dec 06, 2011 at 05:28:58PM +0200, Felipe Contreras wrote:
> >> On Thu, Jun 30, 2011 at 12:51 PM, Felipe Balbi <balbi@ti.com> wrote:
> >> > ... and use threaded IRQ infrastructure. Later
> >> > patches will come dropping both workqueues and
> >> > setting the nested thread flag.
> >>
> >> There's something wrong. When I boot my Nokia N900 with this, as soon
> >> as I press a key from the keyboard the whole device hangs. I don't see
> >> anything on the console framebuffer, and with a jig I can't boot from
> >> MMC, so there isn't much more I can provide at the moment.
> >
> > Funny, this booted fine on my beagle and MMC card uses a twl GPIO for
> > card detection. Maybe it's something fairly obvious that I missed :-(
> 
> Did you try typing anything? That's where the problem happened on my side.

beagle doesn't have keyboard ;-)

> I'll try to flash a system to NAND, since I can't seem to just get the
> MMC to work when the cover is open and the device is connected to the
> jig to see if I can see something from serial console.
> 
> You don't have any idea about what to try?

you could cut the back cover so that it just fits the device on the jig
(I did that to an old cover), then it works just fine :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [regression] Re: [PATCH 3/7] mfd: twl4030-irq: drop the kthread

[Jarkko Nikula]

On 12/07/2011 03:39 PM, Felipe Balbi wrote:
>> I'll try to flash a system to NAND, since I can't seem to just get the
>> MMC to work when the cover is open and the device is connected to the
>> jig to see if I can see something from serial console.
>>
>> You don't have any idea about what to try?
>
> you could cut the back cover so that it just fits the device on the jig
> (I did that to an old cover), then it works just fine :-)
>
You could also go without breaking the back cover by putting a magnet on 
top the hal sensor (like tape a piece from a fridge magnet). See here 
the place:

http://talk.maemo.org/showthread.php?p=1008775

Another option is to to set .gpio_cd = -EINVAL for external mmc in 
arch/arm/mach-omap2/board-rx51-peripherals.c.

-- 
Jarkko

Re: [regression] Re: [PATCH 3/7] mfd: twl4030-irq: drop the kthread

[Felipe Contreras]

On Wed, Dec 7, 2011 at 3:59 PM, Jarkko Nikula <jarkko.nikula@bitmer.com> wrote:
> On 12/07/2011 03:39 PM, Felipe Balbi wrote:
>>>
>>> I'll try to flash a system to NAND, since I can't seem to just get the
>>> MMC to work when the cover is open and the device is connected to the
>>> jig to see if I can see something from serial console.
>>>
>>> You don't have any idea about what to try?
>>
>>
>> you could cut the back cover so that it just fits the device on the jig
>> (I did that to an old cover), then it works just fine :-)
>>
> You could also go without breaking the back cover by putting a magnet on top
> the hal sensor (like tape a piece from a fridge magnet). See here the place:
>
> http://talk.maemo.org/showthread.php?p=1008775
>
> Another option is to to set .gpio_cd = -EINVAL for external mmc in
> arch/arm/mach-omap2/board-rx51-peripherals.c.

Nice. That gpio_cd trick did it :)

-- 
Felipe Contreras

Re: [PATCH 3/7] mfd: twl4030-irq: drop the kthread

[Felipe Contreras]

On Thu, Jun 30, 2011 at 12:51 PM, Felipe Balbi <balbi@ti.com> wrote:
> ... and use threaded IRQ infrastructure. Later
> patches will come dropping both workqueues and
> setting the nested thread flag.

...

>  static irqreturn_t handle_twl4030_pih(int irq, void *devid)
>  {
> -       /* Acknowledge, clear *AND* mask the interrupt... */
> -       disable_irq_nosync(irq);
> -       complete(devid);

...

> -       status = request_irq(irq_num, handle_twl4030_pih, IRQF_DISABLED,
> -                               "TWL4030-PIH", &irq_event);
> +       status = request_threaded_irq(irq_num, NULL, handle_twl4030_pih,
> +                       IRQF_DISABLED, "TWL4030-PIH", NULL);

FTR, in this reorganization this code was dropped
disable_irq_nosync(irq), which means the irq's would never be cleared.
IRQF_ONESHOT should have been on the flags.

But Neil Brown already found the issue and fixed it :)
http://article.gmane.org/gmane.linux.kernel/1220705

-- 
Felipe Contreras

[PATCH 4/7] mfd: twl4030-irq: drop mask_work

[Felipe Balbi]

... we can do the synchronization with the
hardware when calling bus_sync_unlock as
we're supposed to. While at that, also make
variable names uniform on all functions.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/mfd/twl4030-irq.c |   82 ++++++++++++++++++--------------------------
 1 files changed, 34 insertions(+), 48 deletions(-)

diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index e1e0944..bf62389 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -430,7 +430,6 @@ struct sih_agent {
 
 	u32			imr;
 	bool			imr_change_pending;
-	struct work_struct	mask_work;
 
 	u32			edge_change;
 	struct work_struct	edge_work;
@@ -438,37 +437,6 @@ struct sih_agent {
 	struct mutex		irq_lock;
 };
 
-static void twl4030_sih_do_mask(struct work_struct *work)
-{
-	struct sih_agent	*agent;
-	const struct sih	*sih;
-	union {
-		u8	bytes[4];
-		u32	word;
-	}			imr;
-	int			status;
-
-	agent = container_of(work, struct sih_agent, mask_work);
-
-	/* see what work we have */
-	if (agent->imr_change_pending) {
-		sih = agent->sih;
-		/* byte[0] gets overwritten as we write ... */
-		imr.word = cpu_to_le32(agent->imr << 8);
-		agent->imr_change_pending = false;
-	} else
-		sih = NULL;
-	if (!sih)
-		return;
-
-	/* write the whole mask ... simpler than subsetting it */
-	status = twl_i2c_write(sih->module, imr.bytes,
-			sih->mask[irq_line].imr_offset, sih->bytes_ixr);
-	if (status)
-		pr_err("twl4030: %s, %s --> %d\n", __func__,
-				"write", status);
-}
-
 static void twl4030_sih_do_edge(struct work_struct *work)
 {
 	struct sih_agent	*agent;
@@ -537,32 +505,30 @@ static void twl4030_sih_do_edge(struct work_struct *work)
 
 static void twl4030_sih_mask(struct irq_data *data)
 {
-	struct sih_agent *sih = irq_data_get_irq_chip_data(data);
+	struct sih_agent *agent = irq_data_get_irq_chip_data(data);
 
-	sih->imr |= BIT(data->irq - sih->irq_base);
-	sih->imr_change_pending = true;
-	queue_work(wq, &sih->mask_work);
+	agent->imr |= BIT(data->irq - agent->irq_base);
+	agent->imr_change_pending = true;
 }
 
 static void twl4030_sih_unmask(struct irq_data *data)
 {
-	struct sih_agent *sih = irq_data_get_irq_chip_data(data);
+	struct sih_agent *agent = irq_data_get_irq_chip_data(data);
 
-	sih->imr &= ~BIT(data->irq - sih->irq_base);
-	sih->imr_change_pending = true;
-	queue_work(wq, &sih->mask_work);
+	agent->imr &= ~BIT(data->irq - agent->irq_base);
+	agent->imr_change_pending = true;
 }
 
 static int twl4030_sih_set_type(struct irq_data *data, unsigned trigger)
 {
-	struct sih_agent *sih = irq_data_get_irq_chip_data(data);
+	struct sih_agent *agent = irq_data_get_irq_chip_data(data);
 
 	if (trigger & ~(IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
 		return -EINVAL;
 
 	if (irqd_get_trigger_type(data) != trigger) {
-		sih->edge_change |= BIT(data->irq - sih->irq_base);
-		queue_work(wq, &sih->edge_work);
+		agent->edge_change |= BIT(data->irq - agent->irq_base);
+		queue_work(wq, &agent->edge_work);
 	}
 
 	return 0;
@@ -570,16 +536,37 @@ static int twl4030_sih_set_type(struct irq_data *data, unsigned trigger)
 
 static void twl4030_sih_bus_lock(struct irq_data *data)
 {
-	struct sih_agent *sih = irq_data_get_irq_chip_data(data);
+	struct sih_agent	*agent = irq_data_get_irq_chip_data(data);
 
-	mutex_lock(&sih->irq_lock);
+	mutex_lock(&agent->irq_lock);
 }
 
 static void twl4030_sih_bus_sync_unlock(struct irq_data *data)
 {
-	struct sih_agent *sih = irq_data_get_irq_chip_data(data);
+	struct sih_agent	*agent = irq_data_get_irq_chip_data(data);
+	const struct sih	*sih = agent->sih;
+	int			status;
+
+	if (agent->imr_change_pending) {
+		union {
+			u32	word;
+			u8	bytes[4];
+		} imr;
+
+		/* byte[0] gets overwriten as we write ... */
+		imr.word = cpu_to_le32(agent->imr << 8);
+		agent->imr_change_pending = false;
+
+		/* write the whole mask ... simpler than subsetting it */
+		status = twl_i2c_write(sih->module, imr.bytes,
+				sih->mask[irq_line].imr_offset,
+				sih->bytes_ixr);
+		if (status)
+			pr_err("twl4030: %s, %s --> %d\n", __func__,
+					"write", status);
+	}
 
-	mutex_unlock(&sih->irq_lock);
+	mutex_unlock(&agent->irq_lock);
 }
 
 static struct irq_chip twl4030_sih_irq_chip = {
@@ -685,7 +672,6 @@ int twl4030_sih_setup(int module)
 	agent->sih = sih;
 	agent->imr = ~0;
 	mutex_init(&agent->irq_lock);
-	INIT_WORK(&agent->mask_work, twl4030_sih_do_mask);
 	INIT_WORK(&agent->edge_work, twl4030_sih_do_edge);
 
 	for (i = 0; i < sih->bits; i++) {
-- 
1.7.6

[PATCH 5/7] mfd: twl4030-irq: drop edge_work

[Felipe Balbi]

... and do all the synchronization with the
hardware during bus_sync_unlock. We can now
remove all the workqueues.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/mfd/twl4030-irq.c |  124 +++++++++++++++++++--------------------------
 1 files changed, 52 insertions(+), 72 deletions(-)

diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index bf62389..1b9ab2f 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -422,8 +422,6 @@ static inline void activate_irq(int irq)
 
 /*----------------------------------------------------------------------*/
 
-static struct workqueue_struct *wq;
-
 struct sih_agent {
 	int			irq_base;
 	const struct sih	*sih;
@@ -432,68 +430,10 @@ struct sih_agent {
 	bool			imr_change_pending;
 
 	u32			edge_change;
-	struct work_struct	edge_work;
 
 	struct mutex		irq_lock;
 };
 
-static void twl4030_sih_do_edge(struct work_struct *work)
-{
-	struct sih_agent	*agent;
-	const struct sih	*sih;
-	u8			bytes[6];
-	u32			edge_change;
-	int			status;
-
-	agent = container_of(work, struct sih_agent, edge_work);
-
-	/* see what work we have */
-	edge_change = agent->edge_change;
-	agent->edge_change = 0;
-	sih = edge_change ? agent->sih : NULL;
-	if (!sih)
-		return;
-
-	/* Read, reserving first byte for write scratch.  Yes, this
-	 * could be cached for some speedup ... but be careful about
-	 * any processor on the other IRQ line, EDR registers are
-	 * shared.
-	 */
-	status = twl_i2c_read(sih->module, bytes + 1,
-			sih->edr_offset, sih->bytes_edr);
-	if (status) {
-		pr_err("twl4030: %s, %s --> %d\n", __func__,
-				"read", status);
-		return;
-	}
-
-	/* Modify only the bits we know must change */
-	while (edge_change) {
-		int		i = fls(edge_change) - 1;
-		struct irq_data	*idata = irq_get_irq_data(i + agent->irq_base);
-		int		byte = 1 + (i >> 2);
-		int		off = (i & 0x3) * 2;
-		unsigned int	type;
-
-		bytes[byte] &= ~(0x03 << off);
-
-		type = irqd_get_trigger_type(idata);
-		if (type & IRQ_TYPE_EDGE_RISING)
-			bytes[byte] |= BIT(off + 1);
-		if (type & IRQ_TYPE_EDGE_FALLING)
-			bytes[byte] |= BIT(off + 0);
-
-		edge_change &= ~BIT(i);
-	}
-
-	/* Write */
-	status = twl_i2c_write(sih->module, bytes,
-			sih->edr_offset, sih->bytes_edr);
-	if (status)
-		pr_err("twl4030: %s, %s --> %d\n", __func__,
-				"write", status);
-}
-
 /*----------------------------------------------------------------------*/
 
 /*
@@ -526,10 +466,8 @@ static int twl4030_sih_set_type(struct irq_data *data, unsigned trigger)
 	if (trigger & ~(IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
 		return -EINVAL;
 
-	if (irqd_get_trigger_type(data) != trigger) {
+	if (irqd_get_trigger_type(data) != trigger)
 		agent->edge_change |= BIT(data->irq - agent->irq_base);
-		queue_work(wq, &agent->edge_work);
-	}
 
 	return 0;
 }
@@ -566,6 +504,56 @@ static void twl4030_sih_bus_sync_unlock(struct irq_data *data)
 					"write", status);
 	}
 
+	if (agent->edge_change) {
+		u32		edge_change;
+		u8		bytes[6];
+
+		edge_change = agent->edge_change;
+		agent->edge_change = 0;
+
+		/*
+		 * Read, reserving first byte for write scratch.  Yes, this
+		 * could be cached for some speedup ... but be careful about
+		 * any processor on the other IRQ line, EDR registers are
+		 * shared.
+		 */
+		status = twl_i2c_read(sih->module, bytes + 1,
+				sih->edr_offset, sih->bytes_edr);
+		if (status) {
+			pr_err("twl4030: %s, %s --> %d\n", __func__,
+					"read", status);
+			return;
+		}
+
+		/* Modify only the bits we know must change */
+		while (edge_change) {
+			int		i = fls(edge_change) - 1;
+			struct irq_data	*idata;
+			int		byte = 1 + (i >> 2);
+			int		off = (i & 0x3) * 2;
+			unsigned int	type;
+
+			idata = irq_get_irq_data(i + agent->irq_base);
+
+			bytes[byte] &= ~(0x03 << off);
+
+			type = irqd_get_trigger_type(idata);
+			if (type & IRQ_TYPE_EDGE_RISING)
+				bytes[byte] |= BIT(off + 1);
+			if (type & IRQ_TYPE_EDGE_FALLING)
+				bytes[byte] |= BIT(off + 0);
+
+			edge_change &= ~BIT(i);
+		}
+
+		/* Write */
+		status = twl_i2c_write(sih->module, bytes,
+				sih->edr_offset, sih->bytes_edr);
+		if (status)
+			pr_err("twl4030: %s, %s --> %d\n", __func__,
+					"write", status);
+	}
+
 	mutex_unlock(&agent->irq_lock);
 }
 
@@ -672,7 +660,6 @@ int twl4030_sih_setup(int module)
 	agent->sih = sih;
 	agent->imr = ~0;
 	mutex_init(&agent->irq_lock);
-	INIT_WORK(&agent->edge_work, twl4030_sih_do_edge);
 
 	for (i = 0; i < sih->bits; i++) {
 		irq = irq_base + i;
@@ -720,12 +707,6 @@ int twl4030_init_irq(int irq_num, unsigned irq_base, unsigned irq_end)
 	if (status < 0)
 		return status;
 
-	wq = create_singlethread_workqueue("twl4030-irqchip");
-	if (!wq) {
-		pr_err("twl4030: workqueue FAIL\n");
-		return -ESRCH;
-	}
-
 	twl4030_irq_base = irq_base;
 
 	/* install an irq handler for each of the SIH modules;
@@ -766,8 +747,7 @@ fail_rqirq:
 fail:
 	for (i = irq_base; i < irq_end; i++)
 		irq_set_chip_and_handler(i, NULL, NULL);
-	destroy_workqueue(wq);
-	wq = NULL;
+
 	return status;
 }
 
-- 
1.7.6

[PATCH 6/7] mfd: twl4030-irq: set irq nested flag

[Felipe Balbi]

threads from twl4030's children will be called
nested in the context of the demultiplexing
handler on twl4030-irq.c.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/mfd/twl4030-irq.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index 1b9ab2f..ff16e9c 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -304,7 +304,7 @@ static irqreturn_t handle_twl4030_pih(int irq, void *devid)
 			pih_isr;
 			pih_isr >>= 1, module_irq++) {
 		if (pih_isr & 0x1)
-			generic_handle_irq(module_irq);
+			handle_nested_irq(module_irq);
 	}
 
 	return IRQ_HANDLED;
@@ -596,9 +596,7 @@ static void handle_twl4030_sih(unsigned irq, struct irq_desc *desc)
 	int isr;
 
 	/* reading ISR acks the IRQs, using clear-on-read mode */
-	local_irq_enable();
 	isr = sih_read_isr(sih);
-	local_irq_disable();
 
 	if (isr < 0) {
 		pr_err("twl4030: %s SIH, read ISR error %d\n",
@@ -613,7 +611,7 @@ static void handle_twl4030_sih(unsigned irq, struct irq_desc *desc)
 		isr &= ~BIT(irq);
 
 		if (irq < sih->bits)
-			generic_handle_irq(agent->irq_base + irq);
+			handle_nested_irq(agent->irq_base + irq);
 		else
 			pr_err("twl4030: %s SIH, invalid ISR bit %d\n",
 				sih->name, irq);
@@ -720,6 +718,7 @@ int twl4030_init_irq(int irq_num, unsigned irq_base, unsigned irq_end)
 	for (i = irq_base; i < irq_end; i++) {
 		irq_set_chip_and_handler(i, &twl4030_irq_chip,
 					 handle_simple_irq);
+		irq_set_nested_thread(i, 1);
 		activate_irq(i);
 	}
 	twl4030_irq_next = i;
@@ -745,8 +744,10 @@ int twl4030_init_irq(int irq_num, unsigned irq_base, unsigned irq_end)
 fail_rqirq:
 	/* clean up twl4030_sih_setup */
 fail:
-	for (i = irq_base; i < irq_end; i++)
+	for (i = irq_base; i < irq_end; i++) {
+		irq_set_nested_thread(i, 0);
 		irq_set_chip_and_handler(i, NULL, NULL);
+	}
 
 	return status;
 }
-- 
1.7.6

Re: [PATCH 6/7] mfd: twl4030-irq: set irq nested flag

[Felipe Contreras]

On Thu, Jun 30, 2011 at 12:51 PM, Felipe Balbi <balbi@ti.com> wrote:
> threads from twl4030's children will be called
> nested in the context of the demultiplexing
> handler on twl4030-irq.c.
>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  drivers/mfd/twl4030-irq.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
> index 1b9ab2f..ff16e9c 100644
> --- a/drivers/mfd/twl4030-irq.c
> +++ b/drivers/mfd/twl4030-irq.c
> @@ -304,7 +304,7 @@ static irqreturn_t handle_twl4030_pih(int irq, void *devid)
>                        pih_isr;
>                        pih_isr >>= 1, module_irq++) {
>                if (pih_isr & 0x1)
> -                       generic_handle_irq(module_irq);
> +                       handle_nested_irq(module_irq);

But handle_nested_irq calls action->thread_fn, which is NULL for the
sih handlers that are set-up below; handle_twl4030_sih will never be
called.

>        }
>
>        return IRQ_HANDLED;
> @@ -596,9 +596,7 @@ static void handle_twl4030_sih(unsigned irq, struct irq_desc *desc)
>        int isr;
>
>        /* reading ISR acks the IRQs, using clear-on-read mode */
> -       local_irq_enable();
>        isr = sih_read_isr(sih);
> -       local_irq_disable();

But handle_twl4030_sih is still not nested.

Fortunately, Neil Brown already fixed the issues in this patch by
making handle_twl4030_sih truly nested:
http://article.gmane.org/gmane.linux.kernel/1220708

Cheers.

-- 
Felipe Contreras

[PATCH 7/7] rtc: twl: move to threaded irq

[Felipe Balbi]

simple trivial conversion to threaded_irq.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/rtc/rtc-twl.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/drivers/rtc/rtc-twl.c b/drivers/rtc/rtc-twl.c
index f9a2799..eca0502 100644
--- a/drivers/rtc/rtc-twl.c
+++ b/drivers/rtc/rtc-twl.c
@@ -362,14 +362,6 @@ static irqreturn_t twl_rtc_interrupt(int irq, void *rtc)
 	int res;
 	u8 rd_reg;
 
-#ifdef CONFIG_LOCKDEP
-	/* WORKAROUND for lockdep forcing IRQF_DISABLED on us, which
-	 * we don't want and can't tolerate.  Although it might be
-	 * friendlier not to borrow this thread context...
-	 */
-	local_irq_enable();
-#endif
-
 	res = twl_rtc_read_u8(&rd_reg, REG_RTC_STATUS_REG);
 	if (res)
 		goto out;
@@ -462,7 +454,7 @@ static int __devinit twl_rtc_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto out1;
 
-	ret = request_irq(irq, twl_rtc_interrupt,
+	ret = request_threaded_irq(irq, NULL, twl_rtc_interrupt,
 				IRQF_TRIGGER_RISING,
 				dev_name(&rtc->dev), rtc);
 	if (ret < 0) {
-- 
1.7.6

Re: [PATCH 0/7] twl4030: finish threaded IRQ conversion

[Mark Brown]

On Thu, Jun 30, 2011 at 12:51:03PM +0300, Felipe Balbi wrote:

> the following patches where boot-tested on beagle xM
> and everything seems fine. MMC root filesystem still
> mounts, /proc/interrupts looks like ps aux | grep irq
> shows our threads, etc.

Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 0/7] twl4030: finish threaded IRQ conversion

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

Hi,

On Sat, Jul 09, 2011 at 05:09:00AM +0100, Mark Brown wrote:
> On Thu, Jun 30, 2011 at 12:51:03PM +0300, Felipe Balbi wrote:
> 
> > the following patches where boot-tested on beagle xM
> > and everything seems fine. MMC root filesystem still
> > mounts, /proc/interrupts looks like ps aux | grep irq
> > shows our threads, etc.
> 
> Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

It has been a long time and nobody complained. Samuel, can you take
these patches in for 3.2 merge window ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 0/7] twl4030: finish threaded IRQ conversion

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]

Hi,

On Fri, Aug 19, 2011 at 12:36:15AM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Sat, Jul 09, 2011 at 05:09:00AM +0100, Mark Brown wrote:
> > On Thu, Jun 30, 2011 at 12:51:03PM +0300, Felipe Balbi wrote:
> > 
> > > the following patches where boot-tested on beagle xM
> > > and everything seems fine. MMC root filesystem still
> > > mounts, /proc/interrupts looks like ps aux | grep irq
> > > shows our threads, etc.
> > 
> > Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> It has been a long time and nobody complained. Samuel, can you take
> these patches in for 3.2 merge window ?

if you want in pull request form, here it is:

The following changes since commit 93ee7a9340d64f20295aacc3fb6a22b759323280:

  Linux 3.1-rc2 (2011-08-14 15:09:08 -0700)

are available in the git repository at:
  master.kernel.org:/pub/scm/linux/kernel/git/balbi/usb.git irq

Felipe Balbi (7):
      mfd: twl4030-irq: remove trailing whitespaces
      mfd: twl4030-irq: implement bus_lock/bus_sync_unlock methods
      mfd: twl4030-irq: drop the kthread
      mfd: twl4030-irq: drop mask_work
      mfd: twl4030-irq: drop edge_work
      mfd: twl4030-irq: set irq nested flag
      rtc: twl: move to threaded irq

 drivers/mfd/twl4030-irq.c |  342 +++++++++++++++++----------------------------
 drivers/rtc/rtc-twl.c     |   10 +--
 2 files changed, 132 insertions(+), 220 deletions(-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 0/7] twl4030: finish threaded IRQ conversion

[Samuel Ortiz]

Hi Felipe,

On Fri, Aug 19, 2011 at 12:36:15AM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Sat, Jul 09, 2011 at 05:09:00AM +0100, Mark Brown wrote:
> > On Thu, Jun 30, 2011 at 12:51:03PM +0300, Felipe Balbi wrote:
> > 
> > > the following patches where boot-tested on beagle xM
> > > and everything seems fine. MMC root filesystem still
> > > mounts, /proc/interrupts looks like ps aux | grep irq
> > > shows our threads, etc.
> > 
> > Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> It has been a long time and nobody complained. Samuel, can you take
> these patches in for 3.2 merge window ?
Yes, that sounds like the right time now.
All 7 patches applied, many thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 0/7] twl4030: finish threaded IRQ conversion

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

Hi,

On Mon, Aug 22, 2011 at 04:54:36PM +0200, Samuel Ortiz wrote:
> On Fri, Aug 19, 2011 at 12:36:15AM +0300, Felipe Balbi wrote:
> > Hi,
> > 
> > On Sat, Jul 09, 2011 at 05:09:00AM +0100, Mark Brown wrote:
> > > On Thu, Jun 30, 2011 at 12:51:03PM +0300, Felipe Balbi wrote:
> > > 
> > > > the following patches where boot-tested on beagle xM
> > > > and everything seems fine. MMC root filesystem still
> > > > mounts, /proc/interrupts looks like ps aux | grep irq
> > > > shows our threads, etc.
> > > 
> > > Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > 
> > It has been a long time and nobody complained. Samuel, can you take
> > these patches in for 3.2 merge window ?
> Yes, that sounds like the right time now.
> All 7 patches applied, many thanks.

thanks ;-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]