[v4,1/4] rtc: omap: Cut down the shutdown time from 2 seconds to 1 sec

Message ID 1531372060-10532-2-git-send-email-j-keerthy@ti.com
State Superseded
Headers show
Series
  • rtc: OMAP: Add support for rtc-only mode
Related show

Commit Message

J, KEERTHY July 12, 2018, 5:07 a.m.
Cut down the shutdown time from 2 seconds to 1 sec. In case of roll
over try again.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---

Changes in v4:

  * Fixed a compilation issue.
  * Extended the roll over check post interrupt programming.

 drivers/rtc/rtc-omap.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Johan Hovold July 19, 2018, 10:02 a.m. | #1
On Thu, Jul 12, 2018 at 10:37:37AM +0530, Keerthy wrote:
> Cut down the shutdown time from 2 seconds to 1 sec. In case of roll
> over try again.
> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
> 
> Changes in v4:
> 
>   * Fixed a compilation issue.
>   * Extended the roll over check post interrupt programming.
> 
>  drivers/rtc/rtc-omap.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index 323ff55..88da927 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c

First, the comment above this function would need to be updated as part
of this patch as it refers to the two-second alarm offset.

> @@ -435,17 +435,23 @@ static void omap_rtc_power_off(void)
>  	struct rtc_time tm;
>  	unsigned long now;
>  	u32 val;
> +	int seconds;
>  
>  	rtc->type->unlock(rtc);
>  	/* enable pmic_power_en control */
>  	val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
>  	rtc_writel(rtc, OMAP_RTC_PMIC_REG, val | OMAP_RTC_PMIC_POWER_EN_EN);
>  
> -	/* set alarm two seconds from now */
> +again:
> +	/* Clear any existing ALARM2 event */
> +	rtc_writel(rtc, OMAP_RTC_STATUS_REG, OMAP_RTC_STATUS_ALARM2);

Why is this needed? Any pending interrupt is cleared at probe, and a
previous attempt to set the alarm really led to the alarm going off, why
would we retry?

> +
> +	/* set alarm one second from now */
>  	omap_rtc_read_time_raw(rtc, &tm);
> +	seconds = tm.tm_sec;
>  	bcd2tm(&tm);
>  	rtc_tm_to_time(&tm, &now);
> -	rtc_time_to_tm(now + 2, &tm);
> +	rtc_time_to_tm(now + 1, &tm);
>  
>  	if (tm2bcd(&tm) < 0) {
>  		dev_err(&rtc->rtc->dev, "power off failed\n");
> @@ -470,6 +476,9 @@ static void omap_rtc_power_off(void)
>  	val = rtc_read(rtc, OMAP_RTC_INTERRUPTS_REG);
>  	rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
>  			val | OMAP_RTC_INTERRUPTS_IT_ALARM2);

Add a newline here.

> +	/* Our calculations started right before the rollover, try again */

Nit: use all lower case unless writing full sentences, which also
matches most of the other comments in this file.

> +	if (seconds != rtc_read(omap_rtc_power_off_rtc, OMAP_RTC_SECONDS_REG))
> +		goto again;

Here the alarm may have gone off as part of the roll over, in which case
you shouldn't retry.

Add a newline here too.

>  	rtc->type->lock(rtc);
>  
>  	/*

Thanks,
Johan
Johan Hovold July 19, 2018, 10:23 a.m. | #2
On Thu, Jul 12, 2018 at 10:37:37AM +0530, Keerthy wrote:
> Cut down the shutdown time from 2 seconds to 1 sec. In case of roll
> over try again.
> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>

> @@ -435,17 +435,23 @@ static void omap_rtc_power_off(void)

> +	/* set alarm one second from now */
>  	omap_rtc_read_time_raw(rtc, &tm);
> +	seconds = tm.tm_sec;
>  	bcd2tm(&tm);
>  	rtc_tm_to_time(&tm, &now);
> -	rtc_time_to_tm(now + 2, &tm);
> +	rtc_time_to_tm(now + 1, &tm);

One more thing, the comment before final mdelay as well as that delay
itself also needs to be updated to reflect this change.

Thanks,
Johan
J, KEERTHY July 19, 2018, 11:53 a.m. | #3
On Thursday 19 July 2018 03:32 PM, Johan Hovold wrote:
> On Thu, Jul 12, 2018 at 10:37:37AM +0530, Keerthy wrote:
>> Cut down the shutdown time from 2 seconds to 1 sec. In case of roll
>> over try again.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>
>> Changes in v4:
>>
>>   * Fixed a compilation issue.
>>   * Extended the roll over check post interrupt programming.
>>
>>  drivers/rtc/rtc-omap.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
>> index 323ff55..88da927 100644
>> --- a/drivers/rtc/rtc-omap.c
>> +++ b/drivers/rtc/rtc-omap.c
> 
> First, the comment above this function would need to be updated as part
> of this patch as it refers to the two-second alarm offset.

Yes, will change that.

> 
>> @@ -435,17 +435,23 @@ static void omap_rtc_power_off(void)
>>  	struct rtc_time tm;
>>  	unsigned long now;
>>  	u32 val;
>> +	int seconds;
>>  
>>  	rtc->type->unlock(rtc);
>>  	/* enable pmic_power_en control */
>>  	val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
>>  	rtc_writel(rtc, OMAP_RTC_PMIC_REG, val | OMAP_RTC_PMIC_POWER_EN_EN);
>>  
>> -	/* set alarm two seconds from now */
>> +again:
>> +	/* Clear any existing ALARM2 event */
>> +	rtc_writel(rtc, OMAP_RTC_STATUS_REG, OMAP_RTC_STATUS_ALARM2);
> 
> Why is this needed? Any pending interrupt is cleared at probe, and a
> previous attempt to set the alarm really led to the alarm going off, why
> would we retry?

Yes this is not needed.

> 
>> +
>> +	/* set alarm one second from now */
>>  	omap_rtc_read_time_raw(rtc, &tm);
>> +	seconds = tm.tm_sec;
>>  	bcd2tm(&tm);
>>  	rtc_tm_to_time(&tm, &now);
>> -	rtc_time_to_tm(now + 2, &tm);
>> +	rtc_time_to_tm(now + 1, &tm);
>>  
>>  	if (tm2bcd(&tm) < 0) {
>>  		dev_err(&rtc->rtc->dev, "power off failed\n");
>> @@ -470,6 +476,9 @@ static void omap_rtc_power_off(void)
>>  	val = rtc_read(rtc, OMAP_RTC_INTERRUPTS_REG);
>>  	rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
>>  			val | OMAP_RTC_INTERRUPTS_IT_ALARM2);
> 
> Add a newline here.

Okay

> 
>> +	/* Our calculations started right before the rollover, try again */
> 
> Nit: use all lower case unless writing full sentences, which also
> matches most of the other comments in this file.

okay

> 
>> +	if (seconds != rtc_read(omap_rtc_power_off_rtc, OMAP_RTC_SECONDS_REG))
>> +		goto again;
> 
> Here the alarm may have gone off as part of the roll over, in which case
> you shouldn't retry.

Ex: We programmed at Sec = 2 and we expect ALARM2 to fire at sec = 3.

In the event of Roll over before setting the
OMAP_RTC_INTERRUPTS_IT_ALARM2 bit in the OMAP_RTC_INTERRUPTS_REG will we
not miss the ALARM2 event? Then poweroff would fail right?

Hence the attempt to retry the next second. This sequence would begin
right at the beginning of a new second and we expect the full sequence
to get over without having to retry again.

Hope i am clear.

> 
> Add a newline here too.

Okay

> 
>>  	rtc->type->lock(rtc);
>>  
>>  	/*
> 
> Thanks,
> Johan
>
J, KEERTHY July 19, 2018, 12:22 p.m. | #4
On Thursday 19 July 2018 05:23 PM, Keerthy wrote:
> 
> 
> On Thursday 19 July 2018 03:32 PM, Johan Hovold wrote:
>> On Thu, Jul 12, 2018 at 10:37:37AM +0530, Keerthy wrote:
>>> Cut down the shutdown time from 2 seconds to 1 sec. In case of roll
>>> over try again.
>>>
>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>> ---
>>>
>>> Changes in v4:
>>>
>>>   * Fixed a compilation issue.
>>>   * Extended the roll over check post interrupt programming.
>>>
>>>  drivers/rtc/rtc-omap.c | 13 +++++++++++--
>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
>>> index 323ff55..88da927 100644
>>> --- a/drivers/rtc/rtc-omap.c
>>> +++ b/drivers/rtc/rtc-omap.c
>>
>> First, the comment above this function would need to be updated as part
>> of this patch as it refers to the two-second alarm offset.
> 
> Yes, will change that.
> 
>>
>>> @@ -435,17 +435,23 @@ static void omap_rtc_power_off(void)
>>>  	struct rtc_time tm;
>>>  	unsigned long now;
>>>  	u32 val;
>>> +	int seconds;
>>>  
>>>  	rtc->type->unlock(rtc);
>>>  	/* enable pmic_power_en control */
>>>  	val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
>>>  	rtc_writel(rtc, OMAP_RTC_PMIC_REG, val | OMAP_RTC_PMIC_POWER_EN_EN);
>>>  
>>> -	/* set alarm two seconds from now */
>>> +again:
>>> +	/* Clear any existing ALARM2 event */
>>> +	rtc_writel(rtc, OMAP_RTC_STATUS_REG, OMAP_RTC_STATUS_ALARM2);
>>
>> Why is this needed? Any pending interrupt is cleared at probe, and a
>> previous attempt to set the alarm really led to the alarm going off, why
>> would we retry?
> 
> Yes this is not needed.
> 
>>
>>> +
>>> +	/* set alarm one second from now */
>>>  	omap_rtc_read_time_raw(rtc, &tm);
>>> +	seconds = tm.tm_sec;
>>>  	bcd2tm(&tm);
>>>  	rtc_tm_to_time(&tm, &now);
>>> -	rtc_time_to_tm(now + 2, &tm);
>>> +	rtc_time_to_tm(now + 1, &tm);
>>>  
>>>  	if (tm2bcd(&tm) < 0) {
>>>  		dev_err(&rtc->rtc->dev, "power off failed\n");
>>> @@ -470,6 +476,9 @@ static void omap_rtc_power_off(void)
>>>  	val = rtc_read(rtc, OMAP_RTC_INTERRUPTS_REG);
>>>  	rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
>>>  			val | OMAP_RTC_INTERRUPTS_IT_ALARM2);
>>
>> Add a newline here.
> 
> Okay
> 
>>
>>> +	/* Our calculations started right before the rollover, try again */
>>
>> Nit: use all lower case unless writing full sentences, which also
>> matches most of the other comments in this file.
> 
> okay
> 
>>
>>> +	if (seconds != rtc_read(omap_rtc_power_off_rtc, OMAP_RTC_SECONDS_REG))
>>> +		goto again;
>>
>> Here the alarm may have gone off as part of the roll over, in which case
>> you shouldn't retry.
> 
> Ex: We programmed at Sec = 2 and we expect ALARM2 to fire at sec = 3.
> 
> In the event of Roll over before setting the
> OMAP_RTC_INTERRUPTS_IT_ALARM2 bit in the OMAP_RTC_INTERRUPTS_REG will we
> not miss the ALARM2 event? Then poweroff would fail right?
> 
> Hence the attempt to retry the next second. This sequence would begin
> right at the beginning of a new second and we expect the full sequence
> to get over without having to retry again.
> 
> Hope i am clear.

I tried to program the interrupt for the same second on the hardware and
it does not fire. So to take care of roll over corner case one attempt
to retry is needed.

> 
>>
>> Add a newline here too.
> 
> Okay
> 
>>
>>>  	rtc->type->lock(rtc);
>>>  
>>>  	/*
>>
>> Thanks,
>> Johan
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Johan Hovold July 19, 2018, 12:36 p.m. | #5
On Thu, Jul 19, 2018 at 05:52:17PM +0530, Keerthy wrote:
> On Thursday 19 July 2018 05:23 PM, Keerthy wrote:
> > On Thursday 19 July 2018 03:32 PM, Johan Hovold wrote:
> >> On Thu, Jul 12, 2018 at 10:37:37AM +0530, Keerthy wrote:

> >>> @@ -470,6 +476,9 @@ static void omap_rtc_power_off(void)
> >>>  	val = rtc_read(rtc, OMAP_RTC_INTERRUPTS_REG);
> >>>  	rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
> >>>  			val | OMAP_RTC_INTERRUPTS_IT_ALARM2);

> >>> +	/* Our calculations started right before the rollover, try again */

> >>> +	if (seconds != rtc_read(omap_rtc_power_off_rtc, OMAP_RTC_SECONDS_REG))
> >>> +		goto again;
> >>
> >> Here the alarm may have gone off as part of the roll over, in which case
> >> you shouldn't retry.
> > 
> > Ex: We programmed at Sec = 2 and we expect ALARM2 to fire at sec = 3.
> > 
> > In the event of Roll over before setting the
> > OMAP_RTC_INTERRUPTS_IT_ALARM2 bit in the OMAP_RTC_INTERRUPTS_REG will we
> > not miss the ALARM2 event? Then poweroff would fail right?

Right, that would fail.

> > Hence the attempt to retry the next second. This sequence would begin
> > right at the beginning of a new second and we expect the full sequence
> > to get over without having to retry again.
> > 
> > Hope i am clear.

Yes, sure, but my point is that could end up retrying also after the
alarm has fired correctly (e.g. due to latencies in turning of the
power).

It may be enough to check OMAP_RTC_STATUS_REG before retrying.

> I tried to program the interrupt for the same second on the hardware and
> it does not fire. So to take care of roll over corner case one attempt
> to retry is needed.

Yes, that's expected.

Thanks,
Johan
J, KEERTHY July 19, 2018, 12:46 p.m. | #6
On Thursday 19 July 2018 06:06 PM, Johan Hovold wrote:
> On Thu, Jul 19, 2018 at 05:52:17PM +0530, Keerthy wrote:
>> On Thursday 19 July 2018 05:23 PM, Keerthy wrote:
>>> On Thursday 19 July 2018 03:32 PM, Johan Hovold wrote:
>>>> On Thu, Jul 12, 2018 at 10:37:37AM +0530, Keerthy wrote:
> 
>>>>> @@ -470,6 +476,9 @@ static void omap_rtc_power_off(void)
>>>>>  	val = rtc_read(rtc, OMAP_RTC_INTERRUPTS_REG);
>>>>>  	rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
>>>>>  			val | OMAP_RTC_INTERRUPTS_IT_ALARM2);
> 
>>>>> +	/* Our calculations started right before the rollover, try again */
> 
>>>>> +	if (seconds != rtc_read(omap_rtc_power_off_rtc, OMAP_RTC_SECONDS_REG))
>>>>> +		goto again;
>>>>
>>>> Here the alarm may have gone off as part of the roll over, in which case
>>>> you shouldn't retry.
>>>
>>> Ex: We programmed at Sec = 2 and we expect ALARM2 to fire at sec = 3.
>>>
>>> In the event of Roll over before setting the
>>> OMAP_RTC_INTERRUPTS_IT_ALARM2 bit in the OMAP_RTC_INTERRUPTS_REG will we
>>> not miss the ALARM2 event? Then poweroff would fail right?
> 
> Right, that would fail.
> 
>>> Hence the attempt to retry the next second. This sequence would begin
>>> right at the beginning of a new second and we expect the full sequence
>>> to get over without having to retry again.
>>>
>>> Hope i am clear.
> 
> Yes, sure, but my point is that could end up retrying also after the
> alarm has fired correctly (e.g. due to latencies in turning of the
> power)>
> It may be enough to check OMAP_RTC_STATUS_REG before retrying.

Ah okay. Yes this makes sense. I will use the status to retry.

> 
>> I tried to program the interrupt for the same second on the hardware and
>> it does not fire. So to take care of roll over corner case one attempt
>> to retry is needed.
> 
> Yes, that's expected.
> 
> Thanks,
> Johan
>
J, KEERTHY July 20, 2018, 10:33 a.m. | #7
On Thursday 19 July 2018 06:16 PM, Keerthy wrote:
> 
> 
> On Thursday 19 July 2018 06:06 PM, Johan Hovold wrote:
>> On Thu, Jul 19, 2018 at 05:52:17PM +0530, Keerthy wrote:
>>> On Thursday 19 July 2018 05:23 PM, Keerthy wrote:
>>>> On Thursday 19 July 2018 03:32 PM, Johan Hovold wrote:
>>>>> On Thu, Jul 12, 2018 at 10:37:37AM +0530, Keerthy wrote:
>>
>>>>>> @@ -470,6 +476,9 @@ static void omap_rtc_power_off(void)
>>>>>>  	val = rtc_read(rtc, OMAP_RTC_INTERRUPTS_REG);
>>>>>>  	rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
>>>>>>  			val | OMAP_RTC_INTERRUPTS_IT_ALARM2);
>>
>>>>>> +	/* Our calculations started right before the rollover, try again */
>>
>>>>>> +	if (seconds != rtc_read(omap_rtc_power_off_rtc, OMAP_RTC_SECONDS_REG))
>>>>>> +		goto again;
>>>>>
>>>>> Here the alarm may have gone off as part of the roll over, in which case
>>>>> you shouldn't retry.
>>>>
>>>> Ex: We programmed at Sec = 2 and we expect ALARM2 to fire at sec = 3.
>>>>
>>>> In the event of Roll over before setting the
>>>> OMAP_RTC_INTERRUPTS_IT_ALARM2 bit in the OMAP_RTC_INTERRUPTS_REG will we
>>>> not miss the ALARM2 event? Then poweroff would fail right?
>>
>> Right, that would fail.
>>
>>>> Hence the attempt to retry the next second. This sequence would begin
>>>> right at the beginning of a new second and we expect the full sequence
>>>> to get over without having to retry again.
>>>>
>>>> Hope i am clear.
>>
>> Yes, sure, but my point is that could end up retrying also after the
>> alarm has fired correctly (e.g. due to latencies in turning of the
>> power)>
>> It may be enough to check OMAP_RTC_STATUS_REG before retrying.

On a second thought. Status gets set only after the next second.

if ALARM2 status bit is set that surely means interrupt has fired but if
it is not set then there are 2 possibilities

1) ALARM2 is missed as the roll over happened
2) ALARM2 yet to fire as we are yet to get to the next second.

On the other hand Seconds gives me clear indication if we missed the
interrupt or we are about to get one.

> 
> Ah okay. Yes this makes sense. I will use the status to retry.
> 
>>
>>> I tried to program the interrupt for the same second on the hardware and
>>> it does not fire. So to take care of roll over corner case one attempt
>>> to retry is needed.
>>
>> Yes, that's expected.
>>
>> Thanks,
>> Johan
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Johan Hovold July 20, 2018, 11:03 a.m. | #8
On Fri, Jul 20, 2018 at 04:03:20PM +0530, Keerthy wrote:
> 
> 
> On Thursday 19 July 2018 06:16 PM, Keerthy wrote:
> > 
> > 
> > On Thursday 19 July 2018 06:06 PM, Johan Hovold wrote:
> >> On Thu, Jul 19, 2018 at 05:52:17PM +0530, Keerthy wrote:
> >>> On Thursday 19 July 2018 05:23 PM, Keerthy wrote:
> >>>> On Thursday 19 July 2018 03:32 PM, Johan Hovold wrote:
> >>>>> On Thu, Jul 12, 2018 at 10:37:37AM +0530, Keerthy wrote:
> >>
> >>>>>> @@ -470,6 +476,9 @@ static void omap_rtc_power_off(void)
> >>>>>>  	val = rtc_read(rtc, OMAP_RTC_INTERRUPTS_REG);
> >>>>>>  	rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
> >>>>>>  			val | OMAP_RTC_INTERRUPTS_IT_ALARM2);
> >>
> >>>>>> +	/* Our calculations started right before the rollover, try again */
> >>
> >>>>>> +	if (seconds != rtc_read(omap_rtc_power_off_rtc, OMAP_RTC_SECONDS_REG))
> >>>>>> +		goto again;
> >>>>>
> >>>>> Here the alarm may have gone off as part of the roll over, in which case
> >>>>> you shouldn't retry.
> >>>>
> >>>> Ex: We programmed at Sec = 2 and we expect ALARM2 to fire at sec = 3.
> >>>>
> >>>> In the event of Roll over before setting the
> >>>> OMAP_RTC_INTERRUPTS_IT_ALARM2 bit in the OMAP_RTC_INTERRUPTS_REG will we
> >>>> not miss the ALARM2 event? Then poweroff would fail right?
> >>
> >> Right, that would fail.
> >>
> >>>> Hence the attempt to retry the next second. This sequence would begin
> >>>> right at the beginning of a new second and we expect the full sequence
> >>>> to get over without having to retry again.
> >>>>
> >>>> Hope i am clear.
> >>
> >> Yes, sure, but my point is that could end up retrying also after the
> >> alarm has fired correctly (e.g. due to latencies in turning of the
> >> power)>
> >> It may be enough to check OMAP_RTC_STATUS_REG before retrying.
> 
> On a second thought. Status gets set only after the next second.
> 
> if ALARM2 status bit is set that surely means interrupt has fired but if
> it is not set then there are 2 possibilities
> 
> 1) ALARM2 is missed as the roll over happened
> 2) ALARM2 yet to fire as we are yet to get to the next second.
> 
> On the other hand Seconds gives me clear indication if we missed the
> interrupt or we are about to get one.

Yes, you still have to check seconds *before* retrying based on status.

That should do, right?

Johan

Patch

diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index 323ff55..88da927 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c
@@ -435,17 +435,23 @@  static void omap_rtc_power_off(void)
 	struct rtc_time tm;
 	unsigned long now;
 	u32 val;
+	int seconds;
 
 	rtc->type->unlock(rtc);
 	/* enable pmic_power_en control */
 	val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
 	rtc_writel(rtc, OMAP_RTC_PMIC_REG, val | OMAP_RTC_PMIC_POWER_EN_EN);
 
-	/* set alarm two seconds from now */
+again:
+	/* Clear any existing ALARM2 event */
+	rtc_writel(rtc, OMAP_RTC_STATUS_REG, OMAP_RTC_STATUS_ALARM2);
+
+	/* set alarm one second from now */
 	omap_rtc_read_time_raw(rtc, &tm);
+	seconds = tm.tm_sec;
 	bcd2tm(&tm);
 	rtc_tm_to_time(&tm, &now);
-	rtc_time_to_tm(now + 2, &tm);
+	rtc_time_to_tm(now + 1, &tm);
 
 	if (tm2bcd(&tm) < 0) {
 		dev_err(&rtc->rtc->dev, "power off failed\n");
@@ -470,6 +476,9 @@  static void omap_rtc_power_off(void)
 	val = rtc_read(rtc, OMAP_RTC_INTERRUPTS_REG);
 	rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
 			val | OMAP_RTC_INTERRUPTS_IT_ALARM2);
+	/* Our calculations started right before the rollover, try again */
+	if (seconds != rtc_read(omap_rtc_power_off_rtc, OMAP_RTC_SECONDS_REG))
+		goto again;
 	rtc->type->lock(rtc);
 
 	/*