On 7/17/2018 2:31 AM, John Stultz wrote:
On Mon, Jul 16, 2018 at 1:40 PM, Mukesh Ojha <[email protected]> wrote:Currently, there exists a corner case assuming when there is only one clocksource e.g RTC, and system failed to go to suspend mode. While resume rtc_resume() injects the sleeptime as timekeeping_rtc_skipresume() returned 'false' (default value of sleeptime_injected) due to which we can see mismatch in timestamps. This issue can also come in a system where more than one clocksource are present and very first suspend fails. Fix this by handling the sleeptime_injected flag properly. Success case: ------------ {sleeptime_injected=false} rtc_suspend() => timekeeping_suspend() => timekeeping_resume() => (sleeptime injected) rtc_resume() Failure case: ------------ {failure in sleep path} {sleeptime_injected=false} rtc_suspend() => rtc_resume() sleeptime injected again which was not required as the suspend failed) Originally-by: Thomas Gleixner <[email protected]> Signed-off-by: Mukesh Ojha <[email protected]> --- Changes in V4: * Changes as suggested by John - Changed the variable name from sleeptime_injected to suspend_timing_needed - Changed the boolean logic.Thanks so much for reworking and resending this again!diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c index d37588f..ee455cc 100644 --- a/drivers/rtc/class.c +++ b/drivers/rtc/class.c @@ -102,7 +102,7 @@ static int rtc_resume(struct device *dev) struct timespec64 sleep_time; int err; - if (timekeeping_rtc_skipresume()) + if (!timekeeping_rtc_skipresume())Hrm... So I'd have instead inverted the logic *in* timekeeping_rtc_skipresume(), rather then here, but this looks to be close enough and I can fix that bit up.
Will take care of yours and Thomas comment in v5.
Can you confirm you've validated this version of the patch resolves the issue you reported?
Yeah, I validated. Thanks Mukesh
thanks -john
