1. Patchew
  2. linux
  3. [PATCH 0/3] PCI: qcom: Binding fix
  4. View patch

[PATCH 2/3] dt-bindings: PCI: qcom: Enforce check for PHY, PERST# and WAKE# properties

Manivannan Sadhasivam posted 3 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH 2/3] dt-bindings: PCI: qcom: Enforce check for PHY, PERST# and WAKE# properties
Posted by Manivannan Sadhasivam 2 months, 1 week ago 
Currently, the binding supports specifying the PHY, PERST#, WAKE#
properties in two ways:

1. Controller node (deprecated)
	- phys
	- perst-gpios
	- wake-gpios

2. Root Port node
	- phys
	- reset-gpios
	- wake-gpios

But there is no check to make sure that the both variants are not mixed.
For instance, if the Controller node specifies 'phys', 'reset-gpios',
'wake-gpios' or if the Root Port node specifies 'phys', 'perst-gpios',
'wake-gpios', then the driver will fail as reported. Hence, enforce the
check in the binding to catch these issues.

It is also possible that DTs could have 'phys' property in Controller node
and 'reset-gpios/wake-gpios' properties in the Root Port node. It will also
be a problem, but it is not possible to catch these cross-node issues in
the binding.

Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Closes: https://lore.kernel.org/linux-pci/8f2e0631-6c59-4298-b36e-060708970ced@oss.qualcomm.com
Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 .../devicetree/bindings/pci/qcom,pcie-common.yaml    | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
index 77f8faf54737e0fab089a368976290dece4f2e7d..6eaecf83d6efd37e9acb044049c1ef95611cbf58 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
@@ -111,6 +111,16 @@ patternProperties:
       phys:
         maxItems: 1
 
+    oneOf:
+      - required:
+          - phys
+          - reset-gpios
+          - wake-gpios
+      - properties:
+          phys: false
+          reset-gpios: false
+          wake-gpios: false
+
     unevaluatedProperties: false
 
 required:
@@ -129,6 +139,16 @@ anyOf:
   - required:
       - msi-map
 
+oneOf:
+  - required:
+      - phys
+      - perst-gpios
+      - wake-gpios
+  - properties:
+      phys: false
+      perst-gpios: false
+      wake-gpios: false
+
 allOf:
   - $ref: /schemas/pci/pci-host-bridge.yaml#
 

-- 
2.48.1
Re: [PATCH 2/3] dt-bindings: PCI: qcom: Enforce check for PHY, PERST# and WAKE# properties
Posted by Rob Herring (Arm) 2 months, 1 week ago 
On Fri, 10 Oct 2025 11:25:48 -0700, Manivannan Sadhasivam wrote:
> Currently, the binding supports specifying the PHY, PERST#, WAKE#
> properties in two ways:
> 
> 1. Controller node (deprecated)
> 	- phys
> 	- perst-gpios
> 	- wake-gpios
> 
> 2. Root Port node
> 	- phys
> 	- reset-gpios
> 	- wake-gpios
> 
> But there is no check to make sure that the both variants are not mixed.
> For instance, if the Controller node specifies 'phys', 'reset-gpios',
> 'wake-gpios' or if the Root Port node specifies 'phys', 'perst-gpios',
> 'wake-gpios', then the driver will fail as reported. Hence, enforce the
> check in the binding to catch these issues.
> 
> It is also possible that DTs could have 'phys' property in Controller node
> and 'reset-gpios/wake-gpios' properties in the Root Port node. It will also
> be a problem, but it is not possible to catch these cross-node issues in
> the binding.
> 
> Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Closes: https://lore.kernel.org/linux-pci/8f2e0631-6c59-4298-b36e-060708970ced@oss.qualcomm.com
> Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/pci/qcom,pcie-common.yaml    | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie-sc8180x.example.dtb: pcie@1c00000 (qcom,pcie-sc8180x): 'oneOf' conditional failed, one must be fixed:
	'perst-gpios' is a required property
	'wake-gpios' is a required property
	False schema does not allow [[4294967295]]
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-sc8180x.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie-sc8180x.example.dtb: pcie@1c00000 (qcom,pcie-sc8180x): Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'bus-range', 'device_type', 'dma-coherent', 'interconnect-names', 'interconnects', 'interrupt-map', 'interrupt-map-mask', 'iommu-map', 'linux,pci-domain', 'num-lanes', 'phy-names', 'phys', 'power-domains', 'ranges' were unexpected)
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-sc8180x.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.example.dtb: pcie@1c08000 (qcom,pcie-sc7280): pcie@0: 'oneOf' conditional failed, one must be fixed:
	'wake-gpios' is a required property
	False schema does not allow [[4294967295]]
	False schema does not allow [[4294967295, 2, 1]]
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-sc7280.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.example.dtb: pcie@1c08000 (qcom,pcie-sc7280): Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'bus-range', 'device_type', 'dma-coherent', 'interrupt-map', 'interrupt-map-mask', 'iommu-map', 'linux,pci-domain', 'num-lanes', 'pcie@0', 'power-domains', 'ranges', 'vddpe-3v3-supply' were unexpected)
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-sc7280.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20251010-pci-binding-v1-2-947c004b5699@oss.qualcomm.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH 2/3] dt-bindings: PCI: qcom: Enforce check for PHY, PERST# and WAKE# properties
Posted by Manivannan Sadhasivam 2 months, 1 week ago 
On Fri, Oct 10, 2025 at 02:33:23PM -0500, Rob Herring (Arm) wrote:
> 
> On Fri, 10 Oct 2025 11:25:48 -0700, Manivannan Sadhasivam wrote:
> > Currently, the binding supports specifying the PHY, PERST#, WAKE#
> > properties in two ways:
> > 
> > 1. Controller node (deprecated)
> > 	- phys
> > 	- perst-gpios
> > 	- wake-gpios
> > 
> > 2. Root Port node
> > 	- phys
> > 	- reset-gpios
> > 	- wake-gpios
> > 
> > But there is no check to make sure that the both variants are not mixed.
> > For instance, if the Controller node specifies 'phys', 'reset-gpios',
> > 'wake-gpios' or if the Root Port node specifies 'phys', 'perst-gpios',
> > 'wake-gpios', then the driver will fail as reported. Hence, enforce the
> > check in the binding to catch these issues.
> > 
> > It is also possible that DTs could have 'phys' property in Controller node
> > and 'reset-gpios/wake-gpios' properties in the Root Port node. It will also
> > be a problem, but it is not possible to catch these cross-node issues in
> > the binding.
> > 
> > Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > Closes: https://lore.kernel.org/linux-pci/8f2e0631-6c59-4298-b36e-060708970ced@oss.qualcomm.com
> > Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  .../devicetree/bindings/pci/qcom,pcie-common.yaml    | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie-sc8180x.example.dtb: pcie@1c00000 (qcom,pcie-sc8180x): 'oneOf' conditional failed, one must be fixed:
> 	'perst-gpios' is a required property
> 	'wake-gpios' is a required property
> 	False schema does not allow [[4294967295]]
> 	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-sc8180x.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie-sc8180x.example.dtb: pcie@1c00000 (qcom,pcie-sc8180x): Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'bus-range', 'device_type', 'dma-coherent', 'interconnect-names', 'interconnects', 'interrupt-map', 'interrupt-map-mask', 'iommu-map', 'linux,pci-domain', 'num-lanes', 'phy-names', 'phys', 'power-domains', 'ranges' were unexpected)
> 	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-sc8180x.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.example.dtb: pcie@1c08000 (qcom,pcie-sc7280): pcie@0: 'oneOf' conditional failed, one must be fixed:
> 	'wake-gpios' is a required property
> 	False schema does not allow [[4294967295]]
> 	False schema does not allow [[4294967295, 2, 1]]
> 	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-sc7280.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie-sc7280.example.dtb: pcie@1c08000 (qcom,pcie-sc7280): Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'bus-range', 'device_type', 'dma-coherent', 'interrupt-map', 'interrupt-map-mask', 'iommu-map', 'linux,pci-domain', 'num-lanes', 'pcie@0', 'power-domains', 'ranges', 'vddpe-3v3-supply' were unexpected)
> 	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-sc7280.yaml#
> 

I need to add the missing properties to the example nodes in these bindings.

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH 2/3] dt-bindings: PCI: qcom: Enforce check for PHY, PERST# and WAKE# properties
Posted by Konrad Dybcio 2 months, 1 week ago 
On 10/10/25 8:25 PM, Manivannan Sadhasivam wrote:
> Currently, the binding supports specifying the PHY, PERST#, WAKE#
> properties in two ways:
> 
> 1. Controller node (deprecated)
> 	- phys
> 	- perst-gpios
> 	- wake-gpios
> 
> 2. Root Port node
> 	- phys
> 	- reset-gpios
> 	- wake-gpios
> 
> But there is no check to make sure that the both variants are not mixed.
> For instance, if the Controller node specifies 'phys', 'reset-gpios',
> 'wake-gpios' or if the Root Port node specifies 'phys', 'perst-gpios',
> 'wake-gpios', then the driver will fail as reported. Hence, enforce the
> check in the binding to catch these issues.
> 
> It is also possible that DTs could have 'phys' property in Controller node
> and 'reset-gpios/wake-gpios' properties in the Root Port node. It will also
> be a problem, but it is not possible to catch these cross-node issues in
> the binding.
> 
> Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Closes: https://lore.kernel.org/linux-pci/8f2e0631-6c59-4298-b36e-060708970ced@oss.qualcomm.com
> Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---

I would also like to add

Reported-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

Since he originally made me aware of this issue

Konrad

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.