[PATCH v4 1/2] dt-bindings: PCI: xilinx-cpm: Add reset-gpios for PCIe RP PERST#

Sai Krishna Musham posted 2 patches 9 months ago
There is a newer version of this series
[PATCH v4 1/2] dt-bindings: PCI: xilinx-cpm: Add reset-gpios for PCIe RP PERST#
Posted by Sai Krishna Musham 9 months ago
Introduce `reset-gpios` property to enable GPIO-based control of
the PCIe RP PERST# signal, generating assert and deassert signals.

Traditionally, the reset was managed in hardware and enabled during
initialization. With this patch set, the reset will be handled by the
driver. Consequently, the `reset-gpios` property must be explicitly
provided to ensure proper functionality.

Add CPM clock and reset control registers base to handle PCIe IP
reset along with PCIe RP PERST# to avoid Link Training errors.

Signed-off-by: Sai Krishna Musham <sai.krishna.musham@amd.com>
---
Changes for v4:
- Add CPM clock and reset control registers base to handle PCIe IP
  reset.
- Update commit message.

Changes for v3:
- None

Changes for v2:
- Add define from include/dt-bindings/gpio/gpio.h for PERST# polarity
- Update commit message
---
 .../bindings/pci/xilinx-versal-cpm.yaml       | 21 ++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
index d674a24c8ccc..904594138af2 100644
--- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
+++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
@@ -24,15 +24,20 @@ properties:
     items:
       - description: CPM system level control and status registers.
       - description: Configuration space region and bridge registers.
+      - description: CPM clock and reset control registers.
       - description: CPM5 control and status registers.
-    minItems: 2
+    minItems: 3
 
   reg-names:
     items:
       - const: cpm_slcr
       - const: cfg
+      - const: cpm_crx
       - const: cpm_csr
-    minItems: 2
+    minItems: 3
+
+  reset-gpios:
+    description: GPIO used as PERST# signal
 
   interrupts:
     maxItems: 1
@@ -64,6 +69,7 @@ properties:
 required:
   - reg
   - reg-names
+  - reset-gpios
   - "#interrupt-cells"
   - interrupts
   - interrupt-map
@@ -76,6 +82,7 @@ unevaluatedProperties: false
 
 examples:
   - |
+    #include <dt-bindings/gpio/gpio.h>
 
     versal {
                #address-cells = <2>;
@@ -98,8 +105,10 @@ examples:
                                 <0x43000000 0x80 0x00000000 0x80 0x00000000 0x0 0x80000000>;
                        msi-map = <0x0 &its_gic 0x0 0x10000>;
                        reg = <0x0 0xfca10000 0x0 0x1000>,
-                             <0x6 0x00000000 0x0 0x10000000>;
-                       reg-names = "cpm_slcr", "cfg";
+                             <0x6 0x00000000 0x0 0x10000000>,
+                             <0x0 0xfca00000 0x0 10000>;
+                       reg-names = "cpm_slcr", "cfg", "cpm_crx";
+                       reset-gpios = <&gpio1 38 GPIO_ACTIVE_LOW>;
                        pcie_intc_0: interrupt-controller {
                                #address-cells = <0>;
                                #interrupt-cells = <1>;
@@ -126,8 +135,10 @@ examples:
                        msi-map = <0x0 &its_gic 0x0 0x10000>;
                        reg = <0x00 0xfcdd0000 0x00 0x1000>,
                              <0x06 0x00000000 0x00 0x1000000>,
+                             <0x00 0xfcdc0000 0x00 0x10000>,
                              <0x00 0xfce20000 0x00 0x1000000>;
-                       reg-names = "cpm_slcr", "cfg", "cpm_csr";
+                       reg-names = "cpm_slcr", "cfg", "cpm_crx", "cpm_csr";
+                       reset-gpios = <&gpio1 38 GPIO_ACTIVE_LOW>;
 
                        pcie_intc_1: interrupt-controller {
                                #address-cells = <0>;
-- 
2.44.1
Re: [PATCH v4 1/2] dt-bindings: PCI: xilinx-cpm: Add reset-gpios for PCIe RP PERST#
Posted by Krzysztof Kozlowski 9 months ago
On 18/03/2025 10:26, Sai Krishna Musham wrote:
> Changes for v2:
> - Add define from include/dt-bindings/gpio/gpio.h for PERST# polarity
> - Update commit message
> ---
>  .../bindings/pci/xilinx-versal-cpm.yaml       | 21 ++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> index d674a24c8ccc..904594138af2 100644
> --- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> @@ -24,15 +24,20 @@ properties:
>      items:
>        - description: CPM system level control and status registers.
>        - description: Configuration space region and bridge registers.
> +      - description: CPM clock and reset control registers.
>        - description: CPM5 control and status registers.

You cannot add items to the middle, that's an ABI break. Adding required
properties is also an ABI break. Why you cannot add it to the end of the
list?

Or at least explain ABI break impact in commit msg?


> -    minItems: 2
> +    minItems: 3
>  
>    reg-names:
>      items:
>        - const: cpm_slcr
>        - const: cfg
> +      - const: cpm_crx
>        - const: cpm_csr
> -    minItems: 2
> +    minItems: 3
> +
> +  reset-gpios:
> +    description: GPIO used as PERST# signal

Isn't this already in pci-bus-common.yaml?


Best regards,
Krzysztof