[PATCH v2 0/4] rk3588 PCIe improvements

[PATCH v2 0/4] rk3588 PCIe improvements

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

Hello,

This series fixes two issues related to the pcie3x4 slot on the rk3588:
1) Adds the atu region, so that the driver can properly detect all 16
inbound iATUs and all 16 outbound iATUs.
2) Adds the dma region, and the related IRQs used by the eDMA, so that it
is possible to offload data transfers using the embedded DMA controller.


Changes since v1:
-Added patches to rockchip-dw-pcie.yaml to make 'make CHECK_DTBS=y' pass.


Kind regards,
Niklas

Niklas Cassel (4):
  dt-bindings: PCI: dwc: rockchip: Add atu property
  arm64: dts: rockchip: add missing mandatory rk3588 PCIe atu property
  dt-bindings: PCI: dwc: rockchip: Add dma properties
  arm64: dts: rockchip: add missing rk3588 PCIe dma properties

 .../bindings/pci/rockchip-dw-pcie.yaml        | 24 ++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3588.dtsi      | 31 ++++++++++++-------
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi     | 14 +++++----
 3 files changed, 52 insertions(+), 17 deletions(-)

-- 
2.41.0

[PATCH v2 0/4] rk3588 PCIe improvements

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

Hello,

This series fixes two issues related to the pcie3x4 slot on the rk3588:
1) Adds the atu region, so that the driver can properly detect all 16
inbound iATUs and all 16 outbound iATUs.
2) Adds the dma region, and the related IRQs used by the eDMA, so that it
is possible to offload data transfers using the embedded DMA controller.


Changes since v1:
-Added patches to rockchip-dw-pcie.yaml to make 'make CHECK_DTBS=y' pass.


Kind regards,
Niklas

Niklas Cassel (4):
  dt-bindings: PCI: dwc: rockchip: Add atu property
  arm64: dts: rockchip: add missing mandatory rk3588 PCIe atu property
  dt-bindings: PCI: dwc: rockchip: Add dma properties
  arm64: dts: rockchip: add missing rk3588 PCIe dma properties

 .../bindings/pci/rockchip-dw-pcie.yaml        | 24 ++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3588.dtsi      | 31 ++++++++++++-------
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi     | 14 +++++----
 3 files changed, 52 insertions(+), 17 deletions(-)

-- 
2.41.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 0/4] rk3588 PCIe improvements

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

Hello,

This series fixes two issues related to the pcie3x4 slot on the rk3588:
1) Adds the atu region, so that the driver can properly detect all 16
inbound iATUs and all 16 outbound iATUs.
2) Adds the dma region, and the related IRQs used by the eDMA, so that it
is possible to offload data transfers using the embedded DMA controller.


Changes since v1:
-Added patches to rockchip-dw-pcie.yaml to make 'make CHECK_DTBS=y' pass.


Kind regards,
Niklas

Niklas Cassel (4):
  dt-bindings: PCI: dwc: rockchip: Add atu property
  arm64: dts: rockchip: add missing mandatory rk3588 PCIe atu property
  dt-bindings: PCI: dwc: rockchip: Add dma properties
  arm64: dts: rockchip: add missing rk3588 PCIe dma properties

 .../bindings/pci/rockchip-dw-pcie.yaml        | 24 ++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3588.dtsi      | 31 ++++++++++++-------
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi     | 14 +++++----
 3 files changed, 52 insertions(+), 17 deletions(-)

-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
using:

allOf:
  - $ref: /schemas/pci/snps,dw-pcie.yaml#

and snps,dw-pcie.yaml does have the atu property defined, in order to be
able to use this property, while still making sure 'make CHECK_DTBS=y'
pass, we need to add this property to rockchip-dw-pcie.yaml.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
index 1ae8dcfa072c..229f8608c535 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
@@ -29,16 +29,20 @@ properties:
           - const: rockchip,rk3568-pcie
 
   reg:
+    minItems: 3
     items:
       - description: Data Bus Interface (DBI) registers
       - description: Rockchip designed configuration registers
       - description: Config registers
+      - description: iATU registers
 
   reg-names:
+    minItems: 3
     items:
       - const: dbi
       - const: apb
       - const: config
+      - const: atu
 
   clocks:
     minItems: 5
-- 
2.41.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
using:

allOf:
  - $ref: /schemas/pci/snps,dw-pcie.yaml#

and snps,dw-pcie.yaml does have the atu property defined, in order to be
able to use this property, while still making sure 'make CHECK_DTBS=y'
pass, we need to add this property to rockchip-dw-pcie.yaml.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
index 1ae8dcfa072c..229f8608c535 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
@@ -29,16 +29,20 @@ properties:
           - const: rockchip,rk3568-pcie
 
   reg:
+    minItems: 3
     items:
       - description: Data Bus Interface (DBI) registers
       - description: Rockchip designed configuration registers
       - description: Config registers
+      - description: iATU registers
 
   reg-names:
+    minItems: 3
     items:
       - const: dbi
       - const: apb
       - const: config
+      - const: atu
 
   clocks:
     minItems: 5
-- 
2.41.0

[PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
using:

allOf:
  - $ref: /schemas/pci/snps,dw-pcie.yaml#

and snps,dw-pcie.yaml does have the atu property defined, in order to be
able to use this property, while still making sure 'make CHECK_DTBS=y'
pass, we need to add this property to rockchip-dw-pcie.yaml.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
index 1ae8dcfa072c..229f8608c535 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
@@ -29,16 +29,20 @@ properties:
           - const: rockchip,rk3568-pcie
 
   reg:
+    minItems: 3
     items:
       - description: Data Bus Interface (DBI) registers
       - description: Rockchip designed configuration registers
       - description: Config registers
+      - description: iATU registers
 
   reg-names:
+    minItems: 3
     items:
       - const: dbi
       - const: apb
       - const: config
+      - const: atu
 
   clocks:
     minItems: 5
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/4] arm64: dts: rockchip: add missing mandatory rk3588 PCIe atu property

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

From the snps,dw-pcie.yaml devicetree binding:
"At least DBI reg-space and peripheral devices CFG-space outbound window
are required for the normal controller work. iATU memory IO region is
also required if the space is unrolled (IP-core version >= 4.80a)."

All the PCIe controllers in rk3588 are using the iATU unroll feature,
and thus have to supply the atu property in the device tree node.

The size of the iATU region equals to:
MAX(num inbound ATU regions, num outbound ATU regions) * 0x200.

Where for each 0x200 region, the registers controlling the
IATU_REGION_OUTBOUND starts at offset 0x0, and the registers controlling
IATU_REGION_INBOUND starts at offset 0x100.

pcie3x4 and pcie3x2 have 16 ATU inbound regions, 16 ATU outbound regions,
so they have size: max(16, 16) * 0x200 = 0x2000

pcie2x1l0, pcie2x1l1, and pcie2x1l2 have 8 ATU inbound regions, 8 ATU
outbound regions, so they have size: max(8, 8) * 0x200 = 0x1000

On the rk3588 based rock-5b board:
Before this patch, dw_pcie_iatu_detect() fails to detect all iATUs:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
rockchip-dw-pcie a41000000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
rockchip-dw-pcie a40800000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G

After this patch, dw_pcie_iatu_detect() succeeds to detect all iATUs:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 16 ob, 16 ib, align 64K, limit 8G
rockchip-dw-pcie a41000000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
rockchip-dw-pcie a40800000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G

Fixes: 8d81b77f4c49 ("arm64: dts: rockchip: add rk3588 PCIe2 support")
Fixes: 0acf4fa7f187 ("arm64: dts: rockchip: add PCIe3 support for rk3588")
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 arch/arm64/boot/dts/rockchip/rk3588.dtsi  | 21 ++++++++++++---------
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 14 ++++++++------
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index 5519c1430cb7..d7998a9c0c43 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -119,10 +119,11 @@ pcie3x4: pcie@fe150000 {
 		ranges = <0x01000000 0x0 0xf0100000 0x0 0xf0100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf0200000 0x0 0xf0200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0x9 0x00000000 0x0 0x40000000>;
-		reg = <0xa 0x40000000 0x0 0x00400000>,
+		reg = <0xa 0x40000000 0x0 0x00300000>,
 		      <0x0 0xfe150000 0x0 0x00010000>,
-		      <0x0 0xf0000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf0000000 0x0 0x00100000>,
+		      <0xa 0x40300000 0x0 0x00002000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE0_POWER_UP>, <&cru SRST_P_PCIE0>;
 		reset-names = "pwr", "pipe";
 		status = "disabled";
@@ -170,10 +171,11 @@ pcie3x2: pcie@fe160000 {
 		ranges = <0x01000000 0x0 0xf1100000 0x0 0xf1100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf1200000 0x0 0xf1200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0x9 0x40000000 0x0 0x40000000>;
-		reg = <0xa 0x40400000 0x0 0x00400000>,
+		reg = <0xa 0x40400000 0x0 0x00300000>,
 		      <0x0 0xfe160000 0x0 0x00010000>,
-		      <0x0 0xf1000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf1000000 0x0 0x00100000>,
+		      <0xa 0x40700000 0x0 0x00002000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE1_POWER_UP>, <&cru SRST_P_PCIE1>;
 		reset-names = "pwr", "pipe";
 		status = "disabled";
@@ -219,10 +221,11 @@ pcie2x1l0: pcie@fe170000 {
 		ranges = <0x01000000 0x0 0xf2100000 0x0 0xf2100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf2200000 0x0 0xf2200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0x9 0x80000000 0x0 0x40000000>;
-		reg = <0xa 0x40800000 0x0 0x00400000>,
+		reg = <0xa 0x40800000 0x0 0x00300000>,
 		      <0x0 0xfe170000 0x0 0x00010000>,
-		      <0x0 0xf2000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf2000000 0x0 0x00100000>,
+		      <0xa 0x40b00000 0x0 0x00001000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE2_POWER_UP>, <&cru SRST_P_PCIE2>;
 		reset-names = "pwr", "pipe";
 		#address-cells = <3>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 5544f66c6ff4..286d7b10b9dd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -1259,10 +1259,11 @@ pcie2x1l1: pcie@fe180000 {
 		ranges = <0x01000000 0x0 0xf3100000 0x0 0xf3100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf3200000 0x0 0xf3200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0x9 0xc0000000 0x0 0x40000000>;
-		reg = <0xa 0x40c00000 0x0 0x00400000>,
+		reg = <0xa 0x40c00000 0x0 0x00300000>,
 		      <0x0 0xfe180000 0x0 0x00010000>,
-		      <0x0 0xf3000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf3000000 0x0 0x00100000>,
+		      <0xa 0x40f00000 0x0 0x00001000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE3_POWER_UP>, <&cru SRST_P_PCIE3>;
 		reset-names = "pwr", "pipe";
 		#address-cells = <3>;
@@ -1310,10 +1311,11 @@ pcie2x1l2: pcie@fe190000 {
 		ranges = <0x01000000 0x0 0xf4100000 0x0 0xf4100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf4200000 0x0 0xf4200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0xa 0x00000000 0x0 0x40000000>;
-		reg = <0xa 0x41000000 0x0 0x00400000>,
+		reg = <0xa 0x41000000 0x0 0x00300000>,
 		      <0x0 0xfe190000 0x0 0x00010000>,
-		      <0x0 0xf4000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf4000000 0x0 0x00100000>,
+		      <0xa 0x41300000 0x0 0x00001000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE4_POWER_UP>, <&cru SRST_P_PCIE4>;
 		reset-names = "pwr", "pipe";
 		#address-cells = <3>;
-- 
2.41.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 2/4] arm64: dts: rockchip: add missing mandatory rk3588 PCIe atu property

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

From the snps,dw-pcie.yaml devicetree binding:
"At least DBI reg-space and peripheral devices CFG-space outbound window
are required for the normal controller work. iATU memory IO region is
also required if the space is unrolled (IP-core version >= 4.80a)."

All the PCIe controllers in rk3588 are using the iATU unroll feature,
and thus have to supply the atu property in the device tree node.

The size of the iATU region equals to:
MAX(num inbound ATU regions, num outbound ATU regions) * 0x200.

Where for each 0x200 region, the registers controlling the
IATU_REGION_OUTBOUND starts at offset 0x0, and the registers controlling
IATU_REGION_INBOUND starts at offset 0x100.

pcie3x4 and pcie3x2 have 16 ATU inbound regions, 16 ATU outbound regions,
so they have size: max(16, 16) * 0x200 = 0x2000

pcie2x1l0, pcie2x1l1, and pcie2x1l2 have 8 ATU inbound regions, 8 ATU
outbound regions, so they have size: max(8, 8) * 0x200 = 0x1000

On the rk3588 based rock-5b board:
Before this patch, dw_pcie_iatu_detect() fails to detect all iATUs:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
rockchip-dw-pcie a41000000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
rockchip-dw-pcie a40800000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G

After this patch, dw_pcie_iatu_detect() succeeds to detect all iATUs:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 16 ob, 16 ib, align 64K, limit 8G
rockchip-dw-pcie a41000000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
rockchip-dw-pcie a40800000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G

Fixes: 8d81b77f4c49 ("arm64: dts: rockchip: add rk3588 PCIe2 support")
Fixes: 0acf4fa7f187 ("arm64: dts: rockchip: add PCIe3 support for rk3588")
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 arch/arm64/boot/dts/rockchip/rk3588.dtsi  | 21 ++++++++++++---------
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 14 ++++++++------
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index 5519c1430cb7..d7998a9c0c43 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -119,10 +119,11 @@ pcie3x4: pcie@fe150000 {
 		ranges = <0x01000000 0x0 0xf0100000 0x0 0xf0100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf0200000 0x0 0xf0200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0x9 0x00000000 0x0 0x40000000>;
-		reg = <0xa 0x40000000 0x0 0x00400000>,
+		reg = <0xa 0x40000000 0x0 0x00300000>,
 		      <0x0 0xfe150000 0x0 0x00010000>,
-		      <0x0 0xf0000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf0000000 0x0 0x00100000>,
+		      <0xa 0x40300000 0x0 0x00002000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE0_POWER_UP>, <&cru SRST_P_PCIE0>;
 		reset-names = "pwr", "pipe";
 		status = "disabled";
@@ -170,10 +171,11 @@ pcie3x2: pcie@fe160000 {
 		ranges = <0x01000000 0x0 0xf1100000 0x0 0xf1100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf1200000 0x0 0xf1200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0x9 0x40000000 0x0 0x40000000>;
-		reg = <0xa 0x40400000 0x0 0x00400000>,
+		reg = <0xa 0x40400000 0x0 0x00300000>,
 		      <0x0 0xfe160000 0x0 0x00010000>,
-		      <0x0 0xf1000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf1000000 0x0 0x00100000>,
+		      <0xa 0x40700000 0x0 0x00002000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE1_POWER_UP>, <&cru SRST_P_PCIE1>;
 		reset-names = "pwr", "pipe";
 		status = "disabled";
@@ -219,10 +221,11 @@ pcie2x1l0: pcie@fe170000 {
 		ranges = <0x01000000 0x0 0xf2100000 0x0 0xf2100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf2200000 0x0 0xf2200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0x9 0x80000000 0x0 0x40000000>;
-		reg = <0xa 0x40800000 0x0 0x00400000>,
+		reg = <0xa 0x40800000 0x0 0x00300000>,
 		      <0x0 0xfe170000 0x0 0x00010000>,
-		      <0x0 0xf2000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf2000000 0x0 0x00100000>,
+		      <0xa 0x40b00000 0x0 0x00001000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE2_POWER_UP>, <&cru SRST_P_PCIE2>;
 		reset-names = "pwr", "pipe";
 		#address-cells = <3>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 5544f66c6ff4..286d7b10b9dd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -1259,10 +1259,11 @@ pcie2x1l1: pcie@fe180000 {
 		ranges = <0x01000000 0x0 0xf3100000 0x0 0xf3100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf3200000 0x0 0xf3200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0x9 0xc0000000 0x0 0x40000000>;
-		reg = <0xa 0x40c00000 0x0 0x00400000>,
+		reg = <0xa 0x40c00000 0x0 0x00300000>,
 		      <0x0 0xfe180000 0x0 0x00010000>,
-		      <0x0 0xf3000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf3000000 0x0 0x00100000>,
+		      <0xa 0x40f00000 0x0 0x00001000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE3_POWER_UP>, <&cru SRST_P_PCIE3>;
 		reset-names = "pwr", "pipe";
 		#address-cells = <3>;
@@ -1310,10 +1311,11 @@ pcie2x1l2: pcie@fe190000 {
 		ranges = <0x01000000 0x0 0xf4100000 0x0 0xf4100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf4200000 0x0 0xf4200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0xa 0x00000000 0x0 0x40000000>;
-		reg = <0xa 0x41000000 0x0 0x00400000>,
+		reg = <0xa 0x41000000 0x0 0x00300000>,
 		      <0x0 0xfe190000 0x0 0x00010000>,
-		      <0x0 0xf4000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf4000000 0x0 0x00100000>,
+		      <0xa 0x41300000 0x0 0x00001000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE4_POWER_UP>, <&cru SRST_P_PCIE4>;
 		reset-names = "pwr", "pipe";
 		#address-cells = <3>;
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/4] arm64: dts: rockchip: add missing mandatory rk3588 PCIe atu property

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

From the snps,dw-pcie.yaml devicetree binding:
"At least DBI reg-space and peripheral devices CFG-space outbound window
are required for the normal controller work. iATU memory IO region is
also required if the space is unrolled (IP-core version >= 4.80a)."

All the PCIe controllers in rk3588 are using the iATU unroll feature,
and thus have to supply the atu property in the device tree node.

The size of the iATU region equals to:
MAX(num inbound ATU regions, num outbound ATU regions) * 0x200.

Where for each 0x200 region, the registers controlling the
IATU_REGION_OUTBOUND starts at offset 0x0, and the registers controlling
IATU_REGION_INBOUND starts at offset 0x100.

pcie3x4 and pcie3x2 have 16 ATU inbound regions, 16 ATU outbound regions,
so they have size: max(16, 16) * 0x200 = 0x2000

pcie2x1l0, pcie2x1l1, and pcie2x1l2 have 8 ATU inbound regions, 8 ATU
outbound regions, so they have size: max(8, 8) * 0x200 = 0x1000

On the rk3588 based rock-5b board:
Before this patch, dw_pcie_iatu_detect() fails to detect all iATUs:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
rockchip-dw-pcie a41000000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
rockchip-dw-pcie a40800000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G

After this patch, dw_pcie_iatu_detect() succeeds to detect all iATUs:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 16 ob, 16 ib, align 64K, limit 8G
rockchip-dw-pcie a41000000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G
rockchip-dw-pcie a40800000.pcie: iATU: unroll T, 8 ob, 8 ib, align 64K, limit 8G

Fixes: 8d81b77f4c49 ("arm64: dts: rockchip: add rk3588 PCIe2 support")
Fixes: 0acf4fa7f187 ("arm64: dts: rockchip: add PCIe3 support for rk3588")
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 arch/arm64/boot/dts/rockchip/rk3588.dtsi  | 21 ++++++++++++---------
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 14 ++++++++------
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index 5519c1430cb7..d7998a9c0c43 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -119,10 +119,11 @@ pcie3x4: pcie@fe150000 {
 		ranges = <0x01000000 0x0 0xf0100000 0x0 0xf0100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf0200000 0x0 0xf0200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0x9 0x00000000 0x0 0x40000000>;
-		reg = <0xa 0x40000000 0x0 0x00400000>,
+		reg = <0xa 0x40000000 0x0 0x00300000>,
 		      <0x0 0xfe150000 0x0 0x00010000>,
-		      <0x0 0xf0000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf0000000 0x0 0x00100000>,
+		      <0xa 0x40300000 0x0 0x00002000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE0_POWER_UP>, <&cru SRST_P_PCIE0>;
 		reset-names = "pwr", "pipe";
 		status = "disabled";
@@ -170,10 +171,11 @@ pcie3x2: pcie@fe160000 {
 		ranges = <0x01000000 0x0 0xf1100000 0x0 0xf1100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf1200000 0x0 0xf1200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0x9 0x40000000 0x0 0x40000000>;
-		reg = <0xa 0x40400000 0x0 0x00400000>,
+		reg = <0xa 0x40400000 0x0 0x00300000>,
 		      <0x0 0xfe160000 0x0 0x00010000>,
-		      <0x0 0xf1000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf1000000 0x0 0x00100000>,
+		      <0xa 0x40700000 0x0 0x00002000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE1_POWER_UP>, <&cru SRST_P_PCIE1>;
 		reset-names = "pwr", "pipe";
 		status = "disabled";
@@ -219,10 +221,11 @@ pcie2x1l0: pcie@fe170000 {
 		ranges = <0x01000000 0x0 0xf2100000 0x0 0xf2100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf2200000 0x0 0xf2200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0x9 0x80000000 0x0 0x40000000>;
-		reg = <0xa 0x40800000 0x0 0x00400000>,
+		reg = <0xa 0x40800000 0x0 0x00300000>,
 		      <0x0 0xfe170000 0x0 0x00010000>,
-		      <0x0 0xf2000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf2000000 0x0 0x00100000>,
+		      <0xa 0x40b00000 0x0 0x00001000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE2_POWER_UP>, <&cru SRST_P_PCIE2>;
 		reset-names = "pwr", "pipe";
 		#address-cells = <3>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 5544f66c6ff4..286d7b10b9dd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -1259,10 +1259,11 @@ pcie2x1l1: pcie@fe180000 {
 		ranges = <0x01000000 0x0 0xf3100000 0x0 0xf3100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf3200000 0x0 0xf3200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0x9 0xc0000000 0x0 0x40000000>;
-		reg = <0xa 0x40c00000 0x0 0x00400000>,
+		reg = <0xa 0x40c00000 0x0 0x00300000>,
 		      <0x0 0xfe180000 0x0 0x00010000>,
-		      <0x0 0xf3000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf3000000 0x0 0x00100000>,
+		      <0xa 0x40f00000 0x0 0x00001000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE3_POWER_UP>, <&cru SRST_P_PCIE3>;
 		reset-names = "pwr", "pipe";
 		#address-cells = <3>;
@@ -1310,10 +1311,11 @@ pcie2x1l2: pcie@fe190000 {
 		ranges = <0x01000000 0x0 0xf4100000 0x0 0xf4100000 0x0 0x00100000>,
 			 <0x02000000 0x0 0xf4200000 0x0 0xf4200000 0x0 0x00e00000>,
 			 <0x03000000 0x0 0x40000000 0xa 0x00000000 0x0 0x40000000>;
-		reg = <0xa 0x41000000 0x0 0x00400000>,
+		reg = <0xa 0x41000000 0x0 0x00300000>,
 		      <0x0 0xfe190000 0x0 0x00010000>,
-		      <0x0 0xf4000000 0x0 0x00100000>;
-		reg-names = "dbi", "apb", "config";
+		      <0x0 0xf4000000 0x0 0x00100000>,
+		      <0xa 0x41300000 0x0 0x00001000>;
+		reg-names = "dbi", "apb", "config", "atu";
 		resets = <&cru SRST_PCIE4_POWER_UP>, <&cru SRST_P_PCIE4>;
 		reset-names = "pwr", "pipe";
 		#address-cells = <3>;
-- 
2.41.0

[PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
using:

allOf:
  - $ref: /schemas/pci/snps,dw-pcie.yaml#

and snps,dw-pcie.yaml does have the dma properties defined, in order to be
able to use these properties, while still making sure 'make CHECK_DTBS=y'
pass, we need to add these properties to rockchip-dw-pcie.yaml.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
index 229f8608c535..633f8e0e884f 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
@@ -35,6 +35,7 @@ properties:
       - description: Rockchip designed configuration registers
       - description: Config registers
       - description: iATU registers
+      - description: eDMA registers
 
   reg-names:
     minItems: 3
@@ -43,6 +44,7 @@ properties:
       - const: apb
       - const: config
       - const: atu
+      - const: dma
 
   clocks:
     minItems: 5
@@ -65,6 +67,7 @@ properties:
       - const: pipe
 
   interrupts:
+    minItems: 5
     items:
       - description:
           Combined system interrupt, which is used to signal the following
@@ -88,14 +91,31 @@ properties:
           interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
           tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
           nf_err_rx, f_err_rx, radm_qoverflow
+      - description:
+          Indicates that the eDMA Tx/Rx transfer is complete or that an
+          error has occurred on the corresponding channel.
+      - description:
+          Indicates that the eDMA Tx/Rx transfer is complete or that an
+          error has occurred on the corresponding channel.
+      - description:
+          Indicates that the eDMA Tx/Rx transfer is complete or that an
+          error has occurred on the corresponding channel.
+      - description:
+          Indicates that the eDMA Tx/Rx transfer is complete or that an
+          error has occurred on the corresponding channel.
 
   interrupt-names:
+    minItems: 5
     items:
       - const: sys
       - const: pmc
       - const: msg
       - const: legacy
       - const: err
+      - const: dma0
+      - const: dma1
+      - const: dma2
+      - const: dma3
 
   legacy-interrupt-controller:
     description: Interrupt controller node for handling legacy PCI interrupts.
-- 
2.41.0

[PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
using:

allOf:
  - $ref: /schemas/pci/snps,dw-pcie.yaml#

and snps,dw-pcie.yaml does have the dma properties defined, in order to be
able to use these properties, while still making sure 'make CHECK_DTBS=y'
pass, we need to add these properties to rockchip-dw-pcie.yaml.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
index 229f8608c535..633f8e0e884f 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
@@ -35,6 +35,7 @@ properties:
       - description: Rockchip designed configuration registers
       - description: Config registers
       - description: iATU registers
+      - description: eDMA registers
 
   reg-names:
     minItems: 3
@@ -43,6 +44,7 @@ properties:
       - const: apb
       - const: config
       - const: atu
+      - const: dma
 
   clocks:
     minItems: 5
@@ -65,6 +67,7 @@ properties:
       - const: pipe
 
   interrupts:
+    minItems: 5
     items:
       - description:
           Combined system interrupt, which is used to signal the following
@@ -88,14 +91,31 @@ properties:
           interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
           tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
           nf_err_rx, f_err_rx, radm_qoverflow
+      - description:
+          Indicates that the eDMA Tx/Rx transfer is complete or that an
+          error has occurred on the corresponding channel.
+      - description:
+          Indicates that the eDMA Tx/Rx transfer is complete or that an
+          error has occurred on the corresponding channel.
+      - description:
+          Indicates that the eDMA Tx/Rx transfer is complete or that an
+          error has occurred on the corresponding channel.
+      - description:
+          Indicates that the eDMA Tx/Rx transfer is complete or that an
+          error has occurred on the corresponding channel.
 
   interrupt-names:
+    minItems: 5
     items:
       - const: sys
       - const: pmc
       - const: msg
       - const: legacy
       - const: err
+      - const: dma0
+      - const: dma1
+      - const: dma2
+      - const: dma3
 
   legacy-interrupt-controller:
     description: Interrupt controller node for handling legacy PCI interrupts.
-- 
2.41.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
using:

allOf:
  - $ref: /schemas/pci/snps,dw-pcie.yaml#

and snps,dw-pcie.yaml does have the dma properties defined, in order to be
able to use these properties, while still making sure 'make CHECK_DTBS=y'
pass, we need to add these properties to rockchip-dw-pcie.yaml.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
index 229f8608c535..633f8e0e884f 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
@@ -35,6 +35,7 @@ properties:
       - description: Rockchip designed configuration registers
       - description: Config registers
       - description: iATU registers
+      - description: eDMA registers
 
   reg-names:
     minItems: 3
@@ -43,6 +44,7 @@ properties:
       - const: apb
       - const: config
       - const: atu
+      - const: dma
 
   clocks:
     minItems: 5
@@ -65,6 +67,7 @@ properties:
       - const: pipe
 
   interrupts:
+    minItems: 5
     items:
       - description:
           Combined system interrupt, which is used to signal the following
@@ -88,14 +91,31 @@ properties:
           interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
           tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
           nf_err_rx, f_err_rx, radm_qoverflow
+      - description:
+          Indicates that the eDMA Tx/Rx transfer is complete or that an
+          error has occurred on the corresponding channel.
+      - description:
+          Indicates that the eDMA Tx/Rx transfer is complete or that an
+          error has occurred on the corresponding channel.
+      - description:
+          Indicates that the eDMA Tx/Rx transfer is complete or that an
+          error has occurred on the corresponding channel.
+      - description:
+          Indicates that the eDMA Tx/Rx transfer is complete or that an
+          error has occurred on the corresponding channel.
 
   interrupt-names:
+    minItems: 5
     items:
       - const: sys
       - const: pmc
       - const: msg
       - const: legacy
       - const: err
+      - const: dma0
+      - const: dma1
+      - const: dma2
+      - const: dma3
 
   legacy-interrupt-controller:
     description: Interrupt controller node for handling legacy PCI interrupts.
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/4] arm64: dts: rockchip: add missing rk3588 PCIe dma properties

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

The rk3588 has 5 PCIe controllers, however, according the the rk3588 TRM
(Technical Reference Manual), only pcie3x4 supports the embedded DMA
controller (eDMA) on the DWC PCIe controller.

The size of the eDMA region equals to:
0x200 + MAX(NUM_DMA_RD_CHAN, NUM_DMA_WR_CHAN) * 0x200.

Where for each 0x200 region, the registers controlling the write channel
starts at offset 0x0, and the registers controlling the read channel
starts at offset 0x100.

pcie3x4 has two DMA read channels and two DMA write channels,
so it has size: 0x200 + max(2, 2) * 0x200 = 0x600

On the rk3588 based rock-5b board, when building with CONFIG_DW_EDMA=y:
Before this patch, only the iATUs are detected:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 16 ob, 16 ib, align 64K, limit 8G

After this patch, both the iATUs and the eDMA channels are detected:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 16 ob, 16 ib, align 64K, limit 8G
rockchip-dw-pcie a40000000.pcie: eDMA: unroll T, 2 wr, 2 rd

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 arch/arm64/boot/dts/rockchip/rk3588.dtsi | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index d7998a9c0c43..e072f5fe655d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -101,8 +101,13 @@ pcie3x4: pcie@fe150000 {
 			     <GIC_SPI 262 IRQ_TYPE_LEVEL_HIGH 0>,
 			     <GIC_SPI 261 IRQ_TYPE_LEVEL_HIGH 0>,
 			     <GIC_SPI 260 IRQ_TYPE_LEVEL_HIGH 0>,
-			     <GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH 0>;
-		interrupt-names = "sys", "pmc", "msg", "legacy", "err";
+			     <GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH 0>;
+		interrupt-names = "sys", "pmc", "msg", "legacy", "err",
+				  "dma0", "dma1", "dma2", "dma3";
 		#interrupt-cells = <1>;
 		interrupt-map-mask = <0 0 0 7>;
 		interrupt-map = <0 0 0 1 &pcie3x4_intc 0>,
@@ -122,8 +127,9 @@ pcie3x4: pcie@fe150000 {
 		reg = <0xa 0x40000000 0x0 0x00300000>,
 		      <0x0 0xfe150000 0x0 0x00010000>,
 		      <0x0 0xf0000000 0x0 0x00100000>,
-		      <0xa 0x40300000 0x0 0x00002000>;
-		reg-names = "dbi", "apb", "config", "atu";
+		      <0xa 0x40300000 0x0 0x00002000>,
+		      <0xa 0x40380000 0x0 0x00000600>;
+		reg-names = "dbi", "apb", "config", "atu", "dma";
 		resets = <&cru SRST_PCIE0_POWER_UP>, <&cru SRST_P_PCIE0>;
 		reset-names = "pwr", "pipe";
 		status = "disabled";
-- 
2.41.0

[PATCH v2 4/4] arm64: dts: rockchip: add missing rk3588 PCIe dma properties

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

The rk3588 has 5 PCIe controllers, however, according the the rk3588 TRM
(Technical Reference Manual), only pcie3x4 supports the embedded DMA
controller (eDMA) on the DWC PCIe controller.

The size of the eDMA region equals to:
0x200 + MAX(NUM_DMA_RD_CHAN, NUM_DMA_WR_CHAN) * 0x200.

Where for each 0x200 region, the registers controlling the write channel
starts at offset 0x0, and the registers controlling the read channel
starts at offset 0x100.

pcie3x4 has two DMA read channels and two DMA write channels,
so it has size: 0x200 + max(2, 2) * 0x200 = 0x600

On the rk3588 based rock-5b board, when building with CONFIG_DW_EDMA=y:
Before this patch, only the iATUs are detected:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 16 ob, 16 ib, align 64K, limit 8G

After this patch, both the iATUs and the eDMA channels are detected:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 16 ob, 16 ib, align 64K, limit 8G
rockchip-dw-pcie a40000000.pcie: eDMA: unroll T, 2 wr, 2 rd

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 arch/arm64/boot/dts/rockchip/rk3588.dtsi | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index d7998a9c0c43..e072f5fe655d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -101,8 +101,13 @@ pcie3x4: pcie@fe150000 {
 			     <GIC_SPI 262 IRQ_TYPE_LEVEL_HIGH 0>,
 			     <GIC_SPI 261 IRQ_TYPE_LEVEL_HIGH 0>,
 			     <GIC_SPI 260 IRQ_TYPE_LEVEL_HIGH 0>,
-			     <GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH 0>;
-		interrupt-names = "sys", "pmc", "msg", "legacy", "err";
+			     <GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH 0>;
+		interrupt-names = "sys", "pmc", "msg", "legacy", "err",
+				  "dma0", "dma1", "dma2", "dma3";
 		#interrupt-cells = <1>;
 		interrupt-map-mask = <0 0 0 7>;
 		interrupt-map = <0 0 0 1 &pcie3x4_intc 0>,
@@ -122,8 +127,9 @@ pcie3x4: pcie@fe150000 {
 		reg = <0xa 0x40000000 0x0 0x00300000>,
 		      <0x0 0xfe150000 0x0 0x00010000>,
 		      <0x0 0xf0000000 0x0 0x00100000>,
-		      <0xa 0x40300000 0x0 0x00002000>;
-		reg-names = "dbi", "apb", "config", "atu";
+		      <0xa 0x40300000 0x0 0x00002000>,
+		      <0xa 0x40380000 0x0 0x00000600>;
+		reg-names = "dbi", "apb", "config", "atu", "dma";
 		resets = <&cru SRST_PCIE0_POWER_UP>, <&cru SRST_P_PCIE0>;
 		reset-names = "pwr", "pipe";
 		status = "disabled";
-- 
2.41.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 4/4] arm64: dts: rockchip: add missing rk3588 PCIe dma properties

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

The rk3588 has 5 PCIe controllers, however, according the the rk3588 TRM
(Technical Reference Manual), only pcie3x4 supports the embedded DMA
controller (eDMA) on the DWC PCIe controller.

The size of the eDMA region equals to:
0x200 + MAX(NUM_DMA_RD_CHAN, NUM_DMA_WR_CHAN) * 0x200.

Where for each 0x200 region, the registers controlling the write channel
starts at offset 0x0, and the registers controlling the read channel
starts at offset 0x100.

pcie3x4 has two DMA read channels and two DMA write channels,
so it has size: 0x200 + max(2, 2) * 0x200 = 0x600

On the rk3588 based rock-5b board, when building with CONFIG_DW_EDMA=y:
Before this patch, only the iATUs are detected:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 16 ob, 16 ib, align 64K, limit 8G

After this patch, both the iATUs and the eDMA channels are detected:
rockchip-dw-pcie a40000000.pcie: iATU: unroll T, 16 ob, 16 ib, align 64K, limit 8G
rockchip-dw-pcie a40000000.pcie: eDMA: unroll T, 2 wr, 2 rd

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 arch/arm64/boot/dts/rockchip/rk3588.dtsi | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index d7998a9c0c43..e072f5fe655d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -101,8 +101,13 @@ pcie3x4: pcie@fe150000 {
 			     <GIC_SPI 262 IRQ_TYPE_LEVEL_HIGH 0>,
 			     <GIC_SPI 261 IRQ_TYPE_LEVEL_HIGH 0>,
 			     <GIC_SPI 260 IRQ_TYPE_LEVEL_HIGH 0>,
-			     <GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH 0>;
-		interrupt-names = "sys", "pmc", "msg", "legacy", "err";
+			     <GIC_SPI 259 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH 0>;
+		interrupt-names = "sys", "pmc", "msg", "legacy", "err",
+				  "dma0", "dma1", "dma2", "dma3";
 		#interrupt-cells = <1>;
 		interrupt-map-mask = <0 0 0 7>;
 		interrupt-map = <0 0 0 1 &pcie3x4_intc 0>,
@@ -122,8 +127,9 @@ pcie3x4: pcie@fe150000 {
 		reg = <0xa 0x40000000 0x0 0x00300000>,
 		      <0x0 0xfe150000 0x0 0x00010000>,
 		      <0x0 0xf0000000 0x0 0x00100000>,
-		      <0xa 0x40300000 0x0 0x00002000>;
-		reg-names = "dbi", "apb", "config", "atu";
+		      <0xa 0x40300000 0x0 0x00002000>,
+		      <0xa 0x40380000 0x0 0x00000600>;
+		reg-names = "dbi", "apb", "config", "atu", "dma";
 		resets = <&cru SRST_PCIE0_POWER_UP>, <&cru SRST_P_PCIE0>;
 		reset-names = "pwr", "pipe";
 		status = "disabled";
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Conor Dooley]

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

On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
> 
> allOf:
>   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> 
> and snps,dw-pcie.yaml does have the atu property defined, in order to be
> able to use this property, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add this property to rockchip-dw-pcie.yaml.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 1ae8dcfa072c..229f8608c535 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -29,16 +29,20 @@ properties:
>            - const: rockchip,rk3568-pcie
>  
>    reg:
> +    minItems: 3
>      items:
>        - description: Data Bus Interface (DBI) registers
>        - description: Rockchip designed configuration registers
>        - description: Config registers
> +      - description: iATU registers

Is this extra register only for the ..88 or for the ..68 and for the
..88 models?

>  
>    reg-names:
> +    minItems: 3
>      items:
>        - const: dbi
>        - const: apb
>        - const: config
> +      - const: atu
>  
>    clocks:
>      minItems: 5
> -- 
> 2.41.0
> 

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

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1604 bytes --]

On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
> 
> allOf:
>   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> 
> and snps,dw-pcie.yaml does have the atu property defined, in order to be
> able to use this property, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add this property to rockchip-dw-pcie.yaml.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 1ae8dcfa072c..229f8608c535 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -29,16 +29,20 @@ properties:
>            - const: rockchip,rk3568-pcie
>  
>    reg:
> +    minItems: 3
>      items:
>        - description: Data Bus Interface (DBI) registers
>        - description: Rockchip designed configuration registers
>        - description: Config registers
> +      - description: iATU registers

Is this extra register only for the ..88 or for the ..68 and for the
..88 models?

>  
>    reg-names:
> +    minItems: 3
>      items:
>        - const: dbi
>        - const: apb
>        - const: config
> +      - const: atu
>  
>    clocks:
>      minItems: 5
> -- 
> 2.41.0
> 

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

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1604 bytes --]

On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
> 
> allOf:
>   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> 
> and snps,dw-pcie.yaml does have the atu property defined, in order to be
> able to use this property, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add this property to rockchip-dw-pcie.yaml.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 1ae8dcfa072c..229f8608c535 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -29,16 +29,20 @@ properties:
>            - const: rockchip,rk3568-pcie
>  
>    reg:
> +    minItems: 3
>      items:
>        - description: Data Bus Interface (DBI) registers
>        - description: Rockchip designed configuration registers
>        - description: Config registers
> +      - description: iATU registers

Is this extra register only for the ..88 or for the ..68 and for the
..88 models?

>  
>    reg-names:
> +    minItems: 3
>      items:
>        - const: dbi
>        - const: apb
>        - const: config
> +      - const: atu
>  
>    clocks:
>      minItems: 5
> -- 
> 2.41.0
> 

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Conor Dooley]

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

On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
> 
> allOf:
>   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> 
> and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> able to use these properties, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add these properties to rockchip-dw-pcie.yaml.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 229f8608c535..633f8e0e884f 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -35,6 +35,7 @@ properties:
>        - description: Rockchip designed configuration registers
>        - description: Config registers
>        - description: iATU registers
> +      - description: eDMA registers

Same here, is this just for one of the two supported models, or for
both?

Cheers,
Conor.

>  
>    reg-names:
>      minItems: 3
> @@ -43,6 +44,7 @@ properties:
>        - const: apb
>        - const: config
>        - const: atu
> +      - const: dma
>  
>    clocks:
>      minItems: 5
> @@ -65,6 +67,7 @@ properties:
>        - const: pipe
>  
>    interrupts:
> +    minItems: 5
>      items:
>        - description:
>            Combined system interrupt, which is used to signal the following
> @@ -88,14 +91,31 @@ properties:
>            interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
>            tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
>            nf_err_rx, f_err_rx, radm_qoverflow
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
>  
>    interrupt-names:
> +    minItems: 5
>      items:
>        - const: sys
>        - const: pmc
>        - const: msg
>        - const: legacy
>        - const: err
> +      - const: dma0
> +      - const: dma1
> +      - const: dma2
> +      - const: dma3
>  
>    legacy-interrupt-controller:
>      description: Interrupt controller node for handling legacy PCI interrupts.
> -- 
> 2.41.0
> 

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

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 3039 bytes --]

On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
> 
> allOf:
>   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> 
> and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> able to use these properties, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add these properties to rockchip-dw-pcie.yaml.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 229f8608c535..633f8e0e884f 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -35,6 +35,7 @@ properties:
>        - description: Rockchip designed configuration registers
>        - description: Config registers
>        - description: iATU registers
> +      - description: eDMA registers

Same here, is this just for one of the two supported models, or for
both?

Cheers,
Conor.

>  
>    reg-names:
>      minItems: 3
> @@ -43,6 +44,7 @@ properties:
>        - const: apb
>        - const: config
>        - const: atu
> +      - const: dma
>  
>    clocks:
>      minItems: 5
> @@ -65,6 +67,7 @@ properties:
>        - const: pipe
>  
>    interrupts:
> +    minItems: 5
>      items:
>        - description:
>            Combined system interrupt, which is used to signal the following
> @@ -88,14 +91,31 @@ properties:
>            interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
>            tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
>            nf_err_rx, f_err_rx, radm_qoverflow
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
>  
>    interrupt-names:
> +    minItems: 5
>      items:
>        - const: sys
>        - const: pmc
>        - const: msg
>        - const: legacy
>        - const: err
> +      - const: dma0
> +      - const: dma1
> +      - const: dma2
> +      - const: dma3
>  
>    legacy-interrupt-controller:
>      description: Interrupt controller node for handling legacy PCI interrupts.
> -- 
> 2.41.0
> 

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

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 3039 bytes --]

On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
> 
> allOf:
>   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> 
> and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> able to use these properties, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add these properties to rockchip-dw-pcie.yaml.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 229f8608c535..633f8e0e884f 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -35,6 +35,7 @@ properties:
>        - description: Rockchip designed configuration registers
>        - description: Config registers
>        - description: iATU registers
> +      - description: eDMA registers

Same here, is this just for one of the two supported models, or for
both?

Cheers,
Conor.

>  
>    reg-names:
>      minItems: 3
> @@ -43,6 +44,7 @@ properties:
>        - const: apb
>        - const: config
>        - const: atu
> +      - const: dma
>  
>    clocks:
>      minItems: 5
> @@ -65,6 +67,7 @@ properties:
>        - const: pipe
>  
>    interrupts:
> +    minItems: 5
>      items:
>        - description:
>            Combined system interrupt, which is used to signal the following
> @@ -88,14 +91,31 @@ properties:
>            interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
>            tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
>            nf_err_rx, f_err_rx, radm_qoverflow
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
>  
>    interrupt-names:
> +    minItems: 5
>      items:
>        - const: sys
>        - const: pmc
>        - const: msg
>        - const: legacy
>        - const: err
> +      - const: dma0
> +      - const: dma1
> +      - const: dma2
> +      - const: dma3
>  
>    legacy-interrupt-controller:
>      description: Interrupt controller node for handling legacy PCI interrupts.
> -- 
> 2.41.0
> 

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Niklas Cassel]

Hello Conor,

On Tue, Oct 24, 2023 at 05:29:28PM +0100, Conor Dooley wrote:
> On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> > From: Niklas Cassel <niklas.cassel@wdc.com>
> > 
> > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > using:
> > 
> > allOf:
> >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > 
> > and snps,dw-pcie.yaml does have the atu property defined, in order to be
> > able to use this property, while still making sure 'make CHECK_DTBS=y'
> > pass, we need to add this property to rockchip-dw-pcie.yaml.
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > ---
> >  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > index 1ae8dcfa072c..229f8608c535 100644
> > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > @@ -29,16 +29,20 @@ properties:
> >            - const: rockchip,rk3568-pcie
> >  
> >    reg:
> > +    minItems: 3
> >      items:
> >        - description: Data Bus Interface (DBI) registers
> >        - description: Rockchip designed configuration registers
> >        - description: Config registers
> > +      - description: iATU registers
> 
> Is this extra register only for the ..88 or for the ..68 and for the
> ..88 models?

Looking at the rk3568 Technical Reference Manual (TRM):
https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf

The iATU register register range exists for all 3 PCIe controllers
found on the rk3568.

This register range is currently not defined in the rk3568.dtsi, so the driver
will currently use the default register offset (which is correct), but with
the driver fallback register size that is only big enough to cover 8 inbound
and 8 outbound iATUs (internal Address Translation Units).

According to the TRM, all three PCIe controllers found on the rk3568 have
16 inbound iATUs and 16 outbound iATUs, so if someone wants to be able to
make use of all the iATUs on the rk3568, they will need to add "atu" to
rk3568.dtsi.

I don't have the board, so I can't test, but I could add a patch that adds
the "atu" range for the controllers in rk3568, if that makes things easier.


Kind regards,
Niklas

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Niklas Cassel]

Hello Conor,

On Tue, Oct 24, 2023 at 05:29:28PM +0100, Conor Dooley wrote:
> On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> > From: Niklas Cassel <niklas.cassel@wdc.com>
> > 
> > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > using:
> > 
> > allOf:
> >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > 
> > and snps,dw-pcie.yaml does have the atu property defined, in order to be
> > able to use this property, while still making sure 'make CHECK_DTBS=y'
> > pass, we need to add this property to rockchip-dw-pcie.yaml.
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > ---
> >  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > index 1ae8dcfa072c..229f8608c535 100644
> > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > @@ -29,16 +29,20 @@ properties:
> >            - const: rockchip,rk3568-pcie
> >  
> >    reg:
> > +    minItems: 3
> >      items:
> >        - description: Data Bus Interface (DBI) registers
> >        - description: Rockchip designed configuration registers
> >        - description: Config registers
> > +      - description: iATU registers
> 
> Is this extra register only for the ..88 or for the ..68 and for the
> ..88 models?

Looking at the rk3568 Technical Reference Manual (TRM):
https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf

The iATU register register range exists for all 3 PCIe controllers
found on the rk3568.

This register range is currently not defined in the rk3568.dtsi, so the driver
will currently use the default register offset (which is correct), but with
the driver fallback register size that is only big enough to cover 8 inbound
and 8 outbound iATUs (internal Address Translation Units).

According to the TRM, all three PCIe controllers found on the rk3568 have
16 inbound iATUs and 16 outbound iATUs, so if someone wants to be able to
make use of all the iATUs on the rk3568, they will need to add "atu" to
rk3568.dtsi.

I don't have the board, so I can't test, but I could add a patch that adds
the "atu" range for the controllers in rk3568, if that makes things easier.


Kind regards,
Niklas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Niklas Cassel]

Hello Conor,

On Tue, Oct 24, 2023 at 05:29:28PM +0100, Conor Dooley wrote:
> On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> > From: Niklas Cassel <niklas.cassel@wdc.com>
> > 
> > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > using:
> > 
> > allOf:
> >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > 
> > and snps,dw-pcie.yaml does have the atu property defined, in order to be
> > able to use this property, while still making sure 'make CHECK_DTBS=y'
> > pass, we need to add this property to rockchip-dw-pcie.yaml.
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > ---
> >  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > index 1ae8dcfa072c..229f8608c535 100644
> > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > @@ -29,16 +29,20 @@ properties:
> >            - const: rockchip,rk3568-pcie
> >  
> >    reg:
> > +    minItems: 3
> >      items:
> >        - description: Data Bus Interface (DBI) registers
> >        - description: Rockchip designed configuration registers
> >        - description: Config registers
> > +      - description: iATU registers
> 
> Is this extra register only for the ..88 or for the ..68 and for the
> ..88 models?

Looking at the rk3568 Technical Reference Manual (TRM):
https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf

The iATU register register range exists for all 3 PCIe controllers
found on the rk3568.

This register range is currently not defined in the rk3568.dtsi, so the driver
will currently use the default register offset (which is correct), but with
the driver fallback register size that is only big enough to cover 8 inbound
and 8 outbound iATUs (internal Address Translation Units).

According to the TRM, all three PCIe controllers found on the rk3568 have
16 inbound iATUs and 16 outbound iATUs, so if someone wants to be able to
make use of all the iATUs on the rk3568, they will need to add "atu" to
rk3568.dtsi.

I don't have the board, so I can't test, but I could add a patch that adds
the "atu" range for the controllers in rk3568, if that makes things easier.


Kind regards,
Niklas
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Niklas Cassel]

Hello Conor,

On Tue, Oct 24, 2023 at 05:30:22PM +0100, Conor Dooley wrote:
> On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> > From: Niklas Cassel <niklas.cassel@wdc.com>
> > 
> > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > using:
> > 
> > allOf:
> >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > 
> > and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> > able to use these properties, while still making sure 'make CHECK_DTBS=y'
> > pass, we need to add these properties to rockchip-dw-pcie.yaml.
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > ---
> >  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > index 229f8608c535..633f8e0e884f 100644
> > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > @@ -35,6 +35,7 @@ properties:
> >        - description: Rockchip designed configuration registers
> >        - description: Config registers
> >        - description: iATU registers
> > +      - description: eDMA registers
> 
> Same here, is this just for one of the two supported models, or for
> both?

For the 3 controllers found in rk3568, this range exists for all
(all of the controllers were synthesized with the eDMA controller).

For the 5 controllers found in rk3588, this range exists for only one of them
(only one of the controllers was synthesized with the eDMA controller).


> >    interrupt-names:
> > +    minItems: 5
> >      items:
> >        - const: sys
> >        - const: pmc
> >        - const: msg
> >        - const: legacy
> >        - const: err
> > +      - const: dma0
> > +      - const: dma1
> > +      - const: dma2
> > +      - const: dma3

While all the PCIe controllers on the rk3568 have the embedded DMA controller
as part of the PCIe controller, they don't have separate IRQs for the eDMA.
(They will need to use the combined "sys" irq, so the driver will need to read
an additional register to see that it was an eDMA irq.)

For the rk3588, only one of the 5 PCIe controllers have the eDMA, and that
controller also has dedicated IRQs for the eDMA.
(It should also be able to use the combined "sys" irq, but that would be less
efficient, and AFAICT, the driver currently only works with dedicated IRQs.)


Kind regards,
Niklas

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Niklas Cassel]

Hello Conor,

On Tue, Oct 24, 2023 at 05:30:22PM +0100, Conor Dooley wrote:
> On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> > From: Niklas Cassel <niklas.cassel@wdc.com>
> > 
> > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > using:
> > 
> > allOf:
> >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > 
> > and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> > able to use these properties, while still making sure 'make CHECK_DTBS=y'
> > pass, we need to add these properties to rockchip-dw-pcie.yaml.
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > ---
> >  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > index 229f8608c535..633f8e0e884f 100644
> > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > @@ -35,6 +35,7 @@ properties:
> >        - description: Rockchip designed configuration registers
> >        - description: Config registers
> >        - description: iATU registers
> > +      - description: eDMA registers
> 
> Same here, is this just for one of the two supported models, or for
> both?

For the 3 controllers found in rk3568, this range exists for all
(all of the controllers were synthesized with the eDMA controller).

For the 5 controllers found in rk3588, this range exists for only one of them
(only one of the controllers was synthesized with the eDMA controller).


> >    interrupt-names:
> > +    minItems: 5
> >      items:
> >        - const: sys
> >        - const: pmc
> >        - const: msg
> >        - const: legacy
> >        - const: err
> > +      - const: dma0
> > +      - const: dma1
> > +      - const: dma2
> > +      - const: dma3

While all the PCIe controllers on the rk3568 have the embedded DMA controller
as part of the PCIe controller, they don't have separate IRQs for the eDMA.
(They will need to use the combined "sys" irq, so the driver will need to read
an additional register to see that it was an eDMA irq.)

For the rk3588, only one of the 5 PCIe controllers have the eDMA, and that
controller also has dedicated IRQs for the eDMA.
(It should also be able to use the combined "sys" irq, but that would be less
efficient, and AFAICT, the driver currently only works with dedicated IRQs.)


Kind regards,
Niklas
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Niklas Cassel]

Hello Conor,

On Tue, Oct 24, 2023 at 05:30:22PM +0100, Conor Dooley wrote:
> On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> > From: Niklas Cassel <niklas.cassel@wdc.com>
> > 
> > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > using:
> > 
> > allOf:
> >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > 
> > and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> > able to use these properties, while still making sure 'make CHECK_DTBS=y'
> > pass, we need to add these properties to rockchip-dw-pcie.yaml.
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > ---
> >  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > index 229f8608c535..633f8e0e884f 100644
> > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > @@ -35,6 +35,7 @@ properties:
> >        - description: Rockchip designed configuration registers
> >        - description: Config registers
> >        - description: iATU registers
> > +      - description: eDMA registers
> 
> Same here, is this just for one of the two supported models, or for
> both?

For the 3 controllers found in rk3568, this range exists for all
(all of the controllers were synthesized with the eDMA controller).

For the 5 controllers found in rk3588, this range exists for only one of them
(only one of the controllers was synthesized with the eDMA controller).


> >    interrupt-names:
> > +    minItems: 5
> >      items:
> >        - const: sys
> >        - const: pmc
> >        - const: msg
> >        - const: legacy
> >        - const: err
> > +      - const: dma0
> > +      - const: dma1
> > +      - const: dma2
> > +      - const: dma3

While all the PCIe controllers on the rk3568 have the embedded DMA controller
as part of the PCIe controller, they don't have separate IRQs for the eDMA.
(They will need to use the combined "sys" irq, so the driver will need to read
an additional register to see that it was an eDMA irq.)

For the rk3588, only one of the 5 PCIe controllers have the eDMA, and that
controller also has dedicated IRQs for the eDMA.
(It should also be able to use the combined "sys" irq, but that would be less
efficient, and AFAICT, the driver currently only works with dedicated IRQs.)


Kind regards,
Niklas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Niklas Cassel]

On Wed, Oct 25, 2023 at 10:07:02PM +0200, Niklas Cassel wrote:
> Hello Conor,
> 
> On Tue, Oct 24, 2023 at 05:30:22PM +0100, Conor Dooley wrote:
> > On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > 
> > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > using:
> > > 
> > > allOf:
> > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > 
> > > and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> > > able to use these properties, while still making sure 'make CHECK_DTBS=y'
> > > pass, we need to add these properties to rockchip-dw-pcie.yaml.
> > > 
> > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > ---
> > >  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > index 229f8608c535..633f8e0e884f 100644
> > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > @@ -35,6 +35,7 @@ properties:
> > >        - description: Rockchip designed configuration registers
> > >        - description: Config registers
> > >        - description: iATU registers
> > > +      - description: eDMA registers
> > 
> > Same here, is this just for one of the two supported models, or for
> > both?
> 
> For the 3 controllers found in rk3568, this range exists for all
> (all of the controllers were synthesized with the eDMA controller).
> 
> For the 5 controllers found in rk3588, this range exists for only one of them
> (only one of the controllers was synthesized with the eDMA controller).
> 
> 
> > >    interrupt-names:
> > > +    minItems: 5
> > >      items:
> > >        - const: sys
> > >        - const: pmc
> > >        - const: msg
> > >        - const: legacy
> > >        - const: err
> > > +      - const: dma0
> > > +      - const: dma1
> > > +      - const: dma2
> > > +      - const: dma3
> 
> While all the PCIe controllers on the rk3568 have the embedded DMA controller
> as part of the PCIe controller, they don't have separate IRQs for the eDMA.
> (They will need to use the combined "sys" irq, so the driver will need to read
> an additional register to see that it was an eDMA irq.)
> 
> For the rk3588, only one of the 5 PCIe controllers have the eDMA, and that
> controller also has dedicated IRQs for the eDMA.
> (It should also be able to use the combined "sys" irq, but that would be less
> efficient, and AFAICT, the driver currently only works with dedicated IRQs.)

We could go with Sebastian's suggestion to define a 1MB range for "atu", see:
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/pci/snps%2Cdw-pcie.yaml#L76-L85

Which would allow the driver to probe if the eDMA is there or not
(even if this is strictly bigger than the real ATU_CAP size, the size is still
within the PCIe core's register map).

That would solve the problem that some pcie controllers, with the exact same
compatible, has a "dma" range while others do not.
(All controllers would have a 1MB atu range, and none of them would have the
dma range specified.)

However, we would still have the problem that for the exact same compatible,
some controllers have eDMA irqs specified in interrupts, and some do not...

But perhaps having mandatory atu range (and no dma range) + optional dma irqs
is better than mandatory atu range + optional dma range + optional dma irqs?
(At least from a DT schema maintainability PoV.)


Kind regards,
Niklas

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Niklas Cassel]

On Wed, Oct 25, 2023 at 10:07:02PM +0200, Niklas Cassel wrote:
> Hello Conor,
> 
> On Tue, Oct 24, 2023 at 05:30:22PM +0100, Conor Dooley wrote:
> > On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > 
> > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > using:
> > > 
> > > allOf:
> > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > 
> > > and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> > > able to use these properties, while still making sure 'make CHECK_DTBS=y'
> > > pass, we need to add these properties to rockchip-dw-pcie.yaml.
> > > 
> > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > ---
> > >  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > index 229f8608c535..633f8e0e884f 100644
> > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > @@ -35,6 +35,7 @@ properties:
> > >        - description: Rockchip designed configuration registers
> > >        - description: Config registers
> > >        - description: iATU registers
> > > +      - description: eDMA registers
> > 
> > Same here, is this just for one of the two supported models, or for
> > both?
> 
> For the 3 controllers found in rk3568, this range exists for all
> (all of the controllers were synthesized with the eDMA controller).
> 
> For the 5 controllers found in rk3588, this range exists for only one of them
> (only one of the controllers was synthesized with the eDMA controller).
> 
> 
> > >    interrupt-names:
> > > +    minItems: 5
> > >      items:
> > >        - const: sys
> > >        - const: pmc
> > >        - const: msg
> > >        - const: legacy
> > >        - const: err
> > > +      - const: dma0
> > > +      - const: dma1
> > > +      - const: dma2
> > > +      - const: dma3
> 
> While all the PCIe controllers on the rk3568 have the embedded DMA controller
> as part of the PCIe controller, they don't have separate IRQs for the eDMA.
> (They will need to use the combined "sys" irq, so the driver will need to read
> an additional register to see that it was an eDMA irq.)
> 
> For the rk3588, only one of the 5 PCIe controllers have the eDMA, and that
> controller also has dedicated IRQs for the eDMA.
> (It should also be able to use the combined "sys" irq, but that would be less
> efficient, and AFAICT, the driver currently only works with dedicated IRQs.)

We could go with Sebastian's suggestion to define a 1MB range for "atu", see:
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/pci/snps%2Cdw-pcie.yaml#L76-L85

Which would allow the driver to probe if the eDMA is there or not
(even if this is strictly bigger than the real ATU_CAP size, the size is still
within the PCIe core's register map).

That would solve the problem that some pcie controllers, with the exact same
compatible, has a "dma" range while others do not.
(All controllers would have a 1MB atu range, and none of them would have the
dma range specified.)

However, we would still have the problem that for the exact same compatible,
some controllers have eDMA irqs specified in interrupts, and some do not...

But perhaps having mandatory atu range (and no dma range) + optional dma irqs
is better than mandatory atu range + optional dma range + optional dma irqs?
(At least from a DT schema maintainability PoV.)


Kind regards,
Niklas
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Niklas Cassel]

On Wed, Oct 25, 2023 at 10:07:02PM +0200, Niklas Cassel wrote:
> Hello Conor,
> 
> On Tue, Oct 24, 2023 at 05:30:22PM +0100, Conor Dooley wrote:
> > On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > 
> > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > using:
> > > 
> > > allOf:
> > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > 
> > > and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> > > able to use these properties, while still making sure 'make CHECK_DTBS=y'
> > > pass, we need to add these properties to rockchip-dw-pcie.yaml.
> > > 
> > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > ---
> > >  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > index 229f8608c535..633f8e0e884f 100644
> > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > @@ -35,6 +35,7 @@ properties:
> > >        - description: Rockchip designed configuration registers
> > >        - description: Config registers
> > >        - description: iATU registers
> > > +      - description: eDMA registers
> > 
> > Same here, is this just for one of the two supported models, or for
> > both?
> 
> For the 3 controllers found in rk3568, this range exists for all
> (all of the controllers were synthesized with the eDMA controller).
> 
> For the 5 controllers found in rk3588, this range exists for only one of them
> (only one of the controllers was synthesized with the eDMA controller).
> 
> 
> > >    interrupt-names:
> > > +    minItems: 5
> > >      items:
> > >        - const: sys
> > >        - const: pmc
> > >        - const: msg
> > >        - const: legacy
> > >        - const: err
> > > +      - const: dma0
> > > +      - const: dma1
> > > +      - const: dma2
> > > +      - const: dma3
> 
> While all the PCIe controllers on the rk3568 have the embedded DMA controller
> as part of the PCIe controller, they don't have separate IRQs for the eDMA.
> (They will need to use the combined "sys" irq, so the driver will need to read
> an additional register to see that it was an eDMA irq.)
> 
> For the rk3588, only one of the 5 PCIe controllers have the eDMA, and that
> controller also has dedicated IRQs for the eDMA.
> (It should also be able to use the combined "sys" irq, but that would be less
> efficient, and AFAICT, the driver currently only works with dedicated IRQs.)

We could go with Sebastian's suggestion to define a 1MB range for "atu", see:
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/pci/snps%2Cdw-pcie.yaml#L76-L85

Which would allow the driver to probe if the eDMA is there or not
(even if this is strictly bigger than the real ATU_CAP size, the size is still
within the PCIe core's register map).

That would solve the problem that some pcie controllers, with the exact same
compatible, has a "dma" range while others do not.
(All controllers would have a 1MB atu range, and none of them would have the
dma range specified.)

However, we would still have the problem that for the exact same compatible,
some controllers have eDMA irqs specified in interrupts, and some do not...

But perhaps having mandatory atu range (and no dma range) + optional dma irqs
is better than mandatory atu range + optional dma range + optional dma irqs?
(At least from a DT schema maintainability PoV.)


Kind regards,
Niklas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Serge Semin]

Hi Niklas

On Wed, Oct 25, 2023 at 08:55:33PM +0000, Niklas Cassel wrote:
> On Wed, Oct 25, 2023 at 10:07:02PM +0200, Niklas Cassel wrote:
> > Hello Conor,
> > 
> > On Tue, Oct 24, 2023 at 05:30:22PM +0100, Conor Dooley wrote:
> > > On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> > > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > > 
> > > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > > using:
> > > > 
> > > > allOf:
> > > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > 
> > > > and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> > > > able to use these properties, while still making sure 'make CHECK_DTBS=y'
> > > > pass, we need to add these properties to rockchip-dw-pcie.yaml.
> > > > 
> > > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > > ---
> > > >  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
> > > >  1 file changed, 20 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > index 229f8608c535..633f8e0e884f 100644
> > > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > @@ -35,6 +35,7 @@ properties:
> > > >        - description: Rockchip designed configuration registers
> > > >        - description: Config registers
> > > >        - description: iATU registers

> > > > +      - description: eDMA registers

Are you sure the eDMA regs aren't just a part of the same space as
iATU and available within the default offset DEFAULT_DBI_DMA_OFFSET?
If it is then drop separate eDMA reg, having just 'atu' will be enough.

> > > 
> > > Same here, is this just for one of the two supported models, or for
> > > both?
> > 
> > For the 3 controllers found in rk3568, this range exists for all
> > (all of the controllers were synthesized with the eDMA controller).
> > 
> > For the 5 controllers found in rk3588, this range exists for only one of them
> > (only one of the controllers was synthesized with the eDMA controller).
> > 
> > 
> > > >    interrupt-names:
> > > > +    minItems: 5
> > > >      items:
> > > >        - const: sys
> > > >        - const: pmc
> > > >        - const: msg
> > > >        - const: legacy
> > > >        - const: err

> > > > +      - const: dma0
> > > > +      - const: dma1
> > > > +      - const: dma2
> > > > +      - const: dma3
> > 

> > While all the PCIe controllers on the rk3568 have the embedded DMA controller
> > as part of the PCIe controller, they don't have separate IRQs for the eDMA.
> > (They will need to use the combined "sys" irq, so the driver will need to read
> > an additional register to see that it was an eDMA irq.)

If it's some vendor-specific register, then the best solution would be
to create a custom IRQ domain with 'sys' IRQ being a parental IRQ and
with the "dma*" IRQs being the child IRQs. Then you can re-define the
dw_pcie_edma_ops.irq_vector method (and edma.nr_irqs field), which
would return the DMA IRQs from the new sub-domain.

If it's just all the IRQs combined in a single "sys" IRQ line with no
custom way to distinguish one from another, then you can re-define the
dw_pcie_edma_ops.irq_vector method with a method returning the 'sys'
IRQ vector (don't forget to set the edma.nr_irqs field). eDMA IRQ
handler will check whether the IRQ was originated by the eDMA
controller. 

> > 

> > For the rk3588, only one of the 5 PCIe controllers have the eDMA, and that
> > controller also has dedicated IRQs for the eDMA.
> > (It should also be able to use the combined "sys" irq, but that would be less
> > efficient, and AFAICT, the driver currently only works with dedicated IRQs.)

This is a standard way of the eDMA IRQs definition. So supposedly there won't
be problem with rk3588

> 
> We could go with Sebastian's suggestion to define a 1MB range for "atu", see:
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/pci/snps%2Cdw-pcie.yaml#L76-L85

Right, That's what I asked in my very first message.

> 
> Which would allow the driver to probe if the eDMA is there or not
> (even if this is strictly bigger than the real ATU_CAP size, the size is still
> within the PCIe core's register map).
> 
> That would solve the problem that some pcie controllers, with the exact same
> compatible, has a "dma" range while others do not.
> (All controllers would have a 1MB atu range, and none of them would have the
> dma range specified.)
> 

> However, we would still have the problem that for the exact same compatible,
> some controllers have eDMA irqs specified in interrupts, and some do not...

You can define the normal eDMA IRQs as optional for all Rockchip
controllers (or for rk3588 only). They will be auto-detected by the
DW PCIe core driver as expected if the standard eDMA IRQs are
available. But for the rk3568 you'll need to redefine the
dw_edma_chip.ops operation to have a custom irq_vector() callback as I
described above.

> 
> But perhaps having mandatory atu range (and no dma range) + optional dma irqs
> is better than mandatory atu range + optional dma range + optional dma irqs?
> (At least from a DT schema maintainability PoV.)

As I mentioned in the first message, if eDMA CSRs are available within
the standard offset PCIE_DMA_UNROLL_BASE with respect to the iATU CSRs
base, then that is a single CSRs space (selected by the DBI_CS2=1 and
CDM_SB=1 internal IP-core signals) and just 'atu' reg is supposed to
be specified.

-Serge(y)

> 
> 
> Kind regards,
> Niklas

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Serge Semin]

Hi Niklas

On Wed, Oct 25, 2023 at 08:55:33PM +0000, Niklas Cassel wrote:
> On Wed, Oct 25, 2023 at 10:07:02PM +0200, Niklas Cassel wrote:
> > Hello Conor,
> > 
> > On Tue, Oct 24, 2023 at 05:30:22PM +0100, Conor Dooley wrote:
> > > On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> > > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > > 
> > > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > > using:
> > > > 
> > > > allOf:
> > > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > 
> > > > and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> > > > able to use these properties, while still making sure 'make CHECK_DTBS=y'
> > > > pass, we need to add these properties to rockchip-dw-pcie.yaml.
> > > > 
> > > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > > ---
> > > >  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
> > > >  1 file changed, 20 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > index 229f8608c535..633f8e0e884f 100644
> > > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > @@ -35,6 +35,7 @@ properties:
> > > >        - description: Rockchip designed configuration registers
> > > >        - description: Config registers
> > > >        - description: iATU registers

> > > > +      - description: eDMA registers

Are you sure the eDMA regs aren't just a part of the same space as
iATU and available within the default offset DEFAULT_DBI_DMA_OFFSET?
If it is then drop separate eDMA reg, having just 'atu' will be enough.

> > > 
> > > Same here, is this just for one of the two supported models, or for
> > > both?
> > 
> > For the 3 controllers found in rk3568, this range exists for all
> > (all of the controllers were synthesized with the eDMA controller).
> > 
> > For the 5 controllers found in rk3588, this range exists for only one of them
> > (only one of the controllers was synthesized with the eDMA controller).
> > 
> > 
> > > >    interrupt-names:
> > > > +    minItems: 5
> > > >      items:
> > > >        - const: sys
> > > >        - const: pmc
> > > >        - const: msg
> > > >        - const: legacy
> > > >        - const: err

> > > > +      - const: dma0
> > > > +      - const: dma1
> > > > +      - const: dma2
> > > > +      - const: dma3
> > 

> > While all the PCIe controllers on the rk3568 have the embedded DMA controller
> > as part of the PCIe controller, they don't have separate IRQs for the eDMA.
> > (They will need to use the combined "sys" irq, so the driver will need to read
> > an additional register to see that it was an eDMA irq.)

If it's some vendor-specific register, then the best solution would be
to create a custom IRQ domain with 'sys' IRQ being a parental IRQ and
with the "dma*" IRQs being the child IRQs. Then you can re-define the
dw_pcie_edma_ops.irq_vector method (and edma.nr_irqs field), which
would return the DMA IRQs from the new sub-domain.

If it's just all the IRQs combined in a single "sys" IRQ line with no
custom way to distinguish one from another, then you can re-define the
dw_pcie_edma_ops.irq_vector method with a method returning the 'sys'
IRQ vector (don't forget to set the edma.nr_irqs field). eDMA IRQ
handler will check whether the IRQ was originated by the eDMA
controller. 

> > 

> > For the rk3588, only one of the 5 PCIe controllers have the eDMA, and that
> > controller also has dedicated IRQs for the eDMA.
> > (It should also be able to use the combined "sys" irq, but that would be less
> > efficient, and AFAICT, the driver currently only works with dedicated IRQs.)

This is a standard way of the eDMA IRQs definition. So supposedly there won't
be problem with rk3588

> 
> We could go with Sebastian's suggestion to define a 1MB range for "atu", see:
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/pci/snps%2Cdw-pcie.yaml#L76-L85

Right, That's what I asked in my very first message.

> 
> Which would allow the driver to probe if the eDMA is there or not
> (even if this is strictly bigger than the real ATU_CAP size, the size is still
> within the PCIe core's register map).
> 
> That would solve the problem that some pcie controllers, with the exact same
> compatible, has a "dma" range while others do not.
> (All controllers would have a 1MB atu range, and none of them would have the
> dma range specified.)
> 

> However, we would still have the problem that for the exact same compatible,
> some controllers have eDMA irqs specified in interrupts, and some do not...

You can define the normal eDMA IRQs as optional for all Rockchip
controllers (or for rk3588 only). They will be auto-detected by the
DW PCIe core driver as expected if the standard eDMA IRQs are
available. But for the rk3568 you'll need to redefine the
dw_edma_chip.ops operation to have a custom irq_vector() callback as I
described above.

> 
> But perhaps having mandatory atu range (and no dma range) + optional dma irqs
> is better than mandatory atu range + optional dma range + optional dma irqs?
> (At least from a DT schema maintainability PoV.)

As I mentioned in the first message, if eDMA CSRs are available within
the standard offset PCIE_DMA_UNROLL_BASE with respect to the iATU CSRs
base, then that is a single CSRs space (selected by the DBI_CS2=1 and
CDM_SB=1 internal IP-core signals) and just 'atu' reg is supposed to
be specified.

-Serge(y)

> 
> 
> Kind regards,
> Niklas

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Serge Semin]

Hi Niklas

On Wed, Oct 25, 2023 at 08:55:33PM +0000, Niklas Cassel wrote:
> On Wed, Oct 25, 2023 at 10:07:02PM +0200, Niklas Cassel wrote:
> > Hello Conor,
> > 
> > On Tue, Oct 24, 2023 at 05:30:22PM +0100, Conor Dooley wrote:
> > > On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> > > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > > 
> > > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > > using:
> > > > 
> > > > allOf:
> > > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > 
> > > > and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> > > > able to use these properties, while still making sure 'make CHECK_DTBS=y'
> > > > pass, we need to add these properties to rockchip-dw-pcie.yaml.
> > > > 
> > > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > > ---
> > > >  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
> > > >  1 file changed, 20 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > index 229f8608c535..633f8e0e884f 100644
> > > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > @@ -35,6 +35,7 @@ properties:
> > > >        - description: Rockchip designed configuration registers
> > > >        - description: Config registers
> > > >        - description: iATU registers

> > > > +      - description: eDMA registers

Are you sure the eDMA regs aren't just a part of the same space as
iATU and available within the default offset DEFAULT_DBI_DMA_OFFSET?
If it is then drop separate eDMA reg, having just 'atu' will be enough.

> > > 
> > > Same here, is this just for one of the two supported models, or for
> > > both?
> > 
> > For the 3 controllers found in rk3568, this range exists for all
> > (all of the controllers were synthesized with the eDMA controller).
> > 
> > For the 5 controllers found in rk3588, this range exists for only one of them
> > (only one of the controllers was synthesized with the eDMA controller).
> > 
> > 
> > > >    interrupt-names:
> > > > +    minItems: 5
> > > >      items:
> > > >        - const: sys
> > > >        - const: pmc
> > > >        - const: msg
> > > >        - const: legacy
> > > >        - const: err

> > > > +      - const: dma0
> > > > +      - const: dma1
> > > > +      - const: dma2
> > > > +      - const: dma3
> > 

> > While all the PCIe controllers on the rk3568 have the embedded DMA controller
> > as part of the PCIe controller, they don't have separate IRQs for the eDMA.
> > (They will need to use the combined "sys" irq, so the driver will need to read
> > an additional register to see that it was an eDMA irq.)

If it's some vendor-specific register, then the best solution would be
to create a custom IRQ domain with 'sys' IRQ being a parental IRQ and
with the "dma*" IRQs being the child IRQs. Then you can re-define the
dw_pcie_edma_ops.irq_vector method (and edma.nr_irqs field), which
would return the DMA IRQs from the new sub-domain.

If it's just all the IRQs combined in a single "sys" IRQ line with no
custom way to distinguish one from another, then you can re-define the
dw_pcie_edma_ops.irq_vector method with a method returning the 'sys'
IRQ vector (don't forget to set the edma.nr_irqs field). eDMA IRQ
handler will check whether the IRQ was originated by the eDMA
controller. 

> > 

> > For the rk3588, only one of the 5 PCIe controllers have the eDMA, and that
> > controller also has dedicated IRQs for the eDMA.
> > (It should also be able to use the combined "sys" irq, but that would be less
> > efficient, and AFAICT, the driver currently only works with dedicated IRQs.)

This is a standard way of the eDMA IRQs definition. So supposedly there won't
be problem with rk3588

> 
> We could go with Sebastian's suggestion to define a 1MB range for "atu", see:
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/pci/snps%2Cdw-pcie.yaml#L76-L85

Right, That's what I asked in my very first message.

> 
> Which would allow the driver to probe if the eDMA is there or not
> (even if this is strictly bigger than the real ATU_CAP size, the size is still
> within the PCIe core's register map).
> 
> That would solve the problem that some pcie controllers, with the exact same
> compatible, has a "dma" range while others do not.
> (All controllers would have a 1MB atu range, and none of them would have the
> dma range specified.)
> 

> However, we would still have the problem that for the exact same compatible,
> some controllers have eDMA irqs specified in interrupts, and some do not...

You can define the normal eDMA IRQs as optional for all Rockchip
controllers (or for rk3588 only). They will be auto-detected by the
DW PCIe core driver as expected if the standard eDMA IRQs are
available. But for the rk3568 you'll need to redefine the
dw_edma_chip.ops operation to have a custom irq_vector() callback as I
described above.

> 
> But perhaps having mandatory atu range (and no dma range) + optional dma irqs
> is better than mandatory atu range + optional dma range + optional dma irqs?
> (At least from a DT schema maintainability PoV.)

As I mentioned in the first message, if eDMA CSRs are available within
the standard offset PCIE_DMA_UNROLL_BASE with respect to the iATU CSRs
base, then that is a single CSRs space (selected by the DBI_CS2=1 and
CDM_SB=1 internal IP-core signals) and just 'atu' reg is supposed to
be specified.

-Serge(y)

> 
> 
> Kind regards,
> Niklas

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Serge Semin]

On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
> 
> allOf:
>   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> 
> and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> able to use these properties, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add these properties to rockchip-dw-pcie.yaml.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 229f8608c535..633f8e0e884f 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -35,6 +35,7 @@ properties:
>        - description: Rockchip designed configuration registers
>        - description: Config registers
>        - description: iATU registers
> +      - description: eDMA registers
>  
>    reg-names:
>      minItems: 3
> @@ -43,6 +44,7 @@ properties:
>        - const: apb
>        - const: config
>        - const: atu
> +      - const: dma
>  
>    clocks:
>      minItems: 5
> @@ -65,6 +67,7 @@ properties:
>        - const: pipe
>  
>    interrupts:
> +    minItems: 5
>      items:
>        - description:
>            Combined system interrupt, which is used to signal the following
> @@ -88,14 +91,31 @@ properties:
>            interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
>            tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
>            nf_err_rx, f_err_rx, radm_qoverflow

> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.

They aren't identical. Some IRQs indicate events on the write eDMA
channels, some - read eDMA channels. The respective channel ID would
be also useful to have in the description.

-Serge(y)

>  
>    interrupt-names:
> +    minItems: 5
>      items:
>        - const: sys
>        - const: pmc
>        - const: msg
>        - const: legacy
>        - const: err
> +      - const: dma0
> +      - const: dma1
> +      - const: dma2
> +      - const: dma3
>  
>    legacy-interrupt-controller:
>      description: Interrupt controller node for handling legacy PCI interrupts.
> -- 
> 2.41.0
> 

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Serge Semin]

On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
> 
> allOf:
>   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> 
> and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> able to use these properties, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add these properties to rockchip-dw-pcie.yaml.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 229f8608c535..633f8e0e884f 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -35,6 +35,7 @@ properties:
>        - description: Rockchip designed configuration registers
>        - description: Config registers
>        - description: iATU registers
> +      - description: eDMA registers
>  
>    reg-names:
>      minItems: 3
> @@ -43,6 +44,7 @@ properties:
>        - const: apb
>        - const: config
>        - const: atu
> +      - const: dma
>  
>    clocks:
>      minItems: 5
> @@ -65,6 +67,7 @@ properties:
>        - const: pipe
>  
>    interrupts:
> +    minItems: 5
>      items:
>        - description:
>            Combined system interrupt, which is used to signal the following
> @@ -88,14 +91,31 @@ properties:
>            interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
>            tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
>            nf_err_rx, f_err_rx, radm_qoverflow

> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.

They aren't identical. Some IRQs indicate events on the write eDMA
channels, some - read eDMA channels. The respective channel ID would
be also useful to have in the description.

-Serge(y)

>  
>    interrupt-names:
> +    minItems: 5
>      items:
>        - const: sys
>        - const: pmc
>        - const: msg
>        - const: legacy
>        - const: err
> +      - const: dma0
> +      - const: dma1
> +      - const: dma2
> +      - const: dma3
>  
>    legacy-interrupt-controller:
>      description: Interrupt controller node for handling legacy PCI interrupts.
> -- 
> 2.41.0
> 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Serge Semin]

On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
> 
> allOf:
>   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> 
> and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> able to use these properties, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add these properties to rockchip-dw-pcie.yaml.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 229f8608c535..633f8e0e884f 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -35,6 +35,7 @@ properties:
>        - description: Rockchip designed configuration registers
>        - description: Config registers
>        - description: iATU registers
> +      - description: eDMA registers
>  
>    reg-names:
>      minItems: 3
> @@ -43,6 +44,7 @@ properties:
>        - const: apb
>        - const: config
>        - const: atu
> +      - const: dma
>  
>    clocks:
>      minItems: 5
> @@ -65,6 +67,7 @@ properties:
>        - const: pipe
>  
>    interrupts:
> +    minItems: 5
>      items:
>        - description:
>            Combined system interrupt, which is used to signal the following
> @@ -88,14 +91,31 @@ properties:
>            interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
>            tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
>            nf_err_rx, f_err_rx, radm_qoverflow

> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.
> +      - description:
> +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> +          error has occurred on the corresponding channel.

They aren't identical. Some IRQs indicate events on the write eDMA
channels, some - read eDMA channels. The respective channel ID would
be also useful to have in the description.

-Serge(y)

>  
>    interrupt-names:
> +    minItems: 5
>      items:
>        - const: sys
>        - const: pmc
>        - const: msg
>        - const: legacy
>        - const: err
> +      - const: dma0
> +      - const: dma1
> +      - const: dma2
> +      - const: dma3
>  
>    legacy-interrupt-controller:
>      description: Interrupt controller node for handling legacy PCI interrupts.
> -- 
> 2.41.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Rob Herring]

On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>

The subject says 'atu property', but 'atu' is not a property.

> 
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
> 
> allOf:
>   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> 
> and snps,dw-pcie.yaml does have the atu property defined, in order to be
> able to use this property, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add this property to rockchip-dw-pcie.yaml.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 1ae8dcfa072c..229f8608c535 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -29,16 +29,20 @@ properties:
>            - const: rockchip,rk3568-pcie
>  
>    reg:
> +    minItems: 3
>      items:
>        - description: Data Bus Interface (DBI) registers
>        - description: Rockchip designed configuration registers
>        - description: Config registers
> +      - description: iATU registers
>  
>    reg-names:
> +    minItems: 3
>      items:
>        - const: dbi
>        - const: apb
>        - const: config
> +      - const: atu
>  
>    clocks:
>      minItems: 5
> -- 
> 2.41.0
> 

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Rob Herring]

On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>

The subject says 'atu property', but 'atu' is not a property.

> 
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
> 
> allOf:
>   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> 
> and snps,dw-pcie.yaml does have the atu property defined, in order to be
> able to use this property, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add this property to rockchip-dw-pcie.yaml.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 1ae8dcfa072c..229f8608c535 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -29,16 +29,20 @@ properties:
>            - const: rockchip,rk3568-pcie
>  
>    reg:
> +    minItems: 3
>      items:
>        - description: Data Bus Interface (DBI) registers
>        - description: Rockchip designed configuration registers
>        - description: Config registers
> +      - description: iATU registers
>  
>    reg-names:
> +    minItems: 3
>      items:
>        - const: dbi
>        - const: apb
>        - const: config
> +      - const: atu
>  
>    clocks:
>      minItems: 5
> -- 
> 2.41.0
> 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Rob Herring]

On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>

The subject says 'atu property', but 'atu' is not a property.

> 
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
> 
> allOf:
>   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> 
> and snps,dw-pcie.yaml does have the atu property defined, in order to be
> able to use this property, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add this property to rockchip-dw-pcie.yaml.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 1ae8dcfa072c..229f8608c535 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -29,16 +29,20 @@ properties:
>            - const: rockchip,rk3568-pcie
>  
>    reg:
> +    minItems: 3
>      items:
>        - description: Data Bus Interface (DBI) registers
>        - description: Rockchip designed configuration registers
>        - description: Config registers
> +      - description: iATU registers
>  
>    reg-names:
> +    minItems: 3
>      items:
>        - const: dbi
>        - const: apb
>        - const: config
> +      - const: atu
>  
>    clocks:
>      minItems: 5
> -- 
> 2.41.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Rob Herring]

On Wed, Oct 25, 2023 at 08:02:32PM +0000, Niklas Cassel wrote:
> Hello Conor,
> 
> On Tue, Oct 24, 2023 at 05:29:28PM +0100, Conor Dooley wrote:
> > On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > 
> > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > using:
> > > 
> > > allOf:
> > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > 
> > > and snps,dw-pcie.yaml does have the atu property defined, in order to be
> > > able to use this property, while still making sure 'make CHECK_DTBS=y'
> > > pass, we need to add this property to rockchip-dw-pcie.yaml.
> > > 
> > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > ---
> > >  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > index 1ae8dcfa072c..229f8608c535 100644
> > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > @@ -29,16 +29,20 @@ properties:
> > >            - const: rockchip,rk3568-pcie
> > >  
> > >    reg:
> > > +    minItems: 3
> > >      items:
> > >        - description: Data Bus Interface (DBI) registers
> > >        - description: Rockchip designed configuration registers
> > >        - description: Config registers
> > > +      - description: iATU registers
> > 
> > Is this extra register only for the ..88 or for the ..68 and for the
> > ..88 models?
> 
> Looking at the rk3568 Technical Reference Manual (TRM):
> https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf
> 
> The iATU register register range exists for all 3 PCIe controllers
> found on the rk3568.
> 
> This register range is currently not defined in the rk3568.dtsi, so the driver
> will currently use the default register offset (which is correct), but with
> the driver fallback register size that is only big enough to cover 8 inbound
> and 8 outbound iATUs (internal Address Translation Units).

We should probably make the driver smarter instead or in addition. We 
have the DBI size, Just make atu_size = dbi_size - DEFAULT_DBI_ATU_OFFSET.

> According to the TRM, all three PCIe controllers found on the rk3568 have
> 16 inbound iATUs and 16 outbound iATUs, so if someone wants to be able to
> make use of all the iATUs on the rk3568, they will need to add "atu" to
> rk3568.dtsi.

At least for host side, the number of regions used is based on ranges. 
You'd be hard pressed to need more than 8. That or no h/w with 16 is 
probably why I said 8 was enough at the time.

Rob

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Rob Herring]

On Wed, Oct 25, 2023 at 08:02:32PM +0000, Niklas Cassel wrote:
> Hello Conor,
> 
> On Tue, Oct 24, 2023 at 05:29:28PM +0100, Conor Dooley wrote:
> > On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > 
> > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > using:
> > > 
> > > allOf:
> > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > 
> > > and snps,dw-pcie.yaml does have the atu property defined, in order to be
> > > able to use this property, while still making sure 'make CHECK_DTBS=y'
> > > pass, we need to add this property to rockchip-dw-pcie.yaml.
> > > 
> > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > ---
> > >  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > index 1ae8dcfa072c..229f8608c535 100644
> > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > @@ -29,16 +29,20 @@ properties:
> > >            - const: rockchip,rk3568-pcie
> > >  
> > >    reg:
> > > +    minItems: 3
> > >      items:
> > >        - description: Data Bus Interface (DBI) registers
> > >        - description: Rockchip designed configuration registers
> > >        - description: Config registers
> > > +      - description: iATU registers
> > 
> > Is this extra register only for the ..88 or for the ..68 and for the
> > ..88 models?
> 
> Looking at the rk3568 Technical Reference Manual (TRM):
> https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf
> 
> The iATU register register range exists for all 3 PCIe controllers
> found on the rk3568.
> 
> This register range is currently not defined in the rk3568.dtsi, so the driver
> will currently use the default register offset (which is correct), but with
> the driver fallback register size that is only big enough to cover 8 inbound
> and 8 outbound iATUs (internal Address Translation Units).

We should probably make the driver smarter instead or in addition. We 
have the DBI size, Just make atu_size = dbi_size - DEFAULT_DBI_ATU_OFFSET.

> According to the TRM, all three PCIe controllers found on the rk3568 have
> 16 inbound iATUs and 16 outbound iATUs, so if someone wants to be able to
> make use of all the iATUs on the rk3568, they will need to add "atu" to
> rk3568.dtsi.

At least for host side, the number of regions used is based on ranges. 
You'd be hard pressed to need more than 8. That or no h/w with 16 is 
probably why I said 8 was enough at the time.

Rob

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Rob Herring]

On Wed, Oct 25, 2023 at 08:02:32PM +0000, Niklas Cassel wrote:
> Hello Conor,
> 
> On Tue, Oct 24, 2023 at 05:29:28PM +0100, Conor Dooley wrote:
> > On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > 
> > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > using:
> > > 
> > > allOf:
> > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > 
> > > and snps,dw-pcie.yaml does have the atu property defined, in order to be
> > > able to use this property, while still making sure 'make CHECK_DTBS=y'
> > > pass, we need to add this property to rockchip-dw-pcie.yaml.
> > > 
> > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > ---
> > >  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > index 1ae8dcfa072c..229f8608c535 100644
> > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > @@ -29,16 +29,20 @@ properties:
> > >            - const: rockchip,rk3568-pcie
> > >  
> > >    reg:
> > > +    minItems: 3
> > >      items:
> > >        - description: Data Bus Interface (DBI) registers
> > >        - description: Rockchip designed configuration registers
> > >        - description: Config registers
> > > +      - description: iATU registers
> > 
> > Is this extra register only for the ..88 or for the ..68 and for the
> > ..88 models?
> 
> Looking at the rk3568 Technical Reference Manual (TRM):
> https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf
> 
> The iATU register register range exists for all 3 PCIe controllers
> found on the rk3568.
> 
> This register range is currently not defined in the rk3568.dtsi, so the driver
> will currently use the default register offset (which is correct), but with
> the driver fallback register size that is only big enough to cover 8 inbound
> and 8 outbound iATUs (internal Address Translation Units).

We should probably make the driver smarter instead or in addition. We 
have the DBI size, Just make atu_size = dbi_size - DEFAULT_DBI_ATU_OFFSET.

> According to the TRM, all three PCIe controllers found on the rk3568 have
> 16 inbound iATUs and 16 outbound iATUs, so if someone wants to be able to
> make use of all the iATUs on the rk3568, they will need to add "atu" to
> rk3568.dtsi.

At least for host side, the number of regions used is based on ranges. 
You'd be hard pressed to need more than 8. That or no h/w with 16 is 
probably why I said 8 was enough at the time.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Niklas Cassel]

Hello Rob,

On Thu, Oct 26, 2023 at 01:35:01PM -0500, Rob Herring wrote:
> On Wed, Oct 25, 2023 at 08:02:32PM +0000, Niklas Cassel wrote:
> > Hello Conor,
> > 
> > On Tue, Oct 24, 2023 at 05:29:28PM +0100, Conor Dooley wrote:
> > > On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> > > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > > 
> > > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > > using:
> > > > 
> > > > allOf:
> > > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > 
> > > > and snps,dw-pcie.yaml does have the atu property defined, in order to be
> > > > able to use this property, while still making sure 'make CHECK_DTBS=y'
> > > > pass, we need to add this property to rockchip-dw-pcie.yaml.
> > > > 
> > > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > index 1ae8dcfa072c..229f8608c535 100644
> > > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > @@ -29,16 +29,20 @@ properties:
> > > >            - const: rockchip,rk3568-pcie
> > > >  
> > > >    reg:
> > > > +    minItems: 3
> > > >      items:
> > > >        - description: Data Bus Interface (DBI) registers
> > > >        - description: Rockchip designed configuration registers
> > > >        - description: Config registers
> > > > +      - description: iATU registers
> > > 
> > > Is this extra register only for the ..88 or for the ..68 and for the
> > > ..88 models?
> > 
> > Looking at the rk3568 Technical Reference Manual (TRM):
> > https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf
> > 
> > The iATU register register range exists for all 3 PCIe controllers
> > found on the rk3568.
> > 
> > This register range is currently not defined in the rk3568.dtsi, so the driver
> > will currently use the default register offset (which is correct), but with
> > the driver fallback register size that is only big enough to cover 8 inbound
> > and 8 outbound iATUs (internal Address Translation Units).
> 
> We should probably make the driver smarter instead or in addition. We 
> have the DBI size, Just make atu_size = dbi_size - DEFAULT_DBI_ATU_OFFSET.

I though about that, but it seems that some drivers don't use
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi")

but instead set pci->dbi_base from non-common code, e.g.:
drivers/pci/controller/dwc/pci-dra7xx.c:        pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "ep_dbics");
drivers/pci/controller/dwc/pci-dra7xx.c:        pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "rc_dbics");
drivers/pci/controller/dwc/pci-imx6.c:  pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, 0, &dbi_base);
drivers/pci/controller/dwc/pci-keystone.c:      pci->dbi_base = base;
drivers/pci/controller/dwc/pci-layerscape-ep.c: dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
drivers/pci/controller/dwc/pci-layerscape-ep.c: pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
drivers/pci/controller/dwc/pci-layerscape.c:    dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
drivers/pci/controller/dwc/pci-layerscape.c:    pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
drivers/pci/controller/dwc/pci-meson.c: pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "elbi");
drivers/pci/controller/dwc/pcie-al.c:   void __iomem *dbi_base = pcie->dbi_base;
drivers/pci/controller/dwc/pcie-al.c:   al_pcie->dbi_base = devm_pci_remap_cfg_resource(dev, res);
drivers/pci/controller/dwc/pcie-armada8k.c:     pci->dbi_base = devm_pci_remap_cfg_resource(dev, base);
drivers/pci/controller/dwc/pcie-designware.c:           pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res);
drivers/pci/controller/dwc/pcie-histb.c:        pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "rc-dbi");
drivers/pci/controller/dwc/pcie-qcom-ep.c:      pci->dbi_base = devm_pci_remap_cfg_resource(dev, res);
drivers/pci/controller/dwc/pcie-tegra194-acpi.c:        pcie_ecam->dbi_base = cfg->win + SZ_512K;

So I don't think that we can always get the size of the dbi.
And a solution that does not work for all platforms is not
that appealing.


Kind regards,
Niklas

Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Niklas Cassel]

On Thu, Oct 26, 2023 at 05:32:44PM +0300, Serge Semin wrote:
> On Tue, Oct 24, 2023 at 05:10:10PM +0200, Niklas Cassel wrote:
> > From: Niklas Cassel <niklas.cassel@wdc.com>
> > 
> > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > using:
> > 
> > allOf:
> >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > 
> > and snps,dw-pcie.yaml does have the dma properties defined, in order to be
> > able to use these properties, while still making sure 'make CHECK_DTBS=y'
> > pass, we need to add these properties to rockchip-dw-pcie.yaml.
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > ---
> >  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > index 229f8608c535..633f8e0e884f 100644
> > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > @@ -35,6 +35,7 @@ properties:
> >        - description: Rockchip designed configuration registers
> >        - description: Config registers
> >        - description: iATU registers
> > +      - description: eDMA registers
> >  
> >    reg-names:
> >      minItems: 3
> > @@ -43,6 +44,7 @@ properties:
> >        - const: apb
> >        - const: config
> >        - const: atu
> > +      - const: dma
> >  
> >    clocks:
> >      minItems: 5
> > @@ -65,6 +67,7 @@ properties:
> >        - const: pipe
> >  
> >    interrupts:
> > +    minItems: 5
> >      items:
> >        - description:
> >            Combined system interrupt, which is used to signal the following
> > @@ -88,14 +91,31 @@ properties:
> >            interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
> >            tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
> >            nf_err_rx, f_err_rx, radm_qoverflow
> 
> > +      - description:
> > +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> > +          error has occurred on the corresponding channel.
> > +      - description:
> > +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> > +          error has occurred on the corresponding channel.
> > +      - description:
> > +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> > +          error has occurred on the corresponding channel.
> > +      - description:
> > +          Indicates that the eDMA Tx/Rx transfer is complete or that an
> > +          error has occurred on the corresponding channel.
> 
> They aren't identical. Some IRQs indicate events on the write eDMA
> channels, some - read eDMA channels. The respective channel ID would
> be also useful to have in the description.

Hello Serge,

As you know, the IRQs for the write channels have to be specified
before the IRQs for the read channels in "interrupts".

So for rk3588, it will be:
dma0: wr0
dma1: wr1
dma2: rd0
dma3: rd1

However, e.g. rk3568 only has one channel for reads and one for writes.
(Now this SoC doesn't have dedicated IRQs for the eDMA, but let's pretend
that it did.)

So for rk3568, it would then instead be:
dma0: wr0
dma1: rd0
dma2: <unused>
dma3: <unused>

So I would rather not add read/write or channel number to the descriptions,
as both the channel number and read/write will depend on how the eDMA in the
specific controller was synthesized. So I'd rather have no description rather
than a description that will be wrong for all rockchip SoCs other than rk3588.


Kind regards,
Niklas

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Rob Herring]

On Fri, Oct 27, 2023 at 9:35 AM Niklas Cassel <Niklas.Cassel@wdc.com> wrote:
>
> Hello Rob,
>
> On Thu, Oct 26, 2023 at 01:35:01PM -0500, Rob Herring wrote:
> > On Wed, Oct 25, 2023 at 08:02:32PM +0000, Niklas Cassel wrote:
> > > Hello Conor,
> > >
> > > On Tue, Oct 24, 2023 at 05:29:28PM +0100, Conor Dooley wrote:
> > > > On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> > > > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > > >
> > > > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > > > using:
> > > > >
> > > > > allOf:
> > > > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > >
> > > > > and snps,dw-pcie.yaml does have the atu property defined, in order to be
> > > > > able to use this property, while still making sure 'make CHECK_DTBS=y'
> > > > > pass, we need to add this property to rockchip-dw-pcie.yaml.
> > > > >
> > > > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > > index 1ae8dcfa072c..229f8608c535 100644
> > > > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > > @@ -29,16 +29,20 @@ properties:
> > > > >            - const: rockchip,rk3568-pcie
> > > > >
> > > > >    reg:
> > > > > +    minItems: 3
> > > > >      items:
> > > > >        - description: Data Bus Interface (DBI) registers
> > > > >        - description: Rockchip designed configuration registers
> > > > >        - description: Config registers
> > > > > +      - description: iATU registers
> > > >
> > > > Is this extra register only for the ..88 or for the ..68 and for the
> > > > ..88 models?
> > >
> > > Looking at the rk3568 Technical Reference Manual (TRM):
> > > https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf
> > >
> > > The iATU register register range exists for all 3 PCIe controllers
> > > found on the rk3568.
> > >
> > > This register range is currently not defined in the rk3568.dtsi, so the driver
> > > will currently use the default register offset (which is correct), but with
> > > the driver fallback register size that is only big enough to cover 8 inbound
> > > and 8 outbound iATUs (internal Address Translation Units).
> >
> > We should probably make the driver smarter instead or in addition. We
> > have the DBI size, Just make atu_size = dbi_size - DEFAULT_DBI_ATU_OFFSET.
>
> I though about that, but it seems that some drivers don't use
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi")
>
> but instead set pci->dbi_base from non-common code, e.g.:
> drivers/pci/controller/dwc/pci-dra7xx.c:        pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "ep_dbics");
> drivers/pci/controller/dwc/pci-dra7xx.c:        pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "rc_dbics");
> drivers/pci/controller/dwc/pci-imx6.c:  pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, 0, &dbi_base);
> drivers/pci/controller/dwc/pci-keystone.c:      pci->dbi_base = base;
> drivers/pci/controller/dwc/pci-layerscape-ep.c: dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> drivers/pci/controller/dwc/pci-layerscape-ep.c: pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> drivers/pci/controller/dwc/pci-layerscape.c:    dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> drivers/pci/controller/dwc/pci-layerscape.c:    pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> drivers/pci/controller/dwc/pci-meson.c: pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "elbi");
> drivers/pci/controller/dwc/pcie-al.c:   void __iomem *dbi_base = pcie->dbi_base;
> drivers/pci/controller/dwc/pcie-al.c:   al_pcie->dbi_base = devm_pci_remap_cfg_resource(dev, res);
> drivers/pci/controller/dwc/pcie-armada8k.c:     pci->dbi_base = devm_pci_remap_cfg_resource(dev, base);
> drivers/pci/controller/dwc/pcie-designware.c:           pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res);
> drivers/pci/controller/dwc/pcie-histb.c:        pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "rc-dbi");
> drivers/pci/controller/dwc/pcie-qcom-ep.c:      pci->dbi_base = devm_pci_remap_cfg_resource(dev, res);
> drivers/pci/controller/dwc/pcie-tegra194-acpi.c:        pcie_ecam->dbi_base = cfg->win + SZ_512K;
>
> So I don't think that we can always get the size of the dbi.
> And a solution that does not work for all platforms is not
> that appealing.

Do I get a chance to respond before you send a new version?

Does something like the patch below not work for everyone? We could
store the DBI size as well if we want more than 8 regions to work
without a 'dbi' nor 'atu' region defined. We shouldn't have new users
doing that though.

diff --git a/drivers/pci/controller/dwc/pcie-designware.c
b/drivers/pci/controller/dwc/pcie-designware.c
index 250cf7f40b85..3dc71ea7fa76 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -105,11 +105,13 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
        struct platform_device *pdev = to_platform_device(pci->dev);
        struct device_node *np = dev_of_node(pci->dev);
        struct resource *res;
+       size_t dbi_size = DEFAULT_DBI_ATU_OFFSET + SZ_4K;
        int ret;

        if (!pci->dbi_base) {
                res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
                pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res);
+               dbi_size = resource_size(res);
                if (IS_ERR(pci->dbi_base))
                        return PTR_ERR(pci->dbi_base);
        }
@@ -136,13 +138,10 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
                                return PTR_ERR(pci->atu_base);
                } else {
                        pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET;
+                       pci->atu_size = dbi_size - DEFAULT_DBI_ATU_OFFSET;
                }
        }

-       /* Set a default value suitable for at most 8 in and 8 out windows */
-       if (!pci->atu_size)
-               pci->atu_size = SZ_4K;
-
        /* eDMA region can be mapped to a custom base address */
        if (!pci->edma.reg_base) {
                res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dma");

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Niklas Cassel]

On Fri, Oct 27, 2023 at 10:56:50AM -0500, Rob Herring wrote:
> On Fri, Oct 27, 2023 at 9:35 AM Niklas Cassel <Niklas.Cassel@wdc.com> wrote:
> >
> > Hello Rob,
> >
> > On Thu, Oct 26, 2023 at 01:35:01PM -0500, Rob Herring wrote:
> > > On Wed, Oct 25, 2023 at 08:02:32PM +0000, Niklas Cassel wrote:
> > > > Hello Conor,
> > > >
> > > > On Tue, Oct 24, 2023 at 05:29:28PM +0100, Conor Dooley wrote:
> > > > > On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> > > > > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > > > >
> > > > > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > > > > using:
> > > > > >
> > > > > > allOf:
> > > > > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > > >
> > > > > > and snps,dw-pcie.yaml does have the atu property defined, in order to be
> > > > > > able to use this property, while still making sure 'make CHECK_DTBS=y'
> > > > > > pass, we need to add this property to rockchip-dw-pcie.yaml.
> > > > > >
> > > > > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
> > > > > >  1 file changed, 4 insertions(+)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > > > index 1ae8dcfa072c..229f8608c535 100644
> > > > > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > > > @@ -29,16 +29,20 @@ properties:
> > > > > >            - const: rockchip,rk3568-pcie
> > > > > >
> > > > > >    reg:
> > > > > > +    minItems: 3
> > > > > >      items:
> > > > > >        - description: Data Bus Interface (DBI) registers
> > > > > >        - description: Rockchip designed configuration registers
> > > > > >        - description: Config registers
> > > > > > +      - description: iATU registers
> > > > >
> > > > > Is this extra register only for the ..88 or for the ..68 and for the
> > > > > ..88 models?
> > > >
> > > > Looking at the rk3568 Technical Reference Manual (TRM):
> > > > https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf
> > > >
> > > > The iATU register register range exists for all 3 PCIe controllers
> > > > found on the rk3568.
> > > >
> > > > This register range is currently not defined in the rk3568.dtsi, so the driver
> > > > will currently use the default register offset (which is correct), but with
> > > > the driver fallback register size that is only big enough to cover 8 inbound
> > > > and 8 outbound iATUs (internal Address Translation Units).
> > >
> > > We should probably make the driver smarter instead or in addition. We
> > > have the DBI size, Just make atu_size = dbi_size - DEFAULT_DBI_ATU_OFFSET.
> >
> > I though about that, but it seems that some drivers don't use
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi")
> >
> > but instead set pci->dbi_base from non-common code, e.g.:
> > drivers/pci/controller/dwc/pci-dra7xx.c:        pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "ep_dbics");
> > drivers/pci/controller/dwc/pci-dra7xx.c:        pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "rc_dbics");
> > drivers/pci/controller/dwc/pci-imx6.c:  pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, 0, &dbi_base);
> > drivers/pci/controller/dwc/pci-keystone.c:      pci->dbi_base = base;
> > drivers/pci/controller/dwc/pci-layerscape-ep.c: dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> > drivers/pci/controller/dwc/pci-layerscape-ep.c: pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> > drivers/pci/controller/dwc/pci-layerscape.c:    dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> > drivers/pci/controller/dwc/pci-layerscape.c:    pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> > drivers/pci/controller/dwc/pci-meson.c: pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "elbi");
> > drivers/pci/controller/dwc/pcie-al.c:   void __iomem *dbi_base = pcie->dbi_base;
> > drivers/pci/controller/dwc/pcie-al.c:   al_pcie->dbi_base = devm_pci_remap_cfg_resource(dev, res);
> > drivers/pci/controller/dwc/pcie-armada8k.c:     pci->dbi_base = devm_pci_remap_cfg_resource(dev, base);
> > drivers/pci/controller/dwc/pcie-designware.c:           pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res);
> > drivers/pci/controller/dwc/pcie-histb.c:        pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "rc-dbi");
> > drivers/pci/controller/dwc/pcie-qcom-ep.c:      pci->dbi_base = devm_pci_remap_cfg_resource(dev, res);
> > drivers/pci/controller/dwc/pcie-tegra194-acpi.c:        pcie_ecam->dbi_base = cfg->win + SZ_512K;
> >
> > So I don't think that we can always get the size of the dbi.
> > And a solution that does not work for all platforms is not
> > that appealing.
> 
> Do I get a chance to respond before you send a new version?
> 
> Does something like the patch below not work for everyone? We could
> store the DBI size as well if we want more than 8 regions to work
> without a 'dbi' nor 'atu' region defined. We shouldn't have new users
> doing that though.

For the drivers listed above, they assign pci->dbi_base in non-common code.

> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> b/drivers/pci/controller/dwc/pcie-designware.c
> index 250cf7f40b85..3dc71ea7fa76 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -105,11 +105,13 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
>         struct platform_device *pdev = to_platform_device(pci->dev);
>         struct device_node *np = dev_of_node(pci->dev);
>         struct resource *res;
> +       size_t dbi_size = DEFAULT_DBI_ATU_OFFSET + SZ_4K;
>         int ret;
>

So those drivers will not enter this if-statement:

>         if (!pci->dbi_base) {
>                 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
>                 pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res);
> +               dbi_size = resource_size(res);
>                 if (IS_ERR(pci->dbi_base))
>                         return PTR_ERR(pci->dbi_base);
>         }

So for those platforms default size will still be 4k.


But sure, this will avoid the need for some platforms to specify "atu" reg,
so I'm all for it. Could you please send it as a real patch?

If you do, could you please also update the description of the dt binding:
https://github.com/torvalds/linux/blob/v6.6-rc7/Documentation/devicetree/bindings/pci/snps%2Cdw-pcie.yaml#L41-L43

That says that iATU memory region is required if the space is unrolled.


Kind regards,
Niklas

Re: [PATCH v2 1/4] dt-bindings: PCI: dwc: rockchip: Add atu property

[Niklas Cassel]

On Fri, Oct 27, 2023 at 10:56:50AM -0500, Rob Herring wrote:
> On Fri, Oct 27, 2023 at 9:35 AM Niklas Cassel <Niklas.Cassel@wdc.com> wrote:
> >
> > Hello Rob,
> >
> > On Thu, Oct 26, 2023 at 01:35:01PM -0500, Rob Herring wrote:
> > > On Wed, Oct 25, 2023 at 08:02:32PM +0000, Niklas Cassel wrote:
> > > > Hello Conor,
> > > >
> > > > On Tue, Oct 24, 2023 at 05:29:28PM +0100, Conor Dooley wrote:
> > > > > On Tue, Oct 24, 2023 at 05:10:08PM +0200, Niklas Cassel wrote:
> > > > > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > > > > >
> > > > > > Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> > > > > > using:
> > > > > >
> > > > > > allOf:
> > > > > >   - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > > >
> > > > > > and snps,dw-pcie.yaml does have the atu property defined, in order to be
> > > > > > able to use this property, while still making sure 'make CHECK_DTBS=y'
> > > > > > pass, we need to add this property to rockchip-dw-pcie.yaml.
> > > > > >
> > > > > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml | 4 ++++
> > > > > >  1 file changed, 4 insertions(+)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > > > index 1ae8dcfa072c..229f8608c535 100644
> > > > > > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > > > > > @@ -29,16 +29,20 @@ properties:
> > > > > >            - const: rockchip,rk3568-pcie
> > > > > >
> > > > > >    reg:
> > > > > > +    minItems: 3
> > > > > >      items:
> > > > > >        - description: Data Bus Interface (DBI) registers
> > > > > >        - description: Rockchip designed configuration registers
> > > > > >        - description: Config registers
> > > > > > +      - description: iATU registers
> > > > >
> > > > > Is this extra register only for the ..88 or for the ..68 and for the
> > > > > ..88 models?
> > > >
> > > > Looking at the rk3568 Technical Reference Manual (TRM):
> > > > https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf
> > > >
> > > > The iATU register register range exists for all 3 PCIe controllers
> > > > found on the rk3568.
> > > >
> > > > This register range is currently not defined in the rk3568.dtsi, so the driver
> > > > will currently use the default register offset (which is correct), but with
> > > > the driver fallback register size that is only big enough to cover 8 inbound
> > > > and 8 outbound iATUs (internal Address Translation Units).
> > >
> > > We should probably make the driver smarter instead or in addition. We
> > > have the DBI size, Just make atu_size = dbi_size - DEFAULT_DBI_ATU_OFFSET.
> >
> > I though about that, but it seems that some drivers don't use
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi")
> >
> > but instead set pci->dbi_base from non-common code, e.g.:
> > drivers/pci/controller/dwc/pci-dra7xx.c:        pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "ep_dbics");
> > drivers/pci/controller/dwc/pci-dra7xx.c:        pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "rc_dbics");
> > drivers/pci/controller/dwc/pci-imx6.c:  pci->dbi_base = devm_platform_get_and_ioremap_resource(pdev, 0, &dbi_base);
> > drivers/pci/controller/dwc/pci-keystone.c:      pci->dbi_base = base;
> > drivers/pci/controller/dwc/pci-layerscape-ep.c: dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> > drivers/pci/controller/dwc/pci-layerscape-ep.c: pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> > drivers/pci/controller/dwc/pci-layerscape.c:    dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> > drivers/pci/controller/dwc/pci-layerscape.c:    pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> > drivers/pci/controller/dwc/pci-meson.c: pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "elbi");
> > drivers/pci/controller/dwc/pcie-al.c:   void __iomem *dbi_base = pcie->dbi_base;
> > drivers/pci/controller/dwc/pcie-al.c:   al_pcie->dbi_base = devm_pci_remap_cfg_resource(dev, res);
> > drivers/pci/controller/dwc/pcie-armada8k.c:     pci->dbi_base = devm_pci_remap_cfg_resource(dev, base);
> > drivers/pci/controller/dwc/pcie-designware.c:           pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res);
> > drivers/pci/controller/dwc/pcie-histb.c:        pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "rc-dbi");
> > drivers/pci/controller/dwc/pcie-qcom-ep.c:      pci->dbi_base = devm_pci_remap_cfg_resource(dev, res);
> > drivers/pci/controller/dwc/pcie-tegra194-acpi.c:        pcie_ecam->dbi_base = cfg->win + SZ_512K;
> >
> > So I don't think that we can always get the size of the dbi.
> > And a solution that does not work for all platforms is not
> > that appealing.
> 
> Do I get a chance to respond before you send a new version?
> 
> Does something like the patch below not work for everyone? We could
> store the DBI size as well if we want more than 8 regions to work
> without a 'dbi' nor 'atu' region defined. We shouldn't have new users
> doing that though.

For the drivers listed above, they assign pci->dbi_base in non-common code.

> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> b/drivers/pci/controller/dwc/pcie-designware.c
> index 250cf7f40b85..3dc71ea7fa76 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -105,11 +105,13 @@ int dw_pcie_get_resources(struct dw_pcie *pci)
>         struct platform_device *pdev = to_platform_device(pci->dev);
>         struct device_node *np = dev_of_node(pci->dev);
>         struct resource *res;
> +       size_t dbi_size = DEFAULT_DBI_ATU_OFFSET + SZ_4K;
>         int ret;
>

So those drivers will not enter this if-statement:

>         if (!pci->dbi_base) {
>                 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
>                 pci->dbi_base = devm_pci_remap_cfg_resource(pci->dev, res);
> +               dbi_size = resource_size(res);
>                 if (IS_ERR(pci->dbi_base))
>                         return PTR_ERR(pci->dbi_base);
>         }

So for those platforms default size will still be 4k.


But sure, this will avoid the need for some platforms to specify "atu" reg,
so I'm all for it. Could you please send it as a real patch?

If you do, could you please also update the description of the dt binding:
https://github.com/torvalds/linux/blob/v6.6-rc7/Documentation/devicetree/bindings/pci/snps%2Cdw-pcie.yaml#L41-L43

That says that iATU memory region is required if the space is unrolled.


Kind regards,
Niklas
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip