Skip to contentSkip to conversation

ARM: clock-mx28: Fix end of loop condition

PATCH New Fabio Estevam 4 participants 9 messages Jan 16, 2012 – Jan 17, 2012

Subsystems (3)
Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape
lore
Share & tools
lore

Fabio Estevam

author
#0 lore raw

PATCH 1/1 ARM: clock-mx28: Fix end of loop condition

From: Fabio Estevam <[email protected]>

Selecting audio on a mx28evk the saif driver was failing:

[    0.660000] saif0_clk_set_rate: divider writing timeout                     
[    0.670000] mxs-sgtl5000: probe of mxs-sgtl5000.0 failed with error -110    
[    0.670000] ALSA device list:                                               
[    0.680000]   No soundcards found.

The timeout on saif0_clk_set_rate was due to the wrong condition for the end of loop.

After fixing it, the audio driver can be correctly probed and becomes functional.

While at it, fix the other locations where the similar issue occurs.

Signed-off-by Fabio Estevam <[email protected]>
---
 arch/arm/mach-mxs/clock-mx28.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
arch/arm/mach-mxs/clock-mx28.c +4 -4
index 5d68e41..ee9e04b 100644
@@ -396,7 +396,7 @@ static int name##_set_rate(struct clk *clk, unsigned long rate)		\
 	}								\
 	__raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_##dr);		\
 									\
-	for (i = 10000; i; i--)						\
+	for (i = 10000; i < 0; i--)					\
 		if (!(__raw_readl(CLKCTRL_BASE_ADDR +			\
 			HW_CLKCTRL_##dr) & bm_busy))			\
 			break;						\

@@ -439,7 +439,7 @@ static int name##_set_rate(struct clk *clk, unsigned long rate)		\
 	}								\
 	__raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_##dr);		\
 									\
-	for (i = 10000; i; i--)						\
+	for (i = 10000; i < 0; i--)					\
 		if (!(__raw_readl(CLKCTRL_BASE_ADDR +			\
 			HW_CLKCTRL_##dr) & BM_CLKCTRL_##dr##_BUSY))	\
 			break;						\

@@ -479,7 +479,7 @@ static int name##_set_rate(struct clk *clk, unsigned long rate)		\
 	reg |= div << BP_CLKCTRL_##rs##_DIV;				\
 	__raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs);		\
 									\
-	for (i = 10000; i; i--)						\
+	for (i = 10000; i < 0; i--)					\
 		if (!(__raw_readl(CLKCTRL_BASE_ADDR +			\
 			HW_CLKCTRL_##rs) & BM_CLKCTRL_##rs##_BUSY))	\
 			break;						\

@@ -756,7 +756,7 @@ static int clk_misc_init(void)
 	reg |= 3 << BP_CLKCTRL_HBUS_DIV;
 	__raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_HBUS);
 
-	for (i = 10000; i; i--)
+	for (i = 10000; i < 0; i--)
 		if (!(__raw_readl(CLKCTRL_BASE_ADDR +
 			HW_CLKCTRL_HBUS) & BM_CLKCTRL_HBUS_ASM_BUSY))
 			break;
Signature · 1 line
1.7.1
reviewer
#1 lore raw

PATCH 1/1 ARM: clock-mx28: Fix end of loop condition

Quoted 9 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

Selecting audio on a mx28evk the saif driver was failing:

[ ? ?0.660000] saif0_clk_set_rate: divider writing timeout
[ ? ?0.670000] mxs-sgtl5000: probe of mxs-sgtl5000.0 failed with error -110 [ ? ?0.670000] ALSA device list:
[ ? ?0.680000] ? No soundcards found.

Quoted 7 lines

The timeout on saif0_clk_set_rate was due to the wrong condition for the end of loop.

After fixing it, the audio driver can be correctly probed and becomes functional.

While at it, fix the other locations where the similar issue occurs.

Signed-off-by Fabio Estevam <[email protected]>

Please disregard this version.

#2 lore raw

PATCH 1/1 ARM: clock-mx28: Fix end of loop condition

Quoted 15 lines

On Mon, Jan 16, 2012 at 5:01 PM, Fabio Estevam

Quoted 12 lines · <[email protected]> wrote:

From: Fabio Estevam <[email protected]>

Selecting audio on a mx28evk the saif driver was failing:

[    0.660000] saif0_clk_set_rate: divider writing timeout
[    0.670000] mxs-sgtl5000: probe of mxs-sgtl5000.0 failed with error
-110 [    0.670000] ALSA device list:
[    0.680000]   No soundcards found.

The timeout on saif0_clk_set_rate was due to the wrong condition for the end of loop.

Quoted 8 lines
Quoted 6 lines

After fixing it, the audio driver can be correctly probed and becomes functional.

While at it, fix the other locations where the similar issue occurs.

Signed-off-by Fabio Estevam <[email protected]>

Please disregard this version.

I see ... ignore my previous comment ;-)

M

reviewer
#3 lore raw

PATCH 1/1 ARM: clock-mx28: Fix end of loop condition

Quoted 1 line · Marek Vasut

I see ... ignore my previous comment ;-)

Sorry for the noise of my previous patch. I think this issue can be solved by just increasing the timeout value.

I will make more tests to confirm and post a new patch.

Regards,

Fabio Estevam

#4 lore raw

PATCH 1/1 ARM: clock-mx28: Fix end of loop condition

Quoted 8 lines · Fabio Estevam
Quoted 1 line · Marek Vasut

I see ... ignore my previous comment ;-)

Sorry for the noise of my previous patch. I think this issue can be solved by just increasing the timeout value.

I will make more tests to confirm and post a new patch.

Please make it proper and use some real timeout value (measured in us) instead of this random loop counter.

Signature · 9 lines
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/20120116/c9645063/attachment.sig>
#5 lore raw

PATCH 1/1 ARM: clock-mx28: Fix end of loop condition

Quoted 11 lines · Fabio Estevam
Quoted 7 lines · Marek Vasut
Quoted 1 line

I see ... ignore my previous comment ;-)

Sorry for the noise of my previous patch. I think this issue can be solved by just increasing the timeout value.

I will make more tests to confirm and post a new patch.

Please make it proper and use some real timeout value (measured in us) instead of this random loop counter.

Good point

reviewer
#6 lore raw

PATCH 1/1 ARM: clock-mx28: Fix end of loop condition

In reply to Fabio Estevam
Quoted 1 line · Fabio Estevam

I will make more tests to confirm and post a new patch.

On my tests I am always getting the BUSY bit of HW_CLKCTRL_SAIF0 as 1, no matter the timeout I add.

BUSY bit is bit 29 of this register and matches the mx28 reference manual description.

Looking at the Freescale kernel this BUSY bit is defined as 1 instead.

If I do the change below:

(quoted diff) +0 -0

b/arch/arm/mach-mxs/regs-clkctrl-mx28.h
index 7d1b061..1813ff0 100644

arch/arm/mach-mxs/regs-clkctrl-mx28.h +2 -2
@@ -288,7 +288,7 @@
 
 #define BP_CLKCTRL_SAIF0_CLKGATE	31
 #define BM_CLKCTRL_SAIF0_CLKGATE	0x80000000
-#define BM_CLKCTRL_SAIF0_BUSY	0x20000000
+#define BM_CLKCTRL_SAIF0_BUSY		1
 #define BM_CLKCTRL_SAIF0_DIV_FRAC_EN	0x00010000
 #define BP_CLKCTRL_SAIF0_DIV	0
 #define BM_CLKCTRL_SAIF0_DIV	0x0000FFFF

@@ -299,7 +299,7 @@
 
 #define BP_CLKCTRL_SAIF1_CLKGATE	31
 #define BM_CLKCTRL_SAIF1_CLKGATE	0x80000000
-#define BM_CLKCTRL_SAIF1_BUSY	0x20000000
+#define BM_CLKCTRL_SAIF1_BUSY		1
 #define BM_CLKCTRL_SAIF1_DIV_FRAC_EN	0x00010000
 #define BP_CLKCTRL_SAIF1_DIV	0
 #define BM_CLKCTRL_SAIF1_DIV	0x0000FFFF

,then audio can correctly work.

Will clarify with Freescale team about this.

I plan to also send a patch for replacing the for loops with a proper timeout mechanism as suggested by Wolfram.

Regards,

Fabio Estevam

reviewer
#7 lore raw

PATCH 1/1 ARM: clock-mx28: Fix end of loop condition

Quoted 11 lines · Fabio Estevam
Quoted 1 line · Fabio Estevam

I will make more tests to confirm and post a new patch.

On my tests I am always getting the BUSY bit of HW_CLKCTRL_SAIF0 as 1, no matter the timeout I add.

BUSY bit is bit 29 of this register and matches the mx28 reference manual description.

Looking at the Freescale kernel this BUSY bit is defined as 1 instead.

Not really, I looked at the wrong register.

The issue can be solved by clearing CLKGATE bit prior to writing to DIV field, as described in the mx28 reference manual.

Just sent the updated patch that allows audio to work on mx28.

Regards,

Fabio Estevam

#8 lore raw

PATCH 1/1 ARM: clock-mx28: Fix end of loop condition

In reply to Fabio Estevam
Quoted 9 lines

From: Fabio Estevam <[email protected]>

Selecting audio on a mx28evk the saif driver was failing:

[    0.660000] saif0_clk_set_rate: divider writing timeout
[    0.670000] mxs-sgtl5000: probe of mxs-sgtl5000.0 failed with error -110
[    0.670000] ALSA device list:
[    0.680000]   No soundcards found.
Quoted 7 lines

The timeout on saif0_clk_set_rate was due to the wrong condition for the end of loop.

After fixing it, the audio driver can be correctly probed and becomes functional.

While at it, fix the other locations where the similar issue occurs.

What? The patch doesn't make sense.

arch/arm/mach-mxs/clock-mx28.c static int name##_set_rate(struct clk *clk, unsigned · line 396

Quoted 12 lines
Signed-off-by Fabio Estevam <[email protected]>
---
 arch/arm/mach-mxs/clock-mx28.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
(quoted diff) +0 -0

b/arch/arm/mach-mxs/clock-mx28.c index 5d68e41..ee9e04b 100644

arch/arm/mach-mxs/clock-mx28.c +0 -0
@@ -396,7 +396,7 @@ static int name##_set_rate(struct clk *clk, unsigned
 long rate)		\ }

\

arch/arm/mach-mxs/clock-mx28.c static int name##_set_rate(struct clk *clk, unsigned · line 439

Quoted 9 lines
arch/arm/mach-mxs/clock-mx28.c +1 -1
 	__raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_##dr);		\
 									\
-	for (i = 10000; i; i--)						\
+	for (i = 10000; i < 0; i--)					\
 		if (!(__raw_readl(CLKCTRL_BASE_ADDR +			\
 			HW_CLKCTRL_##dr) & bm_busy))			\
 			break;						\

@@ -439,7 +439,7 @@ static int name##_set_rate(struct clk *clk, unsigned
 long rate)		\ }

\

arch/arm/mach-mxs/clock-mx28.c static int name##_set_rate(struct clk *clk, unsigned · line 479

Quoted 9 lines
arch/arm/mach-mxs/clock-mx28.c +1 -1
 	__raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_##dr);		\
 									\
-	for (i = 10000; i; i--)						\
+	for (i = 10000; i < 0; i--)					\
 		if (!(__raw_readl(CLKCTRL_BASE_ADDR +			\
 			HW_CLKCTRL_##dr) & BM_CLKCTRL_##dr##_BUSY))	\
 			break;						\

@@ -479,7 +479,7 @@ static int name##_set_rate(struct clk *clk, unsigned
 long rate)		\ reg |= div << BP_CLKCTRL_##rs##_DIV;

\

Quoted 16 lines
arch/arm/mach-mxs/clock-mx28.c +2 -2
 	__raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs);		\
 									\
-	for (i = 10000; i; i--)						\
+	for (i = 10000; i < 0; i--)					\
 		if (!(__raw_readl(CLKCTRL_BASE_ADDR +			\
 			HW_CLKCTRL_##rs) & BM_CLKCTRL_##rs##_BUSY))	\
 			break;						\

@@ -756,7 +756,7 @@ static int clk_misc_init(void)
 	reg |= 3 << BP_CLKCTRL_HBUS_DIV;
 	__raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_HBUS);
 
-	for (i = 10000; i; i--)
+	for (i = 10000; i < 0; i--)
 		if (!(__raw_readl(CLKCTRL_BASE_ADDR +
 			HW_CLKCTRL_HBUS) & BM_CLKCTRL_HBUS_ASM_BUSY))
 			break;