Re: [PATCH RFC v5 01/12] dt-bindings: soc: zte: Add zx297520v3 top clock and reset bindings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `syscon-reboot` node example omits the `value` property and uses `mask` alone, triggering a deprecated legacy fallback in the driver that overwrites the entire register.
- [Low] The child node is explicitly named `syscon-reboot`, violating the Devicetree Specification for generic node names.
--

commit fc119f64e363e1f9ec8efce39f051382b3c78be5
Author: Stefan Dösinger <stefandoesinger@gmail.com>

dt-bindings: soc: zte: Add zx297520v3 top clock and reset bindings

This commit introduces device tree bindings for the Zte zx297520v3 SoC top
clock and reset controller. It defines the yaml schema for the controller,
including clocks, resets, and a syscon-reboot child node. It also adds the
corresponding clock and reset header definitions.

> diff --git a/Documentation/devicetree/bindings/soc/zte/zte,zx297520v3-topcrm.yaml b/Documentation/devicetree/bindings/soc/zte/zte,zx297520v3-topcrm.yaml
> new file mode 100644
> index 0000000000000..6baab5c98d25e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/zte/zte,zx297520v3-topcrm.yaml
> @@ -0,0 +1,85 @@
[ ... ]
> +  "#reset-cells":
> +    const: 1
> +
> +  syscon-reboot:

[Severity: Low]
Does this child node name violate the Devicetree Specification for generic
node names? By explicitly restricting the node name to syscon-reboot,
validating a device tree that uses the standard generic name reboot will
fail since additional properties are not allowed. Could this be renamed to
match the generic class of the device?

> +    type: object
> +    $ref: /schemas/power/reset/syscon-reboot.yaml#
> +    description:
> +      Reboot method for the SoC.
> +
> +required:
[ ... ]
> +        #clock-cells = <1>;
> +        #reset-cells = <1>;
> +
> +        syscon-reboot {
> +          compatible = "syscon-reboot";
> +          offset = <0x0>;
> +          mask = <0x1>;

[Severity: High]
Is it intentional to omit the value property in this example?

In drivers/power/reset/syscon-reboot.c, if the value property is omitted,
the driver interprets mask as the legacy value and forces the actual update
mask to 0xffffffff. When a reboot is requested, it executes
regmap_update_bits(map, 0x0, 0xffffffff, 0x1), which clears all bits except
bit 0.

Since the topcrm register at offset 0x0 controls core clocks and resets,
would overwriting the entire register hang the system ungracefully before
the reboot can proceed? Adding the value property would ensure the mask
operates as intended.

> +        };
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628-zx29clk-v5-0-79ff044e4192@gmail.com?part=1