Re: [PATCH v8 01/12] dt-bindings: clk: zte: Add zx297520v3 top clock and reset controller

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Self-referencing cyclic dependency enforced by required `resets` property on the reset provider itself.
--

--- Patch [1]: [PATCH v8 01/12] dt-bindings: clk: zte: Add zx297520v3 top clock and reset controller ---
commit bd10e0d2ede4b7523e3aa201c9dc948ccee1e352
Author: Stefan Dösinger <[email protected]>
Subject: dt-bindings: clk: zte: Add zx297520v3 top clock and reset controller

This commit adds a new device tree binding for the ZTE zx297520v3 top clock
and reset controller. It describes the clock inputs, PLL outputs, and reset
controls for various peripherals. The binding also exposes USB and HSIC PHYs,
and a syscon-reboot method.

> diff --git a/Documentation/devicetree/bindings/clock/zte,zx297520v3-topcrm.yaml b/Documentation/devicetree/bindings/clock/zte,zx297520v3-topcrm.yaml
> new file mode 100644
> index 0000000000000..9dbca49871a0e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/zte,zx297520v3-topcrm.yaml
[ ... ]
> +  interrupt-names:
> +    items:
> +      - const: usb-up
> +      - const: usb-down
> +      - const: hsic-up
> +      - const: hsic-down
> +
> +  resets:
> +    items:
> +      - description: USB phy reset
> +      - description: HSIC phy reset
> +
> +  reset-names:
> +    items:
> +      - const: usb
> +      - const: hsic
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  "#phy-cells":
> +    const: 1
> +
> +  "#reset-cells":
> +    const: 1
> +
> +  syscon-reboot:
> +    type: object
> +    $ref: /schemas/power/reset/syscon-reboot.yaml#
> +    description:
> +      Reboot method for the SoC.
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - interrupt-names
> +  - resets
> +  - reset-names

[Severity: Medium]
Is it necessary to make the resets property required here?

Since this node is also the reset provider (indicated by #reset-cells),
requiring it to consume its own resets creates a self-referencing cyclic
dependency during device tree probing. 

Could the driver interact with its own registers directly to handle these
resets, rather than acting as a reset consumer of itself?

> +  - syscon-reboot
> +  - "#clock-cells"
> +  - "#phy-cells"
> +  - "#reset-cells"
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/reset/zte,zx297520v3-reset.h>
> +    #include <dt-bindings/phy/zte,zx297520v3-topcrm.h>
> +    #include <dt-bindings/clock/zte,zx297520v3-clk.h>
> +
> +    topcrm: clock-controller@13b000 {
> +        compatible = "zte,zx297520v3-topcrm", "syscon";
> +        reg = <0x0013b000 0x400>;
> +        clocks = <&osc26m>, <&osc32k>;
> +        clock-names = "osc26m", "osc32k";
> +        interrupts = <GIC_SPI 42 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 43 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 45 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 46 IRQ_TYPE_EDGE_RISING>;
> +        interrupt-names = "usb-up", "usb-down", "hsic-up", "hsic-down";
> +        resets = <&topcrm ZX297520V3_USB_PHY_RESET>,
> +                 <&topcrm ZX297520V3_HSIC_PHY_RESET>;

[Severity: Medium]
Does modeling the internal register operations as external consumer
dependencies artificially create the dependency cycles mentioned in the commit
message?

The example explicitly demonstrates the self-referencing cyclic dependency
back to the topcrm phandle. Typically, it is an anti-pattern for a reset
provider to model its own internal signal lines as external dependencies
pointing back to itself.

If the driver handled its own internal reset logic directly, this cyclic
dependency could be avoided without relying on fw_devlink to forcefully break
it.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1