| Message ID | 1340011750-5113-1-git-send-email-linus.walleij@stericsson.com |
|---|---|
| State | Accepted |
| Headers | show |
On Mon, 18 Jun 2012 11:29:10 +0200 Linus Walleij <linus.walleij@stericsson.com> wrote: > The pl031 interrupt is shared between the timer part > and the clockwatch part of the same HW block on the ux500, > so mark it IRQF_SHARED on this variant. > > This patch also adds the IRQF_NO_SUSPEND flag to the rtc > irq on all variants as we don't want this pretty important > IRQ to be disabled in suspend. This patch appears to fix a bug, so I need to decide whether we should merge the fix into 3.5 or earlier kernels. To do that, I (and others!) need to be told what is the end-user impact of the bug. But this info is absent. Please always include it!
On Wed, Jun 20, 2012 at 2:03 AM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Mon, 18 Jun 2012 11:29:10 +0200 > Linus Walleij <linus.walleij@stericsson.com> wrote: > This patch appears to fix a bug, so I need to decide whether we should > merge the fix into 3.5 or earlier kernels. Basically none of the in-tree systems that use them have support for system wide suspend/resume so they will not run into it. So it's not critical. I know of some out-of-tree systems using it but that's another issue... > To do that, I (and others!) need to be told what is the end-user > impact of the bug. But this info is absent. Please always include it! Sorry, basically on a system suspending you would expect it to wake up if there is an RTC alarm. For example John Stultz recently did some interesting work on alarm timers that will exercise this on systems that support suspend and has an always-on RTC. Without this patch the alarm IRQ will be masked off and the system will not wake up, or it will wake up (for purely electric reasons) and then shut down when it can't find an IRQ. It would probably make sense to take a swipe around drivers/rtc and check them all for this shortcoming at some point, but I don't think the issue is that big since only so many systems really support suspend. Yours, Linus Walleij
diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c index e66afb8..08378e3 100644 --- a/drivers/rtc/rtc-pl031.c +++ b/drivers/rtc/rtc-pl031.c @@ -75,11 +75,13 @@ * clockwatch function * @st_weekday: if this is an ST Microelectronics silicon version that need * the weekday fix + * @irqflags: special IRQ flags per variant */ struct pl031_vendor_data { struct rtc_class_ops ops; bool clockwatch; bool st_weekday; + unsigned long irqflags; }; struct pl031_local { @@ -373,7 +375,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id) } if (request_irq(adev->irq[0], pl031_interrupt, - 0, "rtc-pl031", ldata)) { + vendor->irqflags, "rtc-pl031", ldata)) { ret = -EIO; goto out_no_irq; } @@ -403,6 +405,7 @@ static struct pl031_vendor_data arm_pl031 = { .set_alarm = pl031_set_alarm, .alarm_irq_enable = pl031_alarm_irq_enable, }, + .irqflags = IRQF_NO_SUSPEND, }; /* The First ST derivative */ @@ -416,6 +419,7 @@ static struct pl031_vendor_data stv1_pl031 = { }, .clockwatch = true, .st_weekday = true, + .irqflags = IRQF_NO_SUSPEND, }; /* And the second ST derivative */ @@ -429,6 +433,11 @@ static struct pl031_vendor_data stv2_pl031 = { }, .clockwatch = true, .st_weekday = true, + /* + * This variant shares the IRQ with another block and must not + * suspend that IRQ line. + */ + .irqflags = IRQF_SHARED | IRQF_NO_SUSPEND, }; static struct amba_id pl031_ids[] = {