dt-bindings: clock: via,vt8500: Convert to DT Schema
Subsystems (3)
PATCH v4 1/1 dt-bindings: clock: via,vt8500: Convert to DT Schema
Convert the VIA/Wondermedia VT8500 and Wondermedia WM8xxx series SoCs clock controller binding from the legacy text format to DT schema.
---
Changelog:
Changes since v3
- Add schema select matching for via,vt8500-pmc
- Allow hyphen in node names under patternProperties
- Add dependentRequired validation for enable-reg/enable-bit
- Fix example validation against PMC schema
Link to v3:https://lore.kernel.org/all/[email protected]/
Changes since v2:
- Drop redundant description for clocks
- Disable reg property for device clocks
- Fix schema hierarchy to match actual DTS structure
Link to v2:https://lore.kernel.org/all/[email protected]/
Changes since v1:
- Add default value for divisor-mask
- Add required properties compatible and model
- Fix example node name
- Update example size cells and reg value
Link to v1:https://lore.kernel.org/all/[email protected]/
--- .../bindings/clock/via,vt8500-clock.yaml | 209 ++++++++++++++++++ .../devicetree/bindings/clock/vt8500.txt | 74 ------- 2 files changed, 209 insertions(+), 74 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml delete mode 100644 Documentation/devicetree/bindings/clock/vt8500.txt
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +209 -0
@@ -0,0 +1,209 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/via,vt8500-clock.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: VIA/Wondermedia VT8500 Clock Controller + +maintainers: + - Michael Turquette <[email protected]> + - Stephen Boyd <[email protected]> + +description: + Clock controller bindings for VIA/Wondermedia VT8500 and Wondermedia WM8xxx + series SoCs. + +select: + properties: + compatible: + const: via,vt8500-pmc + + required: + - compatible + +properties: + compatible: + const: via,vt8500-pmc + + reg: + maxItems: 1 + + clocks: + type: object + additionalProperties: true + + properties: + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + required: + - "#address-cells" + - "#size-cells" + + patternProperties: + "^[a-z0-9-]+(@[0-9a-f]+)?$": + type: object + + properties: + compatible: + enum: + - via,vt8500-pll-clock + - wm,wm8650-pll-clock + - wm,wm8750-pll-clock + - wm,wm8850-pll-clock + - via,vt8500-device-clock + + reg: + maxItems: 1 + description: + Offset of the PLL register within the PMC register space. + + clocks: + maxItems: 1 + + "#clock-cells": + const: 0 + + enable-reg: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Offset of the clock enable register within the PMC + register space. + + enable-bit: + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 31 + description: + Bit index controlling clock enable. + + divisor-reg: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Offset of the clock divisor register within the PMC + register space. + + divisor-mask: + $ref: /schemas/types.yaml#/definitions/uint32 + default: 0x1f + description: + Bitmask describing the divisor field inside divisor-reg. + + dependentRequired: + enable-reg: + - enable-bit + + enable-bit: + - enable-reg + + required: + - compatible + - "#clock-cells" + + allOf: + - if: + properties: + compatible: + enum: + - via,vt8500-pll-clock + - wm,wm8650-pll-clock + - wm,wm8750-pll-clock + - wm,wm8850-pll-clock + then: + required: + - reg + - clocks + + - if: + properties: + compatible: + const: via,vt8500-device-clock + then: + properties: + reg: false + + required: + - clocks + + anyOf: + - required: + - enable-reg + - enable-bit + + - required: + - divisor-reg + + additionalProperties: false + +required: + - compatible + - reg + - clocks + +additionalProperties: false + +examples: + - | + pmc@d8130000 { + compatible = "via,vt8500-pmc"; + reg = <0xd8130000 0x1000>; + + clocks { + #address-cells = <1>; + #size-cells = <0>; + + ref24: ref24M { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <24000000>; + }; + + ref25: ref25M { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <25000000>; + }; + + plla: clock@200 { + compatible = "wm,wm8650-pll-clock"; + #clock-cells = <0>; + clocks = <&ref25>; + reg = <0x200>; + }; + + pllb: clock@204 { + compatible = "wm,wm8650-pll-clock"; + #clock-cells = <0>; + clocks = <&ref25>; + reg = <0x204>; + }; + + clkarm: arm { + compatible = "via,vt8500-device-clock"; + #clock-cells = <0>; + clocks = <&plla>; + divisor-reg = <0x300>; + }; + + clkuart0: uart0 { + compatible = "via,vt8500-device-clock"; + #clock-cells = <0>; + clocks = <&ref24>; + enable-reg = <0x250>; + enable-bit = <1>; + }; + + clksdhc: sdhc { + compatible = "via,vt8500-device-clock"; + #clock-cells = <0>; + clocks = <&pllb>; + divisor-reg = <0x328>; + divisor-mask = <0x3f>; + enable-reg = <0x254>; + enable-bit = <18>; + }; + }; + };
Documentation/devicetree/bindings/clock/vt8500.txt deleted +0 -74
74 lines not on this page · raw message
Signature · 1 line
2.43.0
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] The `patternProperties` regex enforces strictly lowercase node names, inadvertently rejecting…
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The patternProperties regex enforces strictly lowercase node names, inadvertently rejecting standard fixed-clock nodes unless they violate DT naming conventions by including uppercase letters to bypass validation.
- [Medium] The patch defines a new schema for via,vt8500-pmc but fails to delete the existing legacy text binding for the same compatible string.
- [Low] The divisor-mask property lacks a dependentRequired constraint to enforce that it can only be used when divisor-reg is also present.
--
commit f63cd62ab71a54ca61f786bce852286bf98a7852
Author: Udaya Kiran Challa <[email protected]>
dt-bindings: clock: via,vt8500: Convert to DT Schema
Convert the VIA/Wondermedia VT8500 and Wondermedia WM8xxx series SoCs clock controller binding from the legacy text format to DT schema.
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml
Quoted 5 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +0
-0
[Severity: Medium]
Should this patch also delete the existing text binding for this compatible located at Documentation/devicetree/bindings/arm/vt8500/via,vt8500-pmc.txt? Leaving the old text binding intact causes duplicate and fragmented documentation for the exact same compatible string.
[ ... ]
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 58
Quoted 12 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +12
-0+ patternProperties: + "^[a-z0-9-]+(@[0-9a-f]+)?$": + type: object + + properties: + compatible: + enum: + - via,vt8500-pll-clock + - wm,wm8650-pll-clock + - wm,wm8750-pll-clock + - wm,wm8850-pll-clock + - via,vt8500-device-clock
[Severity: High]
Does this regex unintentionally restrict compliant lowercase node names?
Any standard child node with a compliant lowercase name (like "ref24") will match this pattern and be strictly restricted to the compatibles above, failing if it contains properties like clock-frequency.
It appears the fixed-clock examples in this patch only pass validation because their node names include uppercase letters (e.g., "ref24M"), which evades the regex entirely and falls through to additionalProperties.
[ ... ]
Quoted 8 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +8
-0+ divisor-reg: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Offset of the clock divisor register within the PMC + register space. + + divisor-mask: + $ref: /schemas/types.yaml#/definitions/uint32
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 100
Quoted 10 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +10
-0+ default: 0x1f + description: + Bitmask describing the divisor field inside divisor-reg. + + dependentRequired: + enable-reg: + - enable-bit + + enable-bit: + - enable-reg
[Severity: Low]
Should this section also include a constraint to enforce that divisor-mask can only be used when divisor-reg is present? The legacy text binding indicates the mask is semantically tied to the divisor register.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Quoted 41 lines · Udaya Kiran Challa
Convert the VIA/Wondermedia VT8500 and Wondermedia WM8xxx series SoCs clock controller binding from the legacy text format to DT schema.
Signed-off-by Udaya Kiran Challa <[email protected]>---
Changelog:
Changes since v3
- Add schema select matching for via,vt8500-pmc
- Allow hyphen in node names under patternProperties
- Add dependentRequired validation for enable-reg/enable-bit
- Fix example validation against PMC schemaLink to v3:https://lore.kernel.org/all/[email protected]/
Changes since v2:
- Drop redundant description for clocks
- Disable reg property for device clocks
- Fix schema hierarchy to match actual DTS structureLink to v2:https://lore.kernel.org/all/[email protected]/
Changes since v1:
- Add default value for divisor-mask
- Add required properties compatible and model
- Fix example node name
- Update example size cells and reg valueLink to v1:https://lore.kernel.org/all/[email protected]/
--- .../bindings/clock/via,vt8500-clock.yaml | 209 ++++++++++++++++++ .../devicetree/bindings/clock/vt8500.txt | 74 ------- 2 files changed, 209 insertions(+), 74 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml delete mode 100644 Documentation/devicetree/bindings/clock/vt8500.txtDocumentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +0
-0
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 7
Quoted 7 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +7
-0@@ -0,0 +1,209 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/via,vt8500-clock.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: VIA/Wondermedia VT8500 Clock Controller
How PMC is a clock controller? Really?
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 11
Quoted 4 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +4
-0+ +maintainers: + - Michael Turquette <[email protected]> + - Stephen Boyd <[email protected]>
Subsystem maintainers do not care about PMC. This can be platform maintainer.
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 23
Quoted 12 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +12
-0+ +description: + Clock controller bindings for VIA/Wondermedia VT8500 and Wondermedia WM8xxx + series SoCs. + +select: + properties: + compatible: + const: via,vt8500-pmc + + required: + - compatible
Why do you have select?
I don't understand your changes. This was not at v2 and I did not ask to change that.
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 20
Quoted 4 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +4
-0+ +properties: + compatible: + const: via,vt8500-pmc
So via,vt8500-clock.yaml or pmc? Why aren't you removing the pmc file? Why is this located at clocks?
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 34
Quoted 7 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +7
-0+ + reg: + maxItems: 1 + + clocks: + type: object + additionalProperties: true
No, this cannot be "true".
I finished review here.
Best regards,
Krzysztof
Quoted 9 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 7
Quoted 7 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +7
-0+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/via,vt8500-clock.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: VIA/Wondermedia VT8500 Clock ControllerHow PMC is a clock controller? Really?
No Krzysztof, actually that was a wrong direction in v4.
Quoted 8 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 11
Quoted 4 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +4
-0+ +maintainers: + - Michael Turquette <[email protected]> + - Stephen Boyd <[email protected]>Subsystem maintainers do not care about PMC. This can be platform maintainer.
I agree with you. I changed maintainers accordingly.
Quoted 15 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 23
Quoted 12 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +12
-0+ +description: + Clock controller bindings for VIA/Wondermedia VT8500 and Wondermedia WM8xxx + series SoCs. + +select: + properties: + compatible: + const: via,vt8500-pmc + + required: + - compatibleWhy do you have select?
Quoted 2 lines
I don't understand your changes. This was not at v2 and I did not ask to change that.
The select: block with via,vt8500-pmc and the clocks: type: object were mistakenly added to via,vt8500-clock.yaml in v4 — leftover confusion from trying to handle the PMC node's clock container in the same schema. In v5 these are removed from the clock schema entirely. The PMC binding is now a separate patch (via,vt8500-pmc.yaml) which is the right place for the clock container node description.
Quoted 8 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 20
Quoted 4 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +4
-0+ +properties: + compatible: + const: via,vt8500-pmcSo via,vt8500-clock.yaml or pmc? Why aren't you removing the pmc file? Why is this located at clocks?
In the next revision, this patch is scope only to the clock provider bindings (via,vt8500-device-clock, via,vt8500-pll-clock, wm,*-pll-clock). It no longer models PMC/top-level node properties and does not modify PMC binding files.
Quoted 9 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 34
Quoted 7 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +7
-0+ + reg: + maxItems: 1 + + clocks: + type: object + additionalProperties: trueNo, this cannot be "true".
Agreed, I dropped that structure and kept strict schema validation.
I also fixed remaining related points from review/bot feedback:
- example validation issues
- dependency constraints (enable-reg/enable-bit, divisor-mask/divisor-reg)
- explicit reg: false for via,vt8500-device-clock
Regards,
Udaya Kiran Challa
Quoted 71 lines · Uday Kiran
Quoted 9 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 7
Quoted 7 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +7
-0+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/via,vt8500-clock.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: VIA/Wondermedia VT8500 Clock ControllerHow PMC is a clock controller? Really?
No Krzysztof, actually that was a wrong direction in v4.
Quoted 8 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 11
Quoted 4 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +4
-0+ +maintainers: + - Michael Turquette <[email protected]> + - Stephen Boyd <[email protected]>Subsystem maintainers do not care about PMC. This can be platform maintainer.
I agree with you. I changed maintainers accordingly.
Quoted 15 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 23
Quoted 12 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +12
-0+ +description: + Clock controller bindings for VIA/Wondermedia VT8500 and Wondermedia WM8xxx + series SoCs. + +select: + properties: + compatible: + const: via,vt8500-pmc + + required: + - compatibleWhy do you have select?
Quoted 2 lines
I don't understand your changes. This was not at v2 and I did not ask to change that.
The select: block with via,vt8500-pmc and the clocks: type: object were mistakenly added to via,vt8500-clock.yaml in v4 — leftover confusion from trying to handle the PMC node's clock container in the same schema. In v5 these are removed from the clock schema entirely. The PMC binding is now a separate patch (via,vt8500-pmc.yaml) which is the right place for the clock container node description.
Quoted 8 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 20
Quoted 4 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +4
-0+ +properties: + compatible: + const: via,vt8500-pmcSo via,vt8500-clock.yaml or pmc? Why aren't you removing the pmc file? Why is this located at clocks?
In the next revision, this patch is scope only to the clock provider bindings (via,vt8500-device-clock, via,vt8500-pll-clock, wm,*-pll-clock). It no longer models PMC/top-level node properties and does not modify PMC binding files.
Quoted 9 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml line 34
Quoted 7 lines
Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml new file +7
-0+ + reg: + maxItems: 1 + + clocks: + type: object + additionalProperties: trueNo, this cannot be "true".
Quoted 1 line
Agreed, I dropped that structure and kept strict schema validation.
So open v5 and tell me how did you solve "this cannot be 'true'", part?
Best regards,
Krzysztof