Re: [PATCH 2/5] dt-bindings: clock: altr,agilex72-clkmgr: add Agilex72 clock manager binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The clock manager binding omits the `clocks` and `clock-names` properties, failing to properly model external hardware inputs.
--

--- Patch [2]: [PATCH 2/5] dt-bindings: clock: altr,agilex72-clkmgr: add Agilex72 clock manager binding ---
commit 99e14dc36681e2c2df411ac60959380ed8376f54
Author: Chiam, Evon <evon.chiam@altera.com>

dt-bindings: clock: altr,agilex72-clkmgr: add Agilex72 clock manager binding

Adds the device tree binding and clock ID header for the Altera SoCFPGA
Agilex72 clock manager.

> diff --git a/Documentation/devicetree/bindings/clock/altr,agilex72-clkmgr.yaml b/Documentation/devicetree/bindings/clock/altr,agilex72-clkmgr.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/altr,agilex72-clkmgr.yaml

[ ... ]

> +properties:
> +  compatible:
> +    const: altr,agilex72-clkmgr
> +
> +  reg:

[Severity: Medium]
Should this properties block declare the clocks and clock-names properties?

Because additionalProperties is set to false, any device tree attempting to
properly link the external reference clocks to the clkmgr node via standard
phandles will be rejected by dt_binding_check.

[ ... ]

> +examples:
> +  - |
> +    /* External reference clocks supplied to the clock manager */
> +    clocks {
> +      cb_intosc_div2_clk: cb-intosc-div2-clk {
> +        #clock-cells = <0>;

[ ... ]

> +    clkmgr: clock-controller@915c000 {
> +      compatible = "altr,agilex72-clkmgr";
> +      reg = <0x915c000 0x1000>,
> +            <0x915e000 0x1000>,
> +            <0x915f000 0x1000>,
> +            <0x9160000 0x1000>;
> +      reg-names = "clkmgr", "gppll0", "gppll1", "gppll2";
> +      #clock-cells = <1>;
> +    };
> +...

[Severity: Medium]
Does this example accurately reflect the hardware topology?

The external reference clocks are defined just above this node, but the
clkmgr node does not consume them. While older bindings relied on global
fixed-clock lookups by name, newer schemas typically declare their
external clock inputs to support accurate topological representation and
potential parent clock power management.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1786006669.git.evon.chiam@altera.com?part=2