[RFC PATCH 1/1] dt-bindings: clk: Mstar msc313 clkgen mux

[Stephen Boyd]

Quoting Daniel Palmer (2021-02-12 03:16:49)
> Add a devicetree binding yaml for the clkgen muxes used
> in various places in MStar/SigmaStar SoCs.
> 
> Signed-off-by: Daniel Palmer <daniel at 0x0f.com>
> ---

Don't need a cover letter for a single patch.

>  .../clock/mstar,msc313-clkgen-mux.yaml        | 87 +++++++++++++++++++
>  1 file changed, 87 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/mstar,msc313-clkgen-mux.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/mstar,msc313-clkgen-mux.yaml b/Documentation/devicetree/bindings/clock/mstar,msc313-clkgen-mux.yaml
> new file mode 100644
> index 000000000000..7f2ff72a601f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/mstar,msc313-clkgen-mux.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/mstar,msc313-clkgen-mux.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MStar/Sigmastar MSC313 CLKGEN mux
> +
> +maintainers:
> +  - Daniel Palmer <daniel at thingy.jp>
> +
> +description: |
> +  The MStar/SigmaStar MSC313 and later ARMv7 chips has a number of
> +  "CLKGEN"s. Some in the pm area, some in the standard peripheral area
> +  and some in the "scaler" area. Inside of these CLKGENs there are
> +  muxes that either connect the output to an always on clock
> +  (deglitch) or one of a number of clocks that are selectable via
> +  a mux. Each of these muxes also includes a gate for the output.
> +  Most of these are in a nice block with multiple muxes in a single
> +  register. Some of them are embedded within blocks of unrelated
> +  registers like in the pm area. Some seem to be embedded in registers
> +  with unrelated bits. For this reason a syscon is used to access the
> +  registers.
> +
> +properties:
> +  compatible:
> +    const: mstar,msc313-clkgen-mux
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  clocks:
> +    description: |
> +      List of clock sources for this mux. If the mux has a deglitch
> +      bit the last entry should be the source of the deglitch clock.
> +
> +  offset:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Offset in the register map for the mux register (in bytes).
> +
> +  regmap:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: |
> +      Phandle to the register map node.
> +
> +  mstar,gate:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Bit position for the gate bit.
> +
> +  mstar,deglitch:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Bit position of the deglitch bit for muxes that have one.
> +
> +  mstar,mux-shift:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Left shift value for the mux bits.
> +
> +  mstar,mux-width:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: The width of the mux bits.
> +
> +required:
> +  - compatible
> +  - "#clock-cells"
> +  - clocks
> +  - offset
> +  - regmap
> +  - mstar,gate
> +  - mstar,mux-shift
> +  - mstar,mux-width
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    clkgen_mux_mspi0: clkgen_mux_mspi0 {
> +      compatible = "mstar,msc313-clkgen-mux";
> +      regmap = <&clkgen>;
> +      offset = <0xcc>;
> +      #clock-cells = <1>;
> +      mstar,gate = <0>;
> +      mstar,mux-shift = <2>;
> +      mstar,mux-width = <2>;

It looks like a node-per clk sort of binding which has been rejected
multiple times in the past. If the clks are spread across various
devices then it sounds like the mediatek design where they have many
syscon nodes that also register clks inside those register spaces. In
this case, I would expect the clkgen node to be registering clks. Given
that there isn't a reg property and there's these mstar specific
properties like shift/width it looks really wrong. Please don't do this.

> +      clocks = <&clk_mpll_div2_div2>,
> +               <&clk_mpll_div2_div4>,
> +               <&xtal12>;
> +    };
> --