On 13/09/13 14:34, Kristo, Tero wrote:
> On 09/13/2013 10:51 AM, Stefan Roese wrote:
>> 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 *'
> 
> Yea sorry about that, I just quickly hacked the patch together without 
> testing it at all. :P
> 
>>
>> 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.
> 
> Yea, looks good to me, however I guess I would like second opinion on 
> this also.

Tero, can you queue this patch? Or who should handle this?

 Tomi


Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to