[PATCH v4 0/2] Add driver support for Eswin EIC7700 SoC PCIe controller

[PATCH v4 0/2] Add driver support for Eswin EIC7700 SoC PCIe controller

[zhangsenchuan]

From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>

Changes in v4:
- Updates: eswin,eic7700-pcie.yaml
  - Use snps,dw-pcie.yaml instead pci-host-bridge.yaml.

- Updates: snps,dw-pcie-common.yaml
  - Add powerup reset property, our powerup property is somewhat different
    from the general attributes defined by Synopsys DWC binding.

- Updates: pcie-eic7700.c
  - Update the driver submission comment.
  - Alphabetize so the menuconfig entries remain sorted by vendor.
  - Update use PCI_CAP_LIST_NEXT_MASK macro.
  - Use readl_poll_timeout function.
  - Update eswin_pcie_suspend/eswin_pcie_resume name to
    eswin_pcie_suspend_noirq/eswin_pcie_resume_noirq.
  - PM use dw_pcie_suspend_noirq and dw_pcie_resume_noirq function and add
    eswin_pcie_get_ltssm, eswin_pcie_pme_turn_off, eswin_pcie_host_exit
    function adapt to PM.
- Link to V3: https://lore.kernel.org/linux-pci/20250923120946.1218-1-zhangsenchuan@eswincomputing.com/

Changes in v3:
- Updates: eswin,eic7700-pcie.yaml
  - Based on the last patch yaml file, devicetree separates the root port
    node, changing it significantly. Therefore, "Reviewed-by: Krzysztof
    Kozlowski <krzysztof.kozlowski@linaro.org>" is not added.
  - Clock and reset drivers are under review. In yaml, macro definitions
    used in clock and reset can only be replaced by constant values.
  - Move the num-lanes and perst resets to the PCIe Root Port node, make
    it easier to support multiple Root Ports in future versions of the
    hardware.
  - Update the num-lanes attribute and modify define num-lanes as decimal.
  - Optimize the ranges attribute and clear the relocatable flag (bit 31)
    for any regions.
  - Update comment: inte~inth are actual interrupts and these names align
    with the interrupt names in the hardware IP, inte~inth interrupts
    corresponds to Deassert_INTA~Deassert_INTD.
  - Add Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>.

- Updates: pcie-eic7700.c
  - Update the submission comment and add DWC IP revision, data rate, lane
    information.
  - Optimize the "config PCIE_EIC7700" configuration.
  - Optimize the macro definition, add bitfield definition for the mask,
    and remove redundant comments. optimize comments, make use of 80
    columns for comments.
  - Use the dw_pcie_find_capability function to obtain the offset by
    traversing the function list.
  - Remove the sets MPS code and configure it by PCI core.
  - Alphabetize so the menuconfig entries remain sorted by vendor.
  - Configure ESWIN VID:DID for Root Port as the default values are
	invalid,and remove the redundant lane config.
  - Use reverse Xmas order for all local variables in this driver
  - Hardware doesn't support MSI-X but it advertises MSI-X capability, set
    a flag and clear it conditionally.
  - Resets are all necessary, Update the interface function for resets.
  - Since driver does not depend on any parent to power on any resource,
    the pm runtime related functions are removed.
  - Remove "eswin_pcie_shutdown" function, our comment on the shutdown
    function is incorrect. Moreover, when the host powers reboots,it will
    enter the shutdown function, we are using host reset and do not need
    to assert perst. Therefore, the shutdown function is not necessary.
  - remove "eswin_pcie_remove", because it is not safe to remove it during
    runtime, and this driver has been modified to builtin_platform_driver
    and does not support hot plugging, therefore, the remove function is
    not needed.
  - The Suspend function adds link state judgment, and for controllers
    with active devices, resources cannot be turned off.
  - Add Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>.
- Link to V2: https://lore.kernel.org/linux-pci/20250829082021.49-1-zhangsenchuan@eswincomputing.com/

Changes in v2:
- Updates: eswin,eic7700-pcie.yaml
  - Optimize the naming of "clock-names" and "reset-names".
  - Add a reference to "$ref: /schemas/pci/pci-host-bridge.yaml#".
    (The name of the reset attribute in the "snps,dw-pcie-common.yaml"
    file is different from our reset attribute and "snps,dw-pcie.yaml"
    file cannot be directly referenced)
  - Follow DTS coding style to optimize yaml attributes.
  - Remove status = "disabled" from yaml.

- Updates: pcie-eic7700.c
  - Remove unnecessary imported header files.
  - Use dev_err instead of pr_err and remove the WARN_ON function.
  - The eswin_evb_socket_power_on function is removed and not supported.
  - The eswin_pcie_remove function is placed after the probe function.
  - Optimize function alignment.
  - Manage the clock using the devm_clk_bulk_get_all_enabled function.
  - Handle the release of resources after the dw_pcie_host_init function
    call fails.
  - Remove the dev_dbg function and remove __exit_p.
  - Add support for the system pm function.
- Link to V1: https://lore.kernel.org/all/20250516094057.1300-1-zhangsenchuan@eswincomputing.com/

Senchuan Zhang (2):
  dt-bindings: PCI: EIC7700: Add Eswin PCIe host controller
  PCI: EIC7700: Add Eswin PCIe host controller driver

 .../bindings/pci/eswin,eic7700-pcie.yaml      | 166 +++++++
 .../bindings/pci/snps,dw-pcie-common.yaml     |   2 +-
 drivers/pci/controller/dwc/Kconfig            |  11 +
 drivers/pci/controller/dwc/Makefile           |   1 +
 drivers/pci/controller/dwc/pcie-eic7700.c     | 462 ++++++++++++++++++
 5 files changed, 641 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c

--
2.25.1

[PATCH v4 1/2] dt-bindings: PCI: EIC7700: Add Eswin PCIe host controller

[zhangsenchuan]

From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>

Add Device Tree binding documentation for the Eswin EIC7700 PCIe
controller module, the PCIe controller enables the core to correctly
initialize and manage the PCIe bus and connected devices.

Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
---
 .../bindings/pci/eswin,eic7700-pcie.yaml      | 166 ++++++++++++++++++
 .../bindings/pci/snps,dw-pcie-common.yaml     |   2 +-
 2 files changed, 167 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml b/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
new file mode 100644
index 000000000000..e6c05e3a093a
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
@@ -0,0 +1,166 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/eswin,eic7700-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Eswin EIC7700 PCIe host controller
+
+maintainers:
+  - Yu Ning <ningyu@eswincomputing.com>
+  - Senchuan Zhang <zhangsenchuan@eswincomputing.com>
+  - Yanghui Ou <ouyanghui@eswincomputing.com>
+
+description:
+  The PCIe controller on EIC7700 SoC.
+
+properties:
+  compatible:
+    const: eswin,eic7700-pcie
+
+  reg:
+    maxItems: 3
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: config
+      - const: mgmt
+
+  ranges:
+    maxItems: 3
+
+  '#interrupt-cells':
+    const: 1
+
+  interrupt-names:
+    items:
+      - const: msi
+      - const: inta
+      - const: intb
+      - const: intc
+      - const: intd
+
+  interrupt-map:
+    maxItems: 4
+
+  interrupt-map-mask:
+    items:
+      - const: 0
+      - const: 0
+      - const: 0
+      - const: 7
+
+  clocks:
+    maxItems: 4
+
+  clock-names:
+    items:
+      - const: mstr
+      - const: dbi
+      - const: pclk
+      - const: aux
+
+  resets:
+    maxItems: 2
+
+  reset-names:
+    items:
+      - const: dbi
+      - const: powerup
+
+patternProperties:
+  "^pcie@":
+    type: object
+    $ref: /schemas/pci/pci-pci-bridge.yaml#
+
+    properties:
+      reg:
+        maxItems: 1
+
+      num-lanes:
+        maximum: 4
+
+      resets:
+        maxItems: 1
+
+      reset-names:
+        items:
+          - const: perst
+
+    required:
+      - reg
+      - ranges
+      - num-lanes
+      - resets
+      - reset-names
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - ranges
+  - interrupts
+  - interrupt-names
+  - interrupt-map-mask
+  - interrupt-map
+  - '#interrupt-cells'
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie@54000000 {
+            compatible = "eswin,eic7700-pcie";
+            reg = <0x0 0x54000000 0x0 0x4000000>,
+                  <0x0 0x40000000 0x0 0x800000>,
+                  <0x0 0x50000000 0x0 0x100000>;
+            reg-names = "dbi", "config", "mgmt";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            #interrupt-cells = <1>;
+            ranges = <0x01000000 0x0 0x40800000 0x0 0x40800000 0x0 0x800000>,
+                     <0x02000000 0x0 0x41000000 0x0 0x41000000 0x0 0xf000000>,
+                     <0x43000000 0x80 0x00000000 0x80 0x00000000 0x2 0x00000000>;
+            bus-range = <0x00 0xff>;
+            clocks = <&clock 144>,
+                     <&clock 145>,
+                     <&clock 146>,
+                     <&clock 147>;
+            clock-names = "mstr", "dbi", "pclk", "aux";
+            resets = <&reset 97>,
+                     <&reset 98>;
+            reset-names = "dbi", "powerup";
+            interrupts = <220>, <179>, <180>, <181>, <182>, <183>, <184>, <185>, <186>;
+            interrupt-names = "msi", "inta", "intb", "intc", "intd";
+            interrupt-parent = <&plic>;
+            interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+            interrupt-map = <0x0 0x0 0x0 0x1 &plic 179>,
+                            <0x0 0x0 0x0 0x2 &plic 180>,
+                            <0x0 0x0 0x0 0x3 &plic 181>,
+                            <0x0 0x0 0x0 0x4 &plic 182>;
+            device_type = "pci";
+            pcie@0 {
+                reg = <0x0 0x0 0x0 0x0 0x0>;
+                #address-cells = <3>;
+                #size-cells = <2>;
+                ranges;
+                device_type = "pci";
+                num-lanes = <4>;
+                resets = <&reset 99>;
+                reset-names = "perst";
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
index 34594972d8db..cff52d0026b0 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
@@ -176,7 +176,7 @@ properties:
             - description: See native 'phy' reset for details
               enum: [ pciephy, link ]
             - description: See native 'pwr' reset for details
-              enum: [ turnoff ]
+              enum: [ turnoff, powerup ]
 
   phys:
     description:
-- 
2.25.1

Re: [PATCH v4 1/2] dt-bindings: PCI: EIC7700: Add Eswin PCIe host controller

[Krzysztof Kozlowski]

On Thu, Oct 30, 2025 at 04:30:57PM +0800, zhangsenchuan@eswincomputing.com wrote:
> From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> 
> Add Device Tree binding documentation for the Eswin EIC7700 PCIe
> controller module, the PCIe controller enables the core to correctly
> initialize and manage the PCIe bus and connected devices.
> 
> Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> ---
>  .../bindings/pci/eswin,eic7700-pcie.yaml      | 166 ++++++++++++++++++
>  .../bindings/pci/snps,dw-pcie-common.yaml     |   2 +-
>  2 files changed, 167 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml b/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
> new file mode 100644
> index 000000000000..e6c05e3a093a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
> @@ -0,0 +1,166 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/eswin,eic7700-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Eswin EIC7700 PCIe host controller
> +
> +maintainers:
> +  - Yu Ning <ningyu@eswincomputing.com>
> +  - Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> +  - Yanghui Ou <ouyanghui@eswincomputing.com>
> +
> +description:
> +  The PCIe controller on EIC7700 SoC.
> +
> +properties:
> +  compatible:
> +    const: eswin,eic7700-pcie
> +
> +  reg:
> +    maxItems: 3
> +
> +  reg-names:
> +    items:
> +      - const: dbi
> +      - const: config
> +      - const: mgmt

That's deprecated. Read its description. That's just elbi.

> +
> +  ranges:
> +    maxItems: 3
> +
> +  '#interrupt-cells':
> +    const: 1
> +
> +  interrupt-names:
> +    items:
> +      - const: msi
> +      - const: inta
> +      - const: intb
> +      - const: intc
> +      - const: intd

Thse are legacy signals. Why are you using legacy?

> +
> +  interrupt-map:
> +    maxItems: 4
> +
> +  interrupt-map-mask:
> +    items:
> +      - const: 0
> +      - const: 0
> +      - const: 0
> +      - const: 7
> +
> +  clocks:
> +    maxItems: 4
> +
> +  clock-names:
> +    items:
> +      - const: mstr
> +      - const: dbi
> +      - const: pclk

Deprecated name.

> +      - const: aux
> +
> +  resets:
> +    maxItems: 2
> +
> +  reset-names:
> +    items:
> +      - const: dbi
> +      - const: powerup

No such name.

> +
> +patternProperties:
> +  "^pcie@":
> +    type: object
> +    $ref: /schemas/pci/pci-pci-bridge.yaml#
> +
> +    properties:
> +      reg:
> +        maxItems: 1
> +
> +      num-lanes:
> +        maximum: 4
> +
> +      resets:
> +        maxItems: 1
> +
> +      reset-names:
> +        items:
> +          - const: perst
> +
> +    required:
> +      - reg
> +      - ranges
> +      - num-lanes
> +      - resets
> +      - reset-names
> +
> +    unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - ranges
> +  - interrupts
> +  - interrupt-names
> +  - interrupt-map-mask
> +  - interrupt-map
> +  - '#interrupt-cells'
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +
> +allOf:
> +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pcie@54000000 {
> +            compatible = "eswin,eic7700-pcie";
> +            reg = <0x0 0x54000000 0x0 0x4000000>,
> +                  <0x0 0x40000000 0x0 0x800000>,
> +                  <0x0 0x50000000 0x0 0x100000>;
> +            reg-names = "dbi", "config", "mgmt";
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            #interrupt-cells = <1>;
> +            ranges = <0x01000000 0x0 0x40800000 0x0 0x40800000 0x0 0x800000>,
> +                     <0x02000000 0x0 0x41000000 0x0 0x41000000 0x0 0xf000000>,
> +                     <0x43000000 0x80 0x00000000 0x80 0x00000000 0x2 0x00000000>;
> +            bus-range = <0x00 0xff>;
> +            clocks = <&clock 144>,
> +                     <&clock 145>,
> +                     <&clock 146>,
> +                     <&clock 147>;
> +            clock-names = "mstr", "dbi", "pclk", "aux";
> +            resets = <&reset 97>,
> +                     <&reset 98>;
> +            reset-names = "dbi", "powerup";
> +            interrupts = <220>, <179>, <180>, <181>, <182>, <183>, <184>, <185>, <186>;
> +            interrupt-names = "msi", "inta", "intb", "intc", "intd";
> +            interrupt-parent = <&plic>;
> +            interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> +            interrupt-map = <0x0 0x0 0x0 0x1 &plic 179>,
> +                            <0x0 0x0 0x0 0x2 &plic 180>,
> +                            <0x0 0x0 0x0 0x3 &plic 181>,
> +                            <0x0 0x0 0x0 0x4 &plic 182>;
> +            device_type = "pci";
> +            pcie@0 {
> +                reg = <0x0 0x0 0x0 0x0 0x0>;
> +                #address-cells = <3>;
> +                #size-cells = <2>;
> +                ranges;
> +                device_type = "pci";
> +                num-lanes = <4>;
> +                resets = <&reset 99>;
> +                reset-names = "perst";
> +            };
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> index 34594972d8db..cff52d0026b0 100644
> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> @@ -176,7 +176,7 @@ properties:
>              - description: See native 'phy' reset for details
>                enum: [ pciephy, link ]
>              - description: See native 'pwr' reset for details
> -              enum: [ turnoff ]
> +              enum: [ turnoff, powerup ]

NAK, you cannot add more deprecated names. Do you understand what
deprecated/legacy mean?


Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: PCI: EIC7700: Add Eswin PCIe host controller

[Manivannan Sadhasivam]

On Mon, Nov 03, 2025 at 09:48:02AM +0100, Krzysztof Kozlowski wrote:
> On Thu, Oct 30, 2025 at 04:30:57PM +0800, zhangsenchuan@eswincomputing.com wrote:
> > From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > 
> > Add Device Tree binding documentation for the Eswin EIC7700 PCIe
> > controller module, the PCIe controller enables the core to correctly
> > initialize and manage the PCIe bus and connected devices.
> > 
> > Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> > Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> > Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > ---
> >  .../bindings/pci/eswin,eic7700-pcie.yaml      | 166 ++++++++++++++++++
> >  .../bindings/pci/snps,dw-pcie-common.yaml     |   2 +-
> >  2 files changed, 167 insertions(+), 1 deletion(-)
> >  create mode 100644 Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml b/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
> > new file mode 100644
> > index 000000000000..e6c05e3a093a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
> > @@ -0,0 +1,166 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/eswin,eic7700-pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Eswin EIC7700 PCIe host controller
> > +
> > +maintainers:
> > +  - Yu Ning <ningyu@eswincomputing.com>
> > +  - Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > +  - Yanghui Ou <ouyanghui@eswincomputing.com>
> > +
> > +description:
> > +  The PCIe controller on EIC7700 SoC.
> > +
> > +properties:
> > +  compatible:
> > +    const: eswin,eic7700-pcie
> > +
> > +  reg:
> > +    maxItems: 3
> > +
> > +  reg-names:
> > +    items:
> > +      - const: dbi
> > +      - const: config
> > +      - const: mgmt
> 
> That's deprecated. Read its description. That's just elbi.
> 
> > +
> > +  ranges:
> > +    maxItems: 3
> > +
> > +  '#interrupt-cells':
> > +    const: 1
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: msi
> > +      - const: inta
> > +      - const: intb
> > +      - const: intc
> > +      - const: intd
> 
> Thse are legacy signals. Why are you using legacy?
> 

Why not? These INTx signals are still supported by many host platforms/devices.
These are not individual out-of-band signals, but just in-band messages. It is
perfectly fine for a host controller to support them.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 1/2] dt-bindings: PCI: EIC7700: Add Eswin PCIe host controller

[Krzysztof Kozlowski]

On 03/11/2025 10:02, Manivannan Sadhasivam wrote:
>>> +  interrupt-names:
>>> +    items:
>>> +      - const: msi
>>> +      - const: inta
>>> +      - const: intb
>>> +      - const: intc
>>> +      - const: intd
>>
>> Thse are legacy signals. Why are you using legacy?
>>
> 
> Why not? These INTx signals are still supported by many host platforms/devices.
> These are not individual out-of-band signals, but just in-band messages. It is
> perfectly fine for a host controller to support them.

Considering how many other legacy things were used in this binding, I
just have doubts that choice of entries was correct.

Best regards,
Krzysztof

Re: Re: [PATCH v4 1/2] dt-bindings: PCI: EIC7700: Add Eswin PCIe host controller

[zhangsenchuan]

> -----Original Messages-----
> From: "Krzysztof Kozlowski" <krzk@kernel.org>
> Send time:Monday, 03/11/2025 16:48:02
> To: zhangsenchuan@eswincomputing.com
> Cc: bhelgaas@google.com, mani@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, lpieralisi@kernel.org, kwilczynski@kernel.org, robh@kernel.org, p.zabel@pengutronix.de, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, christian.bruel@foss.st.com, mayank.rana@oss.qualcomm.com, shradha.t@samsung.com, krishna.chundru@oss.qualcomm.com, thippeswamy.havalige@amd.com, inochiama@gmail.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, ouyanghui@eswincomputing.com
> Subject: Re: [PATCH v4 1/2] dt-bindings: PCI: EIC7700: Add Eswin PCIe host controller
> 
> On Thu, Oct 30, 2025 at 04:30:57PM +0800, zhangsenchuan@eswincomputing.com wrote:
> > From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > 
> > Add Device Tree binding documentation for the Eswin EIC7700 PCIe
> > controller module, the PCIe controller enables the core to correctly
> > initialize and manage the PCIe bus and connected devices.
> > 
> > Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> > Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> > Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > ---
> >  .../bindings/pci/eswin,eic7700-pcie.yaml      | 166 ++++++++++++++++++
> >  .../bindings/pci/snps,dw-pcie-common.yaml     |   2 +-
> >  2 files changed, 167 insertions(+), 1 deletion(-)
> >  create mode 100644 Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml b/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
> > new file mode 100644
> > index 000000000000..e6c05e3a093a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/eswin,eic7700-pcie.yaml
> > @@ -0,0 +1,166 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/eswin,eic7700-pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Eswin EIC7700 PCIe host controller
> > +
> > +maintainers:
> > +  - Yu Ning <ningyu@eswincomputing.com>
> > +  - Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > +  - Yanghui Ou <ouyanghui@eswincomputing.com>
> > +
> > +description:
> > +  The PCIe controller on EIC7700 SoC.
> > +
> > +properties:
> > +  compatible:
> > +    const: eswin,eic7700-pcie
> > +
> > +  reg:
> > +    maxItems: 3
> > +
> > +  reg-names:
> > +    items:
> > +      - const: dbi
> > +      - const: config
> > +      - const: mgmt
> 
> That's deprecated. Read its description. That's just elbi.
> 
> > +
> > +  ranges:
> > +    maxItems: 3
> > +
> > +  '#interrupt-cells':
> > +    const: 1
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: msi
> > +      - const: inta
> > +      - const: intb
> > +      - const: intc
> > +      - const: intd
> 
> Thse are legacy signals. Why are you using legacy?
> 
> > +
> > +  interrupt-map:
> > +    maxItems: 4
> > +
> > +  interrupt-map-mask:
> > +    items:
> > +      - const: 0
> > +      - const: 0
> > +      - const: 0
> > +      - const: 7
> > +
> > +  clocks:
> > +    maxItems: 4
> > +
> > +  clock-names:
> > +    items:
> > +      - const: mstr
> > +      - const: dbi
> > +      - const: pclk
> 
> Deprecated name.
> 
> > +      - const: aux
> > +
> > +  resets:
> > +    maxItems: 2
> > +
> > +  reset-names:
> > +    items:
> > +      - const: dbi
> > +      - const: powerup
> 
> No such name.
> 
> > +
> > +patternProperties:
> > +  "^pcie@":
> > +    type: object
> > +    $ref: /schemas/pci/pci-pci-bridge.yaml#
> > +
> > +    properties:
> > +      reg:
> > +        maxItems: 1
> > +
> > +      num-lanes:
> > +        maximum: 4
> > +
> > +      resets:
> > +        maxItems: 1
> > +
> > +      reset-names:
> > +        items:
> > +          - const: perst
> > +
> > +    required:
> > +      - reg
> > +      - ranges
> > +      - num-lanes
> > +      - resets
> > +      - reset-names
> > +
> > +    unevaluatedProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - ranges
> > +  - interrupts
> > +  - interrupt-names
> > +  - interrupt-map-mask
> > +  - interrupt-map
> > +  - '#interrupt-cells'
> > +  - clocks
> > +  - clock-names
> > +  - resets
> > +  - reset-names
> > +
> > +allOf:
> > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    soc {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        pcie@54000000 {
> > +            compatible = "eswin,eic7700-pcie";
> > +            reg = <0x0 0x54000000 0x0 0x4000000>,
> > +                  <0x0 0x40000000 0x0 0x800000>,
> > +                  <0x0 0x50000000 0x0 0x100000>;
> > +            reg-names = "dbi", "config", "mgmt";
> > +            #address-cells = <3>;
> > +            #size-cells = <2>;
> > +            #interrupt-cells = <1>;
> > +            ranges = <0x01000000 0x0 0x40800000 0x0 0x40800000 0x0 0x800000>,
> > +                     <0x02000000 0x0 0x41000000 0x0 0x41000000 0x0 0xf000000>,
> > +                     <0x43000000 0x80 0x00000000 0x80 0x00000000 0x2 0x00000000>;
> > +            bus-range = <0x00 0xff>;
> > +            clocks = <&clock 144>,
> > +                     <&clock 145>,
> > +                     <&clock 146>,
> > +                     <&clock 147>;
> > +            clock-names = "mstr", "dbi", "pclk", "aux";
> > +            resets = <&reset 97>,
> > +                     <&reset 98>;
> > +            reset-names = "dbi", "powerup";
> > +            interrupts = <220>, <179>, <180>, <181>, <182>, <183>, <184>, <185>, <186>;
> > +            interrupt-names = "msi", "inta", "intb", "intc", "intd";
> > +            interrupt-parent = <&plic>;
> > +            interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> > +            interrupt-map = <0x0 0x0 0x0 0x1 &plic 179>,
> > +                            <0x0 0x0 0x0 0x2 &plic 180>,
> > +                            <0x0 0x0 0x0 0x3 &plic 181>,
> > +                            <0x0 0x0 0x0 0x4 &plic 182>;
> > +            device_type = "pci";
> > +            pcie@0 {
> > +                reg = <0x0 0x0 0x0 0x0 0x0>;
> > +                #address-cells = <3>;
> > +                #size-cells = <2>;
> > +                ranges;
> > +                device_type = "pci";
> > +                num-lanes = <4>;
> > +                resets = <&reset 99>;
> > +                reset-names = "perst";
> > +            };
> > +        };
> > +    };
> > diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > index 34594972d8db..cff52d0026b0 100644
> > --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > @@ -176,7 +176,7 @@ properties:
> >              - description: See native 'phy' reset for details
> >                enum: [ pciephy, link ]
> >              - description: See native 'pwr' reset for details
> > -              enum: [ turnoff ]
> > +              enum: [ turnoff, powerup ]
> 
> NAK, you cannot add more deprecated names. Do you understand what
> deprecated/legacy mean?
> 

Hi, Krzysztof

Thank you for your reminder. I misunderstood deprecated/legacy.
I have submitted the pcie v5 pacth and made the modifications.

Kind regards,
Senchuan Zhang

[PATCH v4 2/2] PCI: EIC7700: Add Eswin PCIe host controller driver

[zhangsenchuan]

From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>

Add driver for the Eswin EIC7700 PCIe host controller, which is based on
the DesignWare PCIe core, IP revision 6.00a. The PCIe Gen.3 controller
supports a data rate of 8 GT/s and 4 channels, support INTX and MSI
interrupts.

Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
---
 drivers/pci/controller/dwc/Kconfig        |  11 +
 drivers/pci/controller/dwc/Makefile       |   1 +
 drivers/pci/controller/dwc/pcie-eic7700.c | 462 ++++++++++++++++++++++
 3 files changed, 474 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 349d4657393c..6a51978d3c49 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -93,6 +93,17 @@ config PCIE_BT1
 	  Enables support for the PCIe controller in the Baikal-T1 SoC to work
 	  in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core.
 
+config PCIE_EIC7700
+	bool "Eswin PCIe controller"
+	depends on ARCH_ESWIN || COMPILE_TEST
+	depends on PCI_MSI
+	select PCIE_DW_HOST
+	help
+	  Say Y here if you want PCIe controller support for the Eswin.
+	  The PCIe controller on Eswin is based on DesignWare hardware,
+	  enables support for the PCIe controller in the Eswin SoC to
+	  work in host mode.
+
 config PCI_IMX6
 	bool
 
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 7ae28f3b0fb3..04f751c49eba 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
 obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
 obj-$(CONFIG_PCIE_AMD_MDB) += pcie-amd-mdb.o
 obj-$(CONFIG_PCIE_BT1) += pcie-bt1.o
+obj-$(CONFIG_PCIE_EIC7700) += pcie-eic7700.o
 obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
 obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
 obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
diff --git a/drivers/pci/controller/dwc/pcie-eic7700.c b/drivers/pci/controller/dwc/pcie-eic7700.c
new file mode 100644
index 000000000000..0016dd0be743
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-eic7700.c
@@ -0,0 +1,462 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ESWIN PCIe root complex driver
+ *
+ * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
+ *
+ * Authors: Yu Ning <ningyu@eswincomputing.com>
+ *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
+ *          Yanghui Ou <ouyanghui@eswincomputing.com>
+ */
+
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/resource.h>
+#include <linux/reset.h>
+#include <linux/types.h>
+
+#include "pcie-designware.h"
+
+/* PCIe top csr registers */
+#define PCIEMGMT_CTRL0_OFFSET		0x0
+#define PCIEMGMT_STATUS0_OFFSET		0x100
+
+/* LTSSM register fields */
+#define PCIEMGMT_APP_LTSSM_ENABLE	BIT(5)
+
+/* APP_HOLD_PHY_RST register fields */
+#define PCIEMGMT_APP_HOLD_PHY_RST	BIT(6)
+
+/* PM_SEL_AUX_CLK register fields */
+#define PCIEMGMT_PM_SEL_AUX_CLK		BIT(16)
+
+/* ROOT_PORT register fields */
+#define PCIEMGMT_CTRL0_ROOT_PORT_MASK	GENMASK(3, 0)
+
+/* Vendor and device id value */
+#define PCI_VENDOR_ID_ESWIN		0x1fe1
+#define PCI_DEVICE_ID_ESWIN		0x2030
+
+struct eswin_pcie_data {
+	bool msix_cap;
+};
+
+struct eswin_pcie_port {
+	struct list_head list;
+	struct reset_control *perst;
+	int num_lanes;
+};
+
+struct eswin_pcie {
+	struct dw_pcie pci;
+	void __iomem *mgmt_base;
+	struct clk_bulk_data *clks;
+	struct reset_control *powerup_rst;
+	struct reset_control *cfg_rst;
+	struct list_head ports;
+	int num_clks;
+	bool msix_cap;
+	bool linked_up;
+};
+
+#define to_eswin_pcie(x) dev_get_drvdata((x)->dev)
+
+static int eswin_pcie_start_link(struct dw_pcie *pci)
+{
+	struct eswin_pcie *pcie = to_eswin_pcie(pci);
+	u32 val;
+
+	/* Enable LTSSM */
+	val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
+	val |= PCIEMGMT_APP_LTSSM_ENABLE;
+	writel_relaxed(val, pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
+
+	return 0;
+}
+
+static bool eswin_pcie_link_up(struct dw_pcie *pci)
+{
+	struct eswin_pcie *pcie = to_eswin_pcie(pci);
+
+	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u16 val = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
+
+	if (val & PCI_EXP_LNKSTA_DLLLA)
+		pcie->linked_up = true;
+
+	return val & PCI_EXP_LNKSTA_DLLLA;
+}
+
+static enum dw_pcie_ltssm eswin_pcie_get_ltssm(struct dw_pcie *pci)
+{
+	dev_info(pci->dev, "LTSSM_L2 not supported\n");
+	/* Return 0 only ensure skip read_poll_timeout function check */
+	return 0;
+}
+
+static int eswin_pcie_deassert(struct eswin_pcie *pcie)
+{
+	int ret;
+
+	ret = reset_control_deassert(pcie->cfg_rst);
+	if (ret) {
+		dev_err(pcie->pci.dev, "Failed to deassert CFG#");
+		return ret;
+	}
+
+	ret = reset_control_deassert(pcie->powerup_rst);
+	if (ret) {
+		dev_err(pcie->pci.dev, "Failed to deassert POWERUP#");
+		goto err_powerup;
+	}
+
+	return 0;
+
+err_powerup:
+	reset_control_assert(pcie->cfg_rst);
+
+	return ret;
+}
+
+static void eswin_pcie_assert(struct eswin_pcie *pcie)
+{
+	reset_control_assert(pcie->powerup_rst);
+	reset_control_assert(pcie->cfg_rst);
+}
+
+static int eswin_pcie_perst_deassert(struct eswin_pcie_port *port,
+				     struct eswin_pcie *pcie)
+{
+	int ret;
+
+	ret = reset_control_assert(port->perst);
+	if (ret) {
+		dev_err(pcie->pci.dev, "Failed to assert PERST#");
+		return ret;
+	}
+
+	/* Ensure that PERST has been asserted for at least 100 ms */
+	msleep(PCIE_T_PVPERL_MS);
+
+	ret = reset_control_deassert(port->perst);
+	if (ret) {
+		dev_err(pcie->pci.dev, "Failed to deassert PERST#");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int eswin_pcie_parse_port(struct eswin_pcie *pcie,
+				 struct device_node *node)
+{
+	struct device *dev = pcie->pci.dev;
+	struct eswin_pcie_port *port;
+
+	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+	if (!port)
+		return -ENOMEM;
+
+	port->perst = of_reset_control_get(node, "perst");
+	if (IS_ERR(port->perst)) {
+		dev_err(dev, "Failed to get perst reset\n");
+		return PTR_ERR(port->perst);
+	}
+
+	/*
+	 * Since the root port node is separated out by pcie devicetree, the
+	 * DWC core initialization code cannot parse the num-lanes attribute
+	 * in the root port. Before entering the DWC core initialization code,
+	 * the platform driver code parses the root port node. The EIC7700 only
+	 * supports one root port node, and the num-lanes attribute is suitable
+	 * for the case of one root port.
+	 */
+	of_property_read_u32(node, "num-lanes", &port->num_lanes);
+	pcie->pci.num_lanes = port->num_lanes;
+
+	INIT_LIST_HEAD(&port->list);
+	list_add_tail(&port->list, &pcie->ports);
+
+	return 0;
+}
+
+static int eswin_pcie_parse_ports(struct eswin_pcie *pcie)
+{
+	struct device *dev = pcie->pci.dev;
+	struct eswin_pcie_port *port, *tmp;
+	int ret;
+
+	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
+		ret = eswin_pcie_parse_port(pcie, of_port);
+		if (ret)
+			goto err_port;
+	}
+
+	return 0;
+
+err_port:
+	list_for_each_entry_safe(port, tmp, &pcie->ports, list)
+		list_del(&port->list);
+
+	return ret;
+}
+
+static void eswin_pcie_hide_broken_msix_cap(struct dw_pcie *pci)
+{
+	u16 offset, val;
+
+	/*
+	 * Hardware doesn't support MSI-X but it advertises MSI-X capability,
+	 * to avoid this problem, the MSI-X capability in the PCIe capabilities
+	 * linked-list needs to be disabled. Since the PCI Express capability
+	 * structure's next pointer points to the MSI-X capability, and the
+	 * MSI-X capability's next pointer is null (00H), so only the PCI
+	 * Express capability structure's next pointer needs to be set 00H.
+	 */
+	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	val = dw_pcie_readl_dbi(pci, offset);
+	val &= ~PCI_CAP_LIST_NEXT_MASK;
+	dw_pcie_writel_dbi(pci, offset, val);
+}
+
+static int eswin_pcie_host_init(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct eswin_pcie *pcie = to_eswin_pcie(pci);
+	struct eswin_pcie_port *port;
+	u8 msi_cap;
+	u32 val;
+	int ret;
+
+	pcie->num_clks = devm_clk_bulk_get_all_enabled(pci->dev, &pcie->clks);
+	if (pcie->num_clks < 0)
+		return dev_err_probe(pci->dev, pcie->num_clks,
+				     "Failed to get pcie clocks\n");
+
+	ret = eswin_pcie_deassert(pcie);
+	if (ret)
+		return ret;
+
+	/* Configure root port type */
+	val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
+	val &= ~PCIEMGMT_CTRL0_ROOT_PORT_MASK;
+	writel_relaxed(val | PCI_EXP_TYPE_ROOT_PORT,
+		       pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
+
+	list_for_each_entry(port, &pcie->ports, list) {
+		ret = eswin_pcie_perst_deassert(port, pcie);
+			if (ret)
+				goto err_perst;
+	}
+
+	/* Configure app_hold_phy_rst */
+	val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
+	val &= ~PCIEMGMT_APP_HOLD_PHY_RST;
+	writel_relaxed(val, pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
+
+	/* The maximum waiting time for the clock switch lock is 20ms */
+	ret = readl_poll_timeout(pcie->mgmt_base + PCIEMGMT_STATUS0_OFFSET,
+				 val, !(val & PCIEMGMT_PM_SEL_AUX_CLK), 1000,
+				 20000);
+
+	if (ret) {
+		dev_err(pci->dev, "Timeout waiting for PM_SEL_AUX_CLK ready\n");
+		goto err_phy_init;
+	}
+
+	/*
+	 * Configure ESWIN VID:DID for Root Port as the default values are
+	 * invalid.
+	 */
+	dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, PCI_VENDOR_ID_ESWIN);
+	dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, PCI_DEVICE_ID_ESWIN);
+
+	/* Configure support 32 MSI vectors */
+	msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
+	val = dw_pcie_readw_dbi(pci, msi_cap + PCI_MSI_FLAGS);
+	val &= ~PCI_MSI_FLAGS_QMASK;
+	val |= FIELD_PREP(PCI_MSI_FLAGS_QMASK, 5);
+	dw_pcie_writew_dbi(pci, msi_cap + PCI_MSI_FLAGS, val);
+
+	/* Configure disable MSI-X cap */
+	if (!pcie->msix_cap)
+		eswin_pcie_hide_broken_msix_cap(pci);
+
+	return 0;
+
+err_phy_init:
+	list_for_each_entry(port, &pcie->ports, list)
+		reset_control_assert(port->perst);
+err_perst:
+	eswin_pcie_assert(pcie);
+
+	return ret;
+}
+
+static void eswin_pcie_host_exit(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct eswin_pcie *pcie = to_eswin_pcie(pci);
+	struct eswin_pcie_port *port;
+
+	/*
+	 * For controllers with active devices, resources are retained and
+	 * cannot be turned off, like NVMEe.
+	 */
+	if (!dw_pcie_link_up(&pcie->pci)) {
+		list_for_each_entry(port, &pcie->ports, list)
+			reset_control_assert(port->perst);
+		eswin_pcie_assert(pcie);
+		clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
+	}
+}
+
+static void eswin_pcie_pme_turn_off(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+
+	/*
+	 * Hardware doesn't support enter the D3code and L2/L3 states, send
+	 * PME_TURN_OFF message, which will then cause Vmain to be removed and
+	 * controller stop working.
+	 */
+	dev_info(pci->dev, "Can't send PME_TURN_OFF message\n");
+}
+
+static const struct dw_pcie_host_ops eswin_pcie_host_ops = {
+	.init = eswin_pcie_host_init,
+	.deinit = eswin_pcie_host_exit,
+	.pme_turn_off = eswin_pcie_pme_turn_off,
+};
+
+static const struct dw_pcie_ops dw_pcie_ops = {
+	.start_link = eswin_pcie_start_link,
+	.link_up = eswin_pcie_link_up,
+	.get_ltssm = eswin_pcie_get_ltssm,
+};
+
+static int eswin_pcie_probe(struct platform_device *pdev)
+{
+	const struct eswin_pcie_data *data;
+	struct eswin_pcie_port *port, *tmp;
+	struct device *dev = &pdev->dev;
+	struct eswin_pcie *pcie;
+	struct dw_pcie *pci;
+	int ret;
+
+	data = of_device_get_match_data(dev);
+	if (!data)
+		return dev_err_probe(dev, -EINVAL, "OF data missing\n");
+
+	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&pcie->ports);
+
+	pci = &pcie->pci;
+	pci->dev = dev;
+	pci->ops = &dw_pcie_ops;
+	pci->pp.ops = &eswin_pcie_host_ops;
+	pcie->msix_cap = data->msix_cap;
+
+	pcie->mgmt_base = devm_platform_ioremap_resource_byname(pdev, "mgmt");
+	if (IS_ERR(pcie->mgmt_base))
+		return dev_err_probe(dev, PTR_ERR(pcie->mgmt_base),
+				     "Failed to map mgmt registers\n");
+
+	pcie->powerup_rst = devm_reset_control_get(&pdev->dev, "powerup");
+	if (IS_ERR(pcie->powerup_rst))
+		return dev_err_probe(dev, PTR_ERR(pcie->powerup_rst),
+				     "Failed to get powerup reset\n");
+
+	pcie->cfg_rst = devm_reset_control_get(&pdev->dev, "dbi");
+	if (IS_ERR(pcie->cfg_rst))
+		return dev_err_probe(dev, PTR_ERR(pcie->cfg_rst),
+				     "Failed to get dbi reset\n");
+
+	ret = eswin_pcie_parse_ports(pcie);
+	if (ret)
+		return dev_err_probe(pci->dev, ret,
+				     "Failed to parse Root Port: %d\n", ret);
+
+	platform_set_drvdata(pdev, pcie);
+
+	ret = dw_pcie_host_init(&pci->pp);
+	if (ret) {
+		dev_err(dev, "Failed to initialize host\n");
+		goto err_init;
+	}
+
+	return 0;
+
+err_init:
+	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
+		list_del(&port->list);
+		reset_control_put(port->perst);
+	}
+
+	return ret;
+}
+
+static int eswin_pcie_suspend_noirq(struct device *dev)
+{
+	struct eswin_pcie *pcie = dev_get_drvdata(dev);
+
+	return dw_pcie_suspend_noirq(&pcie->pci);
+}
+
+static int eswin_pcie_resume_noirq(struct device *dev)
+{
+	struct eswin_pcie *pcie = dev_get_drvdata(dev);
+	int ret;
+
+	ret = dw_pcie_resume_noirq(&pcie->pci);
+	/*
+	 * If the downstream device is not inserted, linkup will TIMEDOUT. At
+	 * this time, when the resume function return, -ETIMEDOUT shouldn't be
+	 * returned, which will raise "PM: failed to resume noirq: error -110".
+	 * Only log message "Ignore errors, the link may come up later".
+	 */
+	if (ret == -ETIMEDOUT && !pcie->linked_up) {
+		dev_info(dev, "Ignore errors, the link may come up later\n");
+		return 0;
+	}
+
+	return ret;
+}
+
+static const struct dev_pm_ops eswin_pcie_pm_ops = {
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(eswin_pcie_suspend_noirq,
+				  eswin_pcie_resume_noirq)
+};
+
+static const struct eswin_pcie_data eswin_7700_data = {
+	.msix_cap = false,
+};
+
+static const struct of_device_id eswin_pcie_of_match[] = {
+	{ .compatible = "eswin,eic7700-pcie", .data = &eswin_7700_data },
+	{},
+};
+
+static struct platform_driver eswin_pcie_driver = {
+	.probe = eswin_pcie_probe,
+	.driver = {
+		.name = "eic7700-pcie",
+		.of_match_table = eswin_pcie_of_match,
+		.suppress_bind_attrs = true,
+		.pm = &eswin_pcie_pm_ops,
+	},
+};
+builtin_platform_driver(eswin_pcie_driver);
+
+MODULE_DESCRIPTION("PCIe host controller driver for EIC7700 SoCs");
+MODULE_AUTHOR("Yu Ning <ningyu@eswincomputing.com>");
+MODULE_AUTHOR("Senchuan Zhang <zhangsenchuan@eswincomputing.com>");
+MODULE_AUTHOR("Yanghui Ou <ouyanghui@eswincomputing.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1

Re: [PATCH v4 2/2] PCI: EIC7700: Add Eswin PCIe host controller driver

[Philipp Zabel]

On Do, 2025-10-30 at 16:31 +0800, zhangsenchuan@eswincomputing.com
wrote:
> From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> 
> Add driver for the Eswin EIC7700 PCIe host controller, which is based on
> the DesignWare PCIe core, IP revision 6.00a. The PCIe Gen.3 controller
> supports a data rate of 8 GT/s and 4 channels, support INTX and MSI
> interrupts.
> 
> Signed-off-by: Yu Ning <ningyu@eswincomputing.com>
> Signed-off-by: Yanghui Ou <ouyanghui@eswincomputing.com>
> Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> ---
>  drivers/pci/controller/dwc/Kconfig        |  11 +
>  drivers/pci/controller/dwc/Makefile       |   1 +
>  drivers/pci/controller/dwc/pcie-eic7700.c | 462 ++++++++++++++++++++++
>  3 files changed, 474 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-eic7700.c
> 
[...]
> diff --git a/drivers/pci/controller/dwc/pcie-eic7700.c b/drivers/pci/controller/dwc/pcie-eic7700.c
> new file mode 100644
> index 000000000000..0016dd0be743
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-eic7700.c
> @@ -0,0 +1,462 @@
[...]
> +static int eswin_pcie_deassert(struct eswin_pcie *pcie)
> +{
> +	int ret;
> +
> +	ret = reset_control_deassert(pcie->cfg_rst);
> +	if (ret) {
> +		dev_err(pcie->pci.dev, "Failed to deassert CFG#");
> +		return ret;
> +	}
> +
> +	ret = reset_control_deassert(pcie->powerup_rst);
> +	if (ret) {
> +		dev_err(pcie->pci.dev, "Failed to deassert POWERUP#");
> +		goto err_powerup;
> +	}
> +
> +	return 0;
> +
> +err_powerup:
> +	reset_control_assert(pcie->cfg_rst);
> +
> +	return ret;
> +}
> +
> +static void eswin_pcie_assert(struct eswin_pcie *pcie)
> +{
> +	reset_control_assert(pcie->powerup_rst);
> +	reset_control_assert(pcie->cfg_rst);
> +}

These look like cfg and powerup resets could be controlled together via
reset_control_bulk_assert/deassert().

[...]
> +static int eswin_pcie_parse_port(struct eswin_pcie *pcie,
> +				 struct device_node *node)
> +{		return -ENOMEM;
[...]
> +	port->perst = of_reset_control_get(node, "perst");

Please use of_reset_control_get_exclusive() directly.


[...]
> +static int eswin_pcie_probe(struct platform_device *pdev)
> +{
[...]
> +	pcie->powerup_rst = devm_reset_control_get(&pdev->dev, "powerup");
> +	if (IS_ERR(pcie->powerup_rst))
> +		return dev_err_probe(dev, PTR_ERR(pcie->powerup_rst),
> +				     "Failed to get powerup reset\n");
> +
> +	pcie->cfg_rst = devm_reset_control_get(&pdev->dev, "dbi");
> +	if (IS_ERR(pcie->cfg_rst))
> +		return dev_err_probe(dev, PTR_ERR(pcie->cfg_rst),
> +				     "Failed to get dbi reset\n");

Please use devm_reset_control_get_exclusive() directly.

Alternatively, you could get powerup and cfg/dbi resets in bulk via
devm_reset_control_bulk_get_exclusive().

regards
Philipp

Re: [PATCH v4 2/2] PCI: EIC7700: Add Eswin PCIe host controller driver

[Bjorn Helgaas]

Nit: if you run "git log --oneline drivers/pci/controller/", you'll
notice that the driver tags ("EIC7700" here) are all lower-case.
Same for the DT bindings.

On Thu, Oct 30, 2025 at 04:31:42PM +0800, zhangsenchuan@eswincomputing.com wrote:
> From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> 
> Add driver for the Eswin EIC7700 PCIe host controller, which is based on
> the DesignWare PCIe core, IP revision 6.00a. The PCIe Gen.3 controller
> supports a data rate of 8 GT/s and 4 channels, support INTX and MSI
> interrupts.

s/INTX/INTx/ to match spec usage.

> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -93,6 +93,17 @@ config PCIE_BT1
>  	  Enables support for the PCIe controller in the Baikal-T1 SoC to work
>  	  in host mode. It's based on the Synopsys DWC PCIe v4.60a IP-core.
>  
> +config PCIE_EIC7700
> +	bool "Eswin PCIe controller"

I think this should mention EIC7700.

> +	depends on ARCH_ESWIN || COMPILE_TEST
> +	depends on PCI_MSI
> +	select PCIE_DW_HOST
> +	help
> +	  Say Y here if you want PCIe controller support for the Eswin.
> +	  The PCIe controller on Eswin is based on DesignWare hardware,
> +	  enables support for the PCIe controller in the Eswin SoC to
> +	  work in host mode.

Mention EIC7700 here also.

> +++ b/drivers/pci/controller/dwc/pcie-eic7700.c
> @@ -0,0 +1,462 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ESWIN PCIe root complex driver

Probably here also.

> + *
> + * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
> + *
> + * Authors: Yu Ning <ningyu@eswincomputing.com>
> + *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> + *          Yanghui Ou <ouyanghui@eswincomputing.com>
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/resource.h>
> +#include <linux/reset.h>
> +#include <linux/types.h>
> +
> +#include "pcie-designware.h"
> +
> +/* PCIe top csr registers */
> +#define PCIEMGMT_CTRL0_OFFSET		0x0
> +#define PCIEMGMT_STATUS0_OFFSET		0x100
> +
> +/* LTSSM register fields */
> +#define PCIEMGMT_APP_LTSSM_ENABLE	BIT(5)
> +
> +/* APP_HOLD_PHY_RST register fields */
> +#define PCIEMGMT_APP_HOLD_PHY_RST	BIT(6)
> +
> +/* PM_SEL_AUX_CLK register fields */
> +#define PCIEMGMT_PM_SEL_AUX_CLK		BIT(16)
> +
> +/* ROOT_PORT register fields */
> +#define PCIEMGMT_CTRL0_ROOT_PORT_MASK	GENMASK(3, 0)

Looks like this is actually a "device type" field, not a "root port"
field, since you OR in the PCI_EXP_TYPE_ROOT_PORT device type below.

Maybe you could name it simply "PCIEMGMT_CTRL0_DEV_TYPE" or similar?

> +/* Vendor and device id value */

s/id/ID/

> +#define PCI_VENDOR_ID_ESWIN		0x1fe1
> +#define PCI_DEVICE_ID_ESWIN		0x2030
> +
> +struct eswin_pcie_data {

Generally speaking the prefix for structs and functions matches the
driver filename, i.e., "eic7700" in this case.

  $ git grep "^struct .*_pcie {" drivers/pci/controller/dwc
  drivers/pci/controller/dwc/pci-dra7xx.c:struct dra7xx_pcie {
  drivers/pci/controller/dwc/pci-exynos.c:struct exynos_pcie {
  drivers/pci/controller/dwc/pci-imx6.c:struct imx_pcie {
  drivers/pci/controller/dwc/pci-keystone.c:struct keystone_pcie {
  ...

> +static int eswin_pcie_perst_deassert(struct eswin_pcie_port *port,
> +				     struct eswin_pcie *pcie)
> +{
> +	int ret;
> +
> +	ret = reset_control_assert(port->perst);
> +	if (ret) {
> +		dev_err(pcie->pci.dev, "Failed to assert PERST#");
> +		return ret;
> +	}
> +
> +	/* Ensure that PERST has been asserted for at least 100 ms */

s/PERST/PERST#/

> +	msleep(PCIE_T_PVPERL_MS);
> +
> +	ret = reset_control_deassert(port->perst);
> +	if (ret) {
> +		dev_err(pcie->pci.dev, "Failed to deassert PERST#");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int eswin_pcie_parse_port(struct eswin_pcie *pcie,
> +				 struct device_node *node)
> +{
> +	struct device *dev = pcie->pci.dev;
> +	struct eswin_pcie_port *port;
> +
> +	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> +	if (!port)
> +		return -ENOMEM;
> +
> +	port->perst = of_reset_control_get(node, "perst");
> +	if (IS_ERR(port->perst)) {
> +		dev_err(dev, "Failed to get perst reset\n");

s/perst/PERST#/ to match spec usage and messages above.

> +static int eswin_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct eswin_pcie *pcie = to_eswin_pcie(pci);
> +	struct eswin_pcie_port *port;
> +	u8 msi_cap;
> +	u32 val;
> +	int ret;
> +
> +	pcie->num_clks = devm_clk_bulk_get_all_enabled(pci->dev, &pcie->clks);
> +	if (pcie->num_clks < 0)
> +		return dev_err_probe(pci->dev, pcie->num_clks,
> +				     "Failed to get pcie clocks\n");
> +
> +	ret = eswin_pcie_deassert(pcie);
> +	if (ret)
> +		return ret;
> +
> +	/* Configure root port type */
> +	val = readl_relaxed(pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);
> +	val &= ~PCIEMGMT_CTRL0_ROOT_PORT_MASK;
> +	writel_relaxed(val | PCI_EXP_TYPE_ROOT_PORT,
> +		       pcie->mgmt_base + PCIEMGMT_CTRL0_OFFSET);

Use FIELD_PREP() here to remove the assumption that
PCIEMGMT_CTRL0_ROOT_PORT_MASK is in the low-order bits.

> +static void eswin_pcie_host_exit(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct eswin_pcie *pcie = to_eswin_pcie(pci);
> +	struct eswin_pcie_port *port;
> +
> +	/*
> +	 * For controllers with active devices, resources are retained and
> +	 * cannot be turned off, like NVMEe.

s/NVMEe/NVMe/

I'm a little skeptical about having behavior here that depends on
specific kinds of downstream devices.

Maybe there's some general requirement that these resources need to be
retained if the link is up, and there's no need to mention NVMe
specifically?  I don't see similar code in other drivers, though.

> +	 */
> +	if (!dw_pcie_link_up(&pcie->pci)) {
> +		list_for_each_entry(port, &pcie->ports, list)
> +			reset_control_assert(port->perst);
> +		eswin_pcie_assert(pcie);
> +		clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
> +	}
> +}
> +
> +static void eswin_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> +	/*
> +	 * Hardware doesn't support enter the D3code and L2/L3 states, send
> +	 * PME_TURN_OFF message, which will then cause Vmain to be removed and
> +	 * controller stop working.
> +	 */
> +	dev_info(pci->dev, "Can't send PME_TURN_OFF message\n");

s/PME_TURN_OFF/PME_Turn_Off/ to match spec usage.

> +}
> +
> +static const struct dw_pcie_host_ops eswin_pcie_host_ops = {
> +	.init = eswin_pcie_host_init,
> +	.deinit = eswin_pcie_host_exit,

Please include "deinit" in this function name so it's connected to the
.deinit structure member.

> +static int eswin_pcie_probe(struct platform_device *pdev)
> +{
> +	const struct eswin_pcie_data *data;
> +	struct eswin_pcie_port *port, *tmp;
> +	struct device *dev = &pdev->dev;
> +	struct eswin_pcie *pcie;
> +	struct dw_pcie *pci;
> +	int ret;
> +
> +	data = of_device_get_match_data(dev);
> +	if (!data)
> +		return dev_err_probe(dev, -EINVAL, "OF data missing\n");
> +
> +	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> +	if (!pcie)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&pcie->ports);
> +
> +	pci = &pcie->pci;
> +	pci->dev = dev;
> +	pci->ops = &dw_pcie_ops;
> +	pci->pp.ops = &eswin_pcie_host_ops;
> +	pcie->msix_cap = data->msix_cap;

I'm not sure there's really any value in copying msix_cap, since
data->msix_cap is a read-only item anyway.

For example, pcie-qcom.c has a per-SoC struct qcom_pcie_cfg, and it
just saves the qcom_pcie_cfg pointer in struct qcom_pcie:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pcie-qcom.c?id=v6.17#n286

> +static int eswin_pcie_resume_noirq(struct device *dev)
> +{
> +	struct eswin_pcie *pcie = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = dw_pcie_resume_noirq(&pcie->pci);

Add blank line here.

> +	/*
> +	 * If the downstream device is not inserted, linkup will TIMEDOUT. At
> +	 * this time, when the resume function return, -ETIMEDOUT shouldn't be
> +	 * returned, which will raise "PM: failed to resume noirq: error -110".
> +	 * Only log message "Ignore errors, the link may come up later".
> +	 */
> +	if (ret == -ETIMEDOUT && !pcie->linked_up) {
> +		dev_info(dev, "Ignore errors, the link may come up later\n");
> +		return 0;
> +	}
> +
> +	return ret;
> +}

> +MODULE_DESCRIPTION("PCIe host controller driver for EIC7700 SoCs");

Include vendor ("Eswin") in the description.  You can use this to see
the typical style:

  $ git grep MODULE_DESCRIPTION drivers/pci/controller/