> >> +Other required properties for all clocks(all from common clock binding):
> >> +- #clock-cells : should be set to 1 as each of the SCPI clocks have multiple
> >> + outputs. The clock specifier will be the index to an entry in the list
> >> + of output clocks.
> >
> > Huh? That's somewhat a circular definition.
> >
> > What does that number correspond to in the HW? If it's just the number
> > that the FW expects, that's a reasonable definition.
> >
>
> Not exactly. The clock specifier are used by the consumers and they just
> indicate the index into the list of clock outputs provided by the clock
> provider. The consumers need not know the exact identifier used by the
> provider to identify the clock(either via H/W or F/W)
Currently the definition is circular because clock-indices is misued. If
you sort that out then this should become grounded and well-defined.
[...]
> >> +scpi_protocol: scpi@2e000000 {
> >> + compatible = "arm,scpi";
> >> + mboxes = <&mailbox 0 &mailbox 1>;
> >> + shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
> >> +
> >> + clocks {
> >> + compatible = "arm,scpi-clocks";
> >> +
> >> + scpi_dvfs: scpi_clocks@0 {
> >> + compatible = "arm,scpi-dvfs-clocks";
> >> + #clock-cells = <1>;
> >> + clock-indices = <0>, <1>, <2>;
> >> + clock-output-names = "vbig", "vlittle", "vgpu";
> >> + };
> >> + scpi_clk: scpi_clocks@3 {
> >> + compatible = "arm,scpi-variable-clocks";
> >> + #clock-cells = <1>;
> >> + clock-indices = <3>, <4>;
> >> + clock-output-names = "pxlclk0", "pxlclk1";
> >> + };
> >> + };
> >> +};
> >> +
> >> +cpu@0 {
> >> + ...
> >> + reg = <0 0>;
> >> + clocks = <&scpi_dvfs 0>;
> >> + clock-names = "vbig";
> >> +};
> >> +
> >> +hdlcd@7ff60000 {
> >> + ...
> >> + reg = <0 0x7ff60000 0 0x1000>;
> >> + clocks = <&scpi_clk 1>;
> >> + clock-names = "pxlclk";
> >> +};
> >> +
> >> +In the above example, the #clock-cells is set to 1 as required.
> >> +scpi_dvfs has 3 output clocks namely: vbig, vlittle and vgpu with 0, 1
> >> +and 2 as clock-indices. scpi_clk has 2 output clocks namely: pxlclk0 and
> >> +pxlclk1 with 3 and 4 as clock-indices.
> >> +
> >> +The first consumer in the example is cpu@0 and it has vbig as input clock.
> >> +The index '0' in the clock specifier here points to the first entry in the
> >> +output clocks of scpi_dvfs for which clock_id asrequired by the firmware
> >> +is 0.
> >> +
> >> +Similarly the second example is hdlcd@7ff60000 and it has pxlclk0 as input
> >> +clock. The index '1' in the clock specifier here points to the second entry
> >> +in the output clocks of scpi_clocks for which clock_id as required by the
> >> +firmware is 4.
> >
> > To the best of my knowledge, this is wrong. Per the example in
> > Documentation/devicetree/bindings/clock/clock-bindings.txt, the
> > clock-indices apply to the logical value in the clock-specifier.
> >
> > So <&scpi_clk 3>, <&scpi_clk 4> exist, (and are named "pxlclk0",
> > "pxlclk1" respectively), but <&scpi_clk 0>, <&scpi_clk 1> do not (or at
> > least don't have names).
> >
>
> That depends, if your clock provider provides a callback for decoding
> clock and does this translation, then they can exist.
Sure, hence the "(or at least don't have names)".
> Since SCPI is using standard/default callback(of_clk_src_onecell_get),
> only <&scpi_clk 0>, <&scpi_clk 1> in above example. For any value >=2,
> of_clk_src_onecell_get will bail out as we have only 2 clocks
> registered from that provider.
That's in violation of the semantics of clock-indices, which was added
to map from a non-contiguous set of clock-specifier values to a list of
strings. Take a look at of_clk_get_parent_name (which this won't work
with).
Also see Documentation/devicetree/bindings/clock/clock-bindings.txt
(relevant portion duplicated below):
----
clock-indices: If the identifying number for the clocks in the node
is not linear from zero, then this allows the mapping of
identifiers into the clock-output-names array.
For example, if we have two clocks <&oscillator 1> and <&oscillator 3>:
oscillator {
compatible = "myclocktype";
#clock-cells = <1>;
clock-indices = <1>, <3>;
clock-output-names = "clka", "clkb";
}
This ensures we do not have any empty strings in clock-output-names
----
Note that the indices are the clock-specifier values, not the raw HW/FW
values.
Either you should be using <&scpi_clk 3> and <&scpi_clk 4>, or you need
a different property to map your logical indices to raw HW values.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
