[PATCH] mfd: twl6030: Setup irq_wake infrastructure.

[PATCH] mfd: twl6030: Setup irq_wake infrastructure.

[Santosh Shilimkar]

TWL6030 devices have an interrupt line which is connected to
application processor like OMAP. These devices support multiple features
such as MMC card detect, USB cable detect, RTC interrupt, etc. that must
wake up the application processor.

With this change, TWL6030 client drivers can make use of
irq_wake() if the wakeup is desirable on it's irq events.

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
cc: Samuel Ortiz <sameo@linux.intel.com>
---
 drivers/mfd/twl6030-irq.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c
index eb3b5f8..4477134 100644
--- a/drivers/mfd/twl6030-irq.c
+++ b/drivers/mfd/twl6030-irq.c
@@ -187,6 +187,13 @@ static inline void activate_irq(int irq)
 #endif
 }
 
+int twl6030_irq_set_wake(struct irq_data *d, unsigned int on)
+{
+	int twl_irq = (int)irq_get_chip_data(d->irq);
+
+	return irq_set_irq_wake(twl_irq, on);
+}
+
 /*----------------------------------------------------------------------*/
 
 static unsigned twl6030_irq_next;
@@ -318,10 +325,12 @@ int twl6030_init_irq(int irq_num, unsigned irq_base, unsigned irq_end)
 	twl6030_irq_chip = dummy_irq_chip;
 	twl6030_irq_chip.name = "twl6030";
 	twl6030_irq_chip.irq_set_type = NULL;
+	twl6030_irq_chip.irq_set_wake = twl6030_irq_set_wake;
 
 	for (i = irq_base; i < irq_end; i++) {
 		irq_set_chip_and_handler(i, &twl6030_irq_chip,
 					 handle_simple_irq);
+		irq_set_chip_data(i, (void *)irq_num);
 		activate_irq(i);
 	}
 
-- 
1.7.4.1

Re: [PATCH] mfd: twl6030: Setup irq_wake infrastructure.

[Samuel Ortiz]

Hi Santosh,

On Tue, Sep 06, 2011 at 09:29:30PM +0530, Santosh Shilimkar wrote:
> TWL6030 devices have an interrupt line which is connected to
> application processor like OMAP. These devices support multiple features
> such as MMC card detect, USB cable detect, RTC interrupt, etc. that must
> wake up the application processor.
> 
> With this change, TWL6030 client drivers can make use of
> irq_wake() if the wakeup is desirable on it's irq events.
Patch applied to my for-next branch, thanks.

Cheers,
Samuel.

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

Re: [PATCH] mfd: twl6030: Setup irq_wake infrastructure.

[Shilimkar, Santosh]

On Thu, Sep 15, 2011 at 10:32 PM, Samuel Ortiz <sameo@linux.intel.com> wrote:
> Hi Santosh,
>
> On Tue, Sep 06, 2011 at 09:29:30PM +0530, Santosh Shilimkar wrote:
>> TWL6030 devices have an interrupt line which is connected to
>> application processor like OMAP. These devices support multiple features
>> such as MMC card detect, USB cable detect, RTC interrupt, etc. that must
>> wake up the application processor.
>>
>> With this change, TWL6030 client drivers can make use of
>> irq_wake() if the wakeup is desirable on it's irq events.
> Patch applied to my for-next branch, thanks.
>
Thanks Samuel.

Regards
Santosh

Re: [PATCH] mfd: twl6030: Setup irq_wake infrastructure.

[Todd Poynor]

On Tue, Sep 06, 2011 at 09:29:30PM +0530, Santosh Shilimkar wrote:
> TWL6030 devices have an interrupt line which is connected to
> application processor like OMAP. These devices support multiple features
> such as MMC card detect, USB cable detect, RTC interrupt, etc. that must
> wake up the application processor.
> 
> With this change, TWL6030 client drivers can make use of
> irq_wake() if the wakeup is desirable on it's irq events.
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> cc: Samuel Ortiz <sameo@linux.intel.com>
> ---
>  drivers/mfd/twl6030-irq.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c
> index eb3b5f8..4477134 100644
> --- a/drivers/mfd/twl6030-irq.c
> +++ b/drivers/mfd/twl6030-irq.c
> @@ -187,6 +187,13 @@ static inline void activate_irq(int irq)
>  #endif
>  }
>  
> +int twl6030_irq_set_wake(struct irq_data *d, unsigned int on)
> +{
> +	int twl_irq = (int)irq_get_chip_data(d->irq);
> +
> +	return irq_set_irq_wake(twl_irq, on);

Note that when running with this previously, LOCKDEP warnings
were seen.  LOCKDEP explicitly sets all irq_desc locks as a single
lock-class, causing "possible recursive locking detected" when the TWL
RTC (or other) driver calls through enable_irq_wake to
twl6030_irq_set_wake, which recursively calls irq_set_irq_wake.
Although the irq_desc and lock are different, LOCKDEP treats these as
equivalent, presumably due to problems that can be incurred when
locking more than one irq_desc.

Currently we're running with twl6030_irq_set_wake keeping a count of
sub-module wakeirqs, and setting the wakeup status for the twl_irq
enabled or disabled as needed at suspend time.  Can send a patch if
wanted.

> +}
> +
>  /*----------------------------------------------------------------------*/
>  
>  static unsigned twl6030_irq_next;
> @@ -318,10 +325,12 @@ int twl6030_init_irq(int irq_num, unsigned irq_base, unsigned irq_end)
>  	twl6030_irq_chip = dummy_irq_chip;
>  	twl6030_irq_chip.name = "twl6030";
>  	twl6030_irq_chip.irq_set_type = NULL;
> +	twl6030_irq_chip.irq_set_wake = twl6030_irq_set_wake;
>  
>  	for (i = irq_base; i < irq_end; i++) {
>  		irq_set_chip_and_handler(i, &twl6030_irq_chip,
>  					 handle_simple_irq);
> +		irq_set_chip_data(i, (void *)irq_num);
>  		activate_irq(i);
>  	}
>  
> -- 
> 1.7.4.1
> 
> --
> 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

Re: [PATCH] mfd: twl6030: Setup irq_wake infrastructure.

[Shilimkar, Santosh]

On Fri, Sep 16, 2011 at 2:07 AM, Todd Poynor <toddpoynor@google.com> wrote:
> On Tue, Sep 06, 2011 at 09:29:30PM +0530, Santosh Shilimkar wrote:
>> TWL6030 devices have an interrupt line which is connected to
>> application processor like OMAP. These devices support multiple features
>> such as MMC card detect, USB cable detect, RTC interrupt, etc. that must
>> wake up the application processor.
>>
>> With this change, TWL6030 client drivers can make use of
>> irq_wake() if the wakeup is desirable on it's irq events.
>>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> cc: Samuel Ortiz <sameo@linux.intel.com>
>> ---
>>  drivers/mfd/twl6030-irq.c |    9 +++++++++
>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c
>> index eb3b5f8..4477134 100644
>> --- a/drivers/mfd/twl6030-irq.c
>> +++ b/drivers/mfd/twl6030-irq.c
>> @@ -187,6 +187,13 @@ static inline void activate_irq(int irq)
>>  #endif
>>  }
>>
>> +int twl6030_irq_set_wake(struct irq_data *d, unsigned int on)
>> +{
>> +     int twl_irq = (int)irq_get_chip_data(d->irq);
>> +
>> +     return irq_set_irq_wake(twl_irq, on);
>
> Note that when running with this previously, LOCKDEP warnings
> were seen.  LOCKDEP explicitly sets all irq_desc locks as a single
> lock-class, causing "possible recursive locking detected" when the TWL
> RTC (or other) driver calls through enable_irq_wake to
> twl6030_irq_set_wake, which recursively calls irq_set_irq_wake.
> Although the irq_desc and lock are different, LOCKDEP treats these as
> equivalent, presumably due to problems that can be incurred when
> locking more than one irq_desc.
>
> Currently we're running with twl6030_irq_set_wake keeping a count of
> sub-module wakeirqs, and setting the wakeup status for the twl_irq
> enabled or disabled as needed at suspend time.  Can send a patch if
> wanted.
>
Please do that.

Regards
Santosh
--
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

Re: [PATCH] mfd: twl6030: Setup irq_wake infrastructure.

[Samuel Ortiz]

Hi Todd,

On Thu, Sep 15, 2011 at 01:37:38PM -0700, Todd Poynor wrote:
> On Tue, Sep 06, 2011 at 09:29:30PM +0530, Santosh Shilimkar wrote:
> > TWL6030 devices have an interrupt line which is connected to
> > application processor like OMAP. These devices support multiple features
> > such as MMC card detect, USB cable detect, RTC interrupt, etc. that must
> > wake up the application processor.
> > 
> > With this change, TWL6030 client drivers can make use of
> > irq_wake() if the wakeup is desirable on it's irq events.
> > 
> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > cc: Samuel Ortiz <sameo@linux.intel.com>
> > ---
> >  drivers/mfd/twl6030-irq.c |    9 +++++++++
> >  1 files changed, 9 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c
> > index eb3b5f8..4477134 100644
> > --- a/drivers/mfd/twl6030-irq.c
> > +++ b/drivers/mfd/twl6030-irq.c
> > @@ -187,6 +187,13 @@ static inline void activate_irq(int irq)
> >  #endif
> >  }
> >  
> > +int twl6030_irq_set_wake(struct irq_data *d, unsigned int on)
> > +{
> > +	int twl_irq = (int)irq_get_chip_data(d->irq);
> > +
> > +	return irq_set_irq_wake(twl_irq, on);
> 
> Note that when running with this previously, LOCKDEP warnings
> were seen.  LOCKDEP explicitly sets all irq_desc locks as a single
> lock-class, causing "possible recursive locking detected" when the TWL
> RTC (or other) driver calls through enable_irq_wake to
> twl6030_irq_set_wake, which recursively calls irq_set_irq_wake.
> Although the irq_desc and lock are different, LOCKDEP treats these as
> equivalent, presumably due to problems that can be incurred when
> locking more than one irq_desc.
> 
> Currently we're running with twl6030_irq_set_wake keeping a count of
> sub-module wakeirqs, and setting the wakeup status for the twl_irq
> enabled or disabled as needed at suspend time.  Can send a patch if
> wanted.
Please do. I'm currently considering removing Santosh's patch without this
LOCKDEP warning fix.

Cheers,
Samuel.

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

Re: [PATCH] mfd: twl6030: Setup irq_wake infrastructure.

[Santosh Shilimkar]

Samual,

On Monday 26 September 2011 02:20 PM, Samuel Ortiz wrote:
> Hi Todd,
> 
> On Thu, Sep 15, 2011 at 01:37:38PM -0700, Todd Poynor wrote:
>> On Tue, Sep 06, 2011 at 09:29:30PM +0530, Santosh Shilimkar wrote:
>>> TWL6030 devices have an interrupt line which is connected to
>>> application processor like OMAP. These devices support multiple features
>>> such as MMC card detect, USB cable detect, RTC interrupt, etc. that must
>>> wake up the application processor.
>>>
>>> With this change, TWL6030 client drivers can make use of
>>> irq_wake() if the wakeup is desirable on it's irq events.
>>>
>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>> cc: Samuel Ortiz <sameo@linux.intel.com>
>>> ---
>>>  drivers/mfd/twl6030-irq.c |    9 +++++++++
>>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c
>>> index eb3b5f8..4477134 100644
>>> --- a/drivers/mfd/twl6030-irq.c
>>> +++ b/drivers/mfd/twl6030-irq.c
>>> @@ -187,6 +187,13 @@ static inline void activate_irq(int irq)
>>>  #endif
>>>  }
>>>  
>>> +int twl6030_irq_set_wake(struct irq_data *d, unsigned int on)
>>> +{
>>> +	int twl_irq = (int)irq_get_chip_data(d->irq);
>>> +
>>> +	return irq_set_irq_wake(twl_irq, on);
>>
>> Note that when running with this previously, LOCKDEP warnings
>> were seen.  LOCKDEP explicitly sets all irq_desc locks as a single
>> lock-class, causing "possible recursive locking detected" when the TWL
>> RTC (or other) driver calls through enable_irq_wake to
>> twl6030_irq_set_wake, which recursively calls irq_set_irq_wake.
>> Although the irq_desc and lock are different, LOCKDEP treats these as
>> equivalent, presumably due to problems that can be incurred when
>> locking more than one irq_desc.
>>
>> Currently we're running with twl6030_irq_set_wake keeping a count of
>> sub-module wakeirqs, and setting the wakeup status for the twl_irq
>> enabled or disabled as needed at suspend time.  Can send a patch if
>> wanted.
> Please do. I'm currently considering removing Santosh's patch without this
> LOCKDEP warning fix.
> 
I guess the lock dep issue as Todd pointed out is because LOCKDEP treats
the irq_desc and lock are equivalent. And IIUC, Todd didn't fix anything
on this patch. he just added some logic in client driver to avoid the
warning. Real fix should be in LOCKDEP code which is exposed by this
patch. This patch should be fine as such.

Regards
Santosh

Re: [PATCH] mfd: twl6030: Setup irq_wake infrastructure.

[Todd Poynor]

On Mon, Sep 26, 2011 at 03:01:54PM +0530, Santosh Shilimkar wrote:
> Samual,
> 
> On Monday 26 September 2011 02:20 PM, Samuel Ortiz wrote:
> > Hi Todd,
> > 
> > On Thu, Sep 15, 2011 at 01:37:38PM -0700, Todd Poynor wrote:
> >> On Tue, Sep 06, 2011 at 09:29:30PM +0530, Santosh Shilimkar wrote:
> >>> TWL6030 devices have an interrupt line which is connected to
> >>> application processor like OMAP. These devices support multiple features
> >>> such as MMC card detect, USB cable detect, RTC interrupt, etc. that must
> >>> wake up the application processor.
> >>>
> >>> With this change, TWL6030 client drivers can make use of
> >>> irq_wake() if the wakeup is desirable on it's irq events.
> >>>
> >>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> >>> cc: Samuel Ortiz <sameo@linux.intel.com>
> >>> ---
> >>>  drivers/mfd/twl6030-irq.c |    9 +++++++++
> >>>  1 files changed, 9 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c
> >>> index eb3b5f8..4477134 100644
> >>> --- a/drivers/mfd/twl6030-irq.c
> >>> +++ b/drivers/mfd/twl6030-irq.c
> >>> @@ -187,6 +187,13 @@ static inline void activate_irq(int irq)
> >>>  #endif
> >>>  }
> >>>  
> >>> +int twl6030_irq_set_wake(struct irq_data *d, unsigned int on)
> >>> +{
> >>> +	int twl_irq = (int)irq_get_chip_data(d->irq);
> >>> +
> >>> +	return irq_set_irq_wake(twl_irq, on);
> >>
> >> Note that when running with this previously, LOCKDEP warnings
> >> were seen.  LOCKDEP explicitly sets all irq_desc locks as a single
> >> lock-class, causing "possible recursive locking detected" when the TWL
> >> RTC (or other) driver calls through enable_irq_wake to
> >> twl6030_irq_set_wake, which recursively calls irq_set_irq_wake.
> >> Although the irq_desc and lock are different, LOCKDEP treats these as
> >> equivalent, presumably due to problems that can be incurred when
> >> locking more than one irq_desc.
> >>
> >> Currently we're running with twl6030_irq_set_wake keeping a count of
> >> sub-module wakeirqs, and setting the wakeup status for the twl_irq
> >> enabled or disabled as needed at suspend time.  Can send a patch if
> >> wanted.
> > Please do. I'm currently considering removing Santosh's patch without this
> > LOCKDEP warning fix.
> > 
> I guess the lock dep issue as Todd pointed out is because LOCKDEP treats
> the irq_desc and lock are equivalent. And IIUC, Todd didn't fix anything
> on this patch. he just added some logic in client driver to avoid the
> warning. Real fix should be in LOCKDEP code which is exposed by this
> patch. This patch should be fine as such.

Sorry, I've been in crunch mode, I'll generate patches now.

I'm not sure where the mfd tree lives at the moment, let me know if
there's a tree I should use.

I think the above comment refers to some old code in the driver that
claimed to be avoiding LOCKDEP warnings, not the forthcoming patches.


Todd


> 
> Regards
> Santosh
>