lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ