Re: [PATCH 04/27] clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/11/17 00:18, Stephen Boyd wrote:
On 11/06, Tero Kristo wrote:
On 03/11/17 17:43, Stephen Boyd wrote:
On 10/30, Tero Kristo wrote:
@@ -90,7 +91,7 @@ static bool _omap4_is_ready(u32 val)
  static bool _omap4_is_timeout(union omap4_timeout *time, u32 timeout)
  {
-	if (unlikely(_early_timeout)) {
+	if (unlikely(_early_timeout || timekeeping_suspended)) {
  		if (time->cycles++ < timeout) {

This would be the second user of timekeeping_suspended outside of
timekeeping core. Why don't we just udelay(1) every time we call
this function? The loop on ktime without any sort of delay in it
may actually spin faster, especially because we can't get
interrupted here (irqs are off). And irqs + preemption enabled is
typically where you would want to use ktime instead of counting
udelay() calls to see if you hit a timeout.

It actually was originally just udelay() but I changed it to use the
ktime_get() approach way back due to comments provided on some early
revisions of this patch. See:
https://patchwork.kernel.org/patch/7884371/


Ok, so we use ktime to spin faster on the bit than udelay() would
allow us to. That looks to be on purpose because udelay(1) is too
long between bit checks.

What is causing us to call this path after timekeeping has been
suspended? Please add some more specifics to the commit text so
we know exactly where it's happening. Also add a comment above
the if statement describing why we're checking the variable so it
isn't buried in commit text somewhere and Cc timekeeping
maintainers on the patch please.

Ok, I'll comment this in the code and repost.

-Tero


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux