[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240617-zoology-silica-2c8c78363b32@spud>
Date: Mon, 17 Jun 2024 17:44:52 +0100
From: Conor Dooley <conor@...nel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Orson Zhai <orsonzhai@...il.com>,
Baolin Wang <baolin.wang@...ux.alibaba.com>,
Chunyan Zhang <zhang.lyra@...il.com>,
Vadivel Murugan <vadivel.muruganx.ramuthevar@...ux.intel.com>,
Jacky Huang <ychuang3@...oton.com>,
Shan-Chun Hung <schung@...oton.com>,
Khuong Dinh <khuong@...amperecomputing.com>,
Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Lars Povlsen <lars.povlsen@...rochip.com>,
Steen Hegelund <Steen.Hegelund@...rochip.com>,
Daniel Machon <daniel.machon@...rochip.com>,
UNGLinuxDriver@...rochip.com, Nishanth Menon <nm@...com>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Jiaxun Yang <jiaxun.yang@...goat.com>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v2 6/7] dt-bindings: mfd: syscon: Split and enforce
documenting MFD children
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@...aro.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@...nel.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@...0000 {
> + 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@...nel.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
>
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists