Re: [PATCH 06/24] dt-bindings: clock: Add ZTE ZX279128S CRM clocks and resets

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt-bindings: missing minItems constraints for clocks in conditional blocks
- [Medium] dt-bindings: clock: forbids #reset-cells for LSP CRPM hardware
--

commit 3165423c65a32ccd48a20652df13ea5343f45889
Author: Navid Ghahremani <ghahramani.navid@gmail.com>

dt-bindings: clock: Add ZTE ZX279128S CRM clocks and resets

Describe the top CRM and the two low-speed peripheral clock domains,
including the confirmed USB reset bits. Uncertain clock-source rates are
called out in the cover letter.

> diff --git a/Documentation/devicetree/bindings/clock/zte,zx279128s-crm.yaml b/Documentation/devicetree/bindings/clock/zte,zx279128s-crm.yaml
> new file mode 100644
> index 0000000000000..2964c0fd6b040
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/zte,zx279128s-crm.yaml
[ ... ]
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: zte,zx279128s-topcrm
> +    then:
[ ... ]
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: zte,zx279128s-lsp0crpm
> +    then:
> +      properties:
> +        '#reset-cells': false

[Severity: Medium]
The description earlier in the schema states that the LSP modules are "clock
and reset/power modules (CRPM)" and that the top CRM is a reset controller
"too". This indicates the LSP CRPMs are hardware reset providers.

Does explicitly forbidding the '#reset-cells' property for the LSP0 CRPM
hardware in this allOf block encode a current driver limitation (or lack of
documentation for the reset bits) into the hardware schema? This forbids
device trees from modeling the hardware's reset capabilities, causing a
mismatch between the text description and the schema constraints.

> +        clocks:
> +          items:
> +            - description: register (APB) clock of the group
> +            - description: 25 MHz work clock
> +            - description: 32.768 kHz work clock
> +            - description: 100 MHz work clock

[Severity: Medium]
Because minItems: 1 is defined at the top level of the schema, does this
items array implicitly restrict the minimum length in JSON schema?

According to JSON Schema rules, defining an items array limits the valid types
and values for those indices, but does not require all of them to be present
unless minItems is set. A device tree node specifying fewer clocks than
required (e.g., 1 instead of 4 for LSP0) will pass validation. 

Could this lack of a minItems: 4 constraint in the zte,zx279128s-lsp0crpm
allOf condition allow incomplete device tree nodes to silently pass schema
validation, leading to potential runtime driver failures when expected clocks
are missing?

> +        clock-names:
> +          items:
> +            - const: pclk
> +            - const: wclk25m
> +            - const: wclk32k
> +            - const: wclk100m
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: zte,zx279128s-lsp1crpm
> +    then:
> +      properties:
> +        '#reset-cells': false

[Severity: Medium]
Does explicitly forbidding the '#reset-cells' property for the LSP1 CRPM also
contradict its described hardware role as a reset module?

> +        clocks:
> +          items:
> +            - description: register (APB) clock of the group
> +            - description: AXI clock of the group
> +            - description: 25 MHz work clock
> +            - description: 49.152 MHz work clock
> +            - description: 100 MHz work clock

[Severity: Medium]
Does this zte,zx279128s-lsp1crpm condition also need a minItems: 5 constraint
for clocks? Without it, an array of length 1-4 could pass validation since the
top-level minItems is 1, potentially causing runtime failures if nodes are
incomplete.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010130024.177503-1-ghahramani.navid@gmail.com?part=6