mx6qsabresd hangs on linux-next

Commit Message

Maxime Coquelin May 7, 2014, 8:32 a.m. UTC

Hi Shawn,

On 05/07/2014 05:32 AM, Shawn Guo wrote:
...
>>>
>>> Here when a table entry matches the input div this function will return
>>> exactly the input div. This means _next_div() will always return the
>>> same value and clk_divider_bestdiv() has an infinite loop:
>>>
>>> 	for (i = 1; i <= maxdiv; i = _next_div(divider, i)) {
>>> 		...
>>> 	}
>>
>> Hmmm, isn't the first thing that _next_div() does to increment the input
>> div?
>
> I think the infinite loop happens in this case because "i" will never
> exceed maxdiv for a table divider.

You are right. Sorry for the regression.

I thought I tested it, as I had some table-based dividers in my test setup.
After checking again, it appears that I didn't had the 
CLK_SET_RATE_PARENT flag set on these clocks, so I didn't entered the 
for loop...

Could the fix be to initialize "up" variable to INT_MAX in 
_round_up_table (see below)?

I can send the patch if you are fine with it.
I have no hardware to test on this week, I only have compiled this 
patch, not tested it.

Thanks,
Maxime

>
> Shawn
>


                 if (clkt->div == div)

Comments

Fabio Estevam May 7, 2014, 2:39 p.m. UTC | #1

Hi Maxime,

On Wed, May 7, 2014 at 5:32 AM, Maxime Coquelin <[e-mail]> wrote:

> You are right. Sorry for the regression.
>
> I thought I tested it, as I had some table-based dividers in my test setup.
> After checking again, it appears that I didn't had the CLK_SET_RATE_PARENT
> flag set on these clocks, so I didn't entered the for loop...
>
> Could the fix be to initialize "up" variable to INT_MAX in _round_up_table
> (see below)?

This works fine for me:

Tested-by: Fabio Estevam <[e-mail]>

> I can send the patch if you are fine with it.
> I have no hardware to test on this week, I only have compiled this patch,
> not tested it.

Please submit your patch, thanks.

Maxime Coquelin May 7, 2014, 4:54 p.m. UTC | #2

Hi Fabio,

On 05/07/2014 04:39 PM, Fabio Estevam wrote:
> Hi Maxime,
>
> On Wed, May 7, 2014 at 5:32 AM, Maxime Coquelin <[e-mail]> wrote:
>
>> You are right. Sorry for the regression.
>>
>> I thought I tested it, as I had some table-based dividers in my test setup.
>> After checking again, it appears that I didn't had the CLK_SET_RATE_PARENT
>> flag set on these clocks, so I didn't entered the for loop...
>>
>> Could the fix be to initialize "up" variable to INT_MAX in _round_up_table
>> (see below)?
>
> This works fine for me:
>
> Tested-by: Fabio Estevam <[e-mail]>
>
>> I can send the patch if you are fine with it.
>> I have no hardware to test on this week, I only have compiled this patch,
>> not tested it.
>
> Please submit your patch, thanks.
Thanks for testing it, patch sent.

Regards,
Maxime

>

Patch

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index b3c8396..cf9114a 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -158,7 +158,7 @@  static bool _is_valid_div(struct clk_divider 
*divider, unsigned int div)
  static int _round_up_table(const struct clk_div_table *table, int div)
  {
         const struct clk_div_table *clkt;
-       int up = _get_table_maxdiv(table);
+       int up = INT_MAX;

         for (clkt = table; clkt->div; clkt++) {