[PATCH] ARM: clock-mx28: Fix end of loop condition

[PATCH] ARM: clock-mx28: Fix end of loop condition

[Fabio Estevam]

From: Fabio Estevam <festevam@gmail.com>

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 <fabio.estevam@freescale.com>
---
 arch/arm/mach-mxs/clock-mx28.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
index 5d68e41..ee9e04b 100644
--- a/arch/arm/mach-mxs/clock-mx28.c
+++ b/arch/arm/mach-mxs/clock-mx28.c
@@ -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;
-- 
1.7.1

[PATCH] ARM: clock-mx28: Fix end of loop condition

[Fabio Estevam]

On Mon, Jan 16, 2012 at 5:01 PM, Fabio Estevam
<fabio.estevam@freescale.com> wrote:
> From: Fabio Estevam <festevam@gmail.com>
>
> 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 <fabio.estevam@freescale.com>

Please disregard this version.

[PATCH] ARM: clock-mx28: Fix end of loop condition

[Marek Vasut]

> From: Fabio Estevam <festevam@gmail.com>
> 
> 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.

What? The patch doesn't make sense.

> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  arch/arm/mach-mxs/clock-mx28.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-mxs/clock-mx28.c
> b/arch/arm/mach-mxs/clock-mx28.c index 5d68e41..ee9e04b 100644
> --- a/arch/arm/mach-mxs/clock-mx28.c
> +++ b/arch/arm/mach-mxs/clock-mx28.c
> @@ -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;

[PATCH] ARM: clock-mx28: Fix end of loop condition

[Marek Vasut]

> On Mon, Jan 16, 2012 at 5:01 PM, Fabio Estevam
> 
> <fabio.estevam@freescale.com> wrote:
> > From: Fabio Estevam <festevam@gmail.com>
> > 
> > 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 <fabio.estevam@freescale.com>
> 
> Please disregard this version.

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

M

[PATCH] ARM: clock-mx28: Fix end of loop condition

[Fabio Estevam]

On Mon, Jan 16, 2012 at 6:52 PM, Marek Vasut <marek.vasut@gmail.com> wrote:

> 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

[PATCH] ARM: clock-mx28: Fix end of loop condition

[Wolfram Sang]

On Mon, Jan 16, 2012 at 06:55:04PM -0200, Fabio Estevam wrote:
> On Mon, Jan 16, 2012 at 6:52 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> 
> > 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.

-- 
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>

[PATCH] ARM: clock-mx28: Fix end of loop condition

[Marek Vasut]

> On Mon, Jan 16, 2012 at 06:55:04PM -0200, Fabio Estevam wrote:
> > On Mon, Jan 16, 2012 at 6:52 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > > 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

[PATCH] ARM: clock-mx28: Fix end of loop condition

[Fabio Estevam]

On 1/16/12, Fabio Estevam <festevam@gmail.com> wrote:

> 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:

diff --git a/arch/arm/mach-mxs/regs-clkctrl-mx28.h
b/arch/arm/mach-mxs/regs-clkctrl-mx28.h
index 7d1b061..1813ff0 100644
--- a/arch/arm/mach-mxs/regs-clkctrl-mx28.h
+++ b/arch/arm/mach-mxs/regs-clkctrl-mx28.h
@@ -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

[PATCH] ARM: clock-mx28: Fix end of loop condition

[Fabio Estevam]

On 1/17/12, Fabio Estevam <festevam@gmail.com> wrote:
> On 1/16/12, Fabio Estevam <festevam@gmail.com> wrote:
>
>> 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