Re: Odd behavior with dpll4_m4x2_ck on omap3 + DT

[Stefan Roese <stefan.roese@gmail.com>]

On 11.09.2013 09:21, Tomi Valkeinen wrote:
> On 10/09/13 16:17, Tero Kristo wrote:
> 
>> In theory, DPLLs can also be used in their bypass mode to feed customer
>> nodes clocks. I just think the check in the clkoutx2_recalc is wrong,
>> and should be enhanced to actually check what is the target mode for the
>> clock once it is enabled. Maybe something like this would work properly:
>>
>> diff --git a/arch/arm/mach-omap2/dpll3xxx.c
>> b/arch/arm/mach-omap2/dpll3xxx.c
>> index 3a0296c..ba218fb 100644
>> --- a/arch/arm/mach-omap2/dpll3xxx.c
>> +++ b/arch/arm/mach-omap2/dpll3xxx.c
>> @@ -658,14 +658,12 @@ unsigned long omap3_clkoutx2_recalc(struct clk_hw
>> *hw,
>>
>>         dd = pclk->dpll_data;
>>
>> -       WARN_ON(!dd->enable_mask);
>> -
>> -       v = __raw_readl(dd->control_reg) & dd->enable_mask;
>> -       v >>= __ffs(dd->enable_mask);
>> -       if ((v != OMAP3XXX_EN_DPLL_LOCKED) || (dd->flags & DPLL_J_TYPE))
>> +       if ((dd->flags & DPLL_J_TYPE) ||
>> +           __clk_get_rate(dd->clk_bypass) == __clk_get_rate(pclk))
>>                 rate = parent_rate;
>>         else
>>                 rate = parent_rate * 2;
>> +
>>         return rate;
>>  }
> 
> Stefan, are you able to test the above?
> 
> I'd rather have a proper fix for this, than hack omapdss =).

Okay, I finally found some time to test this. The patch above generates
this warning:

arch/arm/mach-omap2/dpll3xxx.c: In function 'omap3_clkoutx2_recalc':
arch/arm/mach-omap2/dpll3xxx.c:663:6: warning: passing argument 1 of '__clk_get_rate' from incompatible pointer type [enabled by default]
include/linux/clk-provider.h:423:15: note: expected 'struct clk *' but argument is of type 'struct clk_hw_omap *'

I then changed it (not 100% sure if correctly) to this version:

+       if ((dd->flags & DPLL_J_TYPE) ||
+           __clk_get_rate(dd->clk_bypass) == __clk_get_rate(pclk->hw.clk))

And this seems to work. At least the clock rate mismatch warning does not
appear with this patch applied (and without the clk_enable) in the
bootlog any more.

Thanks,
Stefan