rtc: snvs: improve timeout handling in snvs_rtc_enable()
PATCH 1/1 rtc: snvs: improve timeout handling in snvs_rtc_enable()
If snvs_rtc_enable() happens to be descheduled for a period longer than the timout period, a timeout may be indicated even if the RTC status has changed meanwhile. Force a status read at the end of the timeout period to prevent bogus timeout errors.
--- drivers/rtc/rtc-snvs.c | 22 +++++++++++++--------- 1 files changed, 13 insertions(+), 9 deletions(-)
Compile tested only due to lack of HW.
drivers/rtc/rtc-snvs.c +13 -9
@@ -81,6 +81,13 @@ static void rtc_write_sync_lp(void __iomem *ioaddr) } } +static int snvs_rtc_enable_done (int enable,void __iomem *addr) +{ +u32 lpcr =readl (addr +SNVS_LPCR ); + +return !!enable ^ !(lpcr &SNVS_LPCR_SRTC_ENV ); +} +static int snvs_rtc_enable (struct snvs_rtc_data *data,bool enable) {unsigned long timeout = jiffies +msecs_to_jiffies (1 );
@@ -99,18 +106,15 @@ static int snvs_rtc_enable(struct snvs_rtc_data *data, bool enable)spin_unlock_irqrestore (&data->lock, flags);while (1 ) { - lpcr =readl (data->ioaddr +SNVS_LPCR ); +if (snvs_rtc_enable_done (enable, data->ioaddr)) +break ; -if (enable) { -if (lpcr &SNVS_LPCR_SRTC_ENV ) -break ; - }else { -if (!(lpcr &SNVS_LPCR_SRTC_ENV )) +if (time_after (jiffies, timeout)) { +if (snvs_rtc_enable_done (enable, data->ioaddr))break ; +else +return -ETIMEDOUT ; } - -if (time_after (jiffies, timeout)) -return -ETIMEDOUT ; }return 0 ;
Signature · 1 line
1.7.2.5
PATCH 1/1 rtc: snvs: improve timeout handling in snvs_rtc_enable()
Quoted 6 lines · Lothar Wa?mann
If
snvs_rtc_enable()happens to be descheduled for a period longer than the timout period, a timeout may be indicated even if the RTC status has changed meanwhile. Force a status read at the end of the timeout period to prevent bogus timeout errors.
Quoted 6 lines
Signed-off-by Lothar Wa?mann <[email protected]>--- drivers/rtc/rtc-snvs.c | 22 +++++++++++++--------- 1 files changed, 13 insertions(+), 9 deletions(-)Compile tested only due to lack of HW.
A small nitpick below.
drivers/rtc/rtc-snvs.c static void rtc_write_sync_lp(void __iomem *ioaddr) · line 84
Quoted 10 lines
drivers/rtc/rtc-snvs.c +1
-0@@ -81,6 +81,13 @@ static void rtc_write_sync_lp(void __iomem *ioaddr) } } +static int snvs_rtc_enable_done (int enable,void __iomem *addr)
Can we rename parameter "addr" to "ioaddr" and put it before "enable", also change "enable" to bool?
Regards,
Shawn
Quoted 37 lines
drivers/rtc/rtc-snvs.c +12
-9+{ +u32 lpcr =readl (addr +SNVS_LPCR ); + +return !!enable ^ !(lpcr &SNVS_LPCR_SRTC_ENV ); +} +static int snvs_rtc_enable (struct snvs_rtc_data *data,bool enable) {unsigned long timeout = jiffies +msecs_to_jiffies (1 );@@ -99,18 +106,15 @@ static int snvs_rtc_enable(struct snvs_rtc_data *data, bool enable)spin_unlock_irqrestore (&data->lock, flags);while (1 ) { - lpcr =readl (data->ioaddr +SNVS_LPCR ); +if (snvs_rtc_enable_done (enable, data->ioaddr)) +break ; -if (enable) { -if (lpcr &SNVS_LPCR_SRTC_ENV ) -break ; - }else { -if (!(lpcr &SNVS_LPCR_SRTC_ENV )) +if (time_after (jiffies, timeout)) { +if (snvs_rtc_enable_done (enable, data->ioaddr))break ; +else +return -ETIMEDOUT ; } - -if (time_after (jiffies, timeout)) -return -ETIMEDOUT ; }return 0 ;Signature · 1 line
1.7.2.5
PATCH 1/1 rtc: snvs: improve timeout handling in snvs_rtc_enable()
Quoted 27 lines · Shawn Guo
Quoted 6 lines · Lothar Wa?mann
If
snvs_rtc_enable()happens to be descheduled for a period longer than the timout period, a timeout may be indicated even if the RTC status has changed meanwhile. Force a status read at the end of the timeout period to prevent bogus timeout errors.Quoted 6 lines
Signed-off-by Lothar Wa?mann <[email protected]>--- drivers/rtc/rtc-snvs.c | 22 +++++++++++++--------- 1 files changed, 13 insertions(+), 9 deletions(-)Compile tested only due to lack of HW.
Tested-by Shawn Guo <[email protected]>A small nitpick below.
Quoted 9 lines
drivers/rtc/rtc-snvs.c +0
-0@@ -81,6 +81,13 @@ static void rtc_write_sync_lp(void __iomem *ioaddr) } }
Quoted 5 lines
drivers/rtc/rtc-snvs.c static void rtc_write_sync_lp(void __iomem *ioaddr) · line 84
Quoted 1 line
drivers/rtc/rtc-snvs.c +1
-0+static int snvs_rtc_enable_done (int enable,void __iomem *addr)Can we rename parameter "addr" to "ioaddr" and put it before "enable", also change "enable" to bool?
OK. The resulting assembler code of the patch below is identical to the previous version of this patch.
--- drivers/rtc/rtc-snvs.c | 22 +++++++++++++--------- 1 files changed, 13 insertions(+), 9 deletions(-)
drivers/rtc/rtc-snvs.c +13 -9
@@ -81,6 +81,13 @@ static void rtc_write_sync_lp(void __iomem *ioaddr) } } +static int snvs_rtc_enable_done (void __iomem *ioaddr,bool enable) +{ +u32 lpcr =readl (ioaddr +SNVS_LPCR ); + +return enable ^ !(lpcr &SNVS_LPCR_SRTC_ENV ); +} +static int snvs_rtc_enable (struct snvs_rtc_data *data,bool enable) {unsigned long timeout = jiffies +msecs_to_jiffies (1 );
@@ -99,18 +106,15 @@ static int snvs_rtc_enable(struct snvs_rtc_data *data, bool enable)spin_unlock_irqrestore (&data->lock, flags);while (1 ) { - lpcr =readl (data->ioaddr +SNVS_LPCR ); +if (snvs_rtc_enable_done (data->ioaddr, enable)) +break ; -if (enable) { -if (lpcr &SNVS_LPCR_SRTC_ENV ) -break ; - }else { -if (!(lpcr &SNVS_LPCR_SRTC_ENV )) +if (time_after (jiffies, timeout)) { +if (snvs_rtc_enable_done (data->ioaddr, enable))break ; +else +return -ETIMEDOUT ; } - -if (time_after (jiffies, timeout)) -return -ETIMEDOUT ; }return 0 ;
Signature · 14 lines
1.7.2.5 Lothar Wa?mann -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________
PATCH 1/1 rtc: snvs: improve timeout handling in snvs_rtc_enable()
Hi Shawn, Lothar,
Quoted 13 lines · Lothar Wa?mann
Quoted 12 lines · Shawn Guo
Quoted 11 lines · Lothar Wa?mann
If
snvs_rtc_enable()happens to be descheduled for a period longer than the timout period, a timeout may be indicated even if the RTC status has changed meanwhile. Force a status read at the end of the timeout period to prevent bogus timeout errors.Signed-off-by Lothar Wa?mann <[email protected]>--- drivers/rtc/rtc-snvs.c | 22 +++++++++++++--------- 1 files changed, 13 insertions(+), 9 deletions(-)
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Compile tested only due to lack of HW.
Tested-by Shawn Guo <[email protected]>
It seems that patch never made it mainline. Can you check if it is still needed?
Quoted 69 lines
Quoted 11 lines
A small nitpick below.
Quoted 9 lines
drivers/rtc/rtc-snvs.c +0
-0@@ -81,6 +81,13 @@ static void rtc_write_sync_lp(void __iomem *ioaddr) } }Quoted 5 lines
drivers/rtc/rtc-snvs.c static void rtc_write_sync_lp(void __iomem *ioaddr) · line 84
Quoted 1 line
drivers/rtc/rtc-snvs.c +1
-0+static int snvs_rtc_enable_done (int enable,void __iomem *addr)Can we rename parameter "addr" to "ioaddr" and put it before "enable", also change "enable" to bool?
OK. The resulting assembler code of the patch below is identical to the previous version of this patch.
Signed-off-by Lothar Wa?mann <[email protected]>Tested-by Shawn Guo <[email protected]>--- drivers/rtc/rtc-snvs.c | 22 +++++++++++++--------- 1 files changed, 13 insertions(+), 9 deletions(-)drivers/rtc/rtc-snvs.c +13
-9@@ -81,6 +81,13 @@ static void rtc_write_sync_lp(void __iomem *ioaddr) } } +static int snvs_rtc_enable_done (void __iomem *ioaddr,bool enable) +{ +u32 lpcr =readl (ioaddr +SNVS_LPCR ); + +return enable ^ !(lpcr &SNVS_LPCR_SRTC_ENV ); +} +static int snvs_rtc_enable (struct snvs_rtc_data *data,bool enable) {unsigned long timeout = jiffies +msecs_to_jiffies (1 );@@ -99,18 +106,15 @@ static int snvs_rtc_enable(struct snvs_rtc_data *data, bool enable)spin_unlock_irqrestore (&data->lock, flags);while (1 ) { - lpcr =readl (data->ioaddr +SNVS_LPCR ); +if (snvs_rtc_enable_done (data->ioaddr, enable)) +break ; -if (enable) { -if (lpcr &SNVS_LPCR_SRTC_ENV ) -break ; - }else { -if (!(lpcr &SNVS_LPCR_SRTC_ENV )) +if (time_after (jiffies, timeout)) { +if (snvs_rtc_enable_done (data->ioaddr, enable))break ; +else +return -ETIMEDOUT ; } - -if (time_after (jiffies, timeout)) -return -ETIMEDOUT ; }return 0 ;
Signature · 3 lines
Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 1/1 rtc: snvs: improve timeout handling in snvs_rtc_enable()
In reply to Lothar WaßmannQuoted 4 lines · On Sat, 18 Aug 2012 10:10:08 +0200 Lothar Wa__mann <[email protected]>…
If
snvs_rtc_enable()happens to be descheduled for a period longer than the timout period, a timeout may be indicated even if the RTC status has changed meanwhile. Force a status read at the end of the timeout period to prevent bogus timeout errors.
(delayed, sorry)
Quoted 31 lines
drivers/rtc/rtc-snvs.c +11
-6@@ -81,6 +81,13 @@ static void rtc_write_sync_lp(void __iomem *ioaddr) } } +static int snvs_rtc_enable_done (int enable,void __iomem *addr) +{ +u32 lpcr =readl (addr +SNVS_LPCR ); + +return !!enable ^ !(lpcr &SNVS_LPCR_SRTC_ENV ); +} +static int snvs_rtc_enable (struct snvs_rtc_data *data,bool enable) {unsigned long timeout = jiffies +msecs_to_jiffies (1 );@@ -99,18 +106,15 @@ static int snvs_rtc_enable(struct snvs_rtc_data *data, bool enable)spin_unlock_irqrestore (&data->lock, flags);while (1 ) { - lpcr =readl (data->ioaddr +SNVS_LPCR ); +if (snvs_rtc_enable_done (enable, data->ioaddr)) +break ; -if (enable) { -if (lpcr &SNVS_LPCR_SRTC_ENV ) -break ; - }else { -if (!(lpcr &SNVS_LPCR_SRTC_ENV )) +if (time_after (jiffies, timeout)) { +if (snvs_rtc_enable_done (enable, data->ioaddr))
drivers/rtc/rtc-snvs.c static int snvs_rtc_enable(struct snvs_rtc_data *data, bool enable) · line 120
Quoted 10 lines
drivers/rtc/rtc-snvs.c +2
-3break ; + else + return -ETIMEDOUT; } - -if (time_after (jiffies, timeout)) -return -ETIMEDOUT ; }return 0 ;
I think this will have been addressed by
http://ozlabs.org/~akpm/mmots/broken-out/rtc-snvs-add-freescale-rtc-snvs-driver-fix.patch?