Re: [PATCH] dt-bindings: clock: ti,da850-pll: Convert to DT schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The device tree schema example contains undefined phandle references (`&ref_clk` and `&pll1_sysclk`), which causes `dt_binding_check` to fail with compilation errors.
- [Medium] The schema fails to enforce strict bounds on `clocks` and `clock-names` inside conditional blocks, allowing invalid configurations to pass validation.
--

--- Patch [1]: [PATCH] dt-bindings: clock: ti,da850-pll: Convert to DT schema ---
commit d9fe4826c7bdb546d8ecd3e9ca8ac0454b95166c
Author: Bhargav Joshi <j.bhargav.u@gmail.com>

dt-bindings: clock: ti,da850-pll: Convert to DT schema
    
Convert TI da850-pll binding from legacy text to dt schema.
No functional change is introduced during conversion.

> diff --git a/Documentation/devicetree/bindings/clock/ti/davinci/ti,da850-pll.yaml b/Documentation/devicetree/bindings/clock/ti/davinci/ti,da850-pll.yaml
> new file mode 100644
> index 0000000000000..5e4109567b75d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ti/davinci/ti,da850-pll.yaml

[ ... ]

> @@ -0,0 +1,177 @@

[ ... ]

> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: ti,da850-pll0
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Primary reference clock input
> +            - description: External clock source

[Severity: Medium]
Since the global clocks property specifies minItems: 1 and maxItems: 2, does
this ti,da850-pll0 block also need minItems: 2?

Without it, a device tree node with only 1 clock could silently pass validation
by falling back to the global limits.

> +        clock-names:
> +          items:
> +            - const: clksrc
> +            - const: extclksrc
> +
> +  - if:
> +      properties:
> +        compatible:
> +          const: ti,da850-pll1
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 1
> +        clock-names:
> +          items:
> +            - const: clksrc

[Severity: Medium]
Does this block need maxItems: 1 for clock-names?

Since the global clock-names property allows maxItems: 2, a node with two
clock-names might pass validation for the ti,da850-pll1 variant.

> +        ti,clkmode-square-wave: false
> +        auxclk: false
> +
> +examples:
> +  - |
> +    clock-controller@11000 {
> +        compatible = "ti,da850-pll0";
> +        reg = <0x11000 0x1000>;
> +        clocks = <&ref_clk>, <&pll1_sysclk 3>;

[Severity: Medium]
Will this cause make dt_binding_check to fail?

The &ref_clk and &pll1_sysclk phandles are not defined in this example, which
usually causes dtc compilation errors (Reference to non-existent node or label)
during validation workflows.

> +        clock-names = "clksrc", "extclksrc";
> +        ti,clkmode-square-wave;

[ ... ]

> +  - |
> +    clock-controller@21a000 {
> +        compatible = "ti,da850-pll1";
> +        reg = <0x21a000 0x1000>;
> +        clocks = <&ref_clk>;

[Severity: Medium]
Similar to the first example, this references &ref_clk without defining it.
Could this lead to a dtc compilation error during make dt_binding_check?

> +        clock-names = "clksrc";
> +
> +        sysclk {
> +            #clock-cells = <1>;
> +        };
> +
> +        obsclk {
> +            #clock-cells = <0>;
> +        };
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-ti-pll-v1-1-f6f9ba8c9b5e@gmail.com?part=1