[PATCH] alarmtimer: Add the verification code for rtc device error.

[PATCH] alarmtimer: Add the verification code for rtc device error.

[Hyogi Gim]

In alarmtimer_suspend(), the error after rtc_read_time() is not checked.
If rtc device fail to read rtc time, we cannot ensure the following process.

Furthermore, the return value of rtc_timer_start() needs to distinguish
-ETIME and other rtc device error. If the error is relevant to rtc device,
suspend is failed unintentionally. In this case, it just returns zero for
a stable suspend. Otherwise, in the worst case, suspend will fail continually.

So, this patch verifies the rtc device error in alarmtimer_suspend(). it
includes "rtc_err goto" statement instead of a direct "return 0" to clarify
the rtc device error.

Signed-off-by: Hyogi Gim <hyogi.gim@lge.com>
---
 kernel/time/alarmtimer.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 88c9c65..8cc43b8 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -261,15 +261,23 @@ static int alarmtimer_suspend(struct device *dev)
 
 	/* Setup an rtc timer to fire that far in the future */
 	rtc_timer_cancel(rtc, &rtctimer);
-	rtc_read_time(rtc, &tm);
+	ret = rtc_read_time(rtc, &tm);
+	if (ret < 0)
+		goto rtc_err;
 	now = rtc_tm_to_ktime(tm);
 	now = ktime_add(now, min);
 
 	/* Set alarm, if in the past reject suspend briefly to handle */
 	ret = rtc_timer_start(rtc, &rtctimer, now, ktime_set(0, 0));
-	if (ret < 0)
+	if (ret == -ETIME)
 		__pm_wakeup_event(ws, MSEC_PER_SEC);
+	else if (ret < 0)
+		goto rtc_err;
+
 	return ret;
+
+rtc_err:
+	return 0;
 }
 #else
 static int alarmtimer_suspend(struct device *dev)
-- 
1.8.3.2

Re: [PATCH] alarmtimer: Add the verification code for rtc device error.

[John Stultz]

On Thu, Jul 10, 2014 at 6:24 PM, Hyogi Gim <hyogi.gim@lge.com> wrote:
> In alarmtimer_suspend(), the error after rtc_read_time() is not checked.
> If rtc device fail to read rtc time, we cannot ensure the following process.
>
> Furthermore, the return value of rtc_timer_start() needs to distinguish
> -ETIME and other rtc device error. If the error is relevant to rtc device,
> suspend is failed unintentionally. In this case, it just returns zero for
> a stable suspend. Otherwise, in the worst case, suspend will fail continually.


Hey! Sorry for the late response here.

So this seems reasonable as always failing suspend is problematic, but
I worry that for the case where we do have a failure to read or set
the RTC, we'd suspend and not wake up as specified, which is an issue
as well. Absorbing the error silently in these cases would make it
difficult to debug. Should we at least print some output out to help
folks hunt down this sort of issue?

thanks
-john

Re: [PATCH] alarmtimer: Add the verification code for rtc device error.

[Hyogi Gim]

On 07/31/2014 02:58 AM, John Stultz wrote:
> 
> Hey! Sorry for the late response here.
> 
> So this seems reasonable as always failing suspend is problematic, but
> I worry that for the case where we do have a failure to read or set
> the RTC, we'd suspend and not wake up as specified, which is an issue
> as well. Absorbing the error silently in these cases would make it
> difficult to debug. Should we at least print some output out to help
> folks hunt down this sort of issue?
> 

I agree. Most RTC device drivers don't print out the error of read_time.        
So, I think the higher level like /drivers/rtc/interface.c should check          
the error and give the information of the RTC device status.                    
                                                                                
I'll try to find a proper point.