| Message ID | e3b4f8848cad42e5a2f4dfb34e342c69c8b95786.1539851865.git.baolin.wang@linaro.org |
|---|---|
| State | Accepted |
| Commit | 3822d1bb0df18aa28930f19bc46e0704aea1be0f |
| Headers | show |
| Series | Fix some issues for RTC alarm function | expand |
Hello, On 18/10/2018 16:52:30+0800, Baolin Wang wrote: > When registering one RTC device, it will check to see if there is an > alarm already set in RTC hardware by reading RTC alarm, at this time > we should always read the normal alarm put in always-on region by > checking the rtc->registered flag. > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > --- > drivers/rtc/rtc-sc27xx.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/rtc/rtc-sc27xx.c b/drivers/rtc/rtc-sc27xx.c > index 72bb002..b4eb3b3 100644 > --- a/drivers/rtc/rtc-sc27xx.c > +++ b/drivers/rtc/rtc-sc27xx.c > @@ -415,10 +415,14 @@ static int sprd_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) > u32 val; > > /* > - * If aie_timer is enabled, we should get the normal alarm time. > + * Before RTC device is registered, it will check to see if there is an > + * alarm already set in RTC hardware, and we always read the normal > + * alarm at this time. > + * > + * Or if aie_timer is enabled, we should get the normal alarm time. > * Otherwise we should get auxiliary alarm time. > */ > - if (rtc->rtc && rtc->rtc->aie_timer.enabled == 0) > + if (rtc->rtc && rtc->rtc->registered && rtc->rtc->aie_timer.enabled == 0) Note that the driver should not access rtc->registered and rtc->aie_timer.enabled and this is a bit fragile. But, on the other hand, I currently don't have anything better to suggest. I was also planning to add an in-kernel API for multiple alarms but I'm not sure it will actually help in your case. Anyway, this is applied. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Hi Alexandre, On 25 October 2018 at 08:34, Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > Hello, > > On 18/10/2018 16:52:30+0800, Baolin Wang wrote: >> When registering one RTC device, it will check to see if there is an >> alarm already set in RTC hardware by reading RTC alarm, at this time >> we should always read the normal alarm put in always-on region by >> checking the rtc->registered flag. >> >> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> >> --- >> drivers/rtc/rtc-sc27xx.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/rtc/rtc-sc27xx.c b/drivers/rtc/rtc-sc27xx.c >> index 72bb002..b4eb3b3 100644 >> --- a/drivers/rtc/rtc-sc27xx.c >> +++ b/drivers/rtc/rtc-sc27xx.c >> @@ -415,10 +415,14 @@ static int sprd_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) >> u32 val; >> >> /* >> - * If aie_timer is enabled, we should get the normal alarm time. >> + * Before RTC device is registered, it will check to see if there is an >> + * alarm already set in RTC hardware, and we always read the normal >> + * alarm at this time. >> + * >> + * Or if aie_timer is enabled, we should get the normal alarm time. >> * Otherwise we should get auxiliary alarm time. >> */ >> - if (rtc->rtc && rtc->rtc->aie_timer.enabled == 0) >> + if (rtc->rtc && rtc->rtc->registered && rtc->rtc->aie_timer.enabled == 0) > > Note that the driver should not access rtc->registered and > rtc->aie_timer.enabled and this is a bit fragile. > But, on the other hand, I currently don't have anything better to > suggest. I was also planning to add an in-kernel API for multiple alarms > but I'm not sure it will actually help in your case. Yes, I understand your concern. I will glad to help to test if you introduce new APIs for multiple alarms to see if they can help in our case. Thanks. -- Baolin Wang Best Regards
diff --git a/drivers/rtc/rtc-sc27xx.c b/drivers/rtc/rtc-sc27xx.c index 72bb002..b4eb3b3 100644 --- a/drivers/rtc/rtc-sc27xx.c +++ b/drivers/rtc/rtc-sc27xx.c @@ -415,10 +415,14 @@ static int sprd_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) u32 val; /* - * If aie_timer is enabled, we should get the normal alarm time. + * Before RTC device is registered, it will check to see if there is an + * alarm already set in RTC hardware, and we always read the normal + * alarm at this time. + * + * Or if aie_timer is enabled, we should get the normal alarm time. * Otherwise we should get auxiliary alarm time. */ - if (rtc->rtc && rtc->rtc->aie_timer.enabled == 0) + if (rtc->rtc && rtc->rtc->registered && rtc->rtc->aie_timer.enabled == 0) return sprd_rtc_read_aux_alarm(dev, alrm); ret = sprd_rtc_get_secs(rtc, SPRD_RTC_ALARM, &secs);
When registering one RTC device, it will check to see if there is an alarm already set in RTC hardware by reading RTC alarm, at this time we should always read the normal alarm put in always-on region by checking the rtc->registered flag. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- drivers/rtc/rtc-sc27xx.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) -- 1.7.9.5