| Message ID | 20231125232821.234631-1-niklas.soderlund+renesas@ragnatech.se |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dt-bindings: renesas: Document preferred compatible naming | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 26/11/2023 00:28, Niklas Söderlund wrote: > Compatibles can come in two formats. Either "vendor,ip-soc" or > "vendor,soc-ip". Add a DT schema file documenting Renesas preferred > policy and enforcing it for all new compatibles, except few existing > patterns. > > Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > * Changes since RFC > - Moved to Documentation/devicetree/bindings/soc/renesas. > - Changed the pattern in the initial select to match on .*-.*. > - Added a lot of missing compatible values. > > Compared to the RFC I did not use make dt_binding_check and manual > reading of the bindings to find all compatibles. Instead I generated > YAML of all renesas related files under arch/arm and arc/arm64 and > parsed all the compatible values. > > For v6.7-rc2 the hits on each pattern where, > > ^renesas,emev2-[a-z0-9-]+$ = 14 > ^renesas,r7s[0-9]+-[a-z0-9-]+$ = 58 > ^renesas,r8a[a-z0-9]+-[a-z0-9-]+$ = 1661 > ^renesas,r9a[0-9]+g[a-z0-9]+-[a-z0-9-]+$ = 160 > ^renesas,rcar-[a-z0-9-]+$ = 4522 > ^renesas,rz-[a-z0-9-]+$ = 26 > ^renesas,rza-[a-z0-9-]+$ = 4 > ^renesas,rza1-[a-z0-9-]+$ = 10 > ^renesas,rza2-[a-z0-9-]+$ = 6 > ^renesas,rzg2l-[a-z0-9-]+$ = 54 > ^renesas,rzn1[a-z0-9]*-[a-z0-9-]+$ = 22 > ^renesas,rzv2m-[a-z0-9-]+$ = 9 > ^renesas,sh-[a-z0-9-]+$ = 36 > ^renesas,sh7[a-z0-9]+-[a-z0-9-]+$ = 27 > renesas,cpg-div6-clock = 45 > renesas,cpg-mstp-clocks = 49 > renesas,intc-irqpin = 20 > renesas,smp-sram = 20 > ^renesas,can-[a-z0-9]+$ = 136 > ^renesas,dmac-[a-z0-9]+$ = 326 > ^renesas,du-[a-z0-9]+$ = 77 > ^renesas,ether-[a-z0-9]+$ = 21 > ^renesas,etheravb-[a-z0-9]+$ = 84 > ^renesas,etheravb-rcar-gen[0-9]$ = 82 > ^renesas,gether-[a-z0-9]+$ = 4 > ^renesas,gpio-[a-z0-9]+$ = 608 > ^renesas,hscif-[a-z0-9]+$ = 336 > ^renesas,i2c-[a-z0-9]+$ = 517 > ^renesas,iic-[a-z0-9]+$ = 118 > ^renesas,intc-ex-[a-z0-9]+$ = 58 > ^renesas,intc-irqpin-[a-z0-9]+$ = 10 > ^renesas,ipmmu-[a-z0-9]+$ = 828 > ^renesas,irqc-[a-z0-9]+$ = 22 > ^renesas,jpu-[a-z0-9]+$ = 6 > ^renesas,mmcif-[a-z0-9]+$ = 29 > ^renesas,msiof-[a-z0-9]+$ = 290 > ^renesas,pci-[a-z0-9]+$ = 38 > ^renesas,pci-rcar-gen[0-9]$ = 36 > ^renesas,pcie-[a-z0-9]+$ = 105 > ^renesas,pcie-rcar-gen[0-9]$ = 105 > ^renesas,pfc-[a-z0-9]+$ = 84 > ^renesas,pwm-[a-z0-9]+$ = 984 > ^renesas,qspi-[a-z0-9]+$ = 21 > ^renesas,rcar_sound-[a-z0-9]+$ = 136 > ^renesas,riic-[a-z0-9]+$ = 64 > ^renesas,rspi-[a-z0-9]+$ = 48 > ^renesas,sata-[a-z0-9]+(-es1)?$ = 38 > ^renesas,scif-[a-z0-9]+$ = 506 > ^renesas,scifa-[a-z0-9]+$ = 108 > ^renesas,scifb-[a-z0-9]+$ = 57 > ^renesas,sdhi-[a-z0-9]+$ = 294 > ^renesas,thermal-[a-z0-9]+$ = 22 > ^renesas,tmu-[a-z0-9]+$ = 298 > ^renesas,tpu-[a-z0-9]+$ = 44 > ^renesas,usb-phy-[a-z0-9]+$ = 18 > ^renesas,usb2-phy-[a-z0-9]+$ = 118 > ^renesas,usbhs-[a-z0-9]+$ = 86 > ^renesas,vin-[a-z0-9]+$ = 523 > ^renesas,xhci-[a-z0-9]+$ = 59 > renesas,bsc-r8a73a4 = 1 > renesas,bsc-sh73a0 = 1 > renesas,dbsc-r8a73a4 = 2 > renesas,dbsc3-r8a7740 = 1 > renesas,em-gio = 5 > renesas,em-sti = 1 > renesas,em-uart = 5 > renesas,fsi2-r8a7740 = 1 > renesas,fsi2-sh73a0 = 1 > renesas,hspi-r8a7778 = 3 > renesas,hspi-r8a7779 = 3 > renesas,imr-lx4 = 36 > renesas,mtu2-r7s72100 = 3 > renesas,rmobile-iic = 116 > renesas,sbsc-sh73a0 = 2 > renesas,sdhi-mmc-r8a77470 = 1 > renesas,shmobile-flctl-sh7372 = 0 > renesas,sysc-r8a73a4 = 1 > renesas,sysc-r8a7740 = 1 > renesas,sysc-rmobile = 3 > renesas,sysc-sh73a0 = 1 > renesas,usb-dmac = 144 > > This do not include the examples in the bindings YAML files. So there is > on hit, renesas,shmobile-flctl-sh7372, that is not used in an upstream > DTS. > --- > .../bindings/soc/renesas/renesas-soc.yaml | 126 ++++++++++++++++++ > 1 file changed, 126 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/renesas/renesas-soc.yaml > > diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas-soc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas-soc.yaml > new file mode 100644 > index 000000000000..4674c1f61c1e > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/renesas/renesas-soc.yaml > @@ -0,0 +1,126 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/soc/renesas/renesas-soc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Renesas SoC compatibles naming convention > + > +maintainers: > + - Geert Uytterhoeven <geert+renesas@glider.be> > + - Niklas Söderlund <niklas.soderlund@ragnatech.se> > + > +description: | > + Guidelines for new compatibles for SoC blocks/components. > + When adding new compatibles in new bindings, use the format:: > + renesas,SoC-IP > + > + For example:: > + renesas,r8a77965-csi2 > + > + When adding new compatibles to existing bindings, use the format in the > + existing binding, even if it contradicts the above. > + > +select: > + properties: > + compatible: > + pattern: "^renesas,.*-.*$" > + required: > + - compatible > + > +properties: > + compatible: > + oneOf: > + # Preferred naming style for compatibles of SoC components: > + - pattern: "^renesas,emev2-[a-z0-9-]+$" > + - pattern: "^renesas,r7s[0-9]+-[a-z0-9-]+$" > + - pattern: "^renesas,r8a[a-z0-9]+-[a-z0-9-]+$" > + - pattern: "^renesas,r9a[0-9]+g[a-z0-9]+-[a-z0-9-]+$" > + - pattern: "^renesas,rcar-[a-z0-9-]+$" > + - pattern: "^renesas,rz-[a-z0-9-]+$" > + - pattern: "^renesas,rza-[a-z0-9-]+$" > + - pattern: "^renesas,rza1-[a-z0-9-]+$" > + - pattern: "^renesas,rza2-[a-z0-9-]+$" > + - pattern: "^renesas,rzg2l-[a-z0-9-]+$" > + - pattern: "^renesas,rzn1[a-z0-9]*-[a-z0-9-]+$" > + - pattern: "^renesas,rzv2m-[a-z0-9-]+$" > + - pattern: "^renesas,sh-[a-z0-9-]+$" > + - pattern: "^renesas,sh7[a-z0-9]+-[a-z0-9-]+$" Why so many different patterns? Why it cannot be for example: "^renesas,rz[a-z0-9]*-[a-z0-9-]+$" to cover multiple entries? The point is not to validate the devices. Other bindings do it. The point is to have one or two patterns to enforce ordering of SoC-block. The size of this file suggests you either over-complicated the thing or there is little benefit of adding it. > + > + # SoC agnostic compatibles - new compatibles are OK: Why new compatibles are ok? > + - enum: > + - renesas,cpg-div6-clock > + - renesas,cpg-mstp-clocks > + - renesas,intc-irqpin > + - renesas,smp-sram smp-sram can have new compatibles? I am sorry, but this needs explanation... > + > + # Legacy namings - variations of existing patterns/compatibles are OK, > + # but do not add completely new entries to these: > + - pattern: "^renesas,can-[a-z0-9]+$" > + - pattern: "^renesas,dmac-[a-z0-9]+$" > + - pattern: "^renesas,du-[a-z0-9]+$" > + - pattern: "^renesas,ether-[a-z0-9]+$" > + - pattern: "^renesas,etheravb-[a-z0-9]+$" > + - pattern: "^renesas,etheravb-rcar-gen[0-9]$" > + - pattern: "^renesas,gether-[a-z0-9]+$" > + - pattern: "^renesas,gpio-[a-z0-9]+$" > + - pattern: "^renesas,hscif-[a-z0-9]+$" > + - pattern: "^renesas,i2c-[a-z0-9]+$" > + - pattern: "^renesas,iic-[a-z0-9]+$" > + - pattern: "^renesas,intc-ex-[a-z0-9]+$" > + - pattern: "^renesas,intc-irqpin-[a-z0-9]+$" > + - pattern: "^renesas,ipmmu-[a-z0-9]+$" > + - pattern: "^renesas,irqc-[a-z0-9]+$" > + - pattern: "^renesas,jpu-[a-z0-9]+$" > + - pattern: "^renesas,mmcif-[a-z0-9]+$" > + - pattern: "^renesas,msiof-[a-z0-9]+$" > + - pattern: "^renesas,pci-[a-z0-9]+$" > + - pattern: "^renesas,pci-rcar-gen[0-9]$" > + - pattern: "^renesas,pcie-[a-z0-9]+$" > + - pattern: "^renesas,pcie-rcar-gen[0-9]$" > + - pattern: "^renesas,pfc-[a-z0-9]+$" > + - pattern: "^renesas,pwm-[a-z0-9]+$" > + - pattern: "^renesas,qspi-[a-z0-9]+$" > + - pattern: "^renesas,rcar_sound-[a-z0-9]+$" > + - pattern: "^renesas,riic-[a-z0-9]+$" > + - pattern: "^renesas,rspi-[a-z0-9]+$" > + - pattern: "^renesas,sata-[a-z0-9]+(-es1)?$" > + - pattern: "^renesas,scif-[a-z0-9]+$" > + - pattern: "^renesas,scifa-[a-z0-9]+$" > + - pattern: "^renesas,scifb-[a-z0-9]+$" > + - pattern: "^renesas,sdhi-[a-z0-9]+$" > + - pattern: "^renesas,thermal-[a-z0-9]+$" > + - pattern: "^renesas,tmu-[a-z0-9]+$" > + - pattern: "^renesas,tpu-[a-z0-9]+$" > + - pattern: "^renesas,usb-phy-[a-z0-9]+$" > + - pattern: "^renesas,usb2-phy-[a-z0-9]+$" > + - pattern: "^renesas,usbhs-[a-z0-9]+$" > + - pattern: "^renesas,vin-[a-z0-9]+$" > + - pattern: "^renesas,xhci-[a-z0-9]+$" No, wait, you basically listed most of the SoC as exceptions. What SoC blocks exactly are you going to cover in such case with your rules? Best regards, Krzysztof
Hi Krzysztof, On Tue, Nov 28, 2023 at 10:51 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 26/11/2023 00:28, Niklas Söderlund wrote: > > Compatibles can come in two formats. Either "vendor,ip-soc" or > > "vendor,soc-ip". Add a DT schema file documenting Renesas preferred > > policy and enforcing it for all new compatibles, except few existing > > patterns. > > > > Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > + # Legacy namings - variations of existing patterns/compatibles are OK, > > + # but do not add completely new entries to these: > > + - pattern: "^renesas,can-[a-z0-9]+$" > > + - pattern: "^renesas,dmac-[a-z0-9]+$" > > + - pattern: "^renesas,du-[a-z0-9]+$" > > + - pattern: "^renesas,ether-[a-z0-9]+$" > > + - pattern: "^renesas,etheravb-[a-z0-9]+$" > > + - pattern: "^renesas,etheravb-rcar-gen[0-9]$" > > + - pattern: "^renesas,gether-[a-z0-9]+$" > > + - pattern: "^renesas,gpio-[a-z0-9]+$" > > + - pattern: "^renesas,hscif-[a-z0-9]+$" > > + - pattern: "^renesas,i2c-[a-z0-9]+$" > > + - pattern: "^renesas,iic-[a-z0-9]+$" > > + - pattern: "^renesas,intc-ex-[a-z0-9]+$" > > + - pattern: "^renesas,intc-irqpin-[a-z0-9]+$" > > + - pattern: "^renesas,ipmmu-[a-z0-9]+$" > > + - pattern: "^renesas,irqc-[a-z0-9]+$" > > + - pattern: "^renesas,jpu-[a-z0-9]+$" > > + - pattern: "^renesas,mmcif-[a-z0-9]+$" > > + - pattern: "^renesas,msiof-[a-z0-9]+$" > > + - pattern: "^renesas,pci-[a-z0-9]+$" > > + - pattern: "^renesas,pci-rcar-gen[0-9]$" > > + - pattern: "^renesas,pcie-[a-z0-9]+$" > > + - pattern: "^renesas,pcie-rcar-gen[0-9]$" > > + - pattern: "^renesas,pfc-[a-z0-9]+$" > > + - pattern: "^renesas,pwm-[a-z0-9]+$" > > + - pattern: "^renesas,qspi-[a-z0-9]+$" > > + - pattern: "^renesas,rcar_sound-[a-z0-9]+$" > > + - pattern: "^renesas,riic-[a-z0-9]+$" > > + - pattern: "^renesas,rspi-[a-z0-9]+$" > > + - pattern: "^renesas,sata-[a-z0-9]+(-es1)?$" > > + - pattern: "^renesas,scif-[a-z0-9]+$" > > + - pattern: "^renesas,scifa-[a-z0-9]+$" > > + - pattern: "^renesas,scifb-[a-z0-9]+$" > > + - pattern: "^renesas,sdhi-[a-z0-9]+$" > > + - pattern: "^renesas,thermal-[a-z0-9]+$" > > + - pattern: "^renesas,tmu-[a-z0-9]+$" > > + - pattern: "^renesas,tpu-[a-z0-9]+$" > > + - pattern: "^renesas,usb-phy-[a-z0-9]+$" > > + - pattern: "^renesas,usb2-phy-[a-z0-9]+$" > > + - pattern: "^renesas,usbhs-[a-z0-9]+$" > > + - pattern: "^renesas,vin-[a-z0-9]+$" > > + - pattern: "^renesas,xhci-[a-z0-9]+$" > > No, wait, you basically listed most of the SoC as exceptions. What SoC > blocks exactly are you going to cover in such case with your rules? As Renesas was an early adopter of DT, there are a lot of compatible values that do not follow current best practices. Unfortunately there is not much we can do about that... Gr{oetje,eeting}s, Geert
On 28/11/2023 11:32, Geert Uytterhoeven wrote: > Hi Krzysztof, > > On Tue, Nov 28, 2023 at 10:51 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> On 26/11/2023 00:28, Niklas Söderlund wrote: >>> Compatibles can come in two formats. Either "vendor,ip-soc" or >>> "vendor,soc-ip". Add a DT schema file documenting Renesas preferred >>> policy and enforcing it for all new compatibles, except few existing >>> patterns. >>> >>> Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > >>> + # Legacy namings - variations of existing patterns/compatibles are OK, >>> + # but do not add completely new entries to these: >>> + - pattern: "^renesas,can-[a-z0-9]+$" >>> + - pattern: "^renesas,dmac-[a-z0-9]+$" >>> + - pattern: "^renesas,du-[a-z0-9]+$" >>> + - pattern: "^renesas,ether-[a-z0-9]+$" >>> + - pattern: "^renesas,etheravb-[a-z0-9]+$" >>> + - pattern: "^renesas,etheravb-rcar-gen[0-9]$" >>> + - pattern: "^renesas,gether-[a-z0-9]+$" >>> + - pattern: "^renesas,gpio-[a-z0-9]+$" >>> + - pattern: "^renesas,hscif-[a-z0-9]+$" >>> + - pattern: "^renesas,i2c-[a-z0-9]+$" >>> + - pattern: "^renesas,iic-[a-z0-9]+$" >>> + - pattern: "^renesas,intc-ex-[a-z0-9]+$" >>> + - pattern: "^renesas,intc-irqpin-[a-z0-9]+$" >>> + - pattern: "^renesas,ipmmu-[a-z0-9]+$" >>> + - pattern: "^renesas,irqc-[a-z0-9]+$" >>> + - pattern: "^renesas,jpu-[a-z0-9]+$" >>> + - pattern: "^renesas,mmcif-[a-z0-9]+$" >>> + - pattern: "^renesas,msiof-[a-z0-9]+$" >>> + - pattern: "^renesas,pci-[a-z0-9]+$" >>> + - pattern: "^renesas,pci-rcar-gen[0-9]$" >>> + - pattern: "^renesas,pcie-[a-z0-9]+$" >>> + - pattern: "^renesas,pcie-rcar-gen[0-9]$" >>> + - pattern: "^renesas,pfc-[a-z0-9]+$" >>> + - pattern: "^renesas,pwm-[a-z0-9]+$" >>> + - pattern: "^renesas,qspi-[a-z0-9]+$" >>> + - pattern: "^renesas,rcar_sound-[a-z0-9]+$" >>> + - pattern: "^renesas,riic-[a-z0-9]+$" >>> + - pattern: "^renesas,rspi-[a-z0-9]+$" >>> + - pattern: "^renesas,sata-[a-z0-9]+(-es1)?$" >>> + - pattern: "^renesas,scif-[a-z0-9]+$" >>> + - pattern: "^renesas,scifa-[a-z0-9]+$" >>> + - pattern: "^renesas,scifb-[a-z0-9]+$" >>> + - pattern: "^renesas,sdhi-[a-z0-9]+$" >>> + - pattern: "^renesas,thermal-[a-z0-9]+$" >>> + - pattern: "^renesas,tmu-[a-z0-9]+$" >>> + - pattern: "^renesas,tpu-[a-z0-9]+$" >>> + - pattern: "^renesas,usb-phy-[a-z0-9]+$" >>> + - pattern: "^renesas,usb2-phy-[a-z0-9]+$" >>> + - pattern: "^renesas,usbhs-[a-z0-9]+$" >>> + - pattern: "^renesas,vin-[a-z0-9]+$" >>> + - pattern: "^renesas,xhci-[a-z0-9]+$" >> >> No, wait, you basically listed most of the SoC as exceptions. What SoC >> blocks exactly are you going to cover in such case with your rules? > > As Renesas was an early adopter of DT, there are a lot of compatible > values that do not follow current best practices. > Unfortunately there is not much we can do about that... > Hm, ok, given how many exceptions you have, just please consider whether this schema will be of any use. IOW, how many of new SoC blocks you have which are not covered by the exceptions? Best regards, Krzysztof
Hi Krzysztof, On 2023-11-28 10:51:03 +0100, Krzysztof Kozlowski wrote: > > +properties: > > + compatible: > > + oneOf: > > + # Preferred naming style for compatibles of SoC components: > > + - pattern: "^renesas,emev2-[a-z0-9-]+$" > > + - pattern: "^renesas,r7s[0-9]+-[a-z0-9-]+$" > > + - pattern: "^renesas,r8a[a-z0-9]+-[a-z0-9-]+$" > > + - pattern: "^renesas,r9a[0-9]+g[a-z0-9]+-[a-z0-9-]+$" > > + - pattern: "^renesas,rcar-[a-z0-9-]+$" > > + - pattern: "^renesas,rz-[a-z0-9-]+$" > > + - pattern: "^renesas,rza-[a-z0-9-]+$" > > + - pattern: "^renesas,rza1-[a-z0-9-]+$" > > + - pattern: "^renesas,rza2-[a-z0-9-]+$" > > + - pattern: "^renesas,rzg2l-[a-z0-9-]+$" > > + - pattern: "^renesas,rzn1[a-z0-9]*-[a-z0-9-]+$" > > + - pattern: "^renesas,rzv2m-[a-z0-9-]+$" > > + - pattern: "^renesas,sh-[a-z0-9-]+$" > > + - pattern: "^renesas,sh7[a-z0-9]+-[a-z0-9-]+$" > > Why so many different patterns? Why it cannot be for example: > "^renesas,rz[a-z0-9]*-[a-z0-9-]+$" to cover multiple entries? > > The point is not to validate the devices. Other bindings do it. The > point is to have one or two patterns to enforce ordering of SoC-block. > > The size of this file suggests you either over-complicated the thing or > there is little benefit of adding it. As you point out below there is a lot of patterns that use the style not preferred and the idea to detect future additions of this I thought it a good idea to make these more specialized. If we think that is a bad idea I can try to make fewer more generic ones. > > > + > > + # SoC agnostic compatibles - new compatibles are OK: > > Why new compatibles are ok? > > > + - enum: > > + - renesas,cpg-div6-clock > > + - renesas,cpg-mstp-clocks > > + - renesas,intc-irqpin > > + - renesas,smp-sram > > smp-sram can have new compatibles? I am sorry, but this needs explanation... The intention is to list SoC agnostic compatibles here, or put another way false positives to the generic pattern "renesas,.*-.*". So no "renesas,smp-sram" can't have new compatibles but there might be new renesas compatible strings that hit the pattern that is not related to a SoC. Does this make sens? > > > + > > + # Legacy namings - variations of existing patterns/compatibles are OK, > > + # but do not add completely new entries to these: > > + - pattern: "^renesas,can-[a-z0-9]+$" > > + - pattern: "^renesas,dmac-[a-z0-9]+$" > > + - pattern: "^renesas,du-[a-z0-9]+$" > > + - pattern: "^renesas,ether-[a-z0-9]+$" > > + - pattern: "^renesas,etheravb-[a-z0-9]+$" > > + - pattern: "^renesas,etheravb-rcar-gen[0-9]$" > > + - pattern: "^renesas,gether-[a-z0-9]+$" > > + - pattern: "^renesas,gpio-[a-z0-9]+$" > > + - pattern: "^renesas,hscif-[a-z0-9]+$" > > + - pattern: "^renesas,i2c-[a-z0-9]+$" > > + - pattern: "^renesas,iic-[a-z0-9]+$" > > + - pattern: "^renesas,intc-ex-[a-z0-9]+$" > > + - pattern: "^renesas,intc-irqpin-[a-z0-9]+$" > > + - pattern: "^renesas,ipmmu-[a-z0-9]+$" > > + - pattern: "^renesas,irqc-[a-z0-9]+$" > > + - pattern: "^renesas,jpu-[a-z0-9]+$" > > + - pattern: "^renesas,mmcif-[a-z0-9]+$" > > + - pattern: "^renesas,msiof-[a-z0-9]+$" > > + - pattern: "^renesas,pci-[a-z0-9]+$" > > + - pattern: "^renesas,pci-rcar-gen[0-9]$" > > + - pattern: "^renesas,pcie-[a-z0-9]+$" > > + - pattern: "^renesas,pcie-rcar-gen[0-9]$" > > + - pattern: "^renesas,pfc-[a-z0-9]+$" > > + - pattern: "^renesas,pwm-[a-z0-9]+$" > > + - pattern: "^renesas,qspi-[a-z0-9]+$" > > + - pattern: "^renesas,rcar_sound-[a-z0-9]+$" > > + - pattern: "^renesas,riic-[a-z0-9]+$" > > + - pattern: "^renesas,rspi-[a-z0-9]+$" > > + - pattern: "^renesas,sata-[a-z0-9]+(-es1)?$" > > + - pattern: "^renesas,scif-[a-z0-9]+$" > > + - pattern: "^renesas,scifa-[a-z0-9]+$" > > + - pattern: "^renesas,scifb-[a-z0-9]+$" > > + - pattern: "^renesas,sdhi-[a-z0-9]+$" > > + - pattern: "^renesas,thermal-[a-z0-9]+$" > > + - pattern: "^renesas,tmu-[a-z0-9]+$" > > + - pattern: "^renesas,tpu-[a-z0-9]+$" > > + - pattern: "^renesas,usb-phy-[a-z0-9]+$" > > + - pattern: "^renesas,usb2-phy-[a-z0-9]+$" > > + - pattern: "^renesas,usbhs-[a-z0-9]+$" > > + - pattern: "^renesas,vin-[a-z0-9]+$" > > + - pattern: "^renesas,xhci-[a-z0-9]+$" > > No, wait, you basically listed most of the SoC as exceptions. What SoC > blocks exactly are you going to cover in such case with your rules? As Geert points out these exists for historical reasons, but we don't want any more of this style. You ask in your reply to Geert that we should reconsider if this is still useful. I think it is as it achieves the over all goal, detect if any new of the not preferred style is added. But I won't press it, if you or Geert think this is a bad route I won't spend more time on this work. @Geert: What do you think?
Hi Krzysztof, On Tue, Nov 28, 2023 at 3:32 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 28/11/2023 11:32, Geert Uytterhoeven wrote: > > On Tue, Nov 28, 2023 at 10:51 AM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> On 26/11/2023 00:28, Niklas Söderlund wrote: > >>> Compatibles can come in two formats. Either "vendor,ip-soc" or > >>> "vendor,soc-ip". Add a DT schema file documenting Renesas preferred > >>> policy and enforcing it for all new compatibles, except few existing > >>> patterns. > >>> > >>> Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > >>> + # Legacy namings - variations of existing patterns/compatibles are OK, > >>> + # but do not add completely new entries to these: > >>> + - pattern: "^renesas,can-[a-z0-9]+$" [ deleted 40 legacy patterns] > >> No, wait, you basically listed most of the SoC as exceptions. What SoC > >> blocks exactly are you going to cover in such case with your rules? > > > > As Renesas was an early adopter of DT, there are a lot of compatible > > values that do not follow current best practices. > > Unfortunately there is not much we can do about that... > > Hm, ok, given how many exceptions you have, just please consider whether > this schema will be of any use. IOW, how many of new SoC blocks you have > which are not covered by the exceptions? Given a modern SoC hardware manual contains +200 sections, there are still lots of opportunities for new SoC block bindings and drivers... We really do not want any new compatible values of the legacy form. I guess the TI people could use a schema to reject new non-standard compatible values, too: https://lore.kernel.org/all/CAMuHMdUYOq=q1j=d+Eac28hthOUAaNUkuvxmRu-mUN1pLKq69g@mail.gmail.com/ Gr{oetje,eeting}s, Geert
On 30/11/2023 14:17, Niklas Söderlund wrote: >> >>> + - enum: >>> + - renesas,cpg-div6-clock >>> + - renesas,cpg-mstp-clocks >>> + - renesas,intc-irqpin >>> + - renesas,smp-sram >> >> smp-sram can have new compatibles? I am sorry, but this needs explanation... > > The intention is to list SoC agnostic compatibles here, or put another > way false positives to the generic pattern "renesas,.*-.*". So no > "renesas,smp-sram" can't have new compatibles but there might be new > renesas compatible strings that hit the pattern that is not related to a > SoC. Does this make sens? Well, now I get the meaning, but I don't find this obvious especially considering that contributors many, many times misunderstand "SoC agnostic" with "generic is OK". Generic compatible is not OK for the SoC. I think it is still the most repeated feedback from me. I hope you will be explaining this for Renesas contributors :) Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas-soc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas-soc.yaml new file mode 100644 index 000000000000..4674c1f61c1e --- /dev/null +++ b/Documentation/devicetree/bindings/soc/renesas/renesas-soc.yaml @@ -0,0 +1,126 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/renesas/renesas-soc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Renesas SoC compatibles naming convention + +maintainers: + - Geert Uytterhoeven <geert+renesas@glider.be> + - Niklas Söderlund <niklas.soderlund@ragnatech.se> + +description: | + Guidelines for new compatibles for SoC blocks/components. + When adding new compatibles in new bindings, use the format:: + renesas,SoC-IP + + For example:: + renesas,r8a77965-csi2 + + When adding new compatibles to existing bindings, use the format in the + existing binding, even if it contradicts the above. + +select: + properties: + compatible: + pattern: "^renesas,.*-.*$" + required: + - compatible + +properties: + compatible: + oneOf: + # Preferred naming style for compatibles of SoC components: + - pattern: "^renesas,emev2-[a-z0-9-]+$" + - pattern: "^renesas,r7s[0-9]+-[a-z0-9-]+$" + - pattern: "^renesas,r8a[a-z0-9]+-[a-z0-9-]+$" + - pattern: "^renesas,r9a[0-9]+g[a-z0-9]+-[a-z0-9-]+$" + - pattern: "^renesas,rcar-[a-z0-9-]+$" + - pattern: "^renesas,rz-[a-z0-9-]+$" + - pattern: "^renesas,rza-[a-z0-9-]+$" + - pattern: "^renesas,rza1-[a-z0-9-]+$" + - pattern: "^renesas,rza2-[a-z0-9-]+$" + - pattern: "^renesas,rzg2l-[a-z0-9-]+$" + - pattern: "^renesas,rzn1[a-z0-9]*-[a-z0-9-]+$" + - pattern: "^renesas,rzv2m-[a-z0-9-]+$" + - pattern: "^renesas,sh-[a-z0-9-]+$" + - pattern: "^renesas,sh7[a-z0-9]+-[a-z0-9-]+$" + + # SoC agnostic compatibles - new compatibles are OK: + - enum: + - renesas,cpg-div6-clock + - renesas,cpg-mstp-clocks + - renesas,intc-irqpin + - renesas,smp-sram + + # Legacy namings - variations of existing patterns/compatibles are OK, + # but do not add completely new entries to these: + - pattern: "^renesas,can-[a-z0-9]+$" + - pattern: "^renesas,dmac-[a-z0-9]+$" + - pattern: "^renesas,du-[a-z0-9]+$" + - pattern: "^renesas,ether-[a-z0-9]+$" + - pattern: "^renesas,etheravb-[a-z0-9]+$" + - pattern: "^renesas,etheravb-rcar-gen[0-9]$" + - pattern: "^renesas,gether-[a-z0-9]+$" + - pattern: "^renesas,gpio-[a-z0-9]+$" + - pattern: "^renesas,hscif-[a-z0-9]+$" + - pattern: "^renesas,i2c-[a-z0-9]+$" + - pattern: "^renesas,iic-[a-z0-9]+$" + - pattern: "^renesas,intc-ex-[a-z0-9]+$" + - pattern: "^renesas,intc-irqpin-[a-z0-9]+$" + - pattern: "^renesas,ipmmu-[a-z0-9]+$" + - pattern: "^renesas,irqc-[a-z0-9]+$" + - pattern: "^renesas,jpu-[a-z0-9]+$" + - pattern: "^renesas,mmcif-[a-z0-9]+$" + - pattern: "^renesas,msiof-[a-z0-9]+$" + - pattern: "^renesas,pci-[a-z0-9]+$" + - pattern: "^renesas,pci-rcar-gen[0-9]$" + - pattern: "^renesas,pcie-[a-z0-9]+$" + - pattern: "^renesas,pcie-rcar-gen[0-9]$" + - pattern: "^renesas,pfc-[a-z0-9]+$" + - pattern: "^renesas,pwm-[a-z0-9]+$" + - pattern: "^renesas,qspi-[a-z0-9]+$" + - pattern: "^renesas,rcar_sound-[a-z0-9]+$" + - pattern: "^renesas,riic-[a-z0-9]+$" + - pattern: "^renesas,rspi-[a-z0-9]+$" + - pattern: "^renesas,sata-[a-z0-9]+(-es1)?$" + - pattern: "^renesas,scif-[a-z0-9]+$" + - pattern: "^renesas,scifa-[a-z0-9]+$" + - pattern: "^renesas,scifb-[a-z0-9]+$" + - pattern: "^renesas,sdhi-[a-z0-9]+$" + - pattern: "^renesas,thermal-[a-z0-9]+$" + - pattern: "^renesas,tmu-[a-z0-9]+$" + - pattern: "^renesas,tpu-[a-z0-9]+$" + - pattern: "^renesas,usb-phy-[a-z0-9]+$" + - pattern: "^renesas,usb2-phy-[a-z0-9]+$" + - pattern: "^renesas,usbhs-[a-z0-9]+$" + - pattern: "^renesas,vin-[a-z0-9]+$" + - pattern: "^renesas,xhci-[a-z0-9]+$" + + # Legacy compatibles - list cannot grow with new bindings: + - enum: + - renesas,bsc-r8a73a4 + - renesas,bsc-sh73a0 + - renesas,dbsc-r8a73a4 + - renesas,dbsc3-r8a7740 + - renesas,em-gio + - renesas,em-sti + - renesas,em-uart + - renesas,fsi2-r8a7740 + - renesas,fsi2-sh73a0 + - renesas,hspi-r8a7778 + - renesas,hspi-r8a7779 + - renesas,imr-lx4 + - renesas,mtu2-r7s72100 + - renesas,rmobile-iic + - renesas,sbsc-sh73a0 + - renesas,sdhi-mmc-r8a77470 + - renesas,shmobile-flctl-sh7372 + - renesas,sysc-r8a73a4 + - renesas,sysc-r8a7740 + - renesas,sysc-rmobile + - renesas,sysc-sh73a0 + - renesas,usb-dmac + +additionalProperties: true
Compatibles can come in two formats. Either "vendor,ip-soc" or "vendor,soc-ip". Add a DT schema file documenting Renesas preferred policy and enforcing it for all new compatibles, except few existing patterns. Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- * Changes since RFC - Moved to Documentation/devicetree/bindings/soc/renesas. - Changed the pattern in the initial select to match on .*-.*. - Added a lot of missing compatible values. Compared to the RFC I did not use make dt_binding_check and manual reading of the bindings to find all compatibles. Instead I generated YAML of all renesas related files under arch/arm and arc/arm64 and parsed all the compatible values. For v6.7-rc2 the hits on each pattern where, ^renesas,emev2-[a-z0-9-]+$ = 14 ^renesas,r7s[0-9]+-[a-z0-9-]+$ = 58 ^renesas,r8a[a-z0-9]+-[a-z0-9-]+$ = 1661 ^renesas,r9a[0-9]+g[a-z0-9]+-[a-z0-9-]+$ = 160 ^renesas,rcar-[a-z0-9-]+$ = 4522 ^renesas,rz-[a-z0-9-]+$ = 26 ^renesas,rza-[a-z0-9-]+$ = 4 ^renesas,rza1-[a-z0-9-]+$ = 10 ^renesas,rza2-[a-z0-9-]+$ = 6 ^renesas,rzg2l-[a-z0-9-]+$ = 54 ^renesas,rzn1[a-z0-9]*-[a-z0-9-]+$ = 22 ^renesas,rzv2m-[a-z0-9-]+$ = 9 ^renesas,sh-[a-z0-9-]+$ = 36 ^renesas,sh7[a-z0-9]+-[a-z0-9-]+$ = 27 renesas,cpg-div6-clock = 45 renesas,cpg-mstp-clocks = 49 renesas,intc-irqpin = 20 renesas,smp-sram = 20 ^renesas,can-[a-z0-9]+$ = 136 ^renesas,dmac-[a-z0-9]+$ = 326 ^renesas,du-[a-z0-9]+$ = 77 ^renesas,ether-[a-z0-9]+$ = 21 ^renesas,etheravb-[a-z0-9]+$ = 84 ^renesas,etheravb-rcar-gen[0-9]$ = 82 ^renesas,gether-[a-z0-9]+$ = 4 ^renesas,gpio-[a-z0-9]+$ = 608 ^renesas,hscif-[a-z0-9]+$ = 336 ^renesas,i2c-[a-z0-9]+$ = 517 ^renesas,iic-[a-z0-9]+$ = 118 ^renesas,intc-ex-[a-z0-9]+$ = 58 ^renesas,intc-irqpin-[a-z0-9]+$ = 10 ^renesas,ipmmu-[a-z0-9]+$ = 828 ^renesas,irqc-[a-z0-9]+$ = 22 ^renesas,jpu-[a-z0-9]+$ = 6 ^renesas,mmcif-[a-z0-9]+$ = 29 ^renesas,msiof-[a-z0-9]+$ = 290 ^renesas,pci-[a-z0-9]+$ = 38 ^renesas,pci-rcar-gen[0-9]$ = 36 ^renesas,pcie-[a-z0-9]+$ = 105 ^renesas,pcie-rcar-gen[0-9]$ = 105 ^renesas,pfc-[a-z0-9]+$ = 84 ^renesas,pwm-[a-z0-9]+$ = 984 ^renesas,qspi-[a-z0-9]+$ = 21 ^renesas,rcar_sound-[a-z0-9]+$ = 136 ^renesas,riic-[a-z0-9]+$ = 64 ^renesas,rspi-[a-z0-9]+$ = 48 ^renesas,sata-[a-z0-9]+(-es1)?$ = 38 ^renesas,scif-[a-z0-9]+$ = 506 ^renesas,scifa-[a-z0-9]+$ = 108 ^renesas,scifb-[a-z0-9]+$ = 57 ^renesas,sdhi-[a-z0-9]+$ = 294 ^renesas,thermal-[a-z0-9]+$ = 22 ^renesas,tmu-[a-z0-9]+$ = 298 ^renesas,tpu-[a-z0-9]+$ = 44 ^renesas,usb-phy-[a-z0-9]+$ = 18 ^renesas,usb2-phy-[a-z0-9]+$ = 118 ^renesas,usbhs-[a-z0-9]+$ = 86 ^renesas,vin-[a-z0-9]+$ = 523 ^renesas,xhci-[a-z0-9]+$ = 59 renesas,bsc-r8a73a4 = 1 renesas,bsc-sh73a0 = 1 renesas,dbsc-r8a73a4 = 2 renesas,dbsc3-r8a7740 = 1 renesas,em-gio = 5 renesas,em-sti = 1 renesas,em-uart = 5 renesas,fsi2-r8a7740 = 1 renesas,fsi2-sh73a0 = 1 renesas,hspi-r8a7778 = 3 renesas,hspi-r8a7779 = 3 renesas,imr-lx4 = 36 renesas,mtu2-r7s72100 = 3 renesas,rmobile-iic = 116 renesas,sbsc-sh73a0 = 2 renesas,sdhi-mmc-r8a77470 = 1 renesas,shmobile-flctl-sh7372 = 0 renesas,sysc-r8a73a4 = 1 renesas,sysc-r8a7740 = 1 renesas,sysc-rmobile = 3 renesas,sysc-sh73a0 = 1 renesas,usb-dmac = 144 This do not include the examples in the bindings YAML files. So there is on hit, renesas,shmobile-flctl-sh7372, that is not used in an upstream DTS. --- .../bindings/soc/renesas/renesas-soc.yaml | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/renesas/renesas-soc.yaml