Re: [RFC v2 2/5] dt-bindings: clk: add document for ralink clock driver

[Stephen Boyd <sboyd@kernel.org>]

Quoting NOGUCHI Hiroshi (2019-04-04 17:01:26)
> Signed-off-by: NOGUCHI Hiroshi <drvlabo@gmail.com>
> ---
>  .../bindings/clock/ralink,rt2880-clock.txt    | 58 +++++++++++++++++++
>  1 file changed, 58 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt b/Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt
> new file mode 100644
> index 000000000000..2fc0d622e01e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ralink,rt2880-clock.txt
> @@ -0,0 +1,58 @@
> +* Clock bindings for Ralink/Mediatek MIPS based SoCs
> +
> +Required properties:
> + - compatible: must be "ralink,rt2880-clock"
> + - #clock-cells: must be 1
> + - ralink,sysctl: a phandle to a ralink syscon register region
> + - clock-indices: identifying number.
> +       These must correspond to the bit number in CLKCFG1.

These look like driver level details that we're putting in the DT so we
can compress the number space that consumers use. Is that right? If so,
I don't get it. Can we not use this property?

> +       Clock consumers use one of them as #clock-cells index.
> + - clock-output-names: array of gating clocks' names
> + - clocks: array of phandles which points the parent clock
> +       for gating clocks.
> +       If gating clock does not need parent clock linkage,
> +       we bind to dummy clock whose frequency is zero.
> +
> +
> +Example:
> +
> +/* dummy parent clock node */
> +dummy_ck: dummy_ck {
> +       #clock-cells = <0>;
> +       compatible = "fixed-clock";
> +       clock-frequency = <0>;
> +};

Would this ever exist in practice? If not, please remove from the
example so we don't get the wrong idea.

> +
> +clkctrl: clkctrl {
> +       compatible = "ralink,rt2880-clock";
> +       #clock-cells = <1>;
> +       ralink,sysctl = <&sysc>;
> +
> +       clock-indices =
> +                       <12>,
> +                       <16>, <17>, <18>, <19>,
> +                       <20>,
> +                       <26>;
> +       clock-output-names =
> +                       "uart0",
> +                       "i2c", "i2s", "spi", "uart1",
> +                       "uart2",
> +                       "pcie0";
> +       clocks =
> +                       <&pll MT7620_CLK_PERIPH>,
> +                       <&pll MT7620_CLK_PERIPH>, <&pll MT7620_CLK_PCMI2S>, <&pll MT7620_CLK_SYS>, <&pll MT7620_CLK_PERIPH>,
> +                       <&pll MT7620_CLK_PERIPH>,
> +                       <&dummy_ck>;
> +       };
> +};
> +
> +/* consumer which refers "uart0" clock */
> +uart0: uartlite@c00 {
> +       compatible = "ns16550a";
> +       reg = <0xc00 0x100>;
> +
> +       clocks = <&clkctrl 12>;

So 12 matches in indices and then that is really "uart0" clk?

> +       clock-names = "uart0";
> +