Linux-Clk Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] clk: ti: clkctrl: Fix failed to enable error with double udelay timeout
@ 2019-09-30 15:40 Tony Lindgren
  2019-10-07  0:57 ` Keerthy
  0 siblings, 1 reply; 3+ messages in thread
From: Tony Lindgren @ 2019-09-30 15:40 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Tero Kristo
  Cc: devicetree, linux-clk, linux-omap, Keerthy

Commit 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if
timekeeping is suspended") added handling for cases when timekeeping is
suspended. But looks like we can still get occasional "failed to enable"
errors on the PM runtime resume path with udelay() returning faster than
expected.

With ti-sysc interconnect target module driver this leads into device
failure with PM runtime failing with "failed to enable" clkctrl error.

Let's fix the issue with a delay of two times the desired delay as in
often done for udelay() to account for the inaccuracy.

Fixes: 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended")
Cc: Keerthy <j-keerthy@ti.com>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/clk/ti/clkctrl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -100,11 +100,12 @@ static bool _omap4_is_timeout(union omap4_timeout *time, u32 timeout)
 	 * can be from a timer that requires pm_runtime access, which
 	 * will eventually bring us here with timekeeping_suspended,
 	 * during both suspend entry and resume paths. This happens
-	 * at least on am43xx platform.
+	 * at least on am43xx platform. Account for flakeyness
+	 * with udelay() by multiplying the timeout value by 2.
 	 */
 	if (unlikely(_early_timeout || timekeeping_suspended)) {
 		if (time->cycles++ < timeout) {
-			udelay(1);
+			udelay(1 * 2);
 			return false;
 		}
 	} else {
-- 
2.23.0

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] clk: ti: clkctrl: Fix failed to enable error with double udelay timeout
  2019-09-30 15:40 [PATCH] clk: ti: clkctrl: Fix failed to enable error with double udelay timeout Tony Lindgren
@ 2019-10-07  0:57 ` Keerthy
  2019-10-10 14:05   ` Tony Lindgren
  0 siblings, 1 reply; 3+ messages in thread
From: Keerthy @ 2019-10-07  0:57 UTC (permalink / raw)
  To: Tony Lindgren, Michael Turquette, Stephen Boyd, Tero Kristo
  Cc: devicetree, linux-clk, linux-omap



On 30/09/19 9:10 PM, Tony Lindgren wrote:
> Commit 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if
> timekeeping is suspended") added handling for cases when timekeeping is
> suspended. But looks like we can still get occasional "failed to enable"
> errors on the PM runtime resume path with udelay() returning faster than
> expected.
> 
> With ti-sysc interconnect target module driver this leads into device
> failure with PM runtime failing with "failed to enable" clkctrl error.
> 
> Let's fix the issue with a delay of two times the desired delay as in
> often done for udelay() to account for the inaccuracy.

Tested for DS0 and rtc+ddr modes on am43 and ds0 on am33.

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

> 
> Fixes: 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended")
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>   drivers/clk/ti/clkctrl.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
> --- a/drivers/clk/ti/clkctrl.c
> +++ b/drivers/clk/ti/clkctrl.c
> @@ -100,11 +100,12 @@ static bool _omap4_is_timeout(union omap4_timeout *time, u32 timeout)
>   	 * can be from a timer that requires pm_runtime access, which
>   	 * will eventually bring us here with timekeeping_suspended,
>   	 * during both suspend entry and resume paths. This happens
> -	 * at least on am43xx platform.
> +	 * at least on am43xx platform. Account for flakeyness
> +	 * with udelay() by multiplying the timeout value by 2.
>   	 */
>   	if (unlikely(_early_timeout || timekeeping_suspended)) {
>   		if (time->cycles++ < timeout) {
> -			udelay(1);
> +			udelay(1 * 2);
>   			return false;
>   		}
>   	} else {
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] clk: ti: clkctrl: Fix failed to enable error with double udelay timeout
  2019-10-07  0:57 ` Keerthy
@ 2019-10-10 14:05   ` Tony Lindgren
  0 siblings, 0 replies; 3+ messages in thread
From: Tony Lindgren @ 2019-10-10 14:05 UTC (permalink / raw)
  To: Keerthy
  Cc: Michael Turquette, Stephen Boyd, Tero Kristo, devicetree,
	linux-clk, linux-omap

* Keerthy <j-keerthy@ti.com> [191007 00:57]:
> 
> 
> On 30/09/19 9:10 PM, Tony Lindgren wrote:
> > Commit 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if
> > timekeeping is suspended") added handling for cases when timekeeping is
> > suspended. But looks like we can still get occasional "failed to enable"
> > errors on the PM runtime resume path with udelay() returning faster than
> > expected.
> > 
> > With ti-sysc interconnect target module driver this leads into device
> > failure with PM runtime failing with "failed to enable" clkctrl error.
> > 
> > Let's fix the issue with a delay of two times the desired delay as in
> > often done for udelay() to account for the inaccuracy.
> 
> Tested for DS0 and rtc+ddr modes on am43 and ds0 on am33.
> 
> Tested-by: Keerthy <j-keerthy@ti.com>

Thanks for testing. This one should be applied into v5.4-rc series
please if no more comments.

Regards,

Tony

> > Fixes: 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended")
> > Cc: Keerthy <j-keerthy@ti.com>
> > Cc: Tero Kristo <t-kristo@ti.com>
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > ---
> >   drivers/clk/ti/clkctrl.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
> > --- a/drivers/clk/ti/clkctrl.c
> > +++ b/drivers/clk/ti/clkctrl.c
> > @@ -100,11 +100,12 @@ static bool _omap4_is_timeout(union omap4_timeout *time, u32 timeout)
> >   	 * can be from a timer that requires pm_runtime access, which
> >   	 * will eventually bring us here with timekeeping_suspended,
> >   	 * during both suspend entry and resume paths. This happens
> > -	 * at least on am43xx platform.
> > +	 * at least on am43xx platform. Account for flakeyness
> > +	 * with udelay() by multiplying the timeout value by 2.
> >   	 */
> >   	if (unlikely(_early_timeout || timekeeping_suspended)) {
> >   		if (time->cycles++ < timeout) {
> > -			udelay(1);
> > +			udelay(1 * 2);
> >   			return false;
> >   		}
> >   	} else {
> > 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-30 15:40 [PATCH] clk: ti: clkctrl: Fix failed to enable error with double udelay timeout Tony Lindgren
2019-10-07  0:57 ` Keerthy
2019-10-10 14:05   ` Tony Lindgren

Linux-Clk Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-clk/0 linux-clk/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-clk linux-clk/ https://lore.kernel.org/linux-clk \
		linux-clk@vger.kernel.org linux-clk@archiver.kernel.org
	public-inbox-index linux-clk

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-clk


AGPL code for this site: git clone https://public-inbox.org/ public-inbox