Thread (8 messages) 8 messages, 4 authors, 2012-01-31

Lifecycle

  1. Posted w.sang@pengutronix.de (Wolfram Sang)

[PATCH] ARM: mx28: clock-mx28: Use a proper timeout mechanism

From: Wolfram Sang <hidden>
Date: 2012-01-23 21:18:54

Hi Fabio,

On Mon, Jan 23, 2012 at 12:41:07PM -0200, Fabio Estevam wrote:
Introduce a function for checking the busy bits of CLKCTRL register that 
uses a proper timeout mechanism.

Remove parts of code that use busy loops and replace them with the 
mxs_clkctrl_timeout() function.

Tested on a mx28evk by performing audio playback.

Suggested-by: Wolfram Sang <redacted>
Signed-off-by: Fabio Estevam <redacted>
Is there a difference in the two versions you sent?
quoted hunk ↗ jump to hunk
---
 arch/arm/mach-mxs/clock-mx28.c |   64 +++++++++++++---------------------------
 1 files changed, 21 insertions(+), 43 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
index 5d68e41..ad5482d 100644
--- a/arch/arm/mach-mxs/clock-mx28.c
+++ b/arch/arm/mach-mxs/clock-mx28.c
@@ -38,6 +38,7 @@
 #define DIGCTRL_BASE_ADDR	MX28_IO_ADDRESS(MX28_DIGCTL_BASE_ADDR)
 
 #define PARENT_RATE_SHIFT	8
+#define CLKCTRL_TIMEOUT		10	/* 10 ms = 1 jiffy */
I'd remove the jiffy-part of the comment to prevent belief that the
relationship will always be true.
quoted hunk ↗ jump to hunk
 static struct clk pll2_clk;
 static struct clk cpu_clk;
@@ -127,6 +128,20 @@ static unsigned long pll2_clk_get_rate(struct clk *clk)
 	return 50000000;
 }
 
+static int mxs_clkctrl_timeout(unsigned int reg_offset, unsigned int mask)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(CLKCTRL_TIMEOUT);
+	while (readl_relaxed(CLKCTRL_BASE_ADDR + reg_offset) & mask) {
Hmm, this is a lot better, yet not perfect. You could be scheduled away here
for 10ms and then you had only checked at t=0. However, I think it makes more
sense to introduce a generic timeout-loop somehow and then convert to it
later. This is my homework, though...
+		if (time_after(jiffies, timeout)) {
+			pr_err("%s: divider writing timeout\n", __func__);
__func__ is probably not useful here. Perhaps reg and mask?
+			return -ETIMEDOUT;
+		}
+
Remove empty line.
+	}
+
+	return 0;
+}
+
Rest looked good from a glimpse.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120123/6bdaf43a/attachment.sig>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help