| Message ID | 1453407813-14646-4-git-send-email-javier@osg.samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 22.01.2016 05:23, Javier Martinez Canillas wrote: > Documentation/timers/timers-howto.txt suggest to use usleep_range() > instead of msleep() for small msec (1ms - 20ms) since msleep() will > often sleep for 20ms for any value in that range. > > This is fine in this case since 16ms is the _minimum_ delay required > by max77686 for an RTC update but by using usleep_range() instead of > msleep(), the driver can support other RTC IP blocks with a shorter > minimum delay (i.e: in the range of usecs instead of msecs). > > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> > Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > > --- > > Changes in v2: > - Add Krzysztof Kozlowski's Reviewed-by tag to patch #3. > - Fix typo error in changelog. Suggested by Krzysztof Kozlowski. > > drivers/rtc/rtc-max77686.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > Tested on Trats2 board with max77686: Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 22 January 2016 01:53 AM, Javier Martinez Canillas wrote: > RTC_SEC = 0, > @@ -130,7 +130,8 @@ static int max77686_rtc_update(struct max77686_rtc_info *info, > __func__, ret, data); > else { > /* Minimum 16ms delay required before RTC update. */ > - msleep(MAX77686_RTC_UPDATE_DELAY); > + usleep_range(MAX77686_RTC_UPDATE_DELAY, > + MAX77686_RTC_UPDATE_DELAY * 2); > } > Instead of making usleep_range(16000, 32000), can we make small range as usleep_range(16000, 17000)? I am using as usleep_range(16000, 16000) always. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Laxman, On 01/22/2016 06:41 AM, Laxman Dewangan wrote: > > On Friday 22 January 2016 01:53 AM, Javier Martinez Canillas wrote: >> RTC_SEC = 0, >> @@ -130,7 +130,8 @@ static int max77686_rtc_update(struct max77686_rtc_info *info, >> __func__, ret, data); >> else { >> /* Minimum 16ms delay required before RTC update. */ >> - msleep(MAX77686_RTC_UPDATE_DELAY); >> + usleep_range(MAX77686_RTC_UPDATE_DELAY, >> + MAX77686_RTC_UPDATE_DELAY * 2); >> } >> > > Instead of making usleep_range(16000, 32000), can we make small range as > usleep_range(16000, 17000)? > Yes, I also didn't know how to make the delay smaller. If I do for example usleep_range(delay, delay + 10000), then the 10000 delta would be too big for max77802 (50 times the minimum required 200 delay). So I used delay * 2 for two reasons: 1) That way is generic enough and can work for any delay 2) My understanding is that most of times the delay should be precise and is not that bad if sometimes the delay is the worst case (2 * X) since after all the delay is the minimum required. I also see that usleep_range(X, X * 2) is a used pattern across the kernel. > I am using as usleep_range(16000, 16000) always. According to Documentation/timers/timers-howto.txt, that's not a good idea since usleep_range() is implemented using high-resolution timers so by not using a range, the kernel won't be able to merge the wakeup with other wakeups which leads to much more interrupts being triggered. Best regards,
On Friday 22 January 2016 05:35 PM, Javier Martinez Canillas wrote: > Hello Laxman, > > On 01/22/2016 06:41 AM, Laxman Dewangan wrote: >> >> On Friday 22 January 2016 01:53 AM, Javier Martinez Canillas wrote: >>> RTC_SEC = 0, >>> @@ -130,7 +130,8 @@ static int max77686_rtc_update(struct >>> max77686_rtc_info *info, >>> __func__, ret, data); >>> else { >>> /* Minimum 16ms delay required before RTC update. */ >>> - msleep(MAX77686_RTC_UPDATE_DELAY); >>> + usleep_range(MAX77686_RTC_UPDATE_DELAY, >>> + MAX77686_RTC_UPDATE_DELAY * 2); >>> } >>> >> >> Instead of making usleep_range(16000, 32000), can we make small range as >> usleep_range(16000, 17000)? >> > > Yes, I also didn't know how to make the delay smaller. If I do for > example > > usleep_range(delay, delay + 10000), then the 10000 delta would be too big > for max77802 (50 times the minimum required 200 delay). > > So I used delay * 2 for two reasons: > > 1) That way is generic enough and can work for any delay > > 2) My understanding is that most of times the delay should be precise and > is not that bad if sometimes the delay is the worst case (2 * X) since > after all the delay is the minimum required. > > I also see that usleep_range(X, X * 2) is a used pattern across the > kernel. OK, fine to me here. Acked-by: Laxman Dewangan <ldewangan@nvidia.com> -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c index 98fabdb308b9..71ef2240b3fc 100644 --- a/drivers/rtc/rtc-max77686.c +++ b/drivers/rtc/rtc-max77686.c @@ -41,7 +41,7 @@ #define ALARM_ENABLE_SHIFT 7 #define ALARM_ENABLE_MASK (1 << ALARM_ENABLE_SHIFT) -#define MAX77686_RTC_UPDATE_DELAY 16 +#define MAX77686_RTC_UPDATE_DELAY 16000 enum { RTC_SEC = 0, @@ -130,7 +130,8 @@ static int max77686_rtc_update(struct max77686_rtc_info *info, __func__, ret, data); else { /* Minimum 16ms delay required before RTC update. */ - msleep(MAX77686_RTC_UPDATE_DELAY); + usleep_range(MAX77686_RTC_UPDATE_DELAY, + MAX77686_RTC_UPDATE_DELAY * 2); } return ret;