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

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

[NeilBrown]

Hi,
  The recent tidying up of twl4030-irq seems to have left it broken.
At least it doesn't work for me on my gta04 (www.gta04.org).  The
first interrupt from the device freezes the whole system (by being
constantly delivered)

The following 4 patches make it work for me and addresses some other
less critical issues like a typo in a comment :-)

Thanks,
NeilBrown


---

NeilBrown (4):
      twl4030-irq: fix typo
      twl4030-irq: set tertiary interrupts to be nested/threaded.
      twl4030-irq: use request_threaded_irq in place of irq_set_chained_handler.
      twl4030-irq: Base interrupt must be one-shot


 drivers/mfd/twl4030-irq.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

-- 
Signature

[PATCH 1/4] twl4030-irq: Base interrupt must be one-shot

[NeilBrown]

As the interrupt source is only cleared by the threaded interrupt
service routine, we need to make the base interrupt IRQF_ONESHOT.
Without this, the first interrupt from the TWL4030 cause the CPU to
enter an infinite loop trying to handle to interrupt but never
clearing it.

Signed-off-by: NeilBrown <neilb@suse.de>
---

 drivers/mfd/twl4030-irq.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index f062c8c..3d3d6ed 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -733,8 +733,9 @@ int twl4030_init_irq(int irq_num, unsigned irq_base, unsigned irq_end)
 	}
 
 	/* install an irq handler to demultiplex the TWL4030 interrupt */
-	status = request_threaded_irq(irq_num, NULL, handle_twl4030_pih, 0,
-					"TWL4030-PIH", NULL);
+	status = request_threaded_irq(irq_num, NULL, handle_twl4030_pih,
+				      IRQF_ONESHOT,
+				      "TWL4030-PIH", NULL);
 	if (status < 0) {
 		pr_err("twl4030: could not claim irq%d: %d\n", irq_num, status);
 		goto fail_rqirq;

[PATCH 4/4] twl4030-irq: fix typo

[NeilBrown]

overwriten -> overwritten

Signed-off-by: NeilBrown <neilb@suse.de>
---

 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 3ac7216..b69bb51 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -492,7 +492,7 @@ static void twl4030_sih_bus_sync_unlock(struct irq_data *data)
 			u8	bytes[4];
 		} imr;
 
-		/* byte[0] gets overwriten as we write ... */
+		/* byte[0] gets overwritten as we write ... */
 		imr.word = cpu_to_le32(agent->imr << 8);
 		agent->imr_change_pending = false;
 

[PATCH 3/4] twl4030-irq: set tertiary interrupts to be nested/threaded.

[NeilBrown]

As tertiary interrupts are handled by handle_twl4030_sih calling
handle_nested_irq, they do not need their own separate irq thread.
So mark them as 'nested_thread' interrupts to avoid the extra thread
creation.

Tested on GTA04 Pheonux.

Signed-off-by: NeilBrown <neilb@suse.de>
---

 drivers/mfd/twl4030-irq.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index 29f11e0..3ac7216 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -667,6 +667,7 @@ int twl4030_sih_setup(int module)
 		irq_set_chip_data(irq, agent);
 		irq_set_chip_and_handler(irq, &twl4030_sih_irq_chip,
 					 handle_edge_irq);
+		irq_set_nested_thread(irq, 1);
 		activate_irq(irq);
 	}
 

[PATCH 2/4] twl4030-irq: use request_threaded_irq in place of irq_set_chained_handler.

[NeilBrown]

irq_set_chained_handler sets 'desc->handle_irq'.
However this irq is called by handle_nested_irq from handle_twl4030_pih,
and that uses action->thread_fn.

So the handled set with irq_set_chained_handler is never called.

So change to use request_threaded_irq instead - that sets the correct field.

Tested on GTA04 Phoenux.

Signed-off-by: NeilBrown <neilb@suse.de>
---

 drivers/mfd/twl4030-irq.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index 3d3d6ed..29f11e0 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -432,6 +432,7 @@ struct sih_agent {
 	u32			edge_change;
 
 	struct mutex		irq_lock;
+	char			*irq_name;
 };
 
 /*----------------------------------------------------------------------*/
@@ -589,7 +590,7 @@ static inline int sih_read_isr(const struct sih *sih)
  * Generic handler for SIH interrupts ... we "know" this is called
  * in task context, with IRQs enabled.
  */
-static void handle_twl4030_sih(unsigned irq, struct irq_desc *desc)
+static irqreturn_t handle_twl4030_sih(int irq, void *data)
 {
 	struct sih_agent *agent = irq_get_handler_data(irq);
 	const struct sih *sih = agent->sih;
@@ -602,7 +603,7 @@ static void handle_twl4030_sih(unsigned irq, struct irq_desc *desc)
 		pr_err("twl4030: %s SIH, read ISR error %d\n",
 			sih->name, isr);
 		/* REVISIT:  recover; eventually mask it all, etc */
-		return;
+		return IRQ_HANDLED;
 	}
 
 	while (isr) {
@@ -616,6 +617,7 @@ static void handle_twl4030_sih(unsigned irq, struct irq_desc *desc)
 			pr_err("twl4030: %s SIH, invalid ISR bit %d\n",
 				sih->name, irq);
 	}
+	return IRQ_HANDLED;
 }
 
 static unsigned twl4030_irq_next;
@@ -668,18 +670,19 @@ int twl4030_sih_setup(int module)
 		activate_irq(irq);
 	}
 
-	status = irq_base;
 	twl4030_irq_next += i;
 
 	/* replace generic PIH handler (handle_simple_irq) */
 	irq = sih_mod + twl4030_irq_base;
 	irq_set_handler_data(irq, agent);
-	irq_set_chained_handler(irq, handle_twl4030_sih);
+	agent->irq_name = kasprintf(GFP_KERNEL, "twl4030_%s", sih->name);
+	status = request_threaded_irq(irq, NULL, handle_twl4030_sih, 0,
+				      agent->irq_name ?: sih->name, NULL);
 
 	pr_info("twl4030: %s (irq %d) chaining IRQs %d..%d\n", sih->name,
 			irq, irq_base, twl4030_irq_next - 1);
 
-	return status;
+	return status < 0 ? status : irq_base;
 }
 
 /* FIXME need a call to reverse twl4030_sih_setup() ... */

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

[Felipe Contreras]

On Sat, Nov 26, 2011 at 10:17 PM, NeilBrown <neilb@suse.de> wrote:
>  The recent tidying up of twl4030-irq seems to have left it broken.
> At least it doesn't work for me on my gta04 (www.gta04.org).  The
> first interrupt from the device freezes the whole system (by being
> constantly delivered)
>
> The following 4 patches make it work for me and addresses some other
> less critical issues like a typo in a comment :-)

I also hit the same issue on the N900. These patches fix the problem for me:

Tested-by: Felipe Contreras <felipe.contreras@gmail.com>

-- 
Felipe Contreras

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

[Samuel Ortiz]

Hi Neil,

On Sun, Nov 27, 2011 at 07:17:41AM +1100, NeilBrown wrote:
> Hi,
>   The recent tidying up of twl4030-irq seems to have left it broken.
> At least it doesn't work for me on my gta04 (www.gta04.org).  The
> first interrupt from the device freezes the whole system (by being
> constantly delivered)
> 
> The following 4 patches make it work for me and addresses some other
> less critical issues like a typo in a comment :-)

Thanks, I applied all 4 of them.

Cheers,
Samuel.

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

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

[NeilBrown]

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

On Mon, 12 Dec 2011 18:38:19 +0100 Samuel Ortiz <sameo@linux.intel.com> wrote:

> Hi Neil,
> 
> On Sun, Nov 27, 2011 at 07:17:41AM +1100, NeilBrown wrote:
> > Hi,
> >   The recent tidying up of twl4030-irq seems to have left it broken.
> > At least it doesn't work for me on my gta04 (www.gta04.org).  The
> > first interrupt from the device freezes the whole system (by being
> > constantly delivered)
> > 
> > The following 4 patches make it work for me and addresses some other
> > less critical issues like a typo in a comment :-)
> 
> Thanks, I applied all 4 of them.

Thanks.  I have a couple of other twl4030 patches, these ones related to
battery charging.   Hopefully I'll be ready to post them later this week.
Would they go through you too?

NeilBrown

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

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

[Felipe Contreras]

Hi,

On Mon, Dec 12, 2011 at 7:38 PM, Samuel Ortiz <sameo@linux.intel.com> wrote:
> On Sun, Nov 27, 2011 at 07:17:41AM +1100, NeilBrown wrote:
>>   The recent tidying up of twl4030-irq seems to have left it broken.
>> At least it doesn't work for me on my gta04 (www.gta04.org).  The
>> first interrupt from the device freezes the whole system (by being
>> constantly delivered)
>>
>> The following 4 patches make it work for me and addresses some other
>> less critical issues like a typo in a comment :-)
>
> Thanks, I applied all 4 of them.

Did you apply them for 3.2 or 3.3? Without the first patch any system
that has a twl4030 chip will immediately hang on the first interrupt,
and many functions of twl4030 will just not work without the second
one.

-- 
Felipe Contreras

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

[Samuel Ortiz]

Hi Neil,

On Tue, Dec 13, 2011 at 08:39:59AM +1100, NeilBrown wrote:
> > > The following 4 patches make it work for me and addresses some other
> > > less critical issues like a typo in a comment :-)
> > 
> > Thanks, I applied all 4 of them.
> 
> Thanks.  I have a couple of other twl4030 patches, these ones related to
> battery charging.   Hopefully I'll be ready to post them later this week.
> Would they go through you too?
They most likely would, yes.

Cheers,
Samuel.

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

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

[Samuel Ortiz]

Hi Felipe,

On Tue, Dec 13, 2011 at 04:12:02PM +0200, Felipe Contreras wrote:
> Hi,
> 
> On Mon, Dec 12, 2011 at 7:38 PM, Samuel Ortiz <sameo@linux.intel.com> wrote:
> > On Sun, Nov 27, 2011 at 07:17:41AM +1100, NeilBrown wrote:
> >>   The recent tidying up of twl4030-irq seems to have left it broken.
> >> At least it doesn't work for me on my gta04 (www.gta04.org).  The
> >> first interrupt from the device freezes the whole system (by being
> >> constantly delivered)
> >>
> >> The following 4 patches make it work for me and addresses some other
> >> less critical issues like a typo in a comment :-)
> >
> > Thanks, I applied all 4 of them.
> 
> Did you apply them for 3.2 or 3.3? Without the first patch any system
> that has a twl4030 chip will immediately hang on the first interrupt,
> and many functions of twl4030 will just not work without the second
> one.
Thanks for the heads up. I applied the first 2 patches to my for-linus branch.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html