Re: [PATCH v8 1/2] dt-bindings: clock: add TI CDCE6214 binding
From: sashiko-bot@kernel.org
Date: 2026-09-09 09:00:11
Also in:
linux-devicetree
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Conditional constraints in the 'allOf' block target the non-existent property 'io-standard' instead of 'ti,io-standard', bypassing validation.
- [Low] The description for 'ti,cmosp-mode' incorrectly refers to 'CMOSN output' and contains typos.
- [Low] The 'ti,cmosp-mode' and 'ti,cmosn-mode' properties lack a minimum constraint, incorrectly allowing the value 0.
- [Low] The 'clock-names' property incorrectly permits an array with duplicate clock names (e.g., ['secref', 'secref']).
--
commit 3a564f8b01873582c4ee765638671ebe3d3d19b4
Author: Sascha Hauer [off-list ref]
dt-bindings: clock: add TI CDCE6214 binding
Add device tree binding for the CDCE6214, an Ultra-Low Power Clock
Generator With One PLL, Four Differential Outputs, Two Inputs, and
Internal EEPROM.
quoted hunk ↗ jump to hunk
diff --git a/Documentation/devicetree/bindings/clock/ti,cdce6214.yaml b/Documentation/devicetree/bindings/clock/ti,cdce6214.yaml new file mode 100644 index 0000000000000..4a9c9155894af --- /dev/null +++ b/Documentation/devicetree/bindings/clock/ti,cdce6214.yaml
[ ... ]
+ clock-names: + minItems: 1 + items: + - enum: [ priref, secref ] + - const: secref
[Severity: Low] Could this allow the clock-names array to have duplicate entries like "secref", "secref"? Should the first item be restricted to just priref to prevent defining the same physical clock reference twice? [ ... ]
+ ti,cmosp-mode: + description: | + Driving mode for CMOSN output: + 1: Low Polarity + 2: High Polrity + 3: Disable
[Severity: Low] There appear to be a couple of typos in this description. It refers to CMOSN output instead of CMOSP, and Polrity instead of Polarity.
+ $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 3
[Severity: Low] Since this references uint32 with a maximum of 3, does it inadvertently allow 0 as a valid state? According to the description and the driver header, 0 is not a valid mode. Would it be better to use an enum of [1, 2, 3] or define a minimum of 1?
+ + ti,cmosn-mode: + description: | + Driving mode for CMOSN output: + 1: Low Polarity + 2: High Polrity + 3: Disable + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 3
[Severity: Low] Similarly, does this property need a minimum constraint to prevent 0 from passing schema validation? [ ... ]
+ allOf: + - if: + properties: + pins: + contains: + const: priref + then: + properties: + io-standard: + enum: [ 1, 5 ]
[Severity: Medium] Do these constraints target the non-existent property io-standard instead of ti,io-standard? If so, an invalid ti,io-standard value in a device tree might erroneously pass schema validation because the constraints aren't applied to it. This pattern appears in the subsequent conditional blocks as well. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260909-clk-cdce6214-v8-0-4dfa355c1f65@pengutronix.de?part=1