Hi Santosh,

On Mon, Jul 27, 2009 at 11:30:48AM +0530, Santosh Shilimkar wrote:
> From: Russell King <[email protected]>
> 
> (Rebased on 2.6.31-rc4)
> 
> The TWL4030 IRQ handler has a bug which leads to spinlock lock-up. It is
> calling the 'unmask' function in a process context. :The mask/unmask/ack
> functions are only designed to be called from the IRQ handler code,
> or the proper API interfaces found in linux/interrupt.h.
> 
> Also there is no need to have IRQ chaining mechanism. The right way to
> handle this is to claim the parent interrupt as a standard interrupt
> and arrange for handle_twl4030_pih to take care of the rest of the devices.
I'd like this one to be split in 2 different patches as you're addressing 2
different issues here.

Cheers,
Samuel.

> Mail thread on this issue can be found at:
> http://marc.info/?l=linux-arm-kernel&m=124629940123396&w=2
> 
> Signed-off-by: Russell King <[email protected]>
> Tested-by: Santosh Shilimkar <[email protected]>
> ---
>  drivers/mfd/twl4030-irq.c |   55 +++++++++++++++++++-------------------------
>  1 files changed, 24 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
> index bae61b2..7d43083 100644
> --- a/drivers/mfd/twl4030-irq.c
> +++ b/drivers/mfd/twl4030-irq.c
> @@ -180,14 +180,9 @@ static struct completion irq_event;
>  static int twl4030_irq_thread(void *data)
>  {
>       long irq = (long)data;
> -     struct irq_desc *desc = irq_to_desc(irq);
>       static unsigned i2c_errors;
>       static const unsigned max_i2c_errors = 100;
>  
> -     if (!desc) {
> -             pr_err("twl4030: Invalid IRQ: %ld\n", irq);
> -             return -EINVAL;
> -     }
>  
>       current->flags |= PF_NOFREEZE;
>  
> @@ -240,7 +235,7 @@ static int twl4030_irq_thread(void *data)
>               }
>               local_irq_enable();
>  
> -             desc->chip->unmask(irq);
> +             enable_irq(irq);
>       }
>  
>       return 0;
> @@ -255,25 +250,13 @@ static int twl4030_irq_thread(void *data)
>   * thread.  All we do here is acknowledge and mask the interrupt and wakeup
>   * the kernel thread.
>   */
> -static void handle_twl4030_pih(unsigned int irq, struct irq_desc *desc)
> +static irqreturn_t handle_twl4030_pih(int irq, void *devid)
>  {
>       /* Acknowledge, clear *AND* mask the interrupt... */
> -     desc->chip->ack(irq);
> -     complete(&irq_event);
> -}
> -
> -static struct task_struct *start_twl4030_irq_thread(long irq)
> -{
> -     struct task_struct *thread;
> -
> -     init_completion(&irq_event);
> -     thread = kthread_run(twl4030_irq_thread, (void *)irq, "twl4030-irq");
> -     if (!thread)
> -             pr_err("twl4030: could not create irq %ld thread!\n", irq);
> -
> -     return thread;
> +     disable_irq_nosync(irq);
> +     complete(devid);
> +     return IRQ_HANDLED;
>  }
> -
>  /*----------------------------------------------------------------------*/
>  
>  /*
> @@ -734,18 +717,28 @@ int twl_init_irq(int irq_num, unsigned irq_base, 
> unsigned irq_end)
>       }
>  
>       /* install an irq handler to demultiplex the TWL4030 interrupt */
> -     task = start_twl4030_irq_thread(irq_num);
> -     if (!task) {
> -             pr_err("twl4030: irq thread FAIL\n");
> -             status = -ESRCH;
> -             goto fail;
> -     }
>  
> -     set_irq_data(irq_num, task);
> -     set_irq_chained_handler(irq_num, handle_twl4030_pih);
>  
> -     return status;
> +     init_completion(&irq_event);
>  
> +     status = request_irq(irq_num, handle_twl4030_pih, IRQF_DISABLED,
> +                             "TWL4030-PIH", &irq_event);
> +     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 *)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:
>       for (i = irq_base; i < irq_end; i++)
>               set_irq_chip_and_handler(i, NULL, NULL);
> -- 
> 1.5.4.7
> 

-- 
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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to