2012-02-20 11:37:12

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH 01/01] MFD: add ADC support to DA9052/53 MFD core v2

Hi Ashish,

On Fri, Jan 13, 2012 at 08:00:51PM +0530, Ashish Jangam wrote:
> +struct completion done;
This shouldn't be a global variable, but a struct da9052 one instead.

Cheers,
Samuel.

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


2012-02-20 12:07:35

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 01/01] MFD: add ADC support to DA9052/53 MFD core v2

On Monday 20 February 2012, Samuel Ortiz wrote:
> On Fri, Jan 13, 2012 at 08:00:51PM +0530, Ashish Jangam wrote:
> > +struct completion done;
> This shouldn't be a global variable, but a struct da9052 one instead.

Right. Since this thread has showed up in my inbox again, I've
also taken a look at the patch now and noticed another thing:

>> + ret = request_threaded_irq(da9052->irq_base + DA9052_IRQ_ADC_EOM,
>> + NULL, da9052_auxadc_irq,
>> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
>> + "adc irq", NULL);

First of all, to clarify: The last argument to this function needs to be
the pointer to da9052 so that it is available in the irq handler function.

Also, I would recommend using request_irq instead of request_threaded_irq
here because the function only has a single "complete()" call in it,
just like a threaded IRQ handler has. There is no point going through
another thread just to wake up the one that is blocked.

Arnd

2012-02-20 15:42:06

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 01/01] MFD: add ADC support to DA9052/53 MFD core v2

On Mon, Feb 20, 2012 at 12:07:27PM +0000, Arnd Bergmann wrote:

> Also, I would recommend using request_irq instead of request_threaded_irq
> here because the function only has a single "complete()" call in it,
> just like a threaded IRQ handler has. There is no point going through
> another thread just to wake up the one that is blocked.

Actually in this case that won't work as the interrupt is generated by
the chip interrupt controller and as the chip is I2C/SPI controlled it
is itself threaded and can only generate threaded interrupts as genirq
can't get back into hard interrupt context.

As a result of this restriction when an interrupt handler doesn't care
what context it runs in it's better to use request_any_context_irq().
This will do a normal IRQ when it can but will bind successfully to a
threaded IRQ if that's what's provided.


Attachments:
(No filename) (851.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments