linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 05/17] dt-bindings: PCI: dwc: Stop selecting generic bindings by default
       [not found] <20220728143427.13617-1-Sergey.Semin@baikalelectronics.ru>
@ 2022-07-28 14:34 ` Serge Semin
  2022-07-28 22:37   ` Rob Herring
  0 siblings, 1 reply; 2+ messages in thread
From: Serge Semin @ 2022-07-28 14:34 UTC (permalink / raw)
  To: Rob Herring, Rob Herring, Krzysztof Kozlowski, Bjorn Helgaas,
	Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel, Richard Zhu,
	Lucas Stach, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, Xiaowei Song, Binghui Wang,
	Paul Walmsley, Greentime Hu, Palmer Dabbelt, Kunihiko Hayashi,
	Masami Hiramatsu, Nobuhiro Iwamatsu
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Krzysztof Wilczyński, Frank Li, Manivannan Sadhasivam,
	linux-pci, devicetree, linux-kernel, linux-arm-kernel,
	linux-riscv

It's highly encouraged to have the separate DT schema for each available
particular device, while the generic schema should be left untouched
representing just a set of the common device properties (mainly advertised
by the IP-core reference manual). Seeing there is no currently DW PCIe
RP/EP dts nodes with only generic compatible string and since there isn't
any vendor-specific compatible string added to the generic DT schema,
before it's too late let's mark the snps,dw-pcie.yaml and
snps,dw-pcie-ep.yaml schemas not selected for checking by default and add
the explicit requirement to have the compatible string containing the
generic device name.

Note due to this modification we need to switch some of the DW PCIe-based
DT-bindings to referring to the common DT-schema instead of evaluating
against the generic DW PCIe DT-bindings. They are already defined as
having the vendor-specific compatible string only. So we can't change that
semantic.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

---

Changelog v3:
- This is a new patch unpinned from the next one:
  https://lore.kernel.org/linux-pci/20220503214638.1895-2-Sergey.Semin@baikalelectronics.ru/
  by the Rob' request. (@Rob)
- Fix compatible property schema so one would work as expected: string
  must contain either generic DW PCIe IP-core name or both generic and
  equipped with IP-core version names.
---
 .../bindings/pci/fsl,imx6q-pcie.yaml          |  3 ++-
 .../bindings/pci/hisilicon,kirin-pcie.yaml    |  3 ++-
 .../bindings/pci/sifive,fu740-pcie.yaml       |  3 ++-
 .../bindings/pci/snps,dw-pcie-ep.yaml         | 24 +++++++++++++++----
 .../devicetree/bindings/pci/snps,dw-pcie.yaml | 24 +++++++++++++++----
 .../pci/socionext,uniphier-pcie-ep.yaml       |  9 +++----
 .../bindings/pci/toshiba,visconti-pcie.yaml   |  3 ++-
 7 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index 252e5b72aee0..6f99baa445a6 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -15,7 +15,8 @@ description: |+
   and thus inherits all the common properties defined in snps,dw-pcie.yaml.
 
 allOf:
-  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+  - $ref: /schemas/pci/pci-bus.yaml#
+  - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
 
 properties:
   compatible:
diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
index c9f04999c9cf..f0d5314f340f 100644
--- a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
@@ -17,7 +17,8 @@ description: |
   Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml.
 
 allOf:
-  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+  - $ref: /schemas/pci/pci-bus.yaml#
+  - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
 
 properties:
   compatible:
diff --git a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
index 195e6afeb169..b0cf8ce99ce3 100644
--- a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
@@ -17,7 +17,8 @@ maintainers:
   - Greentime Hu <greentime.hu@sifive.com>
 
 allOf:
-  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+  - $ref: /schemas/pci/pci-bus.yaml#
+  - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
 
 properties:
   compatible:
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
index fc3b5d4ac245..b04ce7ddb796 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
@@ -13,6 +13,12 @@ maintainers:
 description: |
   Synopsys DesignWare PCIe host controller endpoint
 
+# Please create a separate DT-schema for the particular DWC PCIe Endpoint
+# controller and make sure it's assigned with the vendor-specific
+# compatible string together with the generic Synopsys DWC PCIe strings so
+# the bindings would be evaluated against that schema.
+select: false
+
 allOf:
   - $ref: /schemas/pci/pci-ep.yaml#
   - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
@@ -20,8 +26,18 @@ allOf:
 properties:
   compatible:
     anyOf:
-      - {}
-      - const: snps,dw-pcie-ep
+      - description:
+          DWC PCIe Endpoint controller (IP-core version is explicitly
+          specified in the additional compatible string)
+        contains:
+          allOf:
+            - pattern: '^snps,dw-pcie-ep-[0-9]+\.[0-9]+a?$'
+            - const: snps,dw-pcie-ep
+      - description:
+          DWC PCIe Endpoint controller (IP-core version is either unknown
+          or can be read from the PCIe version register of the PL reg-space)
+        contains:
+          const: snps,dw-pcie-ep
 
   reg:
     description: |
@@ -38,16 +54,16 @@ properties:
       enum: [dbi, dbi2, config, atu, addr_space, link, atu_dma, appl]
 
 required:
+  - compatible
   - reg
   - reg-names
-  - compatible
 
 additionalProperties: true
 
 examples:
   - |
     pcie-ep@dfd00000 {
-      compatible = "snps,dw-pcie-ep";
+      compatible = "vendor,soc-pcie", "snps,dw-pcie-ep";
       reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
             <0xdfc01000 0x0001000>, /* IP registers 2 */
             <0xd0000000 0x2000000>; /* Configuration space */
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 01cedf51e0f8..8b2e3210e3e2 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -13,6 +13,12 @@ maintainers:
 description: |
   Synopsys DesignWare PCIe host controller
 
+# Please create a separate DT-schema for the particular DWC PCIe Root Port
+# controller and make sure it's assigned with the vendor-specific
+# compatible string together with the generic Synopsys DWC PCIe strings so
+# the bindings would be evaluated against that schema.
+select: false
+
 allOf:
   - $ref: /schemas/pci/pci-bus.yaml#
   - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
@@ -20,8 +26,18 @@ allOf:
 properties:
   compatible:
     anyOf:
-      - {}
-      - const: snps,dw-pcie
+      - description:
+          DWC PCIe Root Port controller (IP-core version is explicitly
+          specified in the additional compatible string)
+        contains:
+          allOf:
+            - pattern: '^snps,dw-pcie-[0-9]+\.[0-9]+a?$'
+            - const: snps,dw-pcie
+      - description:
+          DWC PCIe Root Port controller (IP-core version is either unknown
+          or can be read from the PCIe version register of the PL reg-space)
+        contains:
+          const: snps,dw-pcie
 
   reg:
     description: |
@@ -47,14 +63,14 @@ properties:
 additionalProperties: true
 
 required:
+  - compatible
   - reg
   - reg-names
-  - compatible
 
 examples:
   - |
     pcie@dfc00000 {
-      compatible = "snps,dw-pcie";
+      compatible = "vendor,soc-pcie", "snps,dw-pcie";
       device_type = "pci";
       reg = <0xdfc00000 0x0001000>, /* IP registers */
             <0xd0000000 0x0002000>; /* Configuration space */
diff --git a/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml
index 437e61618d06..1719a36952c0 100644
--- a/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml
@@ -20,9 +20,10 @@ allOf:
 
 properties:
   compatible:
-    enum:
-      - socionext,uniphier-pro5-pcie-ep
-      - socionext,uniphier-nx1-pcie-ep
+    contains:
+      enum:
+        - socionext,uniphier-pro5-pcie-ep
+        - socionext,uniphier-nx1-pcie-ep
 
   reg:
     minItems: 4
@@ -92,7 +93,7 @@ unevaluatedProperties: false
 examples:
   - |
     pcie_ep: pcie-ep@66000000 {
-        compatible = "socionext,uniphier-pro5-pcie-ep";
+        compatible = "socionext,uniphier-pro5-pcie-ep", "snps,dw-pcie-ep";
         reg-names = "dbi", "dbi2", "link", "addr_space";
         reg = <0x66000000 0x1000>, <0x66001000 0x1000>,
               <0x66010000 0x10000>, <0x67000000 0x400000>;
diff --git a/Documentation/devicetree/bindings/pci/toshiba,visconti-pcie.yaml b/Documentation/devicetree/bindings/pci/toshiba,visconti-pcie.yaml
index 30b6396d83c8..a08002ce9119 100644
--- a/Documentation/devicetree/bindings/pci/toshiba,visconti-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/toshiba,visconti-pcie.yaml
@@ -13,7 +13,8 @@ description:
   Toshiba Visconti5 SoC PCIe host controller is based on the Synopsys DesignWare PCIe IP.
 
 allOf:
-  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+  - $ref: /schemas/pci/pci-bus.yaml#
+  - $ref: /schemas/pci/snps,dw-pcie-common.yaml#
 
 properties:
   compatible:
-- 
2.35.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v4 05/17] dt-bindings: PCI: dwc: Stop selecting generic bindings by default
  2022-07-28 14:34 ` [PATCH v4 05/17] dt-bindings: PCI: dwc: Stop selecting generic bindings by default Serge Semin
@ 2022-07-28 22:37   ` Rob Herring
  0 siblings, 0 replies; 2+ messages in thread
From: Rob Herring @ 2022-07-28 22:37 UTC (permalink / raw)
  To: Serge Semin
  Cc: Alexey Malahov, Masami Hiramatsu, linux-kernel, Richard Zhu,
	Paul Walmsley, Sascha Hauer, Palmer Dabbelt, Frank Li,
	Binghui Wang, linux-arm-kernel, Pengutronix Kernel Team,
	Krzysztof Wilczyński, Lorenzo Pieralisi, Gustavo Pimentel,
	devicetree, NXP Linux Team, Rob Herring, Fabio Estevam,
	Serge Semin, Nobuhiro Iwamatsu, linux-pci, Greentime Hu,
	Krzysztof Kozlowski, linux-riscv, Bjorn Helgaas, Shawn Guo,
	Manivannan Sadhasivam, Lucas Stach, Xiaowei Song, Jingoo Han,
	Pavel Parkhomenko, Kunihiko Hayashi

On Thu, 28 Jul 2022 17:34:15 +0300, Serge Semin wrote:
> It's highly encouraged to have the separate DT schema for each available
> particular device, while the generic schema should be left untouched
> representing just a set of the common device properties (mainly advertised
> by the IP-core reference manual). Seeing there is no currently DW PCIe
> RP/EP dts nodes with only generic compatible string and since there isn't
> any vendor-specific compatible string added to the generic DT schema,
> before it's too late let's mark the snps,dw-pcie.yaml and
> snps,dw-pcie-ep.yaml schemas not selected for checking by default and add
> the explicit requirement to have the compatible string containing the
> generic device name.
> 
> Note due to this modification we need to switch some of the DW PCIe-based
> DT-bindings to referring to the common DT-schema instead of evaluating
> against the generic DW PCIe DT-bindings. They are already defined as
> having the vendor-specific compatible string only. So we can't change that
> semantic.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> ---
> 
> Changelog v3:
> - This is a new patch unpinned from the next one:
>   https://lore.kernel.org/linux-pci/20220503214638.1895-2-Sergey.Semin@baikalelectronics.ru/
>   by the Rob' request. (@Rob)
> - Fix compatible property schema so one would work as expected: string
>   must contain either generic DW PCIe IP-core name or both generic and
>   equipped with IP-core version names.
> ---
>  .../bindings/pci/fsl,imx6q-pcie.yaml          |  3 ++-
>  .../bindings/pci/hisilicon,kirin-pcie.yaml    |  3 ++-
>  .../bindings/pci/sifive,fu740-pcie.yaml       |  3 ++-
>  .../bindings/pci/snps,dw-pcie-ep.yaml         | 24 +++++++++++++++----
>  .../devicetree/bindings/pci/snps,dw-pcie.yaml | 24 +++++++++++++++----
>  .../pci/socionext,uniphier-pcie-ep.yaml       |  9 +++----
>  .../bindings/pci/toshiba,visconti-pcie.yaml   |  3 ++-
>  7 files changed, 53 insertions(+), 16 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.example.dtb: pcie@e00000000: Unevaluated properties are not allowed ('clocks', 'interrupts' were unexpected)
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/toshiba,visconti-pcie.example.dtb: pcie@28400000: Unevaluated properties are not allowed ('interrupt-names' was unexpected)
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/toshiba,visconti-pcie.yaml
Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.example.dtb:0:0: /example-0/pcie-ep@dfd00000: failed to match any schema with compatible: ['vendor,soc-pcie', 'snps,dw-pcie-ep']
Documentation/devicetree/bindings/pci/snps,dw-pcie.example.dtb:0:0: /example-0/pcie@dfc00000: failed to match any schema with compatible: ['vendor,soc-pcie', 'snps,dw-pcie']
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.example.dtb: pcie@15700000: compatible: 'anyOf' conditional failed, one must be fixed:
	['samsung,exynos5433-pcie'] does not contain items matching the given schema
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.example.dtb: pcie@15700000: Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'bus-range', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'ranges' were unexpected)
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie.example.dtb: pcie@66000000: compatible: 'anyOf' conditional failed, one must be fixed:
	['socionext,uniphier-pcie'] does not contain items matching the given schema
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie.example.dtb: pcie@66000000: Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'bus-range', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'interrupt-names', 'interrupts', 'ranges' were unexpected)
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dtb: pcie@f4000000: Unevaluated properties are not allowed ('interrupt-names', 'interrupts' were unexpected)
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dtb: pcie@f5000000: Unevaluated properties are not allowed ('interrupt-names', 'interrupts' were unexpected)
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-07-28 22:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20220728143427.13617-1-Sergey.Semin@baikalelectronics.ru>
2022-07-28 14:34 ` [PATCH v4 05/17] dt-bindings: PCI: dwc: Stop selecting generic bindings by default Serge Semin
2022-07-28 22:37   ` Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).