+ Tero
On 12/16/2014 12:01 PM, Stephen Boyd wrote:
On 12/15/2014 05:31 PM, Paul Walmsley wrote:
I just took a quick glance at Tero's second patch, and it looks like a
hack to me. Better to fix the problem in the core CCF code if
possible. I don't think there's any reason why a PLL couldn't have
just one parent clock. But I'm fine with merging it as a short-term
fix if fixing the core code is difficult or risky.
Can you describe what's wrong?
Took a closer look at it, at your prompting.
The first observation is that the issue seems to be limited to
TI-specific clock code in drivers/clk/ti. So nothing to worry about in
terms of the CCF core, it looks.
The second observation is that this appears to only be a problem due to
the current DT data in arch/arm/boot/dts/omap3xxx-clocks.dtsi:
dpll3_ck: dpll3_ck {
#clock-cells = <0>;
compatible = "ti,omap3-dpll-core-clock";
clocks = <&sys_ck>, <&sys_ck>;
reg = <0x0d00>, <0x0d20>, <0x0d40>, <0x0d30>;
};
It lists two entries for the "clocks" node for some reason.
of_ti_dpll_setup() only seems to require one.
So unless there's some good reason to list two of them in the DTS file,
the right fix would probably have been to fix the DT data.
Does the PLL have a mux with two inputs that map to the same clock?
I don't think so.
...
Generally speaking, I suspect the way we model PLLs isn't quite right
(although it's been a while since I've looked at that particular code).
This was probably my fault, in the final analysis, for taking a shortcut
here a long time ago.
Unless one wishes to implement support for multiple clock parents, it's
probably not quite right to state that a PLL's reference clock is a
"parent" in a clock tree sense. When a PLL is active, the output clock
originates from a VCO of some kind that is internal to the PLL. The
reference clock is just used by the PLL internal logic to close the
control loop. Some PLL-like clock sources use a third clock that is
used to clock some of the PLL's internal logic (let's call this a
"functional clock.")
So the reference clock and functional clock are (usually) required by
the PLL to operate, and should therefore be required by the PLL clock
driver code in the kernel; but one could claim that they aren't
technically parent clocks of the PLL in a clock tree sense, since the
downstream output clock isn't directly derived from either of those clocks.
Otherwise if we want to represent those clocks accurately in the Linux
clock tree, we probably need to state that clocks can have multiple
"parents" that often need to be active simultaneously, and we should
propagate usecount changes accordingly up all of the parent paths.
...
Anyway, thanks for following up on this,
- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
