On Mon, Apr 14, 2025 at 08:53:03AM GMT, Sai Krishna Musham wrote:
> Add the `cpm_crx` property to manage the PCIe IP reset, and
> `cpm5nc_fw_attr` property to clear firewall after link reset, while
> maintaining backward compatibility with existing device trees.
>
> Also, incorporate `reset-gpios` in example for GPIO-based handling of
> the PCIe Root Port (RP) PERST# signal for enabling assert and deassert
> control.
>
> The `reset-gpios` and `cpm_crx` properties must be provided for CPM,
> CPM5 and CPM5_HOST1. For CPM5NC, all three properties - `reset-gpios`,
> `cpm_crx` and `cpm5nc_fw_attr` must be explicitly defined to ensure
This we see from the diff, but why they must be defined?
> proper functionality.
What functionality?
>
> Include an example DTS node and complete the binding documentation for
> CPM5NC. Also, fix the bridge register address size in the example for
> CPM5.
>
> Signed-off-by: Sai Krishna Musham <sai.krishna.musham@xxxxxxx>
> ---
> Changes for v7:
> - Update CPM5NC device tree binding.
> - Add CPM5NC device tree example node.
> - Update commit message.
>
> Changes for v6:
> - Resolve ABI break.
> - Update commit message.
>
...
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - xlnx,versal-cpm5nc-host
> + then:
> + properties:
> + reg:
> + items:
> + - description: CPM system level control and status registers.
> + - description: Configuration space region and bridge registers.
> + - description: CPM clock and reset control registers.
> + - description: CPM5NC Firewall attribute register.
> + minItems: 2
> + reg-names:
> + items:
> + - const: cpm_slcr
> + - const: cfg
> + - const: cpm_crx
> + - const: cpm5nc_fw_attr
> + minItems: 2
Why interrupts are not required for this variant? Why isn't this an
interrupt controller?
>
> unevaluatedProperties: false
>
> examples:
> - |
> + #include <dt-bindings/gpio/gpio.h>
>
> versal {
> #address-cells = <2>;
> @@ -98,8 +165,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 +195,10 @@ examples:
> msi-map = <0x0 &its_gic 0x0 0x10000>;
> reg = <0x00 0xfcdd0000 0x00 0x1000>,
> <0x06 0x00000000 0x00 0x1000000>,
> - <0x00 0xfce20000 0x00 0x1000000>;
> - reg-names = "cpm_slcr", "cfg", "cpm_csr";
> + <0x00 0xfce20000 0x00 0x10000>,
> + <0x00 0xfcdc0000 0x00 0x10000>;
> + reg-names = "cpm_slcr", "cfg", "cpm_csr", "cpm_crx";
> + reset-gpios = <&gpio1 38 GPIO_ACTIVE_LOW>;
>
> pcie_intc_1: interrupt-controller {
> #address-cells = <0>;
> @@ -136,4 +207,22 @@ examples:
> };
> };
>
> + cpm5nc_pcie: pcie@e4a10000 {
> + compatible = "xlnx,versal-cpm5nc-host";
> + device_type = "pci";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + interrupt-parent = <&gic>;
> + bus-range = <0x00 0xff>;
> + ranges = <0x2000000 0x00 0xa8000000 0x00 0xa8000000 0x00 0x8000000>,
> + <0x43000000 0x1010 0x00 0x1010 0x00 0x08 0x00>;
> + msi-map = <0x0 &its_gic 0x40000 0x10000>;
> + reg = <0x00 0xe4a10000 0x00 0x10000>,
> + <0x00 0xa0000000 0x00 0x8000000>,
> + <0x00 0xe4a00000 0x00 0x10000>,
> + <0x00 0xe4301000 0x00 0x10000>;
Follow DTS coding style. Or just drop this example... it also has
incorrect indentation. :/
> + reg-names = "cpm_slcr", "cfg", "cpm_crx", "cpm5nc_fw_attr";
> + reset-gpios = <&gpio0 22 GPIO_ACTIVE_LOW>;
> + };
> +
> };
> --
> 2.44.1
>
