[PATCH 0/2] riscv: sophgo Add PCIe support to Sophgo SG2044 SoC

[PATCH 0/2] riscv: sophgo Add PCIe support to Sophgo SG2044 SoC

[Inochi Amaoto]

Sophgo's SG2044 SoC uses Synopsys Designware PCIe core
to implement RC mode.

For legacy interrupt, the PCIe controller on SG2044 implement
its own legacy interrupt controller. For MSI/MSI-X, it use an
external interrupt controller to handle.

The external MSI interrupt controller patch can be found on [1].
As SG2044 needs a mirror change to support the way to send MSI
message and different irq number.

[1] https://lore.kernel.org/all/cover.1736921549.git.unicorn_wang@outlook.com/

Inochi Amaoto (2):
  dt-bindings: pci: Add Sophgo SG2044 PCIe host
  PCI: sophgo-dwc: Add Sophgo SG2044 PCIe driver

 .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++
 drivers/pci/controller/dwc/Kconfig            |  10 +
 drivers/pci/controller/dwc/Makefile           |   1 +
 drivers/pci/controller/dwc/pcie-dw-sophgo.c   | 282 ++++++++++++++++++
 4 files changed, 418 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-dw-sophgo.c

--
2.48.1

[PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Inochi Amaoto]

The pcie controller on the SG2044 is designware based with
custom app registers.

Add binding document for SG2044 PCIe host controller.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++++++++++++
 1 file changed, 125 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
new file mode 100644
index 000000000000..040dabe905e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
@@ -0,0 +1,125 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
+
+maintainers:
+  - Inochi Amaoto <inochiama@gmail.com>
+
+description: |+
+  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
+  PCIe IP and thus inherits all the common properties defined in
+  snps,dw-pcie.yaml.
+
+allOf:
+  - $ref: /schemas/pci/pci-host-bridge.yaml#
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+
+properties:
+  compatible:
+    const: sophgo,sg2044-pcie
+
+  reg:
+    items:
+      - description: Data Bus Interface (DBI) registers
+      - description: iATU registers
+      - description: Config registers
+      - description: Sophgo designed configuration registers
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: atu
+      - const: config
+      - const: app
+
+  clocks:
+    items:
+      - description: core clk
+
+  clock-names:
+    items:
+      - const: core
+
+  dma-coherent: true
+
+  interrupt-controller:
+    description: Interrupt controller node for handling legacy PCI interrupts.
+    type: object
+
+    properties:
+      "#address-cells":
+        const: 0
+
+      "#interrupt-cells":
+        const: 1
+
+      interrupt-controller: true
+
+      interrupts:
+        items:
+          - description: combined legacy interrupt
+
+    required:
+      - "#address-cells"
+      - "#interrupt-cells"
+      - interrupt-controller
+      - interrupts
+
+    additionalProperties: false
+
+  msi-parent: true
+
+  ranges:
+    maxItems: 5
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      pcie@6c00400000 {
+        compatible = "sophgo,sg2044-pcie";
+        reg = <0x6c 0x00400000 0x0 0x00001000>,
+              <0x6c 0x00700000 0x0 0x00004000>,
+              <0x40 0x00000000 0x0 0x00001000>,
+              <0x6c 0x00780c00 0x0 0x00000400>;
+        reg-names = "dbi", "atu", "config", "app";
+        #address-cells = <3>;
+        #size-cells = <2>;
+        bus-range = <0x00 0xff>;
+        clocks = <&clk 0>;
+        clock-names = "core";
+        device_type = "pci";
+        dma-coherent;
+        linux,pci-domain = <0>;
+        msi-parent = <&msi>;
+        ranges = <0x01000000 0x0  0x00000000  0x40 0x10000000  0x0 0x00200000>,
+                 <0x42000000 0x0  0x00000000  0x0  0x00000000  0x0 0x04000000>,
+                 <0x02000000 0x0  0x04000000  0x0  0x04000000  0x0 0x04000000>,
+                 <0x43000000 0x42 0x00000000  0x42 0x00000000  0x2 0x00000000>,
+                 <0x03000000 0x41 0x00000000  0x41 0x00000000  0x1 0x00000000>;
+
+        interrupt-controller {
+          #address-cells = <0>;
+          #interrupt-cells = <1>;
+          interrupt-controller;
+          interrupt-parent = <&intc>;
+          interrupts = <64 IRQ_TYPE_LEVEL_HIGH>;
+        };
+      };
+    };
+...
-- 
2.48.1

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4503 bytes --]

On Fri, Feb 21, 2025 at 09:37:55AM +0800, Inochi Amaoto wrote:
> The pcie controller on the SG2044 is designware based with
> custom app registers.
> 
> Add binding document for SG2044 PCIe host controller.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++++++++++++
>  1 file changed, 125 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> new file mode 100644
> index 000000000000..040dabe905e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> @@ -0,0 +1,125 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
> +
> +maintainers:
> +  - Inochi Amaoto <inochiama@gmail.com>
> +
> +description: |+
> +  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
> +  PCIe IP and thus inherits all the common properties defined in
> +  snps,dw-pcie.yaml.
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> +
> +properties:
> +  compatible:
> +    const: sophgo,sg2044-pcie
> +
> +  reg:
> +    items:
> +      - description: Data Bus Interface (DBI) registers
> +      - description: iATU registers
> +      - description: Config registers
> +      - description: Sophgo designed configuration registers
> +
> +  reg-names:
> +    items:
> +      - const: dbi
> +      - const: atu
> +      - const: config
> +      - const: app
> +
> +  clocks:
> +    items:
> +      - description: core clk
> +
> +  clock-names:
> +    items:
> +      - const: core
> +
> +  dma-coherent: true

Why's this here? RISC-V is dma-coherent by default, with dma-noncoherent
used to indicate systems/devices that are not.

Cheers,
Conor.

> +
> +  interrupt-controller:
> +    description: Interrupt controller node for handling legacy PCI interrupts.
> +    type: object
> +
> +    properties:
> +      "#address-cells":
> +        const: 0
> +
> +      "#interrupt-cells":
> +        const: 1
> +
> +      interrupt-controller: true
> +
> +      interrupts:
> +        items:
> +          - description: combined legacy interrupt
> +
> +    required:
> +      - "#address-cells"
> +      - "#interrupt-cells"
> +      - interrupt-controller
> +      - interrupts
> +
> +    additionalProperties: false
> +
> +  msi-parent: true
> +
> +  ranges:
> +    maxItems: 5
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      pcie@6c00400000 {
> +        compatible = "sophgo,sg2044-pcie";
> +        reg = <0x6c 0x00400000 0x0 0x00001000>,
> +              <0x6c 0x00700000 0x0 0x00004000>,
> +              <0x40 0x00000000 0x0 0x00001000>,
> +              <0x6c 0x00780c00 0x0 0x00000400>;
> +        reg-names = "dbi", "atu", "config", "app";
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +        bus-range = <0x00 0xff>;
> +        clocks = <&clk 0>;
> +        clock-names = "core";
> +        device_type = "pci";
> +        dma-coherent;
> +        linux,pci-domain = <0>;
> +        msi-parent = <&msi>;
> +        ranges = <0x01000000 0x0  0x00000000  0x40 0x10000000  0x0 0x00200000>,
> +                 <0x42000000 0x0  0x00000000  0x0  0x00000000  0x0 0x04000000>,
> +                 <0x02000000 0x0  0x04000000  0x0  0x04000000  0x0 0x04000000>,
> +                 <0x43000000 0x42 0x00000000  0x42 0x00000000  0x2 0x00000000>,
> +                 <0x03000000 0x41 0x00000000  0x41 0x00000000  0x1 0x00000000>;
> +
> +        interrupt-controller {
> +          #address-cells = <0>;
> +          #interrupt-cells = <1>;
> +          interrupt-controller;
> +          interrupt-parent = <&intc>;
> +          interrupts = <64 IRQ_TYPE_LEVEL_HIGH>;
> +        };
> +      };
> +    };
> +...
> -- 
> 2.48.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Inochi Amaoto]

On Fri, Feb 21, 2025 at 05:01:41PM +0000, Conor Dooley wrote:
> On Fri, Feb 21, 2025 at 09:37:55AM +0800, Inochi Amaoto wrote:
> > The pcie controller on the SG2044 is designware based with
> > custom app registers.
> > 
> > Add binding document for SG2044 PCIe host controller.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> >  .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++++++++++++
> >  1 file changed, 125 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > new file mode 100644
> > index 000000000000..040dabe905e0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > @@ -0,0 +1,125 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
> > +
> > +maintainers:
> > +  - Inochi Amaoto <inochiama@gmail.com>
> > +
> > +description: |+
> > +  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
> > +  PCIe IP and thus inherits all the common properties defined in
> > +  snps,dw-pcie.yaml.
> > +
> > +allOf:
> > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: sophgo,sg2044-pcie
> > +
> > +  reg:
> > +    items:
> > +      - description: Data Bus Interface (DBI) registers
> > +      - description: iATU registers
> > +      - description: Config registers
> > +      - description: Sophgo designed configuration registers
> > +
> > +  reg-names:
> > +    items:
> > +      - const: dbi
> > +      - const: atu
> > +      - const: config
> > +      - const: app
> > +
> > +  clocks:
> > +    items:
> > +      - description: core clk
> > +
> > +  clock-names:
> > +    items:
> > +      - const: core
> > +
> > +  dma-coherent: true
> 
> Why's this here? RISC-V is dma-coherent by default, with dma-noncoherent
> used to indicate systems/devices that are not.
> 
> Cheers,
> Conor.
> 

The PCIe is dma coherent, but the SoC itself is marked as
dma-noncoherent. So I add dma-coherent to the binding. I
wonder whether dma-coherent is necessary even in this case?

Regards,
Inochi

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2959 bytes --]

On Sat, Feb 22, 2025 at 08:34:10AM +0800, Inochi Amaoto wrote:
> On Fri, Feb 21, 2025 at 05:01:41PM +0000, Conor Dooley wrote:
> > On Fri, Feb 21, 2025 at 09:37:55AM +0800, Inochi Amaoto wrote:
> > > The pcie controller on the SG2044 is designware based with
> > > custom app registers.
> > > 
> > > Add binding document for SG2044 PCIe host controller.
> > > 
> > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > ---
> > >  .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++++++++++++
> > >  1 file changed, 125 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > new file mode 100644
> > > index 000000000000..040dabe905e0
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > @@ -0,0 +1,125 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
> > > +
> > > +maintainers:
> > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > +
> > > +description: |+
> > > +  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
> > > +  PCIe IP and thus inherits all the common properties defined in
> > > +  snps,dw-pcie.yaml.
> > > +
> > > +allOf:
> > > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: sophgo,sg2044-pcie
> > > +
> > > +  reg:
> > > +    items:
> > > +      - description: Data Bus Interface (DBI) registers
> > > +      - description: iATU registers
> > > +      - description: Config registers
> > > +      - description: Sophgo designed configuration registers
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: dbi
> > > +      - const: atu
> > > +      - const: config
> > > +      - const: app
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: core clk
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: core
> > > +
> > > +  dma-coherent: true
> > 
> > Why's this here? RISC-V is dma-coherent by default, with dma-noncoherent
> > used to indicate systems/devices that are not.
> 
> The PCIe is dma coherent, but the SoC itself is marked as
> dma-noncoherent.

By "the SoC itself", do you mean that the bus that this device is on is
marked as dma-noncoherent? IMO, that should not be done if there are
devices on it that are coherent.

> So I add dma-coherent to the binding. I
> wonder whether dma-coherent is necessary even in this case?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Inochi Amaoto]

On Mon, Feb 24, 2025 at 06:54:51PM +0000, Conor Dooley wrote:
> On Sat, Feb 22, 2025 at 08:34:10AM +0800, Inochi Amaoto wrote:
> > On Fri, Feb 21, 2025 at 05:01:41PM +0000, Conor Dooley wrote:
> > > On Fri, Feb 21, 2025 at 09:37:55AM +0800, Inochi Amaoto wrote:
> > > > The pcie controller on the SG2044 is designware based with
> > > > custom app registers.
> > > > 
> > > > Add binding document for SG2044 PCIe host controller.
> > > > 
> > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > ---
> > > >  .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++++++++++++
> > > >  1 file changed, 125 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > new file mode 100644
> > > > index 000000000000..040dabe905e0
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > @@ -0,0 +1,125 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
> > > > +
> > > > +maintainers:
> > > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > > +
> > > > +description: |+
> > > > +  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
> > > > +  PCIe IP and thus inherits all the common properties defined in
> > > > +  snps,dw-pcie.yaml.
> > > > +
> > > > +allOf:
> > > > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: sophgo,sg2044-pcie
> > > > +
> > > > +  reg:
> > > > +    items:
> > > > +      - description: Data Bus Interface (DBI) registers
> > > > +      - description: iATU registers
> > > > +      - description: Config registers
> > > > +      - description: Sophgo designed configuration registers
> > > > +
> > > > +  reg-names:
> > > > +    items:
> > > > +      - const: dbi
> > > > +      - const: atu
> > > > +      - const: config
> > > > +      - const: app
> > > > +
> > > > +  clocks:
> > > > +    items:
> > > > +      - description: core clk
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: core
> > > > +
> > > > +  dma-coherent: true
> > > 
> > > Why's this here? RISC-V is dma-coherent by default, with dma-noncoherent
> > > used to indicate systems/devices that are not.
> > 
> > The PCIe is dma coherent, but the SoC itself is marked as
> > dma-noncoherent.
> 
> By "the SoC itself", do you mean that the bus that this device is on is
> marked as dma-noncoherent? 

Yeah, I was told only PCIe device on SG2044 is dma coherent.
The others are not.

> IMO, that should not be done if there are devices on it that are coherent.
> 

It is OK for me. But I wonder how to handle the non coherent device
in DT? Just Mark the bus coherent and mark all devices except the
PCIe device non coherent?

Regards,
Inochi

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3787 bytes --]

On Tue, Feb 25, 2025 at 07:48:59AM +0800, Inochi Amaoto wrote:
> On Mon, Feb 24, 2025 at 06:54:51PM +0000, Conor Dooley wrote:
> > On Sat, Feb 22, 2025 at 08:34:10AM +0800, Inochi Amaoto wrote:
> > > On Fri, Feb 21, 2025 at 05:01:41PM +0000, Conor Dooley wrote:
> > > > On Fri, Feb 21, 2025 at 09:37:55AM +0800, Inochi Amaoto wrote:
> > > > > The pcie controller on the SG2044 is designware based with
> > > > > custom app registers.
> > > > > 
> > > > > Add binding document for SG2044 PCIe host controller.
> > > > > 
> > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > > ---
> > > > >  .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++++++++++++
> > > > >  1 file changed, 125 insertions(+)
> > > > >  create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..040dabe905e0
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > @@ -0,0 +1,125 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
> > > > > +
> > > > > +maintainers:
> > > > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > > > +
> > > > > +description: |+
> > > > > +  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
> > > > > +  PCIe IP and thus inherits all the common properties defined in
> > > > > +  snps,dw-pcie.yaml.
> > > > > +
> > > > > +allOf:
> > > > > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    const: sophgo,sg2044-pcie
> > > > > +
> > > > > +  reg:
> > > > > +    items:
> > > > > +      - description: Data Bus Interface (DBI) registers
> > > > > +      - description: iATU registers
> > > > > +      - description: Config registers
> > > > > +      - description: Sophgo designed configuration registers
> > > > > +
> > > > > +  reg-names:
> > > > > +    items:
> > > > > +      - const: dbi
> > > > > +      - const: atu
> > > > > +      - const: config
> > > > > +      - const: app
> > > > > +
> > > > > +  clocks:
> > > > > +    items:
> > > > > +      - description: core clk
> > > > > +
> > > > > +  clock-names:
> > > > > +    items:
> > > > > +      - const: core
> > > > > +
> > > > > +  dma-coherent: true
> > > > 
> > > > Why's this here? RISC-V is dma-coherent by default, with dma-noncoherent
> > > > used to indicate systems/devices that are not.
> > > 
> > > The PCIe is dma coherent, but the SoC itself is marked as
> > > dma-noncoherent.
> > 
> > By "the SoC itself", do you mean that the bus that this device is on is
> > marked as dma-noncoherent? 
> 
> Yeah, I was told only PCIe device on SG2044 is dma coherent.
> The others are not.
> 
> > IMO, that should not be done if there are devices on it that are coherent.
> > 
> 
> It is OK for me. But I wonder how to handle the non coherent device
> in DT? Just Mark the bus coherent and mark all devices except the
> PCIe device non coherent?

Don't mark the bus anything (default is coherent) and mark the devices.
That said, Is the PCIe controller actually on the same bus as the other
devices? (Not talking about the same DT node, the actual bus in the
device)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Inochi Amaoto]

On Tue, Feb 25, 2025 at 11:35:23PM +0000, Conor Dooley wrote:
> On Tue, Feb 25, 2025 at 07:48:59AM +0800, Inochi Amaoto wrote:
> > On Mon, Feb 24, 2025 at 06:54:51PM +0000, Conor Dooley wrote:
> > > On Sat, Feb 22, 2025 at 08:34:10AM +0800, Inochi Amaoto wrote:
> > > > On Fri, Feb 21, 2025 at 05:01:41PM +0000, Conor Dooley wrote:
> > > > > On Fri, Feb 21, 2025 at 09:37:55AM +0800, Inochi Amaoto wrote:
> > > > > > The pcie controller on the SG2044 is designware based with
> > > > > > custom app registers.
> > > > > > 
> > > > > > Add binding document for SG2044 PCIe host controller.
> > > > > > 
> > > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > > > ---
> > > > > >  .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++++++++++++
> > > > > >  1 file changed, 125 insertions(+)
> > > > > >  create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..040dabe905e0
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > @@ -0,0 +1,125 @@
> > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
> > > > > > +
> > > > > > +maintainers:
> > > > > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > > > > +
> > > > > > +description: |+
> > > > > > +  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
> > > > > > +  PCIe IP and thus inherits all the common properties defined in
> > > > > > +  snps,dw-pcie.yaml.
> > > > > > +
> > > > > > +allOf:
> > > > > > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > > > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > > > +
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +    const: sophgo,sg2044-pcie
> > > > > > +
> > > > > > +  reg:
> > > > > > +    items:
> > > > > > +      - description: Data Bus Interface (DBI) registers
> > > > > > +      - description: iATU registers
> > > > > > +      - description: Config registers
> > > > > > +      - description: Sophgo designed configuration registers
> > > > > > +
> > > > > > +  reg-names:
> > > > > > +    items:
> > > > > > +      - const: dbi
> > > > > > +      - const: atu
> > > > > > +      - const: config
> > > > > > +      - const: app
> > > > > > +
> > > > > > +  clocks:
> > > > > > +    items:
> > > > > > +      - description: core clk
> > > > > > +
> > > > > > +  clock-names:
> > > > > > +    items:
> > > > > > +      - const: core
> > > > > > +
> > > > > > +  dma-coherent: true
> > > > > 
> > > > > Why's this here? RISC-V is dma-coherent by default, with dma-noncoherent
> > > > > used to indicate systems/devices that are not.
> > > > 
> > > > The PCIe is dma coherent, but the SoC itself is marked as
> > > > dma-noncoherent.
> > > 
> > > By "the SoC itself", do you mean that the bus that this device is on is
> > > marked as dma-noncoherent? 
> > 
> > Yeah, I was told only PCIe device on SG2044 is dma coherent.
> > The others are not.
> > 
> > > IMO, that should not be done if there are devices on it that are coherent.
> > > 
> > 
> > It is OK for me. But I wonder how to handle the non coherent device
> > in DT? Just Mark the bus coherent and mark all devices except the
> > PCIe device non coherent?
> 
> Don't mark the bus anything (default is coherent) and mark the devices.

I think this is OK for me.

> That said, Is the PCIe controller actually on the same bus as the
> other devices? (Not talking about the same DT node, the actual bus
> in the device)

It share the same bus with other device, but on a different
master interface.

Regards,
Inochi

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Inochi Amaoto]

On Fri, Feb 28, 2025 at 02:34:00PM +0800, Inochi Amaoto wrote:
> On Tue, Feb 25, 2025 at 11:35:23PM +0000, Conor Dooley wrote:
> > On Tue, Feb 25, 2025 at 07:48:59AM +0800, Inochi Amaoto wrote:
> > > On Mon, Feb 24, 2025 at 06:54:51PM +0000, Conor Dooley wrote:
> > > > On Sat, Feb 22, 2025 at 08:34:10AM +0800, Inochi Amaoto wrote:
> > > > > On Fri, Feb 21, 2025 at 05:01:41PM +0000, Conor Dooley wrote:
> > > > > > On Fri, Feb 21, 2025 at 09:37:55AM +0800, Inochi Amaoto wrote:
> > > > > > > The pcie controller on the SG2044 is designware based with
> > > > > > > custom app registers.
> > > > > > > 
> > > > > > > Add binding document for SG2044 PCIe host controller.
> > > > > > > 
> > > > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > > > > ---
> > > > > > >  .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++++++++++++
> > > > > > >  1 file changed, 125 insertions(+)
> > > > > > >  create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > 
> > > > > > > diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > new file mode 100644
> > > > > > > index 000000000000..040dabe905e0
> > > > > > > --- /dev/null
> > > > > > > +++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > @@ -0,0 +1,125 @@
> > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > > +%YAML 1.2
> > > > > > > +---
> > > > > > > +$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
> > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > > +
> > > > > > > +title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
> > > > > > > +
> > > > > > > +maintainers:
> > > > > > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > > > > > +
> > > > > > > +description: |+
> > > > > > > +  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
> > > > > > > +  PCIe IP and thus inherits all the common properties defined in
> > > > > > > +  snps,dw-pcie.yaml.
> > > > > > > +
> > > > > > > +allOf:
> > > > > > > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > > > > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > > > > +
> > > > > > > +properties:
> > > > > > > +  compatible:
> > > > > > > +    const: sophgo,sg2044-pcie
> > > > > > > +
> > > > > > > +  reg:
> > > > > > > +    items:
> > > > > > > +      - description: Data Bus Interface (DBI) registers
> > > > > > > +      - description: iATU registers
> > > > > > > +      - description: Config registers
> > > > > > > +      - description: Sophgo designed configuration registers
> > > > > > > +
> > > > > > > +  reg-names:
> > > > > > > +    items:
> > > > > > > +      - const: dbi
> > > > > > > +      - const: atu
> > > > > > > +      - const: config
> > > > > > > +      - const: app
> > > > > > > +
> > > > > > > +  clocks:
> > > > > > > +    items:
> > > > > > > +      - description: core clk
> > > > > > > +
> > > > > > > +  clock-names:
> > > > > > > +    items:
> > > > > > > +      - const: core
> > > > > > > +
> > > > > > > +  dma-coherent: true
> > > > > > 
> > > > > > Why's this here? RISC-V is dma-coherent by default, with dma-noncoherent
> > > > > > used to indicate systems/devices that are not.
> > > > > 
> > > > > The PCIe is dma coherent, but the SoC itself is marked as
> > > > > dma-noncoherent.
> > > > 
> > > > By "the SoC itself", do you mean that the bus that this device is on is
> > > > marked as dma-noncoherent? 
> > > 
> > > Yeah, I was told only PCIe device on SG2044 is dma coherent.
> > > The others are not.
> > > 
> > > > IMO, that should not be done if there are devices on it that are coherent.
> > > > 
> > > 
> > > It is OK for me. But I wonder how to handle the non coherent device
> > > in DT? Just Mark the bus coherent and mark all devices except the
> > > PCIe device non coherent?
> > 
> > Don't mark the bus anything (default is coherent) and mark the devices.
> 
> I think this is OK for me.
> 

In technical, I wonder a better way to "handle dma-noncoherent".
In the binding check, all devices with this property complains 

"Unevaluated properties are not allowed ('dma-noncoherent' was unexpected)"

It is a pain as at least 10 devices' binding need to be modified.
So I wonder whether there is a way to simplify this.

Regars,
Inochi

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Inochi Amaoto]

On Fri, Feb 28, 2025 at 04:46:22PM +0800, Inochi Amaoto wrote:
> On Fri, Feb 28, 2025 at 02:34:00PM +0800, Inochi Amaoto wrote:
> > On Tue, Feb 25, 2025 at 11:35:23PM +0000, Conor Dooley wrote:
> > > On Tue, Feb 25, 2025 at 07:48:59AM +0800, Inochi Amaoto wrote:
> > > > On Mon, Feb 24, 2025 at 06:54:51PM +0000, Conor Dooley wrote:
> > > > > On Sat, Feb 22, 2025 at 08:34:10AM +0800, Inochi Amaoto wrote:
> > > > > > On Fri, Feb 21, 2025 at 05:01:41PM +0000, Conor Dooley wrote:
> > > > > > > On Fri, Feb 21, 2025 at 09:37:55AM +0800, Inochi Amaoto wrote:
> > > > > > > > The pcie controller on the SG2044 is designware based with
> > > > > > > > custom app registers.
> > > > > > > > 
> > > > > > > > Add binding document for SG2044 PCIe host controller.
> > > > > > > > 
> > > > > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > > > > > ---
> > > > > > > >  .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++++++++++++
> > > > > > > >  1 file changed, 125 insertions(+)
> > > > > > > >  create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > > 
> > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > > new file mode 100644
> > > > > > > > index 000000000000..040dabe905e0
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > > @@ -0,0 +1,125 @@
> > > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > > > +%YAML 1.2
> > > > > > > > +---
> > > > > > > > +$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
> > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > > > +
> > > > > > > > +title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
> > > > > > > > +
> > > > > > > > +maintainers:
> > > > > > > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > > > > > > +
> > > > > > > > +description: |+
> > > > > > > > +  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
> > > > > > > > +  PCIe IP and thus inherits all the common properties defined in
> > > > > > > > +  snps,dw-pcie.yaml.
> > > > > > > > +
> > > > > > > > +allOf:
> > > > > > > > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > > > > > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > > > > > +
> > > > > > > > +properties:
> > > > > > > > +  compatible:
> > > > > > > > +    const: sophgo,sg2044-pcie
> > > > > > > > +
> > > > > > > > +  reg:
> > > > > > > > +    items:
> > > > > > > > +      - description: Data Bus Interface (DBI) registers
> > > > > > > > +      - description: iATU registers
> > > > > > > > +      - description: Config registers
> > > > > > > > +      - description: Sophgo designed configuration registers
> > > > > > > > +
> > > > > > > > +  reg-names:
> > > > > > > > +    items:
> > > > > > > > +      - const: dbi
> > > > > > > > +      - const: atu
> > > > > > > > +      - const: config
> > > > > > > > +      - const: app
> > > > > > > > +
> > > > > > > > +  clocks:
> > > > > > > > +    items:
> > > > > > > > +      - description: core clk
> > > > > > > > +
> > > > > > > > +  clock-names:
> > > > > > > > +    items:
> > > > > > > > +      - const: core
> > > > > > > > +
> > > > > > > > +  dma-coherent: true
> > > > > > > 
> > > > > > > Why's this here? RISC-V is dma-coherent by default, with dma-noncoherent
> > > > > > > used to indicate systems/devices that are not.
> > > > > > 
> > > > > > The PCIe is dma coherent, but the SoC itself is marked as
> > > > > > dma-noncoherent.
> > > > > 
> > > > > By "the SoC itself", do you mean that the bus that this device is on is
> > > > > marked as dma-noncoherent? 
> > > > 
> > > > Yeah, I was told only PCIe device on SG2044 is dma coherent.
> > > > The others are not.
> > > > 
> > > > > IMO, that should not be done if there are devices on it that are coherent.
> > > > > 
> > > > 
> > > > It is OK for me. But I wonder how to handle the non coherent device
> > > > in DT? Just Mark the bus coherent and mark all devices except the
> > > > PCIe device non coherent?
> > > 
> > > Don't mark the bus anything (default is coherent) and mark the devices.
> > 
> > I think this is OK for me.
> > 
> 
> In technical, I wonder a better way to "handle dma-noncoherent".
> In the binding check, all devices with this property complains 
> 
> "Unevaluated properties are not allowed ('dma-noncoherent' was unexpected)"
> 

> It is a pain as at least 10 devices' binding need to be modified.
> So I wonder whether there is a way to simplify this.
> 

Ignore this, I misunderstood the dma device. it seems like 
only dmac and eth needs it.

Regards,
Inochi

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5503 bytes --]

On Fri, Feb 28, 2025 at 05:20:28PM +0800, Inochi Amaoto wrote:
> On Fri, Feb 28, 2025 at 04:46:22PM +0800, Inochi Amaoto wrote:
> > On Fri, Feb 28, 2025 at 02:34:00PM +0800, Inochi Amaoto wrote:
> > > On Tue, Feb 25, 2025 at 11:35:23PM +0000, Conor Dooley wrote:
> > > > On Tue, Feb 25, 2025 at 07:48:59AM +0800, Inochi Amaoto wrote:
> > > > > On Mon, Feb 24, 2025 at 06:54:51PM +0000, Conor Dooley wrote:
> > > > > > On Sat, Feb 22, 2025 at 08:34:10AM +0800, Inochi Amaoto wrote:
> > > > > > > On Fri, Feb 21, 2025 at 05:01:41PM +0000, Conor Dooley wrote:
> > > > > > > > On Fri, Feb 21, 2025 at 09:37:55AM +0800, Inochi Amaoto wrote:
> > > > > > > > > The pcie controller on the SG2044 is designware based with
> > > > > > > > > custom app registers.
> > > > > > > > > 
> > > > > > > > > Add binding document for SG2044 PCIe host controller.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > > > > > > ---
> > > > > > > > >  .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++++++++++++
> > > > > > > > >  1 file changed, 125 insertions(+)
> > > > > > > > >  create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > > > 
> > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > > > new file mode 100644
> > > > > > > > > index 000000000000..040dabe905e0
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > > > @@ -0,0 +1,125 @@
> > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > > > > +%YAML 1.2
> > > > > > > > > +---
> > > > > > > > > +$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
> > > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > > > > +
> > > > > > > > > +title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
> > > > > > > > > +
> > > > > > > > > +maintainers:
> > > > > > > > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > > > > > > > +
> > > > > > > > > +description: |+
> > > > > > > > > +  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
> > > > > > > > > +  PCIe IP and thus inherits all the common properties defined in
> > > > > > > > > +  snps,dw-pcie.yaml.
> > > > > > > > > +
> > > > > > > > > +allOf:
> > > > > > > > > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > > > > > > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > > > > > > +
> > > > > > > > > +properties:
> > > > > > > > > +  compatible:
> > > > > > > > > +    const: sophgo,sg2044-pcie
> > > > > > > > > +
> > > > > > > > > +  reg:
> > > > > > > > > +    items:
> > > > > > > > > +      - description: Data Bus Interface (DBI) registers
> > > > > > > > > +      - description: iATU registers
> > > > > > > > > +      - description: Config registers
> > > > > > > > > +      - description: Sophgo designed configuration registers
> > > > > > > > > +
> > > > > > > > > +  reg-names:
> > > > > > > > > +    items:
> > > > > > > > > +      - const: dbi
> > > > > > > > > +      - const: atu
> > > > > > > > > +      - const: config
> > > > > > > > > +      - const: app
> > > > > > > > > +
> > > > > > > > > +  clocks:
> > > > > > > > > +    items:
> > > > > > > > > +      - description: core clk
> > > > > > > > > +
> > > > > > > > > +  clock-names:
> > > > > > > > > +    items:
> > > > > > > > > +      - const: core
> > > > > > > > > +
> > > > > > > > > +  dma-coherent: true
> > > > > > > > 
> > > > > > > > Why's this here? RISC-V is dma-coherent by default, with dma-noncoherent
> > > > > > > > used to indicate systems/devices that are not.
> > > > > > > 
> > > > > > > The PCIe is dma coherent, but the SoC itself is marked as
> > > > > > > dma-noncoherent.
> > > > > > 
> > > > > > By "the SoC itself", do you mean that the bus that this device is on is
> > > > > > marked as dma-noncoherent? 
> > > > > 
> > > > > Yeah, I was told only PCIe device on SG2044 is dma coherent.
> > > > > The others are not.
> > > > > 
> > > > > > IMO, that should not be done if there are devices on it that are coherent.
> > > > > > 
> > > > > 
> > > > > It is OK for me. But I wonder how to handle the non coherent device
> > > > > in DT? Just Mark the bus coherent and mark all devices except the
> > > > > PCIe device non coherent?
> > > > 
> > > > Don't mark the bus anything (default is coherent) and mark the devices.
> > > 
> > > I think this is OK for me.
> > > 
> > 
> > In technical, I wonder a better way to "handle dma-noncoherent".
> > In the binding check, all devices with this property complains 
> > 
> > "Unevaluated properties are not allowed ('dma-noncoherent' was unexpected)"
> > 
> 
> > It is a pain as at least 10 devices' binding need to be modified.
> > So I wonder whether there is a way to simplify this.
> > 
> 
> Ignore this, I misunderstood the dma device. it seems like 
> only dmac and eth needs it.

Nah, not gonna ignore it ;) You do make a valid point about it being
painful, but given you mention a different master for the pci device,
having two different soc@<foo> nodes sounds like it might make sense.
One marked dma-noncoherent w/ the existing devices and one that is
unmarked (since that's default) to represent the master than pci is on?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Inochi Amaoto]

On Mon, Mar 03, 2025 at 05:04:28PM +0000, Conor Dooley wrote:
> On Fri, Feb 28, 2025 at 05:20:28PM +0800, Inochi Amaoto wrote:
> > On Fri, Feb 28, 2025 at 04:46:22PM +0800, Inochi Amaoto wrote:
> > > On Fri, Feb 28, 2025 at 02:34:00PM +0800, Inochi Amaoto wrote:
> > > > On Tue, Feb 25, 2025 at 11:35:23PM +0000, Conor Dooley wrote:
> > > > > On Tue, Feb 25, 2025 at 07:48:59AM +0800, Inochi Amaoto wrote:
> > > > > > On Mon, Feb 24, 2025 at 06:54:51PM +0000, Conor Dooley wrote:
> > > > > > > On Sat, Feb 22, 2025 at 08:34:10AM +0800, Inochi Amaoto wrote:
> > > > > > > > On Fri, Feb 21, 2025 at 05:01:41PM +0000, Conor Dooley wrote:
> > > > > > > > > On Fri, Feb 21, 2025 at 09:37:55AM +0800, Inochi Amaoto wrote:
> > > > > > > > > > The pcie controller on the SG2044 is designware based with
> > > > > > > > > > custom app registers.
> > > > > > > > > > 
> > > > > > > > > > Add binding document for SG2044 PCIe host controller.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > > > > > > > ---
> > > > > > > > > >  .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++++++++++++
> > > > > > > > > >  1 file changed, 125 insertions(+)
> > > > > > > > > >  create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > > > > 
> > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > > > > new file mode 100644
> > > > > > > > > > index 000000000000..040dabe905e0
> > > > > > > > > > --- /dev/null
> > > > > > > > > > +++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > > > > @@ -0,0 +1,125 @@
> > > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > > > > > +%YAML 1.2
> > > > > > > > > > +---
> > > > > > > > > > +$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
> > > > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > > > > > +
> > > > > > > > > > +title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
> > > > > > > > > > +
> > > > > > > > > > +maintainers:
> > > > > > > > > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > > > > > > > > +
> > > > > > > > > > +description: |+
> > > > > > > > > > +  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
> > > > > > > > > > +  PCIe IP and thus inherits all the common properties defined in
> > > > > > > > > > +  snps,dw-pcie.yaml.
> > > > > > > > > > +
> > > > > > > > > > +allOf:
> > > > > > > > > > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > > > > > > > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > > > > > > > +
> > > > > > > > > > +properties:
> > > > > > > > > > +  compatible:
> > > > > > > > > > +    const: sophgo,sg2044-pcie
> > > > > > > > > > +
> > > > > > > > > > +  reg:
> > > > > > > > > > +    items:
> > > > > > > > > > +      - description: Data Bus Interface (DBI) registers
> > > > > > > > > > +      - description: iATU registers
> > > > > > > > > > +      - description: Config registers
> > > > > > > > > > +      - description: Sophgo designed configuration registers
> > > > > > > > > > +
> > > > > > > > > > +  reg-names:
> > > > > > > > > > +    items:
> > > > > > > > > > +      - const: dbi
> > > > > > > > > > +      - const: atu
> > > > > > > > > > +      - const: config
> > > > > > > > > > +      - const: app
> > > > > > > > > > +
> > > > > > > > > > +  clocks:
> > > > > > > > > > +    items:
> > > > > > > > > > +      - description: core clk
> > > > > > > > > > +
> > > > > > > > > > +  clock-names:
> > > > > > > > > > +    items:
> > > > > > > > > > +      - const: core
> > > > > > > > > > +
> > > > > > > > > > +  dma-coherent: true
> > > > > > > > > 
> > > > > > > > > Why's this here? RISC-V is dma-coherent by default, with dma-noncoherent
> > > > > > > > > used to indicate systems/devices that are not.
> > > > > > > > 
> > > > > > > > The PCIe is dma coherent, but the SoC itself is marked as
> > > > > > > > dma-noncoherent.
> > > > > > > 
> > > > > > > By "the SoC itself", do you mean that the bus that this device is on is
> > > > > > > marked as dma-noncoherent? 
> > > > > > 
> > > > > > Yeah, I was told only PCIe device on SG2044 is dma coherent.
> > > > > > The others are not.
> > > > > > 
> > > > > > > IMO, that should not be done if there are devices on it that are coherent.
> > > > > > > 
> > > > > > 
> > > > > > It is OK for me. But I wonder how to handle the non coherent device
> > > > > > in DT? Just Mark the bus coherent and mark all devices except the
> > > > > > PCIe device non coherent?
> > > > > 
> > > > > Don't mark the bus anything (default is coherent) and mark the devices.
> > > > 
> > > > I think this is OK for me.
> > > > 
> > > 
> > > In technical, I wonder a better way to "handle dma-noncoherent".
> > > In the binding check, all devices with this property complains 
> > > 
> > > "Unevaluated properties are not allowed ('dma-noncoherent' was unexpected)"
> > > 
> > 
> > > It is a pain as at least 10 devices' binding need to be modified.
> > > So I wonder whether there is a way to simplify this.
> > > 
> > 
> > Ignore this, I misunderstood the dma device. it seems like 
> > only dmac and eth needs it.
> 
> Nah, not gonna ignore it ;) You do make a valid point about it being
> painful, but given you mention a different master for the pci device,
> having two different soc@<foo> nodes sounds like it might make sense.
> One marked dma-noncoherent w/ the existing devices and one that is
> unmarked (since that's default) to represent the master than pci is on?

Yeah, That make sense. I think it serves as a better way for SG2044.

Regards,
Inochi

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Inochi Amaoto]

On Mon, Mar 03, 2025 at 05:04:28PM +0000, Conor Dooley wrote:
> On Fri, Feb 28, 2025 at 05:20:28PM +0800, Inochi Amaoto wrote:
> > On Fri, Feb 28, 2025 at 04:46:22PM +0800, Inochi Amaoto wrote:
> > > On Fri, Feb 28, 2025 at 02:34:00PM +0800, Inochi Amaoto wrote:
> > > > On Tue, Feb 25, 2025 at 11:35:23PM +0000, Conor Dooley wrote:
> > > > > On Tue, Feb 25, 2025 at 07:48:59AM +0800, Inochi Amaoto wrote:
> > > > > > On Mon, Feb 24, 2025 at 06:54:51PM +0000, Conor Dooley wrote:
> > > > > > > On Sat, Feb 22, 2025 at 08:34:10AM +0800, Inochi Amaoto wrote:
> > > > > > > > On Fri, Feb 21, 2025 at 05:01:41PM +0000, Conor Dooley wrote:
> > > > > > > > > On Fri, Feb 21, 2025 at 09:37:55AM +0800, Inochi Amaoto wrote:
> > > > > > > > > > The pcie controller on the SG2044 is designware based with
> > > > > > > > > > custom app registers.
> > > > > > > > > > 
> > > > > > > > > > Add binding document for SG2044 PCIe host controller.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > > > > > > > > ---
> > > > > > > > > >  .../bindings/pci/sophgo,sg2044-pcie.yaml      | 125 ++++++++++++++++++
> > > > > > > > > >  1 file changed, 125 insertions(+)
> > > > > > > > > >  create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > > > > 
> > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > > > > new file mode 100644
> > > > > > > > > > index 000000000000..040dabe905e0
> > > > > > > > > > --- /dev/null
> > > > > > > > > > +++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> > > > > > > > > > @@ -0,0 +1,125 @@
> > > > > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > > > > > +%YAML 1.2
> > > > > > > > > > +---
> > > > > > > > > > +$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
> > > > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > > > > > +
> > > > > > > > > > +title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
> > > > > > > > > > +
> > > > > > > > > > +maintainers:
> > > > > > > > > > +  - Inochi Amaoto <inochiama@gmail.com>
> > > > > > > > > > +
> > > > > > > > > > +description: |+
> > > > > > > > > > +  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
> > > > > > > > > > +  PCIe IP and thus inherits all the common properties defined in
> > > > > > > > > > +  snps,dw-pcie.yaml.
> > > > > > > > > > +
> > > > > > > > > > +allOf:
> > > > > > > > > > +  - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > > > > > > > +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > > > > > > > +
> > > > > > > > > > +properties:
> > > > > > > > > > +  compatible:
> > > > > > > > > > +    const: sophgo,sg2044-pcie
> > > > > > > > > > +
> > > > > > > > > > +  reg:
> > > > > > > > > > +    items:
> > > > > > > > > > +      - description: Data Bus Interface (DBI) registers
> > > > > > > > > > +      - description: iATU registers
> > > > > > > > > > +      - description: Config registers
> > > > > > > > > > +      - description: Sophgo designed configuration registers
> > > > > > > > > > +
> > > > > > > > > > +  reg-names:
> > > > > > > > > > +    items:
> > > > > > > > > > +      - const: dbi
> > > > > > > > > > +      - const: atu
> > > > > > > > > > +      - const: config
> > > > > > > > > > +      - const: app
> > > > > > > > > > +
> > > > > > > > > > +  clocks:
> > > > > > > > > > +    items:
> > > > > > > > > > +      - description: core clk
> > > > > > > > > > +
> > > > > > > > > > +  clock-names:
> > > > > > > > > > +    items:
> > > > > > > > > > +      - const: core
> > > > > > > > > > +
> > > > > > > > > > +  dma-coherent: true
> > > > > > > > > 
> > > > > > > > > Why's this here? RISC-V is dma-coherent by default, with dma-noncoherent
> > > > > > > > > used to indicate systems/devices that are not.
> > > > > > > > 
> > > > > > > > The PCIe is dma coherent, but the SoC itself is marked as
> > > > > > > > dma-noncoherent.
> > > > > > > 
> > > > > > > By "the SoC itself", do you mean that the bus that this device is on is
> > > > > > > marked as dma-noncoherent? 
> > > > > > 
> > > > > > Yeah, I was told only PCIe device on SG2044 is dma coherent.
> > > > > > The others are not.
> > > > > > 
> > > > > > > IMO, that should not be done if there are devices on it that are coherent.
> > > > > > > 
> > > > > > 
> > > > > > It is OK for me. But I wonder how to handle the non coherent device
> > > > > > in DT? Just Mark the bus coherent and mark all devices except the
> > > > > > PCIe device non coherent?
> > > > > 
> > > > > Don't mark the bus anything (default is coherent) and mark the devices.
> > > > 
> > > > I think this is OK for me.
> > > > 
> > > 
> > > In technical, I wonder a better way to "handle dma-noncoherent".
> > > In the binding check, all devices with this property complains 
> > > 
> > > "Unevaluated properties are not allowed ('dma-noncoherent' was unexpected)"
> > > 
> > 
> > > It is a pain as at least 10 devices' binding need to be modified.
> > > So I wonder whether there is a way to simplify this.
> > > 
> > 
> > Ignore this, I misunderstood the dma device. it seems like 
> > only dmac and eth needs it.
> 
> Nah, not gonna ignore it ;) You do make a valid point about it being
> painful, but given you mention a different master for the pci device,
> having two different soc@<foo> nodes sounds like it might make sense.
> One marked dma-noncoherent w/ the existing devices and one that is
> unmarked (since that's default) to represent the master than pci is on?

Hi, Conor,

After some testing. It has some new problems. As the pcie
register area and ranges mapping are not isolated. It is a
disaster to figure the right mapping. If writing this with
soc ranges, the thing may look like the following:

/* this is for pcie */
soc@6c00000000 {
		 /* pcie ip register area */
	ranges = <0x6c 0x00000000 0x6c 0x00000000 0x0  0xc0000000>,
		 /* pcie vendor registes area and other mapping */
		 <0x40 0x00000000 0x40 0x00000000 0x28 0x00000000>,
		 /* for 32 bit memory */
		 <0x0  0x00000000 0x0  0x00000000 0x0  0x70000000>;
};

/* this is for others */
soc@6800000000 {
		 /* clint and some peripherals */
	ranges = <0x68 0x00000000 0x68 0x00000000 0x4  0x0000000>,
		 /* other peripherals */
		 <0x6d 0x00000000 0x6d 0x00000000 0x13 0x00000040>,
		 /* this one is for the external msi controller */
		 <0x0  0x7ee00000 0x0  0x7ee00000 0x0  0x00000040>;
	dma-noncoherent;
};

It is complete a mess. I think it is more clear to just make the
dmac and eth device as noncoherent, and use one soc bus for all.
Do you have any suggestions on it?

Regards,
Inochi

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 309 bytes --]

On Wed, Mar 05, 2025 at 12:43:54PM +0800, Inochi Amaoto wrote:

> It is complete a mess. I think it is more clear to just make the
> dmac and eth device as noncoherent, and use one soc bus for all.
> Do you have any suggestions on it?


If splitting is worse for read/usability, then yes, keep it as one bus.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 2/2] PCI: sophgo-dwc: Add Sophgo SG2044 PCIe driver

[Inochi Amaoto]

Add support for DesignWare-based PCIe controller in SG2044 SoC.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 drivers/pci/controller/dwc/Kconfig          |  10 +
 drivers/pci/controller/dwc/Makefile         |   1 +
 drivers/pci/controller/dwc/pcie-dw-sophgo.c | 282 ++++++++++++++++++++
 3 files changed, 293 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-dw-sophgo.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index b6d6778b0698..202014acf260 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -341,6 +341,16 @@ config PCIE_ROCKCHIP_DW_EP
 	  Enables support for the DesignWare PCIe controller in the
 	  Rockchip SoC (except RK3399) to work in endpoint mode.
 
+config PCIE_SOPHGO_DW
+	bool "SOPHGO DesignWare PCIe controller"
+	depends on ARCH_SOPHGO || COMPILE_TEST
+	depends on PCI_MSI
+	depends on OF
+	select PCIE_DW_HOST
+	help
+	  Enables support for the DesignWare PCIe controller in the
+	  SOPHGO SoC.
+
 config PCI_EXYNOS
 	tristate "Samsung Exynos PCIe controller"
 	depends on ARCH_EXYNOS || COMPILE_TEST
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index a8308d9ea986..193150056dd3 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_PCIE_QCOM_EP) += pcie-qcom-ep.o
 obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
 obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
 obj-$(CONFIG_PCIE_ROCKCHIP_DW) += pcie-dw-rockchip.o
+obj-$(CONFIG_PCIE_SOPHGO_DW) += pcie-dw-sophgo.o
 obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
 obj-$(CONFIG_PCIE_KEEMBAY) += pcie-keembay.o
 obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o
diff --git a/drivers/pci/controller/dwc/pcie-dw-sophgo.c b/drivers/pci/controller/dwc/pcie-dw-sophgo.c
new file mode 100644
index 000000000000..a4ca4f1e26e0
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-dw-sophgo.c
@@ -0,0 +1,282 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCIe host controller driver for Sophgo SoCs.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/gpio/consumer.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/property.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+#include "pcie-designware.h"
+
+#define to_sophgo_pcie(x)		dev_get_drvdata((x)->dev)
+
+#define PCIE_INT_SIGNAL			0xc48
+#define PCIE_INT_EN			0xca0
+
+#define PCIE_SIGNAL_INTX_SHIFT		5
+
+#define PCIE_INT_EN_INTX_SHIFT		1
+#define PCIE_INT_EN_INT_SII		BIT(0)
+#define PCIE_INT_EN_INT_INTA		BIT(1)
+#define PCIE_INT_EN_INT_INTB		BIT(2)
+#define PCIE_INT_EN_INT_INTC		BIT(3)
+#define PCIE_INT_EN_INT_INTD		BIT(4)
+#define PCIE_INT_EN_INT_MSI		BIT(5)
+
+struct sophgo_pcie {
+	struct dw_pcie pci;
+	void __iomem *app_base;
+	struct clk_bulk_data *clks;
+	unsigned int clk_cnt;
+	struct reset_control *rst;
+	struct irq_domain *irq_domain;
+};
+
+static int sophgo_pcie_readl_app(struct sophgo_pcie *sophgo, u32 reg)
+{
+	return readl_relaxed(sophgo->app_base + reg);
+}
+
+static void sophgo_pcie_writel_app(struct sophgo_pcie *sophgo, u32 val, u32 reg)
+{
+	writel_relaxed(val, sophgo->app_base + reg);
+}
+
+static void sophgo_pcie_intx_handler(struct irq_desc *desc)
+{
+	struct dw_pcie_rp *pp = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct sophgo_pcie *sophgo = to_sophgo_pcie(pci);
+	unsigned long hwirq = PCIE_SIGNAL_INTX_SHIFT;
+	unsigned long reg;
+
+	chained_irq_enter(chip, desc);
+
+	reg = sophgo_pcie_readl_app(sophgo, PCIE_INT_SIGNAL);
+
+	for_each_set_bit_from(hwirq, &reg, PCI_NUM_INTX + PCIE_SIGNAL_INTX_SHIFT)
+		generic_handle_domain_irq(sophgo->irq_domain,
+					  hwirq - PCIE_SIGNAL_INTX_SHIFT);
+
+	chained_irq_exit(chip, desc);
+}
+
+static void sophgo_intx_mask(struct irq_data *d)
+{
+	struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct sophgo_pcie *sophgo = to_sophgo_pcie(pci);
+	unsigned long flags;
+	u32 val;
+
+	raw_spin_lock_irqsave(&pp->lock, flags);
+
+	val = sophgo_pcie_readl_app(sophgo, PCIE_INT_EN);
+	val &= ~BIT(d->hwirq + PCIE_INT_EN_INTX_SHIFT);
+	sophgo_pcie_writel_app(sophgo, val, PCIE_INT_EN);
+
+	raw_spin_unlock_irqrestore(&pp->lock, flags);
+};
+
+static void sophgo_intx_unmask(struct irq_data *d)
+{
+	struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct sophgo_pcie *sophgo = to_sophgo_pcie(pci);
+	unsigned long flags;
+	u32 val;
+
+	raw_spin_lock_irqsave(&pp->lock, flags);
+
+	val = sophgo_pcie_readl_app(sophgo, PCIE_INT_EN);
+	val |= BIT(d->hwirq + PCIE_INT_EN_INTX_SHIFT);
+	sophgo_pcie_writel_app(sophgo, val, PCIE_INT_EN);
+
+	raw_spin_unlock_irqrestore(&pp->lock, flags);
+};
+
+static void sophgo_intx_eoi(struct irq_data *d)
+{
+}
+
+static struct irq_chip sophgo_intx_irq_chip = {
+	.name			= "INTx",
+	.irq_mask		= sophgo_intx_mask,
+	.irq_unmask		= sophgo_intx_unmask,
+	.irq_eoi		= sophgo_intx_eoi,
+};
+
+static int sophgo_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
+				irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(irq, &sophgo_intx_irq_chip, handle_fasteoi_irq);
+	irq_set_chip_data(irq, domain->host_data);
+
+	return 0;
+}
+
+static const struct irq_domain_ops intx_domain_ops = {
+	.map = sophgo_pcie_intx_map,
+};
+
+static int sophgo_pcie_init_irq_domain(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct sophgo_pcie *sophgo = to_sophgo_pcie(pci);
+	struct device *dev = sophgo->pci.dev;
+	struct fwnode_handle *intc;
+	int irq;
+
+	intc = device_get_named_child_node(dev, "interrupt-controller");
+	if (!intc) {
+		dev_err(dev, "missing child interrupt-controller node\n");
+		return -ENODEV;
+	}
+
+	irq = fwnode_irq_get(intc, 0);
+	if (irq < 0) {
+		dev_err(dev, "failed to get INTx irq number\n");
+		fwnode_handle_put(intc);
+		return irq;
+	}
+
+	sophgo->irq_domain = irq_domain_create_linear(intc, PCI_NUM_INTX,
+						      &intx_domain_ops, sophgo);
+	fwnode_handle_put(intc);
+	if (!sophgo->irq_domain) {
+		dev_err(dev, "failed to get a INTx irq domain\n");
+		return -EINVAL;
+	}
+
+	return irq;
+}
+
+static void sophgo_pcie_msi_enable(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct sophgo_pcie *sophgo = to_sophgo_pcie(pci);
+	unsigned long flags;
+	u32 val;
+
+	raw_spin_lock_irqsave(&pp->lock, flags);
+
+	val = sophgo_pcie_readl_app(sophgo, PCIE_INT_EN);
+	val |= PCIE_INT_EN_INT_MSI;
+	sophgo_pcie_writel_app(sophgo, val, PCIE_INT_EN);
+
+	raw_spin_unlock_irqrestore(&pp->lock, flags);
+}
+
+static int sophgo_pcie_host_init(struct dw_pcie_rp *pp)
+{
+	int irq;
+
+	irq = sophgo_pcie_init_irq_domain(pp);
+	if (irq < 0)
+		return irq;
+
+	irq_set_chained_handler_and_data(irq, sophgo_pcie_intx_handler,
+					 pp);
+
+	sophgo_pcie_msi_enable(pp);
+
+	return 0;
+}
+
+static const struct dw_pcie_host_ops sophgo_pcie_host_ops = {
+	.init = sophgo_pcie_host_init,
+};
+
+static int sophgo_pcie_clk_init(struct sophgo_pcie *sophgo)
+{
+	struct device *dev = sophgo->pci.dev;
+	int ret;
+
+	ret = devm_clk_bulk_get_all_enabled(dev, &sophgo->clks);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to get clocks\n");
+
+	sophgo->clk_cnt = ret;
+
+	return 0;
+}
+
+static int sophgo_pcie_resource_get(struct platform_device *pdev,
+				    struct sophgo_pcie *sophgo)
+{
+	sophgo->app_base = devm_platform_ioremap_resource_byname(pdev, "app");
+	if (IS_ERR(sophgo->app_base))
+		return dev_err_probe(&pdev->dev, PTR_ERR(sophgo->app_base),
+				     "failed to map app registers\n");
+
+	return 0;
+}
+
+static int sophgo_pcie_configure_rc(struct sophgo_pcie *sophgo)
+{
+	struct dw_pcie_rp *pp;
+
+	pp = &sophgo->pci.pp;
+	pp->ops = &sophgo_pcie_host_ops;
+
+	return dw_pcie_host_init(pp);
+}
+
+static int sophgo_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct sophgo_pcie *sophgo;
+	int ret;
+
+	sophgo = devm_kzalloc(dev, sizeof(*sophgo), GFP_KERNEL);
+	if (!sophgo)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, sophgo);
+
+	sophgo->pci.dev = dev;
+
+	ret = sophgo_pcie_resource_get(pdev, sophgo);
+	if (ret)
+		return ret;
+
+	ret = sophgo_pcie_clk_init(sophgo);
+	if (ret)
+		return ret;
+
+	return sophgo_pcie_configure_rc(sophgo);
+}
+
+static const struct of_device_id sophgo_pcie_of_match[] = {
+	{ .compatible = "sophgo,sg2044-pcie" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sophgo_pcie_acpi_match);
+
+static const struct acpi_device_id sophgo_pcie_acpi_match[] = {
+	{ "SOPHO000", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, sophgo_pcie_acpi_match);
+
+static struct platform_driver sophgo_pcie_driver = {
+	.driver = {
+		.name = "sophgo-dw-pcie",
+		.of_match_table = sophgo_pcie_of_match,
+		.acpi_match_table = sophgo_pcie_acpi_match,
+		.suppress_bind_attrs = true,
+	},
+	.probe = sophgo_pcie_probe,
+};
+builtin_platform_driver(sophgo_pcie_driver);
-- 
2.48.1

Re: [PATCH 2/2] PCI: sophgo-dwc: Add Sophgo SG2044 PCIe driver

[Philipp Zabel]

On Fr, 2025-02-21 at 09:37 +0800, Inochi Amaoto wrote:
> Add support for DesignWare-based PCIe controller in SG2044 SoC.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  drivers/pci/controller/dwc/Kconfig          |  10 +
>  drivers/pci/controller/dwc/Makefile         |   1 +
>  drivers/pci/controller/dwc/pcie-dw-sophgo.c | 282 ++++++++++++++++++++
>  3 files changed, 293 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-dw-sophgo.c
> 
[...]
> diff --git a/drivers/pci/controller/dwc/pcie-dw-sophgo.c b/drivers/pci/controller/dwc/pcie-dw-sophgo.c
> new file mode 100644
> index 000000000000..a4ca4f1e26e0
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-dw-sophgo.c
> @@ -0,0 +1,282 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe host controller driver for Sophgo SoCs.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/phy/phy.h>
> +#include <linux/property.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>

Drop this ...

> +
> +#include "pcie-designware.h"
> +
> +#define to_sophgo_pcie(x)		dev_get_drvdata((x)->dev)
> +
> +#define PCIE_INT_SIGNAL			0xc48
> +#define PCIE_INT_EN			0xca0
> +
> +#define PCIE_SIGNAL_INTX_SHIFT		5
> +
> +#define PCIE_INT_EN_INTX_SHIFT		1
> +#define PCIE_INT_EN_INT_SII		BIT(0)
> +#define PCIE_INT_EN_INT_INTA		BIT(1)
> +#define PCIE_INT_EN_INT_INTB		BIT(2)
> +#define PCIE_INT_EN_INT_INTC		BIT(3)
> +#define PCIE_INT_EN_INT_INTD		BIT(4)
> +#define PCIE_INT_EN_INT_MSI		BIT(5)
> +
> +struct sophgo_pcie {
> +	struct dw_pcie pci;
> +	void __iomem *app_base;
> +	struct clk_bulk_data *clks;
> +	unsigned int clk_cnt;
> +	struct reset_control *rst;

... and this. It is unused.


regards
Philipp

Re: [PATCH 2/2] PCI: sophgo-dwc: Add Sophgo SG2044 PCIe driver

[Inochi Amaoto]

On Fri, Feb 21, 2025 at 10:07:54AM +0100, Philipp Zabel wrote:
> On Fr, 2025-02-21 at 09:37 +0800, Inochi Amaoto wrote:
> > Add support for DesignWare-based PCIe controller in SG2044 SoC.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> >  drivers/pci/controller/dwc/Kconfig          |  10 +
> >  drivers/pci/controller/dwc/Makefile         |   1 +
> >  drivers/pci/controller/dwc/pcie-dw-sophgo.c | 282 ++++++++++++++++++++
> >  3 files changed, 293 insertions(+)
> >  create mode 100644 drivers/pci/controller/dwc/pcie-dw-sophgo.c
> > 
> [...]
> > diff --git a/drivers/pci/controller/dwc/pcie-dw-sophgo.c b/drivers/pci/controller/dwc/pcie-dw-sophgo.c
> > new file mode 100644
> > index 000000000000..a4ca4f1e26e0
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-dw-sophgo.c
> > @@ -0,0 +1,282 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * PCIe host controller driver for Sophgo SoCs.
> > + *
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/irqchip/chained_irq.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/property.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/reset.h>
> 
> Drop this ...
> 
> > +
> > +#include "pcie-designware.h"
> > +
> > +#define to_sophgo_pcie(x)		dev_get_drvdata((x)->dev)
> > +
> > +#define PCIE_INT_SIGNAL			0xc48
> > +#define PCIE_INT_EN			0xca0
> > +
> > +#define PCIE_SIGNAL_INTX_SHIFT		5
> > +
> > +#define PCIE_INT_EN_INTX_SHIFT		1
> > +#define PCIE_INT_EN_INT_SII		BIT(0)
> > +#define PCIE_INT_EN_INT_INTA		BIT(1)
> > +#define PCIE_INT_EN_INT_INTB		BIT(2)
> > +#define PCIE_INT_EN_INT_INTC		BIT(3)
> > +#define PCIE_INT_EN_INT_INTD		BIT(4)
> > +#define PCIE_INT_EN_INT_MSI		BIT(5)
> > +
> > +struct sophgo_pcie {
> > +	struct dw_pcie pci;
> > +	void __iomem *app_base;
> > +	struct clk_bulk_data *clks;
> > +	unsigned int clk_cnt;
> > +	struct reset_control *rst;
> 
> ... and this. It is unused.
> 
> 
> regards
> Philipp

Thanks, I will remove this.

Regards,
Inochi

Re: [PATCH 2/2] PCI: sophgo-dwc: Add Sophgo SG2044 PCIe driver

[Bjorn Helgaas]

On Fri, Feb 21, 2025 at 09:37:56AM +0800, Inochi Amaoto wrote:
> Add support for DesignWare-based PCIe controller in SG2044 SoC.

> @@ -341,6 +341,16 @@ config PCIE_ROCKCHIP_DW_EP
>  	  Enables support for the DesignWare PCIe controller in the
>  	  Rockchip SoC (except RK3399) to work in endpoint mode.
>  
> +config PCIE_SOPHGO_DW
> +	bool "SOPHGO DesignWare PCIe controller"

What's the canonical styling of "SOPHGO"?  I see "Sophgo" in the
subject line and in Chen Wang's SG2042 series.  Pick the official
styling and use it consistently.

Reorder this so the menuconfig menu items remain alphabetically
sorted.

> +	depends on ARCH_SOPHGO || COMPILE_TEST
> +	depends on PCI_MSI
> +	depends on OF
> +	select PCIE_DW_HOST
> +	help
> +	  Enables support for the DesignWare PCIe controller in the
> +	  SOPHGO SoC.
> +
>  config PCI_EXYNOS
>  	tristate "Samsung Exynos PCIe controller"
>  	depends on ARCH_EXYNOS || COMPILE_TEST

> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe host controller driver for Sophgo SoCs.

Looks too generic, since Chen Wang's series says Sophgo SG2042 SoC is
Cadence-based, so this driver apparently doesn't cover all Sophgo
SoCs.

> + *

Spurious blank line.

> + */
> +
> +#include <linux/clk.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/phy/phy.h>
> +#include <linux/property.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-designware.h"
> +
> +#define to_sophgo_pcie(x)		dev_get_drvdata((x)->dev)
> +
> +#define PCIE_INT_SIGNAL			0xc48
> +#define PCIE_INT_EN			0xca0
> +
> +#define PCIE_SIGNAL_INTX_SHIFT		5
> +#define PCIE_INT_EN_INTX_SHIFT		1

Define masks with GENMASK() and get rid of the _SHIFT #defines.

> +#define PCIE_INT_EN_INT_SII		BIT(0)
> +#define PCIE_INT_EN_INT_INTA		BIT(1)
> +#define PCIE_INT_EN_INT_INTB		BIT(2)
> +#define PCIE_INT_EN_INT_INTC		BIT(3)
> +#define PCIE_INT_EN_INT_INTD		BIT(4)

These are unused, drop them.

> +#define PCIE_INT_EN_INT_MSI		BIT(5)
> +
> +struct sophgo_pcie {
> +	struct dw_pcie pci;
> +	void __iomem *app_base;
> +	struct clk_bulk_data *clks;
> +	unsigned int clk_cnt;
> +	struct reset_control *rst;
> +	struct irq_domain *irq_domain;

Indent the member names to align vertically as most other drivers do.

> +};
> +
> +static int sophgo_pcie_readl_app(struct sophgo_pcie *sophgo, u32 reg)
> +{
> +	return readl_relaxed(sophgo->app_base + reg);
> +}
> +
> +static void sophgo_pcie_writel_app(struct sophgo_pcie *sophgo, u32 val, u32 reg)
> +{
> +	writel_relaxed(val, sophgo->app_base + reg);
> +}
> +
> +static void sophgo_pcie_intx_handler(struct irq_desc *desc)
> +{
> +	struct dw_pcie_rp *pp = irq_desc_get_handler_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct sophgo_pcie *sophgo = to_sophgo_pcie(pci);
> +	unsigned long hwirq = PCIE_SIGNAL_INTX_SHIFT;
> +	unsigned long reg;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	reg = sophgo_pcie_readl_app(sophgo, PCIE_INT_SIGNAL);
> +
> +	for_each_set_bit_from(hwirq, &reg, PCI_NUM_INTX + PCIE_SIGNAL_INTX_SHIFT)

Use FIELD_GET() here and iterate through PCI_NUM_INTX.  Then you don't
need for_each_set_bit_from() and shouldn't need PCIE_SIGNAL_INTX_SHIFT
here and below.

> +		generic_handle_domain_irq(sophgo->irq_domain,
> +					  hwirq - PCIE_SIGNAL_INTX_SHIFT);
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
> +static void sophgo_intx_mask(struct irq_data *d)
> +{
> +	struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct sophgo_pcie *sophgo = to_sophgo_pcie(pci);
> +	unsigned long flags;
> +	u32 val;
> +
> +	raw_spin_lock_irqsave(&pp->lock, flags);
> +
> +	val = sophgo_pcie_readl_app(sophgo, PCIE_INT_EN);
> +	val &= ~BIT(d->hwirq + PCIE_INT_EN_INTX_SHIFT);

FIELD_PREP().

> +	sophgo_pcie_writel_app(sophgo, val, PCIE_INT_EN);
> +
> +	raw_spin_unlock_irqrestore(&pp->lock, flags);
> +};
> +
> +static void sophgo_intx_unmask(struct irq_data *d)
> +{
> +	struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct sophgo_pcie *sophgo = to_sophgo_pcie(pci);
> +	unsigned long flags;
> +	u32 val;
> +
> +	raw_spin_lock_irqsave(&pp->lock, flags);
> +
> +	val = sophgo_pcie_readl_app(sophgo, PCIE_INT_EN);
> +	val |= BIT(d->hwirq + PCIE_INT_EN_INTX_SHIFT);

Ditto.

> +	sophgo_pcie_writel_app(sophgo, val, PCIE_INT_EN);
> +
> +	raw_spin_unlock_irqrestore(&pp->lock, flags);
> +};
> +
> +static void sophgo_intx_eoi(struct irq_data *d)
> +{
> +}
> +
> +static struct irq_chip sophgo_intx_irq_chip = {
> +	.name			= "INTx",
> +	.irq_mask		= sophgo_intx_mask,
> +	.irq_unmask		= sophgo_intx_unmask,
> +	.irq_eoi		= sophgo_intx_eoi,

Name these ending with the irq_chip field names, e.g.,
sophgo_intx_irq_mask(), to make them easier to find with grep.

Bjorn

Re: [PATCH 2/2] PCI: sophgo-dwc: Add Sophgo SG2044 PCIe driver

[Inochi Amaoto]

On Fri, Feb 21, 2025 at 05:49:58PM -0600, Bjorn Helgaas wrote:
> On Fri, Feb 21, 2025 at 09:37:56AM +0800, Inochi Amaoto wrote:
> > Add support for DesignWare-based PCIe controller in SG2044 SoC.
> 
> > @@ -341,6 +341,16 @@ config PCIE_ROCKCHIP_DW_EP
> >  	  Enables support for the DesignWare PCIe controller in the
> >  	  Rockchip SoC (except RK3399) to work in endpoint mode.
> >  
> > +config PCIE_SOPHGO_DW
> > +	bool "SOPHGO DesignWare PCIe controller"
> 
> What's the canonical styling of "SOPHGO"?  I see "Sophgo" in the
> subject line and in Chen Wang's SG2042 series.  Pick the official
> styling and use it consistently.
> 

This is my mistake. It should be "Sophgo", I will change it.

> Reorder this so the menuconfig menu items remain alphabetically
> sorted.
> 

I think this order is applied to the entry title in menuconfig,
and is not the config key? If so, I will change it.

> > +	depends on ARCH_SOPHGO || COMPILE_TEST
> > +	depends on PCI_MSI
> > +	depends on OF
> > +	select PCIE_DW_HOST
> > +	help
> > +	  Enables support for the DesignWare PCIe controller in the
> > +	  SOPHGO SoC.
> > +
> >  config PCI_EXYNOS
> >  	tristate "Samsung Exynos PCIe controller"
> >  	depends on ARCH_EXYNOS || COMPILE_TEST
> 
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * PCIe host controller driver for Sophgo SoCs.
> 
> Looks too generic, since Chen Wang's series says Sophgo SG2042 SoC is
> Cadence-based, so this driver apparently doesn't cover all Sophgo
> SoCs.
> 

OK, I will change the description to point it only cover
the controller based on the DesignWare core.

> > + *
> 
> Spurious blank line.
> 
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/irqchip/chained_irq.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/property.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/reset.h>
> > +
> > +#include "pcie-designware.h"
> > +
> > +#define to_sophgo_pcie(x)		dev_get_drvdata((x)->dev)
> > +
> > +#define PCIE_INT_SIGNAL			0xc48
> > +#define PCIE_INT_EN			0xca0
> > +
> > +#define PCIE_SIGNAL_INTX_SHIFT		5
> > +#define PCIE_INT_EN_INTX_SHIFT		1
> 
> Define masks with GENMASK() and get rid of the _SHIFT #defines.
> 
> > +#define PCIE_INT_EN_INT_SII		BIT(0)
> > +#define PCIE_INT_EN_INT_INTA		BIT(1)
> > +#define PCIE_INT_EN_INT_INTB		BIT(2)
> > +#define PCIE_INT_EN_INT_INTC		BIT(3)
> > +#define PCIE_INT_EN_INT_INTD		BIT(4)
> 
> These are unused, drop them.
> 
> > +#define PCIE_INT_EN_INT_MSI		BIT(5)
> > +
> > +struct sophgo_pcie {
> > +	struct dw_pcie pci;
> > +	void __iomem *app_base;
> > +	struct clk_bulk_data *clks;
> > +	unsigned int clk_cnt;
> > +	struct reset_control *rst;
> > +	struct irq_domain *irq_domain;
> 
> Indent the member names to align vertically as most other drivers do.
> 
> > +};
> > +
> > +static int sophgo_pcie_readl_app(struct sophgo_pcie *sophgo, u32 reg)
> > +{
> > +	return readl_relaxed(sophgo->app_base + reg);
> > +}
> > +
> > +static void sophgo_pcie_writel_app(struct sophgo_pcie *sophgo, u32 val, u32 reg)
> > +{
> > +	writel_relaxed(val, sophgo->app_base + reg);
> > +}
> > +
> > +static void sophgo_pcie_intx_handler(struct irq_desc *desc)
> > +{
> > +	struct dw_pcie_rp *pp = irq_desc_get_handler_data(desc);
> > +	struct irq_chip *chip = irq_desc_get_chip(desc);
> > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +	struct sophgo_pcie *sophgo = to_sophgo_pcie(pci);
> > +	unsigned long hwirq = PCIE_SIGNAL_INTX_SHIFT;
> > +	unsigned long reg;
> > +
> > +	chained_irq_enter(chip, desc);
> > +
> > +	reg = sophgo_pcie_readl_app(sophgo, PCIE_INT_SIGNAL);
> > +
> > +	for_each_set_bit_from(hwirq, &reg, PCI_NUM_INTX + PCIE_SIGNAL_INTX_SHIFT)
> 
> Use FIELD_GET() here and iterate through PCI_NUM_INTX.  Then you don't
> need for_each_set_bit_from() and shouldn't need PCIE_SIGNAL_INTX_SHIFT
> here and below.
> 

OK, I will change it

> > +		generic_handle_domain_irq(sophgo->irq_domain,
> > +					  hwirq - PCIE_SIGNAL_INTX_SHIFT);
> > +
> > +	chained_irq_exit(chip, desc);
> > +}
> > +
> > +static void sophgo_intx_mask(struct irq_data *d)
> > +{
> > +	struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
> > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +	struct sophgo_pcie *sophgo = to_sophgo_pcie(pci);
> > +	unsigned long flags;
> > +	u32 val;
> > +
> > +	raw_spin_lock_irqsave(&pp->lock, flags);
> > +
> > +	val = sophgo_pcie_readl_app(sophgo, PCIE_INT_EN);
> > +	val &= ~BIT(d->hwirq + PCIE_INT_EN_INTX_SHIFT);
> 
> FIELD_PREP().
> 
> > +	sophgo_pcie_writel_app(sophgo, val, PCIE_INT_EN);
> > +
> > +	raw_spin_unlock_irqrestore(&pp->lock, flags);
> > +};
> > +
> > +static void sophgo_intx_unmask(struct irq_data *d)
> > +{
> > +	struct dw_pcie_rp *pp = irq_data_get_irq_chip_data(d);
> > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +	struct sophgo_pcie *sophgo = to_sophgo_pcie(pci);
> > +	unsigned long flags;
> > +	u32 val;
> > +
> > +	raw_spin_lock_irqsave(&pp->lock, flags);
> > +
> > +	val = sophgo_pcie_readl_app(sophgo, PCIE_INT_EN);
> > +	val |= BIT(d->hwirq + PCIE_INT_EN_INTX_SHIFT);
> 
> Ditto.
> 
> > +	sophgo_pcie_writel_app(sophgo, val, PCIE_INT_EN);
> > +
> > +	raw_spin_unlock_irqrestore(&pp->lock, flags);
> > +};
> > +
> > +static void sophgo_intx_eoi(struct irq_data *d)
> > +{
> > +}
> > +
> > +static struct irq_chip sophgo_intx_irq_chip = {
> > +	.name			= "INTx",
> > +	.irq_mask		= sophgo_intx_mask,
> > +	.irq_unmask		= sophgo_intx_unmask,
> > +	.irq_eoi		= sophgo_intx_eoi,
> 
> Name these ending with the irq_chip field names, e.g.,
> sophgo_intx_irq_mask(), to make them easier to find with grep.
> 
> Bjorn

Thanks, I will take all the comments and improve the driver.

Regards,
Inochi

Re: [PATCH 2/2] PCI: sophgo-dwc: Add Sophgo SG2044 PCIe driver

[Bjorn Helgaas]

On Sat, Feb 22, 2025 at 08:43:46AM +0800, Inochi Amaoto wrote:
> On Fri, Feb 21, 2025 at 05:49:58PM -0600, Bjorn Helgaas wrote:
> > On Fri, Feb 21, 2025 at 09:37:56AM +0800, Inochi Amaoto wrote:
> > > Add support for DesignWare-based PCIe controller in SG2044 SoC.
> > 
> > > @@ -341,6 +341,16 @@ config PCIE_ROCKCHIP_DW_EP
> > >  	  Enables support for the DesignWare PCIe controller in the
> > >  	  Rockchip SoC (except RK3399) to work in endpoint mode.
> > >  
> > > +config PCIE_SOPHGO_DW
> > > +	bool "SOPHGO DesignWare PCIe controller"
> > 
> > What's the canonical styling of "SOPHGO"?  I see "Sophgo" in the
> > subject line and in Chen Wang's SG2042 series.  Pick the official
> > styling and use it consistently.
> 
> This is my mistake. It should be "Sophgo", I will change it.
> 
> > Reorder this so the menuconfig menu items remain alphabetically
> > sorted.
> 
> I think this order is applied to the entry title in menuconfig,
> and is not the config key? If so, I will change it.

It's the title shown by menuconfig, i.e., "SOPHGO DesignWare PCIe
controller" here.

Re: [PATCH 2/2] PCI: sophgo-dwc: Add Sophgo SG2044 PCIe driver

[Inochi Amaoto]

On Mon, Feb 24, 2025 at 01:47:25PM -0600, Bjorn Helgaas wrote:
> On Sat, Feb 22, 2025 at 08:43:46AM +0800, Inochi Amaoto wrote:
> > On Fri, Feb 21, 2025 at 05:49:58PM -0600, Bjorn Helgaas wrote:
> > > On Fri, Feb 21, 2025 at 09:37:56AM +0800, Inochi Amaoto wrote:
> > > > Add support for DesignWare-based PCIe controller in SG2044 SoC.
> > > 
> > > > @@ -341,6 +341,16 @@ config PCIE_ROCKCHIP_DW_EP
> > > >  	  Enables support for the DesignWare PCIe controller in the
> > > >  	  Rockchip SoC (except RK3399) to work in endpoint mode.
> > > >  
> > > > +config PCIE_SOPHGO_DW
> > > > +	bool "SOPHGO DesignWare PCIe controller"
> > > 
> > > What's the canonical styling of "SOPHGO"?  I see "Sophgo" in the
> > > subject line and in Chen Wang's SG2042 series.  Pick the official
> > > styling and use it consistently.
> > 
> > This is my mistake. It should be "Sophgo", I will change it.
> > 
> > > Reorder this so the menuconfig menu items remain alphabetically
> > > sorted.
> > 
> > I think this order is applied to the entry title in menuconfig,
> > and is not the config key? If so, I will change it.
> 
> It's the title shown by menuconfig, i.e., "SOPHGO DesignWare PCIe
> controller" here.

Thanks, I will reorder it.

Regards,
Inochi

[PATCH 0/2] riscv: sophgo Add PCIe support to Sophgo SG2044 SoC

[Inochi Amaoto]

Sophgo's SG2044 SoC uses Synopsys Designware PCIe core
to implement RC mode.

For legacy interrupt, the PCIe controller on SG2044 implement
its own legacy interrupt controller. For MSI/MSI-X, it use an
external interrupt controller to handle.

The external MSI interrupt controller patch can be found on [1].
As SG2044 needs a mirror change to support the way to send MSI
message and different irq number.

[1] https://lore.kernel.org/all/20250303111648.1337543-1-inochiama@gmail.com

Changed from v1:
- https://lore.kernel.org/all/20250221013758.370936-1-inochiama@gmail.comq
1. patch 1: remove dma-coherent property
2. patch 2: remove unused reset
3. patch 2: fix Kconfig menu title and reorder the entry
4. patch 2: use FIELD_GET/FIELD_PREP to simplify the code.
5. patch 2: rename the irq handle function to match the irq_chip name

Inochi Amaoto (2):
  dt-bindings: pci: Add Sophgo SG2044 PCIe host
  PCI: sophgo-dwc: Add Sophgo SG2044 PCIe driver

 .../bindings/pci/sophgo,sg2044-pcie.yaml      | 122 ++++++++
 drivers/pci/controller/dwc/Kconfig            |  10 +
 drivers/pci/controller/dwc/Makefile           |   1 +
 drivers/pci/controller/dwc/pcie-dw-sophgo.c   | 270 ++++++++++++++++++
 4 files changed, 403 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-dw-sophgo.c

--
2.48.1

[PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Inochi Amaoto]

The pcie controller on the SG2044 is designware based with
custom app registers.

Add binding document for SG2044 PCIe host controller.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 .../bindings/pci/sophgo,sg2044-pcie.yaml      | 122 ++++++++++++++++++
 1 file changed, 122 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
new file mode 100644
index 000000000000..2860d0f13146
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
@@ -0,0 +1,122 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
+
+maintainers:
+  - Inochi Amaoto <inochiama@gmail.com>
+
+description: |+
+  SG2044 SoC PCIe Root Complex controller is based on the Synopsys DesignWare
+  PCIe IP and thus inherits all the common properties defined in
+  snps,dw-pcie.yaml.
+
+allOf:
+  - $ref: /schemas/pci/pci-host-bridge.yaml#
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+
+properties:
+  compatible:
+    const: sophgo,sg2044-pcie
+
+  reg:
+    items:
+      - description: Data Bus Interface (DBI) registers
+      - description: iATU registers
+      - description: Config registers
+      - description: Sophgo designed configuration registers
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: atu
+      - const: config
+      - const: app
+
+  clocks:
+    items:
+      - description: core clk
+
+  clock-names:
+    items:
+      - const: core
+
+  interrupt-controller:
+    description: Interrupt controller node for handling legacy PCI interrupts.
+    type: object
+
+    properties:
+      "#address-cells":
+        const: 0
+
+      "#interrupt-cells":
+        const: 1
+
+      interrupt-controller: true
+
+      interrupts:
+        items:
+          - description: combined legacy interrupt
+
+    required:
+      - "#address-cells"
+      - "#interrupt-cells"
+      - interrupt-controller
+      - interrupts
+
+    additionalProperties: false
+
+  msi-parent: true
+
+  ranges:
+    maxItems: 5
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      pcie@6c00400000 {
+        compatible = "sophgo,sg2044-pcie";
+        reg = <0x6c 0x00400000 0x0 0x00001000>,
+              <0x6c 0x00700000 0x0 0x00004000>,
+              <0x40 0x00000000 0x0 0x00001000>,
+              <0x6c 0x00780c00 0x0 0x00000400>;
+        reg-names = "dbi", "atu", "config", "app";
+        #address-cells = <3>;
+        #size-cells = <2>;
+        bus-range = <0x00 0xff>;
+        clocks = <&clk 0>;
+        clock-names = "core";
+        device_type = "pci";
+        linux,pci-domain = <0>;
+        msi-parent = <&msi>;
+        ranges = <0x01000000 0x0  0x00000000  0x40 0x10000000  0x0 0x00200000>,
+                 <0x42000000 0x0  0x00000000  0x0  0x00000000  0x0 0x04000000>,
+                 <0x02000000 0x0  0x04000000  0x0  0x04000000  0x0 0x04000000>,
+                 <0x43000000 0x42 0x00000000  0x42 0x00000000  0x2 0x00000000>,
+                 <0x03000000 0x41 0x00000000  0x41 0x00000000  0x1 0x00000000>;
+
+        interrupt-controller {
+          #address-cells = <0>;
+          #interrupt-cells = <1>;
+          interrupt-controller;
+          interrupt-parent = <&intc>;
+          interrupts = <64 IRQ_TYPE_LEVEL_HIGH>;
+        };
+      };
+    };
+...
-- 
2.48.1

Re: [PATCH 1/2] dt-bindings: pci: Add Sophgo SG2044 PCIe host

[Rob Herring]

On Tue, Mar 04, 2025 at 03:12:37PM +0800, Inochi Amaoto wrote:
> The pcie controller on the SG2044 is designware based with
> custom app registers.
> 
> Add binding document for SG2044 PCIe host controller.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
>  .../bindings/pci/sophgo,sg2044-pcie.yaml      | 122 ++++++++++++++++++
>  1 file changed, 122 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> new file mode 100644
> index 000000000000..2860d0f13146
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/sophgo,sg2044-pcie.yaml
> @@ -0,0 +1,122 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/sophgo,sg2044-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: DesignWare based PCIe Root Complex controller on Sophgo SoCs
> +
> +maintainers:
> +  - Inochi Amaoto <inochiama@gmail.com>
> +
> +description: |+

Don't need '|+' if no formatting to preserve.

With that,

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>