| Message ID | 20240616-dt-bindings-mfd-syscon-split-v2-6-571b5850174a@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: mfd: syscon: Document more compatibles and require simpe-mfd description | expand |
On Sun, Jun 16, 2024 at 03:19:26PM +0200, Krzysztof Kozlowski wrote: > Simple syscon nodes can be documented in common syscon.yaml, however > devices with simple-mfd compatible, thus with some children, should have > their own schema listing these children. Such listing makes the binding > specific, allows better validation (so the incorrect child would not > appear in the simple-mfd node) and actually enforces repeated rule for > simple-mfd devices: > > "simple-mfd" is only for simple devices, where the children do not > depend on the parent. > > Currently the syscon+simple-mfd binding is quite broad and allows > any child or property, thus above rule cannot be enforced. > > Split the syscon.yaml binding into: > 1. Common syscon properties, used potentially by many bindings. > 2. Simple syscon devices (NO simple-mfd!). > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > Depends on: > 1. Patch in MFD: https://lore.kernel.org/all/171828959006.2643902.8308227314531523435.b4-ty@kernel.org/ > 2. Previous patches in the series. > --- > .../devicetree/bindings/mfd/syscon-common.yaml | 72 +++++ > Documentation/devicetree/bindings/mfd/syscon.yaml | 294 +++++++++++++-------- > 2 files changed, 251 insertions(+), 115 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mfd/syscon-common.yaml b/Documentation/devicetree/bindings/mfd/syscon-common.yaml > new file mode 100644 > index 000000000000..c3ff3a7afce3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/syscon-common.yaml > @@ -0,0 +1,72 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/syscon-common.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: System Controller Registers R/W Common Properties > + > +description: | This | can go, right? > + System controller node represents a register region containing a set > + of miscellaneous registers. The registers are not cohesive enough to > + represent as any specific type of device. The typical use-case is > + for some other node's driver, or platform-specific code, to acquire > + a reference to the syscon node (e.g. by phandle, node path, or > + search using a specific compatible value), interrogate the node (or > + associated OS driver) to determine the location of the registers, > + and access the registers directly. > + > +maintainers: > + - Lee Jones <lee@kernel.org> > + > +select: > + properties: > + compatible: > + contains: > + enum: And this can be const, given it's unlikely to grow? > + - syscon > + > + required: > + - compatible > + > +properties: > + compatible: > + contains: > + const: syscon > + minItems: 2 > + maxItems: 5 # Should be enough > + > + reg: > + maxItems: 1 > + > + reg-io-width: > + description: | Same with this one. > + The size (in bytes) of the IO accesses that should be performed > + on the device. > + enum: [1, 2, 4, 8] > + > +required: > + - compatible > + - reg > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + const: simple-mfd > + then: > + properties: > + compatible: > + minItems: 3 > + maxItems: 5 > + > +additionalProperties: true > + > +examples: > + - | > + syscon: syscon@1c00000 { > + compatible = "allwinner,sun8i-h3-system-controller", "syscon"; > + reg = <0x01c00000 0x1000>; > + }; > +... > diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml > index d6fa58c9e4de..d4e9533cf3fe 100644 > --- a/Documentation/devicetree/bindings/mfd/syscon.yaml > +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml > @@ -4,7 +4,7 @@ > $id: http://devicetree.org/schemas/mfd/syscon.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -title: System Controller Registers R/W > +title: System Controller Devices > > description: | > System controller node represents a register region containing a set > @@ -19,123 +19,196 @@ description: | > maintainers: > - Lee Jones <lee@kernel.org> > > +# Need a select with all compatibles listed for compatibility with older > +# dtschema (<2024.02), so this will not be selected for other schemas having > +# syscon fallback. > select: > properties: > compatible: > contains: > enum: > - - syscon Wow, this is noisy. Is it not possible to achieve something similar by making the select check for not: compatible: contains: simple-mfd? Or did I misunderstand the intention here? Thanks, Conor. > - > + - al,alpine-sysfabric-servic > + - allwinner,sun8i-a83t-system-controller > + - allwinner,sun8i-h3-system-controller > + - allwinner,sun8i-v3s-system-controller > + - allwinner,sun50i-a64-system-controller > + - altr,l3regs > + - altr,sdr-ctl > + - amd,pensando-elba-syscon > + - amlogic,meson-mx-assist > + - amlogic,meson-mx-bootrom > + - amlogic,meson8-analog-top > + - amlogic,meson8b-analog-top > + - amlogic,meson8-pmu > + - amlogic,meson8b-pmu > + - apm,xgene-csw > + - apm,xgene-efuse > + - apm,xgene-mcb > + - apm,xgene-rb > + - apm,xgene-scu > + - atmel,sama5d2-sfrbu > + - atmel,sama5d3-nfc-io > + - atmel,sama5d3-sfrbu > + - atmel,sama5d4-sfrbu > + - axis,artpec6-syscon > + - brcm,cru-clkset > + - brcm,sr-cdru > + - brcm,sr-mhb > + - cirrus,ep7209-syscon1 > + - cirrus,ep7209-syscon2 > + - cirrus,ep7209-syscon3 > + - cnxt,cx92755-uc > + - freecom,fsg-cs2-system-controller > + - fsl,imx93-aonmix-ns-syscfg > + - fsl,imx93-wakeupmix-syscfg > + - fsl,ls1088a-reset > + - fsl,vf610-anatop > + - fsl,vf610-mscm-cpucfg > + - hisilicon,dsa-subctrl > + - hisilicon,hi6220-sramctrl > + - hisilicon,hip04-ppe > + - hisilicon,pcie-sas-subctrl > + - hisilicon,peri-subctrl > + - hpe,gxp-sysreg > + - loongson,ls1b-syscon > + - loongson,ls1c-syscon > + - lsi,axxia-syscon > + - marvell,armada-3700-cpu-misc > + - marvell,armada-3700-nb-pm > + - marvell,armada-3700-avs > + - marvell,armada-3700-usb2-host-misc > + - marvell,dove-global-config > + - mediatek,mt2701-pctl-a-syscfg > + - mediatek,mt2712-pctl-a-syscfg > + - mediatek,mt6397-pctl-pmic-syscfg > + - mediatek,mt8135-pctl-a-syscfg > + - mediatek,mt8135-pctl-b-syscfg > + - mediatek,mt8173-pctl-a-syscfg > + - mediatek,mt8365-syscfg > + - microchip,lan966x-cpu-syscon > + - microchip,sam9x60-sfr > + - microchip,sama7g5-ddr3phy > + - mscc,ocelot-cpu-syscon > + - mstar,msc313-pmsleep > + - nuvoton,ma35d1-sys > + - nuvoton,wpcm450-shm > + - rockchip,px30-qos > + - rockchip,rk3036-qos > + - rockchip,rk3066-qos > + - rockchip,rk3128-qos > + - rockchip,rk3228-qos > + - rockchip,rk3288-qos > + - rockchip,rk3368-qos > + - rockchip,rk3399-qos > + - rockchip,rk3568-qos > + - rockchip,rk3588-qos > + - rockchip,rv1126-qos > + - st,spear1340-misc > + - stericsson,nomadik-pmu > + - starfive,jh7100-sysmain > + - ti,am62-usb-phy-ctrl > + - ti,am625-dss-oldi-io-ctrl > + - ti,am62p-cpsw-mac-efuse > + - ti,am654-dss-oldi-io-ctrl > + - ti,j784s4-pcie-ctrl > + - ti,keystone-pllctrl > required: > - compatible > > properties: > compatible: > - anyOf: > - - items: > - - enum: > - - al,alpine-sysfabric-service > - - allwinner,sun8i-a83t-system-controller > - - allwinner,sun8i-h3-system-controller > - - allwinner,sun8i-v3s-system-controller > - - allwinner,sun50i-a64-system-controller > - - altr,l3regs > - - altr,sdr-ctl > - - amd,pensando-elba-syscon > - - amlogic,meson-mx-assist > - - amlogic,meson-mx-bootrom > - - amlogic,meson8-analog-top > - - amlogic,meson8b-analog-top > - - amlogic,meson8-pmu > - - amlogic,meson8b-pmu > - - apm,xgene-csw > - - apm,xgene-efuse > - - apm,xgene-mcb > - - apm,xgene-rb > - - apm,xgene-scu > - - atmel,sama5d2-sfrbu > - - atmel,sama5d3-nfc-io > - - atmel,sama5d3-sfrbu > - - atmel,sama5d4-sfrbu > - - axis,artpec6-syscon > - - brcm,cru-clkset > - - brcm,sr-cdru > - - brcm,sr-mhb > - - cirrus,ep7209-syscon1 > - - cirrus,ep7209-syscon2 > - - cirrus,ep7209-syscon3 > - - cnxt,cx92755-uc > - - freecom,fsg-cs2-system-controller > - - fsl,imx93-aonmix-ns-syscfg > - - fsl,imx93-wakeupmix-syscfg > - - fsl,ls1088a-reset > - - fsl,vf610-anatop > - - fsl,vf610-mscm-cpucfg > - - hisilicon,dsa-subctrl > - - hisilicon,hi6220-sramctrl > - - hisilicon,hip04-ppe > - - hisilicon,pcie-sas-subctrl > - - hisilicon,peri-subctrl > - - hpe,gxp-sysreg > - - loongson,ls1b-syscon > - - loongson,ls1c-syscon > - - lsi,axxia-syscon > - - marvell,armada-3700-cpu-misc > - - marvell,armada-3700-nb-pm > - - marvell,armada-3700-avs > - - marvell,armada-3700-usb2-host-misc > - - marvell,dove-global-config > - - mediatek,mt2701-pctl-a-syscfg > - - mediatek,mt2712-pctl-a-syscfg > - - mediatek,mt6397-pctl-pmic-syscfg > - - mediatek,mt8135-pctl-a-syscfg > - - mediatek,mt8135-pctl-b-syscfg > - - mediatek,mt8173-pctl-a-syscfg > - - mediatek,mt8365-syscfg > - - microchip,lan966x-cpu-syscon > - - microchip,sam9x60-sfr > - - microchip,sama7g5-ddr3phy > - - mscc,ocelot-cpu-syscon > - - mstar,msc313-pmsleep > - - nuvoton,ma35d1-sys > - - nuvoton,wpcm450-shm > - - rockchip,px30-qos > - - rockchip,rk3036-qos > - - rockchip,rk3066-qos > - - rockchip,rk3128-qos > - - rockchip,rk3228-qos > - - rockchip,rk3288-qos > - - rockchip,rk3368-qos > - - rockchip,rk3399-qos > - - rockchip,rk3568-qos > - - rockchip,rk3588-qos > - - rockchip,rv1126-qos > - - st,spear1340-misc > - - stericsson,nomadik-pmu > - - starfive,jh7100-sysmain > - - ti,am62-usb-phy-ctrl > - - ti,am625-dss-oldi-io-ctrl > - - ti,am62p-cpsw-mac-efuse > - - ti,am654-dss-oldi-io-ctrl > - - ti,j784s4-pcie-ctrl > - - ti,keystone-pllctrl > - > - - const: syscon > - > - - contains: > - const: syscon > - minItems: 2 > - maxItems: 5 # Should be enough > + items: > + - enum: > + - al,alpine-sysfabric-service > + - allwinner,sun8i-a83t-system-controller > + - allwinner,sun8i-h3-system-controller > + - allwinner,sun8i-v3s-system-controller > + - allwinner,sun50i-a64-system-controller > + - altr,l3regs > + - altr,sdr-ctl > + - amd,pensando-elba-syscon > + - amlogic,meson-mx-assist > + - amlogic,meson-mx-bootrom > + - amlogic,meson8-analog-top > + - amlogic,meson8b-analog-top > + - amlogic,meson8-pmu > + - amlogic,meson8b-pmu > + - apm,xgene-csw > + - apm,xgene-efuse > + - apm,xgene-mcb > + - apm,xgene-rb > + - apm,xgene-scu > + - atmel,sama5d2-sfrbu > + - atmel,sama5d3-nfc-io > + - atmel,sama5d3-sfrbu > + - atmel,sama5d4-sfrbu > + - axis,artpec6-syscon > + - brcm,cru-clkset > + - brcm,sr-cdru > + - brcm,sr-mhb > + - cirrus,ep7209-syscon1 > + - cirrus,ep7209-syscon2 > + - cirrus,ep7209-syscon3 > + - cnxt,cx92755-uc > + - freecom,fsg-cs2-system-controller > + - fsl,imx93-aonmix-ns-syscfg > + - fsl,imx93-wakeupmix-syscfg > + - fsl,ls1088a-reset > + - fsl,vf610-anatop > + - fsl,vf610-mscm-cpucfg > + - hisilicon,dsa-subctrl > + - hisilicon,hi6220-sramctrl > + - hisilicon,hip04-ppe > + - hisilicon,pcie-sas-subctrl > + - hisilicon,peri-subctrl > + - hpe,gxp-sysreg > + - loongson,ls1b-syscon > + - loongson,ls1c-syscon > + - lsi,axxia-syscon > + - marvell,armada-3700-cpu-misc > + - marvell,armada-3700-nb-pm > + - marvell,armada-3700-avs > + - marvell,armada-3700-usb2-host-misc > + - marvell,dove-global-config > + - mediatek,mt2701-pctl-a-syscfg > + - mediatek,mt2712-pctl-a-syscfg > + - mediatek,mt6397-pctl-pmic-syscfg > + - mediatek,mt8135-pctl-a-syscfg > + - mediatek,mt8135-pctl-b-syscfg > + - mediatek,mt8173-pctl-a-syscfg > + - mediatek,mt8365-syscfg > + - microchip,lan966x-cpu-syscon > + - microchip,sam9x60-sfr > + - microchip,sama7g5-ddr3phy > + - mscc,ocelot-cpu-syscon > + - mstar,msc313-pmsleep > + - nuvoton,ma35d1-sys > + - nuvoton,wpcm450-shm > + - rockchip,px30-qos > + - rockchip,rk3036-qos > + - rockchip,rk3066-qos > + - rockchip,rk3128-qos > + - rockchip,rk3228-qos > + - rockchip,rk3288-qos > + - rockchip,rk3368-qos > + - rockchip,rk3399-qos > + - rockchip,rk3568-qos > + - rockchip,rk3588-qos > + - rockchip,rv1126-qos > + - st,spear1340-misc > + - stericsson,nomadik-pmu > + - starfive,jh7100-sysmain > + - ti,am62-usb-phy-ctrl > + - ti,am625-dss-oldi-io-ctrl > + - ti,am62p-cpsw-mac-efuse > + - ti,am654-dss-oldi-io-ctrl > + - ti,j784s4-pcie-ctrl > + - ti,keystone-pllctrl > + - const: syscon > > reg: > maxItems: 1 > > - reg-io-width: > - description: | > - The size (in bytes) of the IO accesses that should be performed > - on the device. > - enum: [1, 2, 4, 8] > - > resets: > maxItems: 1 > > @@ -144,18 +217,9 @@ required: > - reg > > allOf: > - - if: > - properties: > - compatible: > - contains: > - const: simple-mfd > - then: > - properties: > - compatible: > - minItems: 3 > - maxItems: 5 > + - $ref: syscon-common.yaml# > > -additionalProperties: true > +unevaluatedProperties: false > > examples: > - | > > -- > 2.43.0 >
On Mon, Jun 17, 2024 at 05:44:52PM +0100, Conor Dooley wrote: > On Sun, Jun 16, 2024 at 03:19:26PM +0200, Krzysztof Kozlowski wrote: > > Simple syscon nodes can be documented in common syscon.yaml, however > > devices with simple-mfd compatible, thus with some children, should have > > their own schema listing these children. Such listing makes the binding > > specific, allows better validation (so the incorrect child would not > > appear in the simple-mfd node) and actually enforces repeated rule for > > simple-mfd devices: > > > > "simple-mfd" is only for simple devices, where the children do not > > depend on the parent. > > > > Currently the syscon+simple-mfd binding is quite broad and allows > > any child or property, thus above rule cannot be enforced. > > > > Split the syscon.yaml binding into: > > 1. Common syscon properties, used potentially by many bindings. > > 2. Simple syscon devices (NO simple-mfd!). > > > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > > > --- > > > > Depends on: > > 1. Patch in MFD: https://lore.kernel.org/all/171828959006.2643902.8308227314531523435.b4-ty@kernel.org/ > > 2. Previous patches in the series. > > --- > > .../devicetree/bindings/mfd/syscon-common.yaml | 72 +++++ > > Documentation/devicetree/bindings/mfd/syscon.yaml | 294 +++++++++++++-------- > > 2 files changed, 251 insertions(+), 115 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/mfd/syscon-common.yaml b/Documentation/devicetree/bindings/mfd/syscon-common.yaml > > new file mode 100644 > > index 000000000000..c3ff3a7afce3 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mfd/syscon-common.yaml > > @@ -0,0 +1,72 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mfd/syscon-common.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: System Controller Registers R/W Common Properties > > + > > +description: | > > This | can go, right? > > > + System controller node represents a register region containing a set > > + of miscellaneous registers. The registers are not cohesive enough to > > + represent as any specific type of device. The typical use-case is > > + for some other node's driver, or platform-specific code, to acquire > > + a reference to the syscon node (e.g. by phandle, node path, or > > + search using a specific compatible value), interrogate the node (or > > + associated OS driver) to determine the location of the registers, > > + and access the registers directly. > > + > > +maintainers: > > + - Lee Jones <lee@kernel.org> > > + > > +select: > > + properties: > > + compatible: > > + contains: > > + enum: > > And this can be const, given it's unlikely to grow? > > > + - syscon > > + > > + required: > > + - compatible > > + > > +properties: > > + compatible: > > + contains: > > + const: syscon > > + minItems: 2 > > + maxItems: 5 # Should be enough > > + > > + reg: > > + maxItems: 1 > > + > > + reg-io-width: > > + description: | > > Same with this one. > > > + The size (in bytes) of the IO accesses that should be performed > > + on the device. > > + enum: [1, 2, 4, 8] > > + > > +required: > > + - compatible > > + - reg > > + > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: simple-mfd > > + then: > > + properties: > > + compatible: > > + minItems: 3 > > + maxItems: 5 > > + > > +additionalProperties: true > > + > > +examples: > > + - | > > + syscon: syscon@1c00000 { > > + compatible = "allwinner,sun8i-h3-system-controller", "syscon"; > > + reg = <0x01c00000 0x1000>; > > + }; > > +... > > diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml > > index d6fa58c9e4de..d4e9533cf3fe 100644 > > --- a/Documentation/devicetree/bindings/mfd/syscon.yaml > > +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml > > @@ -4,7 +4,7 @@ > > $id: http://devicetree.org/schemas/mfd/syscon.yaml# > > $schema: http://devicetree.org/meta-schemas/core.yaml# > > > > -title: System Controller Registers R/W > > +title: System Controller Devices > > > > description: | > > System controller node represents a register region containing a set > > @@ -19,123 +19,196 @@ description: | > > maintainers: > > - Lee Jones <lee@kernel.org> > > > > +# Need a select with all compatibles listed for compatibility with older > > +# dtschema (<2024.02), so this will not be selected for other schemas having > > +# syscon fallback. > > select: > > properties: > > compatible: > > contains: > > enum: > > - - syscon > > Wow, this is noisy. Is it not possible to achieve something similar by > making the select check for not: compatible: contains: simple-mfd? Or > did I misunderstand the intention here? Ah, you'd match things then like the intel,lgm-syscon, right?
On 17/06/2024 18:44, Conor Dooley wrote: >> +$id: http://devicetree.org/schemas/mfd/syscon-common.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: System Controller Registers R/W Common Properties >> + >> +description: | > > This | can go, right? Ack > >> + System controller node represents a register region containing a set >> + of miscellaneous registers. The registers are not cohesive enough to >> + represent as any specific type of device. The typical use-case is >> + for some other node's driver, or platform-specific code, to acquire >> + a reference to the syscon node (e.g. by phandle, node path, or >> + search using a specific compatible value), interrogate the node (or >> + associated OS driver) to determine the location of the registers, >> + and access the registers directly. >> + >> +maintainers: >> + - Lee Jones <lee@kernel.org> >> + >> +select: >> + properties: >> + compatible: >> + contains: >> + enum: > > And this can be const, given it's unlikely to grow? ack > >> + - syscon >> + >> + required: >> + - compatible >> + >> +properties: >> + compatible: >> + contains: >> + const: syscon >> + minItems: 2 >> + maxItems: 5 # Should be enough >> + >> + reg: >> + maxItems: 1 >> + >> + reg-io-width: >> + description: | > > Same with this one. ack > >> + The size (in bytes) of the IO accesses that should be performed >> + on the device. >> + enum: [1, 2, 4, 8] >> + >> +required: >> + - compatible >> + - reg >> + >> +allOf: >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: simple-mfd >> + then: >> + properties: >> + compatible: >> + minItems: 3 >> + maxItems: 5 >> + >> +additionalProperties: true >> + >> +examples: >> + - | >> + syscon: syscon@1c00000 { >> + compatible = "allwinner,sun8i-h3-system-controller", "syscon"; >> + reg = <0x01c00000 0x1000>; >> + }; >> +... >> diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml >> index d6fa58c9e4de..d4e9533cf3fe 100644 >> --- a/Documentation/devicetree/bindings/mfd/syscon.yaml >> +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml >> @@ -4,7 +4,7 @@ >> $id: http://devicetree.org/schemas/mfd/syscon.yaml# >> $schema: http://devicetree.org/meta-schemas/core.yaml# >> >> -title: System Controller Registers R/W >> +title: System Controller Devices >> >> description: | >> System controller node represents a register region containing a set >> @@ -19,123 +19,196 @@ description: | >> maintainers: >> - Lee Jones <lee@kernel.org> >> >> +# Need a select with all compatibles listed for compatibility with older >> +# dtschema (<2024.02), so this will not be selected for other schemas having >> +# syscon fallback. >> select: >> properties: >> compatible: >> contains: >> enum: >> - - syscon > > Wow, this is noisy. Is it not possible to achieve something similar by > making the select check for not: compatible: contains: simple-mfd? Or > did I misunderstand the intention here? See comment from Rob for v1. This is needed for older schema, although 2024.02 worked fine in my tests. The point is to select all schemas, not by compatible property, because then we match this schema to anything having syscon. Kind of similar how it is woth arm,primecell. Best regards, Krzysztof
On 17/06/2024 18:46, Conor Dooley wrote: >>> >>> +# Need a select with all compatibles listed for compatibility with older >>> +# dtschema (<2024.02), so this will not be selected for other schemas having >>> +# syscon fallback. >>> select: >>> properties: >>> compatible: >>> contains: >>> enum: >>> - - syscon >> >> Wow, this is noisy. Is it not possible to achieve something similar by >> making the select check for not: compatible: contains: simple-mfd? Or >> did I misunderstand the intention here? > > Ah, you'd match things then like the intel,lgm-syscon, right? Yes. Best regards, Krzysztof
On Mon, Jun 17, 2024 at 07:35:07PM +0200, Krzysztof Kozlowski wrote: > On 17/06/2024 18:46, Conor Dooley wrote: > >>> > >>> +# Need a select with all compatibles listed for compatibility with older > >>> +# dtschema (<2024.02), so this will not be selected for other schemas having > >>> +# syscon fallback. > >>> select: > >>> properties: > >>> compatible: > >>> contains: > >>> enum: > >>> - - syscon > >> > >> Wow, this is noisy. Is it not possible to achieve something similar by > >> making the select check for not: compatible: contains: simple-mfd? Or > >> did I misunderstand the intention here? > > > > Ah, you'd match things then like the intel,lgm-syscon, right? > > Yes. With the nits then: Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
On Sun, 16 Jun 2024 15:19:26 +0200, Krzysztof Kozlowski wrote: > Simple syscon nodes can be documented in common syscon.yaml, however > devices with simple-mfd compatible, thus with some children, should have > their own schema listing these children. Such listing makes the binding > specific, allows better validation (so the incorrect child would not > appear in the simple-mfd node) and actually enforces repeated rule for > simple-mfd devices: > > "simple-mfd" is only for simple devices, where the children do not > depend on the parent. > > Currently the syscon+simple-mfd binding is quite broad and allows > any child or property, thus above rule cannot be enforced. > > Split the syscon.yaml binding into: > 1. Common syscon properties, used potentially by many bindings. > 2. Simple syscon devices (NO simple-mfd!). > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > Depends on: > 1. Patch in MFD: https://lore.kernel.org/all/171828959006.2643902.8308227314531523435.b4-ty@kernel.org/ > 2. Previous patches in the series. > --- > .../devicetree/bindings/mfd/syscon-common.yaml | 72 +++++ > Documentation/devicetree/bindings/mfd/syscon.yaml | 294 +++++++++++++-------- > 2 files changed, 251 insertions(+), 115 deletions(-) > Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
diff --git a/Documentation/devicetree/bindings/mfd/syscon-common.yaml b/Documentation/devicetree/bindings/mfd/syscon-common.yaml new file mode 100644 index 000000000000..c3ff3a7afce3 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/syscon-common.yaml @@ -0,0 +1,72 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/syscon-common.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: System Controller Registers R/W Common Properties + +description: | + System controller node represents a register region containing a set + of miscellaneous registers. The registers are not cohesive enough to + represent as any specific type of device. The typical use-case is + for some other node's driver, or platform-specific code, to acquire + a reference to the syscon node (e.g. by phandle, node path, or + search using a specific compatible value), interrogate the node (or + associated OS driver) to determine the location of the registers, + and access the registers directly. + +maintainers: + - Lee Jones <lee@kernel.org> + +select: + properties: + compatible: + contains: + enum: + - syscon + + required: + - compatible + +properties: + compatible: + contains: + const: syscon + minItems: 2 + maxItems: 5 # Should be enough + + reg: + maxItems: 1 + + reg-io-width: + description: | + The size (in bytes) of the IO accesses that should be performed + on the device. + enum: [1, 2, 4, 8] + +required: + - compatible + - reg + +allOf: + - if: + properties: + compatible: + contains: + const: simple-mfd + then: + properties: + compatible: + minItems: 3 + maxItems: 5 + +additionalProperties: true + +examples: + - | + syscon: syscon@1c00000 { + compatible = "allwinner,sun8i-h3-system-controller", "syscon"; + reg = <0x01c00000 0x1000>; + }; +... diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml index d6fa58c9e4de..d4e9533cf3fe 100644 --- a/Documentation/devicetree/bindings/mfd/syscon.yaml +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml @@ -4,7 +4,7 @@ $id: http://devicetree.org/schemas/mfd/syscon.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: System Controller Registers R/W +title: System Controller Devices description: | System controller node represents a register region containing a set @@ -19,123 +19,196 @@ description: | maintainers: - Lee Jones <lee@kernel.org> +# Need a select with all compatibles listed for compatibility with older +# dtschema (<2024.02), so this will not be selected for other schemas having +# syscon fallback. select: properties: compatible: contains: enum: - - syscon - + - al,alpine-sysfabric-servic + - allwinner,sun8i-a83t-system-controller + - allwinner,sun8i-h3-system-controller + - allwinner,sun8i-v3s-system-controller + - allwinner,sun50i-a64-system-controller + - altr,l3regs + - altr,sdr-ctl + - amd,pensando-elba-syscon + - amlogic,meson-mx-assist + - amlogic,meson-mx-bootrom + - amlogic,meson8-analog-top + - amlogic,meson8b-analog-top + - amlogic,meson8-pmu + - amlogic,meson8b-pmu + - apm,xgene-csw + - apm,xgene-efuse + - apm,xgene-mcb + - apm,xgene-rb + - apm,xgene-scu + - atmel,sama5d2-sfrbu + - atmel,sama5d3-nfc-io + - atmel,sama5d3-sfrbu + - atmel,sama5d4-sfrbu + - axis,artpec6-syscon + - brcm,cru-clkset + - brcm,sr-cdru + - brcm,sr-mhb + - cirrus,ep7209-syscon1 + - cirrus,ep7209-syscon2 + - cirrus,ep7209-syscon3 + - cnxt,cx92755-uc + - freecom,fsg-cs2-system-controller + - fsl,imx93-aonmix-ns-syscfg + - fsl,imx93-wakeupmix-syscfg + - fsl,ls1088a-reset + - fsl,vf610-anatop + - fsl,vf610-mscm-cpucfg + - hisilicon,dsa-subctrl + - hisilicon,hi6220-sramctrl + - hisilicon,hip04-ppe + - hisilicon,pcie-sas-subctrl + - hisilicon,peri-subctrl + - hpe,gxp-sysreg + - loongson,ls1b-syscon + - loongson,ls1c-syscon + - lsi,axxia-syscon + - marvell,armada-3700-cpu-misc + - marvell,armada-3700-nb-pm + - marvell,armada-3700-avs + - marvell,armada-3700-usb2-host-misc + - marvell,dove-global-config + - mediatek,mt2701-pctl-a-syscfg + - mediatek,mt2712-pctl-a-syscfg + - mediatek,mt6397-pctl-pmic-syscfg + - mediatek,mt8135-pctl-a-syscfg + - mediatek,mt8135-pctl-b-syscfg + - mediatek,mt8173-pctl-a-syscfg + - mediatek,mt8365-syscfg + - microchip,lan966x-cpu-syscon + - microchip,sam9x60-sfr + - microchip,sama7g5-ddr3phy + - mscc,ocelot-cpu-syscon + - mstar,msc313-pmsleep + - nuvoton,ma35d1-sys + - nuvoton,wpcm450-shm + - rockchip,px30-qos + - rockchip,rk3036-qos + - rockchip,rk3066-qos + - rockchip,rk3128-qos + - rockchip,rk3228-qos + - rockchip,rk3288-qos + - rockchip,rk3368-qos + - rockchip,rk3399-qos + - rockchip,rk3568-qos + - rockchip,rk3588-qos + - rockchip,rv1126-qos + - st,spear1340-misc + - stericsson,nomadik-pmu + - starfive,jh7100-sysmain + - ti,am62-usb-phy-ctrl + - ti,am625-dss-oldi-io-ctrl + - ti,am62p-cpsw-mac-efuse + - ti,am654-dss-oldi-io-ctrl + - ti,j784s4-pcie-ctrl + - ti,keystone-pllctrl required: - compatible properties: compatible: - anyOf: - - items: - - enum: - - al,alpine-sysfabric-service - - allwinner,sun8i-a83t-system-controller - - allwinner,sun8i-h3-system-controller - - allwinner,sun8i-v3s-system-controller - - allwinner,sun50i-a64-system-controller - - altr,l3regs - - altr,sdr-ctl - - amd,pensando-elba-syscon - - amlogic,meson-mx-assist - - amlogic,meson-mx-bootrom - - amlogic,meson8-analog-top - - amlogic,meson8b-analog-top - - amlogic,meson8-pmu - - amlogic,meson8b-pmu - - apm,xgene-csw - - apm,xgene-efuse - - apm,xgene-mcb - - apm,xgene-rb - - apm,xgene-scu - - atmel,sama5d2-sfrbu - - atmel,sama5d3-nfc-io - - atmel,sama5d3-sfrbu - - atmel,sama5d4-sfrbu - - axis,artpec6-syscon - - brcm,cru-clkset - - brcm,sr-cdru - - brcm,sr-mhb - - cirrus,ep7209-syscon1 - - cirrus,ep7209-syscon2 - - cirrus,ep7209-syscon3 - - cnxt,cx92755-uc - - freecom,fsg-cs2-system-controller - - fsl,imx93-aonmix-ns-syscfg - - fsl,imx93-wakeupmix-syscfg - - fsl,ls1088a-reset - - fsl,vf610-anatop - - fsl,vf610-mscm-cpucfg - - hisilicon,dsa-subctrl - - hisilicon,hi6220-sramctrl - - hisilicon,hip04-ppe - - hisilicon,pcie-sas-subctrl - - hisilicon,peri-subctrl - - hpe,gxp-sysreg - - loongson,ls1b-syscon - - loongson,ls1c-syscon - - lsi,axxia-syscon - - marvell,armada-3700-cpu-misc - - marvell,armada-3700-nb-pm - - marvell,armada-3700-avs - - marvell,armada-3700-usb2-host-misc - - marvell,dove-global-config - - mediatek,mt2701-pctl-a-syscfg - - mediatek,mt2712-pctl-a-syscfg - - mediatek,mt6397-pctl-pmic-syscfg - - mediatek,mt8135-pctl-a-syscfg - - mediatek,mt8135-pctl-b-syscfg - - mediatek,mt8173-pctl-a-syscfg - - mediatek,mt8365-syscfg - - microchip,lan966x-cpu-syscon - - microchip,sam9x60-sfr - - microchip,sama7g5-ddr3phy - - mscc,ocelot-cpu-syscon - - mstar,msc313-pmsleep - - nuvoton,ma35d1-sys - - nuvoton,wpcm450-shm - - rockchip,px30-qos - - rockchip,rk3036-qos - - rockchip,rk3066-qos - - rockchip,rk3128-qos - - rockchip,rk3228-qos - - rockchip,rk3288-qos - - rockchip,rk3368-qos - - rockchip,rk3399-qos - - rockchip,rk3568-qos - - rockchip,rk3588-qos - - rockchip,rv1126-qos - - st,spear1340-misc - - stericsson,nomadik-pmu - - starfive,jh7100-sysmain - - ti,am62-usb-phy-ctrl - - ti,am625-dss-oldi-io-ctrl - - ti,am62p-cpsw-mac-efuse - - ti,am654-dss-oldi-io-ctrl - - ti,j784s4-pcie-ctrl - - ti,keystone-pllctrl - - - const: syscon - - - contains: - const: syscon - minItems: 2 - maxItems: 5 # Should be enough + items: + - enum: + - al,alpine-sysfabric-service + - allwinner,sun8i-a83t-system-controller + - allwinner,sun8i-h3-system-controller + - allwinner,sun8i-v3s-system-controller + - allwinner,sun50i-a64-system-controller + - altr,l3regs + - altr,sdr-ctl + - amd,pensando-elba-syscon + - amlogic,meson-mx-assist + - amlogic,meson-mx-bootrom + - amlogic,meson8-analog-top + - amlogic,meson8b-analog-top + - amlogic,meson8-pmu + - amlogic,meson8b-pmu + - apm,xgene-csw + - apm,xgene-efuse + - apm,xgene-mcb + - apm,xgene-rb + - apm,xgene-scu + - atmel,sama5d2-sfrbu + - atmel,sama5d3-nfc-io + - atmel,sama5d3-sfrbu + - atmel,sama5d4-sfrbu + - axis,artpec6-syscon + - brcm,cru-clkset + - brcm,sr-cdru + - brcm,sr-mhb + - cirrus,ep7209-syscon1 + - cirrus,ep7209-syscon2 + - cirrus,ep7209-syscon3 + - cnxt,cx92755-uc + - freecom,fsg-cs2-system-controller + - fsl,imx93-aonmix-ns-syscfg + - fsl,imx93-wakeupmix-syscfg + - fsl,ls1088a-reset + - fsl,vf610-anatop + - fsl,vf610-mscm-cpucfg + - hisilicon,dsa-subctrl + - hisilicon,hi6220-sramctrl + - hisilicon,hip04-ppe + - hisilicon,pcie-sas-subctrl + - hisilicon,peri-subctrl + - hpe,gxp-sysreg + - loongson,ls1b-syscon + - loongson,ls1c-syscon + - lsi,axxia-syscon + - marvell,armada-3700-cpu-misc + - marvell,armada-3700-nb-pm + - marvell,armada-3700-avs + - marvell,armada-3700-usb2-host-misc + - marvell,dove-global-config + - mediatek,mt2701-pctl-a-syscfg + - mediatek,mt2712-pctl-a-syscfg + - mediatek,mt6397-pctl-pmic-syscfg + - mediatek,mt8135-pctl-a-syscfg + - mediatek,mt8135-pctl-b-syscfg + - mediatek,mt8173-pctl-a-syscfg + - mediatek,mt8365-syscfg + - microchip,lan966x-cpu-syscon + - microchip,sam9x60-sfr + - microchip,sama7g5-ddr3phy + - mscc,ocelot-cpu-syscon + - mstar,msc313-pmsleep + - nuvoton,ma35d1-sys + - nuvoton,wpcm450-shm + - rockchip,px30-qos + - rockchip,rk3036-qos + - rockchip,rk3066-qos + - rockchip,rk3128-qos + - rockchip,rk3228-qos + - rockchip,rk3288-qos + - rockchip,rk3368-qos + - rockchip,rk3399-qos + - rockchip,rk3568-qos + - rockchip,rk3588-qos + - rockchip,rv1126-qos + - st,spear1340-misc + - stericsson,nomadik-pmu + - starfive,jh7100-sysmain + - ti,am62-usb-phy-ctrl + - ti,am625-dss-oldi-io-ctrl + - ti,am62p-cpsw-mac-efuse + - ti,am654-dss-oldi-io-ctrl + - ti,j784s4-pcie-ctrl + - ti,keystone-pllctrl + - const: syscon reg: maxItems: 1 - reg-io-width: - description: | - The size (in bytes) of the IO accesses that should be performed - on the device. - enum: [1, 2, 4, 8] - resets: maxItems: 1 @@ -144,18 +217,9 @@ required: - reg allOf: - - if: - properties: - compatible: - contains: - const: simple-mfd - then: - properties: - compatible: - minItems: 3 - maxItems: 5 + - $ref: syscon-common.yaml# -additionalProperties: true +unevaluatedProperties: false examples: - |
Simple syscon nodes can be documented in common syscon.yaml, however devices with simple-mfd compatible, thus with some children, should have their own schema listing these children. Such listing makes the binding specific, allows better validation (so the incorrect child would not appear in the simple-mfd node) and actually enforces repeated rule for simple-mfd devices: "simple-mfd" is only for simple devices, where the children do not depend on the parent. Currently the syscon+simple-mfd binding is quite broad and allows any child or property, thus above rule cannot be enforced. Split the syscon.yaml binding into: 1. Common syscon properties, used potentially by many bindings. 2. Simple syscon devices (NO simple-mfd!). Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Depends on: 1. Patch in MFD: https://lore.kernel.org/all/171828959006.2643902.8308227314531523435.b4-ty@kernel.org/ 2. Previous patches in the series. --- .../devicetree/bindings/mfd/syscon-common.yaml | 72 +++++ Documentation/devicetree/bindings/mfd/syscon.yaml | 294 +++++++++++++-------- 2 files changed, 251 insertions(+), 115 deletions(-)