| Message ID | 20190321101557.26857-1-alexandre.belloni@bootlin.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [1/2] rtc: da9063: set range | expand |
Hi Alexandre, Thanks. Sorry, I didn't see this immediately because of our e-mail filtering algorithm, On 21 March 2019 10:16, Alexandre Belloni wrote, > Subject: [PATCH 1/2] rtc: da9063: set range > The DA9062 and DA9063 have a year register that can go up to 0x3F. Yep. [...] > index 588120ba372c..09fb4af5edab 100644 > --- a/include/linux/rtc.h > +++ b/include/linux/rtc.h > @@ -164,6 +164,7 @@ struct rtc_device { > /* useful timestamps */ > #define RTC_TIMESTAMP_BEGIN_1900 -2208989361LL /* 1900-01-01 00:00:00 */ > #define RTC_TIMESTAMP_BEGIN_2000 946684800LL /* 2000-01-01 00:00:00 */ > +#define RTC_TIMESTAMP_END_2063 2966371199LL /* 2063-12-31 23:59:59 */ Thanks. Yes. Register min and max values are: 2000-01-01 00:00:00 2063-12-31 23:59:59 Acked-by: Steve Twiss <stwiss.opensource@diasemi.com> Regards, Steve
On Thu, Mar 21, 2019 at 11:15:56AM +0100, Alexandre Belloni wrote: > The DA9062 and DA9063 have a year register that can go up to 0x3F. > > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> Thanks for this patch! Didn't know about the devm_rtc_device_register() conversion going on. Glad I learned about it. Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com> I couldn't test the upper limit (DA9063 hooked to a 32bit system here), but lower limit works and RTC in general works. Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Hi Wolfram, On Mon, Apr 1, 2019 at 10:43 AM Wolfram Sang <wsa@the-dreams.de> wrote: > On Thu, Mar 21, 2019 at 11:15:56AM +0100, Alexandre Belloni wrote: > > The DA9062 and DA9063 have a year register that can go up to 0x3F. > > > > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > > Thanks for this patch! Didn't know about the devm_rtc_device_register() > conversion going on. Glad I learned about it. > > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > I couldn't test the upper limit (DA9063 hooked to a 32bit system here), > but lower limit works and RTC in general works. BTW, does the RTC alarm interrupt work for you? Gr{oetje,eeting}s, Geert
Hi Geert, On 01 April 2019 10:00, Geert Uytterhoeven wrote: > Subject: Re: [PATCH 1/2] rtc: da9063: set range > > Hi Wolfram, > > On Mon, Apr 1, 2019 at 10:43 AM Wolfram Sang <wsa@the-dreams.de> wrote: > > On Thu, Mar 21, 2019 at 11:15:56AM +0100, Alexandre Belloni wrote: > > > The DA9062 and DA9063 have a year register that can go up to 0x3F. > > > > > > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > > > I couldn't test the upper limit (DA9063 hooked to a 32bit system here), > > but lower limit works and RTC in general works. > > BTW, does the RTC alarm interrupt work for you? As far as I can tell, there are no RTC alarm regressions. I am using an i.MX6Q board and with an unmodified v5.1-rc1 kernel, for the alarms, I see everything working okay WITHOUT Alexandre's patches ... Then, WITH Alexandre's patches ... - https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git/commit/?h=rtc-next&id=05e4ffeadecaa6f501218504b86a6cec89202bd9 - https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git/commit/?h=rtc-next&id=da44e0eb7ec6853b5d20faba7dc34c48e4bb35c7 ... applied to v5.1-rc1, and with the same set-up, [ 2.027144] da9063 1-0058: Device detected (chip-ID: 0x61, var-ID: 0x60) [ 2.483699] da9063-rtc da9063-rtc: DMA mask not set [ 2.523279] da9063-rtc da9063-rtc: registered as rtc0 Linux test 5.1.0-rc1 #1 SMP Mon Apr 1 13:05:32 BST 2019 armv7l GNU/Linux [...] [PASS] Setting the current date and time from the da9063-rtc da9063-rtc as 2000-01-01 00:00:00 [PASS] Setting the alarm date and time from the da9063-rtc da9063-rtc as 2000-01-01 00:00:05 (+5 secs into the future) [PASS] Setting the listener on da9063-rtc da9063-rtc then waiting for elapsed timeout of 15 seconds... [PASS] The alarm was triggered on da9063-rtc da9063-rtc within the expected time and the alarm happened at 2000-01-01 00:00:05 [PASS] Setting the current date and time from the da9063-rtc da9063-rtc as 2000-01-01 00:00:00 [PASS] Setting the alarm date and time from the da9063-rtc da9063-rtc as 2000-01-01 00:00:15 (+15 secs into the future) [PASS] Setting the listener on da9063-rtc da9063-rtc then waiting for elapsed timeout of 25 seconds... [PASS] The alarm was triggered on da9063-rtc da9063-rtc within the expected time and the alarm happened at 2000-01-01 00:00:15 > cat /proc/interrupts | grep da90 247: 2 0 0 0 gpio-mxc 11 Level da9063-irq 305: 0 0 2 0 da9063-irq 1 Level ALARM I get an identical result. So as far as I can tell, there are no RTC alarm regressions. Tested-by: Steve Twiss <stwiss.opensource@diasemi.com> Regards, Steve
Hi Steve, On Mon, Apr 1, 2019 at 2:39 PM Steve Twiss <stwiss.opensource@diasemi.com> wrote: > On 01 April 2019 10:00, Geert Uytterhoeven wrote: > > Subject: Re: [PATCH 1/2] rtc: da9063: set range > > On Mon, Apr 1, 2019 at 10:43 AM Wolfram Sang <wsa@the-dreams.de> wrote: > > > On Thu, Mar 21, 2019 at 11:15:56AM +0100, Alexandre Belloni wrote: > > > > The DA9062 and DA9063 have a year register that can go up to 0x3F. > > > > > > > > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > > > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > > > > > I couldn't test the upper limit (DA9063 hooked to a 32bit system here), > > > but lower limit works and RTC in general works. > > > > BTW, does the RTC alarm interrupt work for you? > > As far as I can tell, there are no RTC alarm regressions. > > I am using an i.MX6Q board and with an unmodified v5.1-rc1 kernel, for the > alarms, I see everything working okay WITHOUT Alexandre's patches ... Thanks! I was just wondering whether they work for Wolfram, who has a similar but different board than I have. Last time I tried, they didn't work for me. Gr{oetje,eeting}s, Geert
Hi Geert, On 01 April 2019 13:42, Geert Uytterhoeven wrote > Subject: Re: [PATCH 1/2] rtc: da9063: set range > > Hi Steve, > > On Mon, Apr 1, 2019 at 2:39 PM Steve Twiss wrote: > > As far as I can tell, there are no RTC alarm regressions. > > I am using an i.MX6Q board and with an unmodified v5.1-rc1 kernel, for the > > alarms, I see everything working okay [...] > > Thanks! > > I was just wondering whether they work for Wolfram, who has a similar > but different board than I have. Last time I tried, they didn't work for me. I think I remember, .. was this it? https://lore.kernel.org/lkml/6ED8E3B22081A4459DAC7699F3695FB7022179F236@SW-EX-MBX02.diasemi.com/ I remember previously I wasn't able to quickly provide you with a regression test result after you fixed an IRQ problem that affected the DA9063 RTC (thanks for your help on that btw). Regards, Steve
> I was just wondering whether they work for Wolfram, who has a similar > but different board than I have. Last time I tried, they didn't work for me. How did you test?
Hi Wolfram, On Mon, Apr 1, 2019 at 3:21 PM Wolfram Sang <wsa@the-dreams.de> wrote: > > I was just wondering whether they work for Wolfram, who has a similar > > but different board than I have. Last time I tried, they didn't work for me. > > How did you test? tools/testing/selftests/rtc/rtctest.c cfr. https://lore.kernel.org/lkml/CAMuHMdWbV2=LfSWz0rFTGRakZ=2iXKOYdM_Uwaov8-OsJpUgoA@mail.gmail.com/ Gr{oetje,eeting}s, Geert
On Mon, Apr 01, 2019 at 03:39:43PM +0200, Geert Uytterhoeven wrote: > Hi Wolfram, > > On Mon, Apr 1, 2019 at 3:21 PM Wolfram Sang <wsa@the-dreams.de> wrote: > > > I was just wondering whether they work for Wolfram, who has a similar > > > but different board than I have. Last time I tried, they didn't work for me. > > > > How did you test? > > tools/testing/selftests/rtc/rtctest.c cfr. > https://lore.kernel.org/lkml/CAMuHMdWbV2=LfSWz0rFTGRakZ=2iXKOYdM_Uwaov8-OsJpUgoA@mail.gmail.com/ Hmm, with vanilla v5.1-rc3, I don't even get that far: [==========] Running 7 tests from 2 test cases. [ RUN ] rtc.date_read rtctest.c:49:rtc.date_read:Current RTC date/time is 01/01/2000 04:06:40. [ OK ] rtc.date_read [ RUN ] rtc.uie_read And here it blocks forever. Any pointers?
On 01/04/2019 17:07:42+0200, Wolfram Sang wrote: > On Mon, Apr 01, 2019 at 03:39:43PM +0200, Geert Uytterhoeven wrote: > > Hi Wolfram, > > > > On Mon, Apr 1, 2019 at 3:21 PM Wolfram Sang <wsa@the-dreams.de> wrote: > > > > I was just wondering whether they work for Wolfram, who has a similar > > > > but different board than I have. Last time I tried, they didn't work for me. > > > > > > How did you test? > > > > tools/testing/selftests/rtc/rtctest.c cfr. > > https://lore.kernel.org/lkml/CAMuHMdWbV2=LfSWz0rFTGRakZ=2iXKOYdM_Uwaov8-OsJpUgoA@mail.gmail.com/ > > Hmm, with vanilla v5.1-rc3, I don't even get that far: > > [==========] Running 7 tests from 2 test cases. > [ RUN ] rtc.date_read > rtctest.c:49:rtc.date_read:Current RTC date/time is 01/01/2000 04:06:40. > [ OK ] rtc.date_read > [ RUN ] rtc.uie_read > > And here it blocks forever. Any pointers? > This means that you don't get any interrupt from your RTC (and that I have to fix the timeout for uie_read).
> > [==========] Running 7 tests from 2 test cases. > > [ RUN ] rtc.date_read > > rtctest.c:49:rtc.date_read:Current RTC date/time is 01/01/2000 04:06:40. > > [ OK ] rtc.date_read > > [ RUN ] rtc.uie_read > > > > And here it blocks forever. Any pointers? > > > > This means that you don't get any interrupt from your RTC (and that I Thought so. Well, somehow makes sense that no interrupt gets through, and not only alarm interrupts... > have to fix the timeout for uie_read). :) I am happy to test.
On 01/04/2019 17:52:04+0200, Wolfram Sang wrote: > > > > [==========] Running 7 tests from 2 test cases. > > > [ RUN ] rtc.date_read > > > rtctest.c:49:rtc.date_read:Current RTC date/time is 01/01/2000 04:06:40. > > > [ OK ] rtc.date_read > > > [ RUN ] rtc.uie_read > > > > > > And here it blocks forever. Any pointers? > > > > > > > This means that you don't get any interrupt from your RTC (and that I > > Thought so. Well, somehow makes sense that no interrupt gets through, > and not only alarm interrupts... > > > have to fix the timeout for uie_read). > > :) I am happy to test. > Well, seeing the code, I actually remembered that this test is still there to ensure the core will properly block. If you remove that test, the other ones should all timeout.
> Well, seeing the code, I actually remembered that this test is still > there to ensure the core will properly block. If you remove that test, > the other ones should all timeout. Thanks for your assistance! What I did just now was to make use of the 'uie_unsupported' flag. This is the outcome: [==========] Running 7 tests from 2 test cases. [ RUN ] rtc.date_read rtctest.c:49:rtc.date_read:Current RTC date/time is 01/01/2000 00:13:23. [ OK ] rtc.date_read [ RUN ] rtc.uie_read [ OK ] rtc.uie_read [ RUN ] rtc.uie_select [ OK ] rtc.uie_select [ RUN ] rtc.alarm_alm_set rtctest.c:137:rtc.alarm_alm_set:Alarm time now set to 00:13:32. rtctest.c:148:rtc.alarm_alm_set:Expected 0 (0) != rc (0) rtc.alarm_alm_set: Test terminated by assertion [ FAIL ] rtc.alarm_alm_set [ RUN ] rtc.alarm_wkalm_set rtctest.c:195:rtc.alarm_wkalm_set:Alarm time now set to 01/01/2000 00:13:37. rtctest.c:202:rtc.alarm_wkalm_set:Expected 0 (0) != rc (0) rtc.alarm_wkalm_set: Test terminated by assertion [ FAIL ] rtc.alarm_wkalm_set [ RUN ] rtc.alarm_alm_set_minute rtctest.c:239:rtc.alarm_alm_set_minute:Alarm time now set to 00:14:00. rtctest.c:258:rtc.alarm_alm_set_minute:data: 1a0 [ OK ] rtc.alarm_alm_set_minute [ RUN ] rtc.alarm_wkalm_set_minute rtctest.c:297:rtc.alarm_wkalm_set_minute:Alarm time now set to 01/01/2000 00:15:00. [ OK ] rtc.alarm_wkalm_set_minute [==========] 5 / 7 tests passed. [ FAILED ] I wonder why the_set_minute tests pass, but the other ones fail. I also wonder why I need the uie_unsupported flag? It's been a while since I dug into the RTC subsystem, I may be missing something. But I see the UIE code finally calling into set_alarm for some codepath. We have that for DA9063, but it is not executed for the UIE test of rtctest. However, it seems the driver doesn't support this in an optimal way, because there is a currently unused update interrupt which should be used for UIE, or? I also wonder why all this works fine for Steve. Thanks, Wolfram
On 01/04/2019 21:34:25+0200, Wolfram Sang wrote: > > > Well, seeing the code, I actually remembered that this test is still > > there to ensure the core will properly block. If you remove that test, > > the other ones should all timeout. > > Thanks for your assistance! What I did just now was to make use of the > 'uie_unsupported' flag. This is the outcome: > > > [==========] Running 7 tests from 2 test cases. > [ RUN ] rtc.date_read > rtctest.c:49:rtc.date_read:Current RTC date/time is 01/01/2000 00:13:23. > [ OK ] rtc.date_read > [ RUN ] rtc.uie_read > [ OK ] rtc.uie_read > [ RUN ] rtc.uie_select > [ OK ] rtc.uie_select > [ RUN ] rtc.alarm_alm_set > rtctest.c:137:rtc.alarm_alm_set:Alarm time now set to 00:13:32. > rtctest.c:148:rtc.alarm_alm_set:Expected 0 (0) != rc (0) > rtc.alarm_alm_set: Test terminated by assertion > [ FAIL ] rtc.alarm_alm_set > [ RUN ] rtc.alarm_wkalm_set > rtctest.c:195:rtc.alarm_wkalm_set:Alarm time now set to 01/01/2000 > 00:13:37. > rtctest.c:202:rtc.alarm_wkalm_set:Expected 0 (0) != rc (0) > rtc.alarm_wkalm_set: Test terminated by assertion > [ FAIL ] rtc.alarm_wkalm_set > [ RUN ] rtc.alarm_alm_set_minute > rtctest.c:239:rtc.alarm_alm_set_minute:Alarm time now set to 00:14:00. > rtctest.c:258:rtc.alarm_alm_set_minute:data: 1a0 > [ OK ] rtc.alarm_alm_set_minute > [ RUN ] rtc.alarm_wkalm_set_minute > rtctest.c:297:rtc.alarm_wkalm_set_minute:Alarm time now set to > 01/01/2000 00:15:00. > [ OK ] rtc.alarm_wkalm_set_minute > [==========] 5 / 7 tests passed. > [ FAILED ] > > I wonder why the_set_minute tests pass, but the other ones fail. I also > wonder why I need the uie_unsupported flag? It's been a while since I > dug into the RTC subsystem, I may be missing something. But I see the > UIE code finally calling into set_alarm for some codepath. We have that > for DA9063, but it is not executed for the UIE test of rtctest. However, > it seems the driver doesn't support this in an optimal way, because > there is a currently unused update interrupt which should be used for > UIE, or? I also wonder why all this works fine for Steve. > I had a look at the driver and I guess you have a 9063AD while Steve uses another model. That explains why you need the uie_unsupported flag. The 9063AD can only do alarms on a minute boundary. Since the move to hr_timer, the uie are done using the classic alarm or they are emulated by the core. This improved the situation for many RTCs that don't have a separate UIE but this made it worse for a few (and this is an example). I have plan to work on this but didn't have the time yet. I suggest the following patch: === From 37b2ab7d537e76e42bde64cf4b57701b0ed8e8cd Mon Sep 17 00:00:00 2001 From: Alexandre Belloni <alexandre.belloni@bootlin.com> Date: Tue, 2 Apr 2019 10:06:46 +0200 Subject: [PATCH] rtc: da9063: set uie_unsupported when relevant The DA9063AD doesn't support alarms on any seconds and its granularity is the minute. Set uie_unsupported in that case. Reported-by: Wolfram Sang <wsa@the-dreams.de> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> --- drivers/rtc/rtc-da9063.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c index 1b792bcea3c7..53e690b0f3a2 100644 --- a/drivers/rtc/rtc-da9063.c +++ b/drivers/rtc/rtc-da9063.c @@ -475,6 +475,9 @@ static int da9063_rtc_probe(struct platform_device *pdev) da9063_data_to_tm(data, &rtc->alarm_time, rtc); rtc->rtc_sync = false; + if (config->rtc_data_start != RTC_SEC) + rtc->rtc_dev->uie_unsupported = 1; + irq_alarm = platform_get_irq_byname(pdev, "ALARM"); ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL, da9063_alarm_event,
Hi Alexandre, > I had a look at the driver and I guess you have a 9063AD while Steve > uses another model. > > That explains why you need the uie_unsupported flag. The 9063AD can only > do alarms on a minute boundary. Bingo! Nice catch. I was on the wrong track because we have an early boot quirk handling for the DA on this platform and I was searching there for side effects. Makes all sense now. Thanks a lot for your help! > Since the move to hr_timer, the uie are done using the classic alarm or > they are emulated by the core. This improved the situation for many RTCs > that don't have a separate UIE but this made it worse for a few (and > this is an example). I have plan to work on this but didn't have the > time yet. I understand. That explains why my RTC knowledge from a few years ago feels so outdated :) > I suggest the following patch: > > === > > From 37b2ab7d537e76e42bde64cf4b57701b0ed8e8cd Mon Sep 17 00:00:00 2001 > From: Alexandre Belloni <alexandre.belloni@bootlin.com> > Date: Tue, 2 Apr 2019 10:06:46 +0200 > Subject: [PATCH] rtc: da9063: set uie_unsupported when relevant > > The DA9063AD doesn't support alarms on any seconds and its granularity is > the minute. Set uie_unsupported in that case. > > Reported-by: Wolfram Sang <wsa@the-dreams.de> Please use this address: Reported-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> And probably Geert wants his "+renesas" address, too: Geert Uytterhoeven <geert+renesas@glider.be> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > --- > drivers/rtc/rtc-da9063.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c > index 1b792bcea3c7..53e690b0f3a2 100644 > --- a/drivers/rtc/rtc-da9063.c > +++ b/drivers/rtc/rtc-da9063.c > @@ -475,6 +475,9 @@ static int da9063_rtc_probe(struct platform_device *pdev) > da9063_data_to_tm(data, &rtc->alarm_time, rtc); > rtc->rtc_sync = false; > > + if (config->rtc_data_start != RTC_SEC) > + rtc->rtc_dev->uie_unsupported = 1; > + I think we should have a comment here, like: /* FIXME: Make use of the TICK interrupt once the RTC core supports it */ So, this helps the UIE test: Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com> And I guess we have to live with two of the alarm tests failing because of the minute granularity? Regards, Wolfram
Hi, 02 April 2019 09:53 Alexandre Belloni, wrote: > Subject: Re: [PATCH 1/2] rtc: da9063: set range > On 01/04/2019 21:34:25+0200, Wolfram Sang wrote: > > > > Thanks for your assistance! What I did just now was to make use of the > > 'uie_unsupported' flag. This is the outcome: [...] > > I wonder why the_set_minute tests pass, but the other ones fail. > > [...] > > I also wonder why all this works fine for Steve. > > > > I had a look at the driver and I guess you have a 9063AD while Steve > uses another model. That would be my immediate guess also. https://elixir.bootlin.com/linux/v5.1-rc3/source/include/linux/mfd/da9063/core.h#L39 Reading the PMIC register 0x182 and examining the 4 highest bits of that value will provide the variant code for the DA9063. > That explains why you need the uie_unsupported flag. The 9063AD can only > do alarms on a minute boundary. For AD, alarms only happen to the minute boundary (i.e. alarms to 0 seconds only) https://elixir.bootlin.com/linux/v5.1-rc3/source/drivers/rtc/rtc-da9063.c#L84 Whereas, BB and greater can alarm to any of the second values 0-59 https://elixir.bootlin.com/linux/v5.1-rc3/source/drivers/rtc/rtc-da9063.c#L113 I can confirm that I was only running my previous regression tests with a BB and CA compliant silicon version. I didn't even think to mention what variant I was testing with. [...] > I suggest the following patch: > > === > > From 37b2ab7d537e76e42bde64cf4b57701b0ed8e8cd Mon Sep 17 00:00:00 2001 > From: Alexandre Belloni <alexandre.belloni@bootlin.com> > Date: Tue, 2 Apr 2019 10:06:46 +0200 > Subject: [PATCH] rtc: da9063: set uie_unsupported when relevant > > The DA9063AD doesn't support alarms on any seconds and its granularity is > the minute. Set uie_unsupported in that case. > > Reported-by: Wolfram Sang <wsa@the-dreams.de> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > --- > drivers/rtc/rtc-da9063.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c > index 1b792bcea3c7..53e690b0f3a2 100644 > --- a/drivers/rtc/rtc-da9063.c > +++ b/drivers/rtc/rtc-da9063.c > @@ -475,6 +475,9 @@ static int da9063_rtc_probe(struct platform_device > *pdev) > da9063_data_to_tm(data, &rtc->alarm_time, rtc); > rtc->rtc_sync = false; > > + if (config->rtc_data_start != RTC_SEC) > + rtc->rtc_dev->uie_unsupported = 1; > + > irq_alarm = platform_get_irq_byname(pdev, "ALARM"); > ret = devm_request_threaded_irq(&pdev->dev, irq_alarm, NULL, > da9063_alarm_event, Thanks Alexandre, Acked-by: Steve Twiss <stwiss.opensource@diasemi.com> Apologies, I am unable to test this on DA9063 AD silicon since I no longer have that variant. Regards, Steve
On 02/04/2019 11:33:59+0200, Wolfram Sang wrote: > Hi Alexandre, > > > I had a look at the driver and I guess you have a 9063AD while Steve > > uses another model. > > > > That explains why you need the uie_unsupported flag. The 9063AD can only > > do alarms on a minute boundary. > > Bingo! Nice catch. I was on the wrong track because we have an early > boot quirk handling for the DA on this platform and I was searching > there for side effects. Makes all sense now. Thanks a lot for your help! > > > Since the move to hr_timer, the uie are done using the classic alarm or > > they are emulated by the core. This improved the situation for many RTCs > > that don't have a separate UIE but this made it worse for a few (and > > this is an example). I have plan to work on this but didn't have the > > time yet. > > I understand. That explains why my RTC knowledge from a few years ago > feels so outdated :) > > > I suggest the following patch: > > > > === > > > > From 37b2ab7d537e76e42bde64cf4b57701b0ed8e8cd Mon Sep 17 00:00:00 2001 > > From: Alexandre Belloni <alexandre.belloni@bootlin.com> > > Date: Tue, 2 Apr 2019 10:06:46 +0200 > > Subject: [PATCH] rtc: da9063: set uie_unsupported when relevant > > > > The DA9063AD doesn't support alarms on any seconds and its granularity is > > the minute. Set uie_unsupported in that case. > > > > Reported-by: Wolfram Sang <wsa@the-dreams.de> > > Please use this address: > > Reported-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > > And probably Geert wants his "+renesas" address, too: > > Geert Uytterhoeven <geert+renesas@glider.be> > > > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > > --- > > drivers/rtc/rtc-da9063.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c > > index 1b792bcea3c7..53e690b0f3a2 100644 > > --- a/drivers/rtc/rtc-da9063.c > > +++ b/drivers/rtc/rtc-da9063.c > > @@ -475,6 +475,9 @@ static int da9063_rtc_probe(struct platform_device *pdev) > > da9063_data_to_tm(data, &rtc->alarm_time, rtc); > > rtc->rtc_sync = false; > > > > + if (config->rtc_data_start != RTC_SEC) > > + rtc->rtc_dev->uie_unsupported = 1; > > + > > I think we should have a comment here, like: > > /* FIXME: Make use of the TICK interrupt once the RTC core supports it */ > Well, My plan is to go over all the uie_unsupported users once the infrastructure is in place but I'll put a comment there. > So, this helps the UIE test: > > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > And I guess we have to live with two of the alarm tests failing because > of the minute granularity? > > Regards, > > Wolfram >
> Apologies, I am unable to test this on DA9063 AD silicon since I no longer have > that variant. Geert and I have boards with these. I am happy to help, and I guess Geert, too.
Hi, > > > drivers/rtc/rtc-da9063.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c > > > index 1b792bcea3c7..53e690b0f3a2 100644 > > > --- a/drivers/rtc/rtc-da9063.c > > > +++ b/drivers/rtc/rtc-da9063.c > > > @@ -475,6 +475,9 @@ static int da9063_rtc_probe(struct platform_device *pdev) > > > da9063_data_to_tm(data, &rtc->alarm_time, rtc); > > > rtc->rtc_sync = false; > > > > > > + if (config->rtc_data_start != RTC_SEC) > > > + rtc->rtc_dev->uie_unsupported = 1; > > > + > > > > I think we should have a comment here, like: > > /* FIXME: Make use of the TICK interrupt once the RTC core supports it */ Is this TICK interrupt suggestion to use the DA9063 TICK interrupt to simulate a second granularity in the AD alarm? If I remember correctly, the original DA9063 patch set which was for AD silicon only, and which was sent to LKML before I took over looking at DA9063, used the DA9063 1-second TICK interrupt to count-down the seconds from the nearest minute in order to simulate second resolution on the RTC alarm for AD. ... yes. Here it is. The original patch was from Krystian Garbaciak and tried to support RTC alarms on the AD silicon to a second resolution by counting down the DA9063 TICK interrupt: https://marc.info/?l=lm-sensors&m=134613501230005&w=2 However, I dropped that patch completely and wrote a new RTC device driver because it didn't work in my tests. The problem was: the TICK interrupt was indistinguishable from the ALARM interrupt for a wake event and when I tested AD silicon to wake up an Android device from suspend or power-off using the RTC IRQ, the device woke up on the ALARM minute (0 seconds), discovered it was not the correct time and immediately went back to sleep. Then it woke-up and returned back to sleep every TICK IRQ second until the correct alarm time was reached (up to 59 times!). At which point it woke up properly. Regards, Steve
On 02/04/2019 10:33:37+0000, Steve Twiss wrote: > Hi, > > > > > drivers/rtc/rtc-da9063.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c > > > > index 1b792bcea3c7..53e690b0f3a2 100644 > > > > --- a/drivers/rtc/rtc-da9063.c > > > > +++ b/drivers/rtc/rtc-da9063.c > > > > @@ -475,6 +475,9 @@ static int da9063_rtc_probe(struct platform_device *pdev) > > > > da9063_data_to_tm(data, &rtc->alarm_time, rtc); > > > > rtc->rtc_sync = false; > > > > > > > > + if (config->rtc_data_start != RTC_SEC) > > > > + rtc->rtc_dev->uie_unsupported = 1; > > > > + > > > > > > I think we should have a comment here, like: > > > /* FIXME: Make use of the TICK interrupt once the RTC core supports it */ > > Is this TICK interrupt suggestion to use the DA9063 TICK interrupt to simulate > a second granularity in the AD alarm? > > If I remember correctly, the original DA9063 patch set which was for AD silicon > only, and which was sent to LKML before I took over looking at DA9063, used the > DA9063 1-second TICK interrupt to count-down the seconds from the nearest > minute in order to simulate second resolution on the RTC alarm for AD. > > ... yes. Here it is. The original patch was from Krystian Garbaciak and tried to > support RTC alarms on the AD silicon to a second resolution by counting down > the DA9063 TICK interrupt: > > https://marc.info/?l=lm-sensors&m=134613501230005&w=2 > > However, I dropped that patch completely and wrote a new RTC device driver > because it didn't work in my tests. > > The problem was: the TICK interrupt was indistinguishable from the ALARM > interrupt for a wake event and when I tested AD silicon to wake up an Android > device from suspend or power-off using the RTC IRQ, the device woke up on the > ALARM minute (0 seconds), discovered it was not the correct time and immediately > went back to sleep. Then it woke-up and returned back to sleep every TICK IRQ > second until the correct alarm time was reached (up to 59 times!). At which point > it woke up properly. > No, the suggestion is to use the TICK interrupt to have a proper UIE support even if the alarm has a minute granularity. As stated, this is not yet supported by the core and need some work. Some RTCs have the following in their set_alarm: if (tm->time.tm_sec) { time64_t alarm_time = rtc_tm_to_time64(&tm->time); alarm_time += 60 - tm->time.tm_sec; rtc_time64_to_tm(alarm_time, &tm->time); } But my plan is to actually expose the capability to userspace and the core so this doesn't have to be handled in the driver.
> But my plan is to actually expose the capability to userspace and the > core so this doesn't have to be handled in the driver. I like that plan!
On 02 April 2019 12:14 Wolfram Sang wrote, > Subject: Re: [PATCH 1/2] rtc: da9063: set range > > > But my plan is to actually expose the capability to userspace and the > > core so this doesn't have to be handled in the driver. > > I like that plan! fwiw, +1
diff --git a/drivers/rtc/rtc-da9063.c b/drivers/rtc/rtc-da9063.c index 73b38d207d7e..b7052156e851 100644 --- a/drivers/rtc/rtc-da9063.c +++ b/drivers/rtc/rtc-da9063.c @@ -464,11 +464,14 @@ static int da9063_rtc_probe(struct platform_device *pdev) platform_set_drvdata(pdev, rtc); - rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, DA9063_DRVNAME_RTC, - &da9063_rtc_ops, THIS_MODULE); + rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev); if (IS_ERR(rtc->rtc_dev)) return PTR_ERR(rtc->rtc_dev); + rtc->rtc_dev->ops = &da9063_rtc_ops; + rtc->rtc_dev->range_min = RTC_TIMESTAMP_BEGIN_2000; + rtc->rtc_dev->range_max = RTC_TIMESTAMP_END_2063; + da9063_data_to_tm(data, &rtc->alarm_time, rtc); rtc->rtc_sync = false; @@ -481,7 +484,7 @@ static int da9063_rtc_probe(struct platform_device *pdev) dev_err(&pdev->dev, "Failed to request ALARM IRQ %d: %d\n", irq_alarm, ret); - return ret; + return rtc_register_device(rtc->rtc_dev); } static struct platform_driver da9063_rtc_driver = { diff --git a/include/linux/rtc.h b/include/linux/rtc.h index 588120ba372c..09fb4af5edab 100644 --- a/include/linux/rtc.h +++ b/include/linux/rtc.h @@ -164,6 +164,7 @@ struct rtc_device { /* useful timestamps */ #define RTC_TIMESTAMP_BEGIN_1900 -2208989361LL /* 1900-01-01 00:00:00 */ #define RTC_TIMESTAMP_BEGIN_2000 946684800LL /* 2000-01-01 00:00:00 */ +#define RTC_TIMESTAMP_END_2063 2966371199LL /* 2063-12-31 23:59:59 */ #define RTC_TIMESTAMP_END_2099 4102444799LL /* 2099-12-31 23:59:59 */ #define RTC_TIMESTAMP_END_9999 253402300799LL /* 9999-12-31 23:59:59 */
The DA9062 and DA9063 have a year register that can go up to 0x3F. Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> --- drivers/rtc/rtc-da9063.c | 9 ++++++--- include/linux/rtc.h | 1 + 2 files changed, 7 insertions(+), 3 deletions(-)