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

[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

[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

[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 | 270 ++++++++++++++++++++
 3 files changed, 281 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..004c384e25ad 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -381,6 +381,16 @@ config PCIE_UNIPHIER_EP
 	  Say Y here if you want PCIe endpoint controller support on
 	  UniPhier SoCs. This driver supports Pro5 SoC.
 
+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 PCIE_SPEAR13XX
 	bool "STMicroelectronics SPEAr PCIe controller"
 	depends on ARCH_SPEAR13XX || 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..3ed7cfe0b361
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-dw-sophgo.c
@@ -0,0 +1,270 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sophgo DesignWare based PCIe host controller driver
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/platform_device.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_INT_SIGNAL_INTX		GENMASK(8, 5)
+
+#define PCIE_INT_EN_INTX		GENMASK(4, 1)
+#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 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, reg;
+
+	chained_irq_enter(chip, desc);
+
+	reg = sophgo_pcie_readl_app(sophgo, PCIE_INT_SIGNAL);
+	reg = FIELD_GET(PCIE_INT_SIGNAL_INTX, reg);
+
+	for_each_set_bit(hwirq, &reg, PCI_NUM_INTX)
+		generic_handle_domain_irq(sophgo->irq_domain, hwirq);
+
+	chained_irq_exit(chip, desc);
+}
+
+static void sophgo_intx_irq_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 &= ~FIELD_PREP(PCIE_INT_EN_INTX, BIT(d->hwirq));
+	sophgo_pcie_writel_app(sophgo, val, PCIE_INT_EN);
+
+	raw_spin_unlock_irqrestore(&pp->lock, flags);
+};
+
+static void sophgo_intx_irq_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 |= FIELD_PREP(PCIE_INT_EN_INTX, BIT(d->hwirq));
+	sophgo_pcie_writel_app(sophgo, val, PCIE_INT_EN);
+
+	raw_spin_unlock_irqrestore(&pp->lock, flags);
+};
+
+static void sophgo_intx_irq_eoi(struct irq_data *d)
+{
+}
+
+static struct irq_chip sophgo_intx_irq_chip = {
+	.name			= "INTx",
+	.irq_mask		= sophgo_intx_irq_mask,
+	.irq_unmask		= sophgo_intx_irq_unmask,
+	.irq_eoi		= sophgo_intx_irq_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 0/2] riscv: sophgo Add PCIe support to Sophgo SG2044 SoC

[Chen Wang]

I think it should be v2, right? :)

On 2025/3/4 15:12, Inochi Amaoto wrote:
> 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
>

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

[Inochi Amaoto]

On Tue, Mar 04, 2025 at 03:53:57PM +0800, Chen Wang wrote:
> I think it should be v2, right? :)
> 

Yeah, I have made a mistake. This is v2, not v1.

> On 2025/3/4 15:12, Inochi Amaoto wrote:
> > 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
> > 

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>

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

[Bjorn Helgaas]

On Tue, Mar 04, 2025 at 03:12:38PM +0800, Inochi Amaoto wrote:
> Add support for DesignWare-based PCIe controller in SG2044 SoC.

> +static void sophgo_intx_irq_eoi(struct irq_data *d)
> +{
> +}

The empty .irq_eoi() is unusual.  Why do you need it?

I see that the existence of chip->irq_eoi() makes a difference in
chained_irq_enter() and chained_irq_exit(), but I'm surprised that
this is the only driver in drivers/pci/controller/ that implements an
empty .irq_eoi().

A comment here about what is special would be helpful.

> +static struct irq_chip sophgo_intx_irq_chip = {
> +	.name			= "INTx",
> +	.irq_mask		= sophgo_intx_irq_mask,
> +	.irq_unmask		= sophgo_intx_irq_unmask,
> +	.irq_eoi		= sophgo_intx_irq_eoi,
> +};

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

[Chen Wang]

On 2025/3/4 15:12, 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 | 270 ++++++++++++++++++++
Will this driver work for all sophgo chips using dw's IP? If not, it is 
recommended to rename it to "pcie-dw-sg2044.c".
>   3 files changed, 281 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..004c384e25ad 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -381,6 +381,16 @@ config PCIE_UNIPHIER_EP
>   	  Say Y here if you want PCIe endpoint controller support on
>   	  UniPhier SoCs. This driver supports Pro5 SoC.
>   
> +config PCIE_SOPHGO_DW
Similar to the above question, would it be better to change it to 
"PCIE_SG2044_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 PCIE_SPEAR13XX
>   	bool "STMicroelectronics SPEAr PCIe controller"
>   	depends on ARCH_SPEAR13XX || 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..3ed7cfe0b361
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-dw-sophgo.c
> @@ -0,0 +1,270 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sophgo DesignWare based PCIe host controller driver
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/platform_device.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_INT_SIGNAL_INTX		GENMASK(8, 5)
> +
> +#define PCIE_INT_EN_INTX		GENMASK(4, 1)
> +#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 irq_domain	*irq_domain;
> +};
> +
A similar question is, can structure names and function names use 
prefixes like "sg2044_"? This also makes it easier for other sophgo 
products to reuse this driver file in the future.
> +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, reg;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	reg = sophgo_pcie_readl_app(sophgo, PCIE_INT_SIGNAL);
> +	reg = FIELD_GET(PCIE_INT_SIGNAL_INTX, reg);
> +
> +	for_each_set_bit(hwirq, &reg, PCI_NUM_INTX)
> +		generic_handle_domain_irq(sophgo->irq_domain, hwirq);
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
> +static void sophgo_intx_irq_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 &= ~FIELD_PREP(PCIE_INT_EN_INTX, BIT(d->hwirq));
> +	sophgo_pcie_writel_app(sophgo, val, PCIE_INT_EN);
> +
> +	raw_spin_unlock_irqrestore(&pp->lock, flags);
> +};
> +
> +static void sophgo_intx_irq_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 |= FIELD_PREP(PCIE_INT_EN_INTX, BIT(d->hwirq));
> +	sophgo_pcie_writel_app(sophgo, val, PCIE_INT_EN);
> +
> +	raw_spin_unlock_irqrestore(&pp->lock, flags);
> +};
> +
> +static void sophgo_intx_irq_eoi(struct irq_data *d)
> +{
> +}
Why define this empty callback function? Please add a comment if it is 
really needed.
> +
> +static struct irq_chip sophgo_intx_irq_chip = {
> +	.name			= "INTx",
"INTx" --> "SG2044 INTx"
> +	.irq_mask		= sophgo_intx_irq_mask,
> +	.irq_unmask		= sophgo_intx_irq_unmask,
> +	.irq_eoi		= sophgo_intx_irq_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);

"sophgo"? This variable name looks a bit strange, why not call it "pcie" 
directly?

I have seen similar naming in other functions, please change it as well.

> +	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);

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 2/2] PCI: sophgo-dwc: Add Sophgo SG2044 PCIe driver

[Inochi Amaoto]

On Tue, Mar 04, 2025 at 11:36:32AM -0600, Bjorn Helgaas wrote:
> On Tue, Mar 04, 2025 at 03:12:38PM +0800, Inochi Amaoto wrote:
> > Add support for DesignWare-based PCIe controller in SG2044 SoC.
> 
> > +static void sophgo_intx_irq_eoi(struct irq_data *d)
> > +{
> > +}
> 
> The empty .irq_eoi() is unusual.  Why do you need it?
> 
> I see that the existence of chip->irq_eoi() makes a difference in
> chained_irq_enter() and chained_irq_exit(), but I'm surprised that
> this is the only driver in drivers/pci/controller/ that implements an
> empty .irq_eoi().
> 
> A comment here about what is special would be helpful.
> 

It is more like a mistake that from the vendor code
that I refer to it. This should be a level irq, and
I haven't tested it as I does not have a device use 
INTx. I try to get a test device will replace this 
empty eoi with handle_level_irq if I can confirm it.

Thanks for pointing to this.

Regards,
Inochi

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

[Inochi Amaoto]

On Wed, Mar 05, 2025 at 07:57:14AM +0800, Chen Wang wrote:
> 
> On 2025/3/4 15:12, 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 | 270 ++++++++++++++++++++
> Will this driver work for all sophgo chips using dw's IP? If not, it is
> recommended to rename it to "pcie-dw-sg2044.c".

It should. As the DesignWare PCIe ip is widely used and I saw
limit changed across the vendor. And the implement on SG2044
is almost like the Synopsys one. I think it is possible to 
represent all DesignWare based PCIe driver for the Sophgo SoCs.

Also, please think twice before giving some suggestion like
this. Although it is a good practice to keep the driver
specifc. keeping everythings specifc make us miss some common
things. 

Regards,
Inochi

> >   3 files changed, 281 insertions(+)
> >   create mode 100644 drivers/pci/controller/dwc/pcie-dw-sophgo.c
> > 
> > [...]
> > +
> > +static void sophgo_intx_irq_eoi(struct irq_data *d)
> > +{
> > +}
> Why define this empty callback function? Please add a comment if it is
> really needed.

See https://lore.kernel.org/all/bxwsvluj6amgoelk7r55gqhmhjwpewnfyhvvw6zpxen7kqzvwc@fxcebjzo5axk/

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 --]