Messages in this thread | | | Date | Thu, 23 Apr 2009 23:51:41 +0900 (JST) | Subject | Re: [PATCH] rtc: Make rtc_update_irq callable with irqs enabled | From | Atsushi Nemoto <> |
| |
[Add CCs to authers or original committers of each mentioned driver]
On Fri, 10 Apr 2009 00:58:20 +0200, Alessandro Zummo <alessandro.zummo@towertech.it> wrote: > On Thu, 9 Apr 2009 15:39:21 -0700 > Andrew Morton <akpm@linux-foundation.org> wrote: > > > > The rtc_update_irq() might be called with irqs enabled, if a interrupt > > > handler was registered without IRQF_DISABLED. > > > > Why? What are the consequences of not merging the patch? Is it a > > bugfix? If so, what are the user-visible effects of the bug? > > rtc_update_irq() is called by a driver, and a driver > is supposed to know when it's doing the call. > > The driver can either use IRQF_DISABLED or disable the > interrupts in some other ways. > > I also suspect this is some legacy we are carrying on, > so it's better to leave the decision on the interrupt > handling to the driver itself. > > Unless I'm missing something.
Then here is list of (potentialy) broken rtc drivers:
rtc-at32ap700x.c rtc-bfin.c rtc-m48t59.c rtc-pcf50633.c rtc-twl4030.c rtc-wm8350.c
I'm not sure there are any real problem on these drivers. It seems IRQF_DISABLED would be suitable for at32ap700x, bfin and m48t59, and local_irq_disable would be suitable for others.
The IRQF_DISABLED fixes would be better regardless of the rtc_update_irq() API change. And local_irq_disable fixes are not needed (and should be reverted) if the API change was acked, but no harm for short term fix.
Note that just adding IRQF_DISABLED will cause "IRQF_DISABLED is not guaranteed on shared IRQs" warning. So you should consider of removing IRQF_SHARED, or finding other way.
--- Atsushi Nemoto
| |