Patchwork Re: [PATCH] rtc-twl: Switch to using threaded irq

login
register
mail settings
Submitter Sebastian Reichel
Date May 31, 2011, 8:51 a.m.
Message ID <20110531085139.GA7290@earth.universe>
Download mbox | patch
Permalink /patch/97978/
State New
Headers show

Comments

Sebastian Reichel - May 31, 2011, 8:51 a.m.
Hi,

I think the patch should also remove the local_irq_enable() call in
twl_rtc_interrupt, since it's no longer needed with threaded irq. At
least on the Pandaboard the RTC is still working with the appended
patch.

-- Sebastian

> >> On Apr 13, 2011 Krishnamoorthy, Balaji T  wrote:
> >>> On Wed, Mar 16, 2011 at 9:37 PM, Ilkka Koskinen
> >> <ilkka.koskinen@nokia.com> wrote:
> >>>>
> >>>> The driver is accessing to i2c bus in interrupt handler.
> >>>> Therefore, it should use threaded irq.
> >>
> >>> Acked-by: Balaji T K <balajitk@ti.com>
> >>
> >>>>
> >>>> Signed-off-by: Ilkka Koskinen <ilkka.koskinen@nokia.com>
> >>>> ---
> >>>>  drivers/rtc/rtc-twl.c |    2 +-
> >>>>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>>>
> >>>> diff --git a/drivers/rtc/rtc-twl.c b/drivers/rtc/rtc-twl.c
> >>>> index ed1b868..2715b96 100644
> >>>> --- a/drivers/rtc/rtc-twl.c
> >>>> +++ b/drivers/rtc/rtc-twl.c
> >>>> @@ -475,7 +475,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.0.4
Sebastian Reichel - June 27, 2011, 4:45 p.m.
On Tue, May 31, 2011 at 10:51:39AM +0200, Sebastian Reichel wrote:
> > The driver is accessing to i2c bus in interrupt handler.
> > Therefore, it should use threaded irq.
> I think the patch should also remove the local_irq_enable() call in
> twl_rtc_interrupt, since it's no longer needed with threaded irq. At
> least on the Pandaboard the RTC is still working with the appended
> patch.

ping.

Currently the kernel prints a stacktrace for each rtc-twl interrupt,
because its interrupt handler enables interrupts.

-- Sebastian
John Stultz - Aug. 10, 2011, 10:44 p.m.
On Mon, 2011-06-27 at 18:45 +0200, Sebastian Reichel wrote:
> On Tue, May 31, 2011 at 10:51:39AM +0200, Sebastian Reichel wrote:
> > > The driver is accessing to i2c bus in interrupt handler.
> > > Therefore, it should use threaded irq.
> > I think the patch should also remove the local_irq_enable() call in
> > twl_rtc_interrupt, since it's no longer needed with threaded irq. At
> > least on the Pandaboard the RTC is still working with the appended
> > patch.
> 
> ping.
> 
> Currently the kernel prints a stacktrace for each rtc-twl interrupt,
> because its interrupt handler enables interrupts.

I've queued both changes. Thanks for the reminder.

thanks
-john

Patch

--- a/drivers/rtc/rtc-twl.c   2011-05-19 06:06:34.000000000 +0200
+++ b/drivers/rtc/rtc-twl.c   2011-05-26 20:34:03.000000000 +0200
@@ -362,14 +362,6 @@ 
        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 @@ 
        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) {