Re: [PATCH] dt-bindings: clock: sprd,sc9860-clk: convert to YAML

[Krzysztof Kozlowski <krzk@kernel.org>]

On 03/07/2024 15:32, Stanislav Jakubek wrote:
> Convert the Spreadtrum SC9860 clock bindings to DT schema.
> 
> Signed-off-by: Stanislav Jakubek <stano.jakubek@gmail.com>
> ---
>  .../bindings/clock/sprd,sc9860-clk.txt        | 63 -------------
>  .../bindings/clock/sprd,sc9860-clk.yaml       | 90 +++++++++++++++++++
>  2 files changed, 90 insertions(+), 63 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/clock/sprd,sc9860-clk.txt
>  create mode 100644 Documentation/devicetree/bindings/clock/sprd,sc9860-clk.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/sprd,sc9860-clk.txt b/Documentation/devicetree/bindings/clock/sprd,sc9860-clk.txt
> deleted file mode 100644
> index aaaf02ca2a6a..000000000000
> --- a/Documentation/devicetree/bindings/clock/sprd,sc9860-clk.txt
> +++ /dev/null
> @@ -1,63 +0,0 @@
> -Spreadtrum SC9860 Clock Binding
> -------------------------
> -
> -Required properties:
> -- compatible: should contain the following compatible strings:
> -	- "sprd,sc9860-pmu-gate"
> -	- "sprd,sc9860-pll"
> -	- "sprd,sc9860-ap-clk"
> -	- "sprd,sc9860-aon-prediv"
> -	- "sprd,sc9860-apahb-gate"
> -	- "sprd,sc9860-aon-gate"
> -	- "sprd,sc9860-aonsecure-clk"
> -	- "sprd,sc9860-agcp-gate"
> -	- "sprd,sc9860-gpu-clk"
> -	- "sprd,sc9860-vsp-clk"
> -	- "sprd,sc9860-vsp-gate"
> -	- "sprd,sc9860-cam-clk"
> -	- "sprd,sc9860-cam-gate"
> -	- "sprd,sc9860-disp-clk"
> -	- "sprd,sc9860-disp-gate"
> -	- "sprd,sc9860-apapb-gate"
> -
> -- #clock-cells: must be 1
> -
> -- clocks : Should be the input parent clock(s) phandle for the clock, this
> -	   property here just simply shows which clock group the clocks'
> -	   parents are in, since each clk node would represent many clocks
> -	   which are defined in the driver.  The detailed dependency
> -	   relationship (i.e. how many parents and which are the parents)
> -	   are implemented in driver code.
> -
> -Optional properties:
> -
> -- reg:	Contain the registers base address and length. It must be configured
> -	only if no 'sprd,syscon' under the node.
> -
> -- sprd,syscon: phandle to the syscon which is in the same address area with
> -	       the clock, and so we can get regmap for the clocks from the
> -	       syscon device.
> -
> -Example:
> -
> -	pmu_gate: pmu-gate {
> -		compatible = "sprd,sc9860-pmu-gate";
> -		sprd,syscon = <&pmu_regs>;
> -		clocks = <&ext_26m>;
> -		#clock-cells = <1>;
> -	};
> -
> -	pll: pll {
> -		compatible = "sprd,sc9860-pll";
> -		sprd,syscon = <&ana_regs>;
> -		clocks = <&pmu_gate 0>;
> -		#clock-cells = <1>;
> -	};
> -
> -	ap_clk: clock-controller@20000000 {
> -		compatible = "sprd,sc9860-ap-clk";
> -		reg = <0 0x20000000 0 0x400>;
> -		clocks = <&ext_26m>, <&pll 0>,
> -			 <&pmu_gate 0>;
> -		#clock-cells = <1>;
> -	};
> diff --git a/Documentation/devicetree/bindings/clock/sprd,sc9860-clk.yaml b/Documentation/devicetree/bindings/clock/sprd,sc9860-clk.yaml
> new file mode 100644
> index 000000000000..21ed023a928c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/sprd,sc9860-clk.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/sprd,sc9860-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Spreadtrum SC9860 clock
> +
> +maintainers:
> +  - Orson Zhai <orsonzhai@gmail.com>
> +  - Baolin Wang <baolin.wang7@gmail.com>
> +  - Chunyan Zhang <zhang.lyra@gmail.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - sprd,sc9860-agcp-gate
> +      - sprd,sc9860-aonsecure-clk
> +      - sprd,sc9860-aon-gate
> +      - sprd,sc9860-aon-prediv
> +      - sprd,sc9860-apahb-gate
> +      - sprd,sc9860-apapb-gate
> +      - sprd,sc9860-ap-clk
> +      - sprd,sc9860-cam-clk
> +      - sprd,sc9860-cam-gate
> +      - sprd,sc9860-disp-clk
> +      - sprd,sc9860-disp-gate
> +      - sprd,sc9860-gpu-clk
> +      - sprd,sc9860-pll
> +      - sprd,sc9860-pmu-gate
> +      - sprd,sc9860-vsp-clk
> +      - sprd,sc9860-vsp-gate
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 1
> +    maxItems: 3
> +    description:
> +      Should be the input parent clock(s) phandle for the clock, this
> +      property just simply shows which clock group the clocks' parents
> +      are in, since each clk node would represent many clocks which are
> +      defined in the driver. The detailed dependency relationship
> +      (i.e. how many parents and which are the parents) are implemented
> +      in driver code.

Description is redundant and not accurate. Which clocks are needed
cannot be defined by the driver, but by binding.

This should be constrained per variant and each item explicitly listed.

> +
> +  '#clock-cells':
> +    const: 1
> +
> +  sprd,syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      phandle to the syscon which is in the same address area with the
> +      clock, and so we can get regmap for the clocks from the syscon device
> +
> +oneOf:
> +  - required:
> +      - reg
> +  - required:
> +      - sprd,syscon
> +
> +required:
> +  - compatible
> +  - clocks
> +  - '#clock-cells'

allOf:if:then:, instead above oneOf, so you narrow which variant has reg
and which uses regmap.


Best regards,
Krzysztof