On Tue, 10 Sep 2013, 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;
>  }
> 

[...]

> Getting comment from someone like Paul would probably help here.

Looks like this is a regression from the CCF port.

Seems to me that the code above assumes that when the DPLL's rate is set 
to the same rate as the bypass clock's rate, we can assume that the DPLL 
is in bypass.  I wonder if that's valid in a case where a previous OS or 
bootloader may have programmed the DPLL.

Sounds to me like the best way to fix it would be to test whether this 
code is intended to return the "target rate" (when the struct clk 
representing the DPLL is disabled), versus the "current rate" (when the 
struct clk representing the DPLL is enabled).  If it's the target rate, 
then there's no point checking the current state of the hardware.  A check 
similar to the above would probably be fine.  Otherwise, seems best to use 
the existing code that does test the PLL state.



- Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to