[RFC][PATCH] rtc: rtc-pl031: Set wakeup flag prior to registering rtcdev

[RFC][PATCH] rtc: rtc-pl031: Set wakeup flag prior to registering rtcdev

[John Stultz]

In some recent testing, I noticed the CLOCK_REALTIME_ALARM clockid
wasn't functioning on my vexpress qemu environment. Looking into it
I noticed the pl031 rtc driver doesn't set the wakeup flag on the
device until after registering the device with the RTC subsystem.

This causes the alarmtimer subsystem to not see the pl031 driver
as a valid backing device, and that resuls in alarm clockids
getting ENOTSUPP errors.

Thus be sure to set the wakeup flag on the device prior to
registering the rtcdev so the pl031 rtc driver can be used as
the backing alarmtimer device.

Let me know if you have any feedback or objections to this.
If it looks ok, I'll queue it for 3.13. I suspect there 
may be other RTC drivers with the same problem.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: "Jon Medhurst (Tixy)" <tixy@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/rtc-pl031.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-pl031.c b/drivers/rtc/rtc-pl031.c
index 0f0609b..e3b2571 100644
--- a/drivers/rtc/rtc-pl031.c
+++ b/drivers/rtc/rtc-pl031.c
@@ -371,6 +371,7 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 		}
 	}
 
+	device_init_wakeup(&adev->dev, 1);
 	ldata->rtc = rtc_device_register("pl031", &adev->dev, ops,
 					THIS_MODULE);
 	if (IS_ERR(ldata->rtc)) {
@@ -384,8 +385,6 @@ static int pl031_probe(struct amba_device *adev, const struct amba_id *id)
 		goto out_no_irq;
 	}
 
-	device_init_wakeup(&adev->dev, 1);
-
 	return 0;
 
 out_no_irq:
-- 
1.8.1.2

Re: [RFC][PATCH] rtc: rtc-pl031: Set wakeup flag prior to registering rtcdev

[Linus Walleij]

On Tue, Sep 17, 2013 at 8:29 PM, John Stultz <john.stultz@linaro.org> wrote:

> In some recent testing, I noticed the CLOCK_REALTIME_ALARM clockid
> wasn't functioning on my vexpress qemu environment. Looking into it
> I noticed the pl031 rtc driver doesn't set the wakeup flag on the
> device until after registering the device with the RTC subsystem.
>
> This causes the alarmtimer subsystem to not see the pl031 driver
> as a valid backing device, and that resuls in alarm clockids
> getting ENOTSUPP errors.
>
> Thus be sure to set the wakeup flag on the device prior to
> registering the rtcdev so the pl031 rtc driver can be used as
> the backing alarmtimer device.
>
> Let me know if you have any feedback or objections to this.
> If it looks ok, I'll queue it for 3.13. I suspect there
> may be other RTC drivers with the same problem.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: "Jon Medhurst (Tixy)" <tixy@linaro.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Makes perfect sense. I have only tested the driver from regular
userspace using ioctl()s. Is these a simple way to test
an alarmtimer on a barebones system?

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Are you sure this should not be for -rc:s and stable?

Yours,
Linus Walleij

Re: [RFC][PATCH] rtc: rtc-pl031: Set wakeup flag prior to registering rtcdev

[John Stultz]

On Wed, Sep 18, 2013 at 6:30 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Sep 17, 2013 at 8:29 PM, John Stultz <john.stultz@linaro.org> wrote:
>
> > In some recent testing, I noticed the CLOCK_REALTIME_ALARM clockid
> > wasn't functioning on my vexpress qemu environment. Looking into it
> > I noticed the pl031 rtc driver doesn't set the wakeup flag on the
> > device until after registering the device with the RTC subsystem.
> >
> > This causes the alarmtimer subsystem to not see the pl031 driver
> > as a valid backing device, and that resuls in alarm clockids
> > getting ENOTSUPP errors.
> >
> > Thus be sure to set the wakeup flag on the device prior to
> > registering the rtcdev so the pl031 rtc driver can be used as
> > the backing alarmtimer device.
> >
> > Let me know if you have any feedback or objections to this.
> > If it looks ok, I'll queue it for 3.13. I suspect there
> > may be other RTC drivers with the same problem.
> >
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Alessandro Zummo <a.zummo@towertech.it>
> > Cc: "Jon Medhurst (Tixy)" <tixy@linaro.org>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
>
> Makes perfect sense. I have only tested the driver from regular
> userspace using ioctl()s. Is these a simple way to test
> an alarmtimer on a barebones system?

The easiest way is to try to read CLOCK_REALTIME_ALARM which will only
function if there's a backing rtc.

I've got some further timekeeping tests here that will test timers etc
on the _ALARM clockids:
https://github.com/johnstultz-work/timetests


> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> Are you sure this should not be for -rc:s and stable?

Its not a regression (never worked before), so I don't think so,
unless someone finds the functionality critical for long-term kernels.

thanks
-john