RFC v2: Zynq Clock Controller

[Sören Brinkmann]

On Mon, Mar 25, 2013 at 05:29:33PM -0600, Stephen Warren wrote:
> On 03/22/2013 04:41 PM, Sören Brinkmann wrote:
> > Hi Lars,
> > 
> > On Thu, Mar 21, 2013 at 07:32:52PM +0100, Lars-Peter Clausen wrote:
> >> On 03/21/2013 12:56 AM, Sören Brinkmann wrote:
> >>> Hi,
> >>>
> >>> I spent some time working on this and incorporating feedback. Here's an updated proposal for a clock controller for Zynq:
> >>>
> >>> Required properties:
> >>>  - #clock-cells : Must be 1
> >>>  - compatible : "xlnx,ps7-clkc"  (this may become 'xlnx,zynq-clkc' terminology differs a bit between Xilinx internal and mainline)
> >>>  - ps-clk-frequency : Frequency of the oscillator providing ps_clk in HZ
> >>>                      (usually 33 MHz oscillators are used for Zynq platforms)
> >>>  - clock-output-names : List of strings used to name the clock outputs. Shall be a list of the outputs given below.
> >>>
> >>> Optional properties:
> >>>  - clocks : as described in the clock bindings
> >>>  - clock-names : as described in the clock bindings
> >>>
> >>> Clock inputs:
> >>> The following strings are optional parameters to the 'clock-names' property in
> >>> order to provide optional (E)MIO clock sources.
> >>>  - swdt_ext_clk
> >>>  - gem0_emio_clk
> >>>  - gem1_emio_clk
> >>>  - mio_clk_XX          # with XX = 00..53
> >>>
> >>> Example:
> >>>         clkc: clkc {
> >>>                 #clock-cells = <1>;
> >>>                 compatible = "xlnx,ps7-clkc";
> >>>                 ps-clk-frequency = <33333333>;
> >>
> >> The input frequency should be a clock as well.
> >
> > Again, monolithic vs split. I don't see a reason not to just internally
> > call clk_register_fixed_rate(). That way its children do not have to
> > cope with a variable name for the xtal.
> > Also, with my proposal 'clocks' and 'clock-names' would be purely
> > optional properties, only required if optional external inputs are
> > present. Having the xtal defined externally would add mandatory entries for
> > those props.
> 
> But isn't the clock source board-specific? It's a completely separate
> object from Zynq's own clock controller HW, and as such should be
> represented by a separate DT node, right?
Well, the ps-clk-frequency property would be board specific right. But
the same would apply for a fixed-rate clock. This is how it's currently
done in mainline. The zynq dtsi file defines the fixed-rate clock and
every board dts file overrides the fixed rate clock's frequency
property.
In my approach the fixed rate clock is within the clkc block and every
board dts has to provide this property accordingly.


> 
> The issue with parent clock names is simply a red herring. A solution is
> needed to registered clock with a parent clock object, rather than a
> parent clock name. Then, the parent names are completely irrelevant.
But we'd trade the naming problems with issues regarding the order of
how clocks are probed.
And it may not be as easy as just probe from the root towards the leafs
of the clock tree.
In Zynq I can route clocks generated in the processor part into the FPGA
and back using them as an input for the clock-controller. Ant the IP in
the FPGA could add another layer of clock providers in that loop.
With such circular dependencies a clock object based
registration/probing does not seem too easy to me.

	Sören