Add support for CPM5 controller-1.

PATCH Superseded Thippeswamy Havalige 5 participants 15 messages Sep 6, 2024 – Sep 12, 2024

Subsystems (5)

Device tree bindings PCI Arm/Zynq architecture PCI native host bridge and endpoint drivers +1 more subsystems

lore

Share & tools

lore

Thippeswamy Havalige

author

2024-09-06 09:32 UTC #0 lore raw

cover Add support for CPM5 controller-1.

This patch series adds support for the Xilinx Versal Premium Controller-1 as a Root Port.

The Versal Premium device supports two controllers, both operating at Gen5. speed. Only one controller can be used at any given time.

The primary difference between the two controllers are the register offsets and bits related to platform-specific errors

Thippeswamy Havalige (2):
dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 host1.
PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller 1

 .../bindings/pci/xilinx-versal-cpm.yaml       |  1 +
 drivers/pci/controller/pcie-xilinx-cpm.c      | 39 +++++++++++++++----
 2 files changed, 33 insertions(+), 7 deletions(-)

Signature · 1 line

2.34.1

Thippeswamy Havalige

author

2024-09-06 09:32 UTC #1 lore raw

PATCH 1/2 dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.

The Xilinx Versal premium series has CPM5 block which supports two typeA Root Port controller functionality at Gen5 speed.

Add compatible string to distinguish between two CPM5 rootport controller1.

Error interrupt register and bits for both the controllers are at different.

Signed-off-by Thippeswamy Havalige <[email protected]>

---
 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 +
 1 file changed, 1 insertion(+)

Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml +1 -0

index 989fb0fa2577..b63a759ec2d7 100644
@@ -17,6 +17,7 @@ properties:
     enum:
       - xlnx,versal-cpm-host-1.00
       - xlnx,versal-cpm5-host
+      - xlnx,versal-cpm5-host1
 
   reg:
     items:

Signature · 1 line

2.34.1

Krzysztof Kozlowski

2024-09-06 09:56 UTC #2 lore raw

Re: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.

Quoted 4 lines · Thippeswamy Havalige

The Xilinx Versal premium series has CPM5 block which supports two typeA Root Port controller functionality at Gen5 speed.
Add compatible string to distinguish between two CPM5 rootport controller1.

Subjects NEVER end with full stops.

Quoted 14 lines

Error interrupt register and bits for both the controllers are at different.
Signed-off-by Thippeswamy Havalige <[email protected]>
---
 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml +0 -0
index 989fb0fa2577..b63a759ec2d7 100644

Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml properties: · line 20

Quoted 4 lines

Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml +1 -0

@@ -17,6 +17,7 @@ properties:
     enum:
       - xlnx,versal-cpm-host-1.00
       - xlnx,versal-cpm5-host
+      - xlnx,versal-cpm5-host1

That's poor naming. "-1.00" and now "1". Get your naming reasonable...

Best regards,
Krzysztof

Havalige, Thippeswamy

2024-09-06 10:43 UTC #3 lore raw

RE: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.

Hi Krzysztof

Quoted 13 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, September 6, 2024 3:26 PM
To: Havalige, Thippeswamy <[email protected]>; [email protected]; [email protected]; linux- [email protected]; [email protected]; linux-arm- [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Gogada, Bharat Kumar <[email protected]>; Simek, Michal <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.

Quoted 8 lines

Quoted 4 lines · Thippeswamy Havalige
The Xilinx Versal premium series has CPM5 block which supports two typeA Root Port controller functionality at Gen5 speed.
Add compatible string to distinguish between two CPM5 rootport
controller1.
Subjects NEVER end with full stops.

Thanks, Update in the next patch series.

Quoted 9 lines

Quoted 9 lines
Error interrupt register and bits for both the controllers are at different.
Signed-off-by Thippeswamy Havalige <[email protected]>
---
 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 +
 1 file changed, 1 insertion(+)

Quoted 13 lines

Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml properties: · line 20
Quoted 11 lines
diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml index 989fb0fa2577..b63a759ec2d7 100644
Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml +1 -0
@@ -17,6 +17,7 @@ properties:
     enum:
       - xlnx,versal-cpm-host-1.00
       - xlnx,versal-cpm5-host
+      - xlnx,versal-cpm5-host1
That's poor naming. "-1.00" and now "1". Get your naming reasonable...

Here 1.00 represents the IP versioning and host1 represents controller-1.

Quoted 3 lines

Best regards,
Krzysztof

Krzysztof Kozlowski

2024-09-06 10:46 UTC #4 lore raw

Re: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.

Quoted 46 lines · Thippeswamy

Hi Krzysztof
Quoted 13 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, September 6, 2024 3:26 PM
To: Havalige, Thippeswamy <[email protected]>; [email protected]; [email protected]; linux- [email protected]; [email protected]; linux-arm- [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Gogada, Bharat Kumar <[email protected]>; Simek, Michal <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.
Quoted 8 lines
Quoted 4 lines · Thippeswamy Havalige
The Xilinx Versal premium series has CPM5 block which supports two typeA Root Port controller functionality at Gen5 speed.
Add compatible string to distinguish between two CPM5 rootport
controller1.
Subjects NEVER end with full stops.
Thanks, Update in the next patch series.
Quoted 9 lines
Quoted 9 lines
Error interrupt register and bits for both the controllers are at different.
Signed-off-by Thippeswamy Havalige <[email protected]>
---
 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 +
 1 file changed, 1 insertion(+)
Quoted 13 lines
Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml properties: · line 20
Quoted 11 lines
diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml index 989fb0fa2577..b63a759ec2d7 100644
Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml +1 -0
@@ -17,6 +17,7 @@ properties:
     enum:
       - xlnx,versal-cpm-host-1.00
       - xlnx,versal-cpm5-host
+      - xlnx,versal-cpm5-host1
That's poor naming. "-1.00" and now "1". Get your naming reasonable...

Quoted 1 line

Here 1.00 represents the IP versioning and host1 represents controller-1.

I understand but you repeating the same is not helping. Make it better and next time upstream "host1-1" compatible.

Number of ports, BTW, comes from ports, right? So no need for the compatible.

Best regards,
Krzysztof

Havalige, Thippeswamy

2024-09-06 10:50 UTC #5 lore raw

RE: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.

Hi Krzysztof,

Quoted 61 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, September 6, 2024 4:16 PM
To: Havalige, Thippeswamy <[email protected]>; [email protected]; [email protected]; linux- [email protected]; [email protected]; linux-arm- [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Gogada, Bharat Kumar <[email protected]>; Simek, Michal <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.
Quoted 11 lines · Thippeswamy
Hi Krzysztof
Quoted 9 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, September 6, 2024 3:26 PM
To: Havalige, Thippeswamy <[email protected]>; [email protected]; [email protected]; linux- [email protected]; [email protected]; linux-arm- [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Gogada, Bharat Kumar <[email protected]>; Simek,
Michal
Quoted 35 lines
Quoted 4 lines
<[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.
Quoted 8 lines
Quoted 4 lines · Thippeswamy Havalige
The Xilinx Versal premium series has CPM5 block which supports two typeA Root Port controller functionality at Gen5 speed.
Add compatible string to distinguish between two CPM5 rootport
controller1.
Subjects NEVER end with full stops.
Thanks, Update in the next patch series.
Quoted 22 lines
Quoted 16 lines
Error interrupt register and bits for both the controllers are at different.
Signed-off-by Thippeswamy Havalige <[email protected]>
---
 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 +
 1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml index 989fb0fa2577..b63a759ec2d7 100644
Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml properties: · line 20
Quoted 4 lines
Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml +1 -0
@@ -17,6 +17,7 @@ properties:
     enum:
       - xlnx,versal-cpm-host-1.00
       - xlnx,versal-cpm5-host
+      - xlnx,versal-cpm5-host1
That's poor naming. "-1.00" and now "1". Get your naming reasonable...

Quoted 7 lines

Quoted 1 line
Here 1.00 represents the IP versioning and host1 represents controller-1.
I understand but you repeating the same is not helping. Make it better and next time upstream "host1-1" compatible.
Number of ports, BTW, comes from ports, right? So no need for the compatible.

To differentiate between the registers for Controller-0 and Controller-1, I am utilizing a compatible string in the driver. This approach enables the driver to identify and manage the registers associated with each controller based on the specified compatible string.

Quoted 2 lines

Best regards,
Krzysztof

Regards,
Thippeswamy H

Krzysztof Kozlowski

2024-09-06 12:19 UTC #6 lore raw

Re: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.

Quoted 65 lines · Thippeswamy

Hi Krzysztof,
Quoted 63 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, September 6, 2024 4:16 PM
To: Havalige, Thippeswamy <[email protected]>; [email protected]; [email protected]; linux- [email protected]; [email protected]; linux-arm- [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Gogada, Bharat Kumar <[email protected]>; Simek, Michal <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.
Quoted 11 lines · Thippeswamy
Hi Krzysztof
Quoted 9 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, September 6, 2024 3:26 PM
To: Havalige, Thippeswamy <[email protected]>; [email protected]; [email protected]; linux- [email protected]; [email protected]; linux-arm- [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Gogada, Bharat Kumar <[email protected]>; Simek,
Michal
Quoted 36 lines
Quoted 4 lines
<[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.
Quoted 8 lines
Quoted 4 lines · Thippeswamy Havalige
The Xilinx Versal premium series has CPM5 block which supports two typeA Root Port controller functionality at Gen5 speed.
Add compatible string to distinguish between two CPM5 rootport
controller1.
Subjects NEVER end with full stops.
Thanks, Update in the next patch series.
Quoted 9 lines
Quoted 9 lines
Error interrupt register and bits for both the controllers are at different.
Signed-off-by Thippeswamy Havalige <[email protected]>
---
 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 +
 1 file changed, 1 insertion(+)
Quoted 13 lines
Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml properties: · line 20
Quoted 11 lines
diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml index 989fb0fa2577..b63a759ec2d7 100644
Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml +1 -0
@@ -17,6 +17,7 @@ properties:
     enum:
       - xlnx,versal-cpm-host-1.00
       - xlnx,versal-cpm5-host
+      - xlnx,versal-cpm5-host1
That's poor naming. "-1.00" and now "1". Get your naming reasonable...
Here 1.00 represents the IP versioning and host1 represents controller-1.

Quoted 8 lines

Quoted 5 lines
I understand but you repeating the same is not helping. Make it better and next time upstream "host1-1" compatible.
Number of ports, BTW, comes from ports, right? So no need for the compatible.
To differentiate between the registers for Controller-0 and Controller-1, I am utilizing a compatible string in the driver. This approach enables the driver to identify and manage the registers associated with each controller based on the specified compatible string.

Please don't state the obvious... I know how Linux kernel works. But maybe I wasn't clear - do you have ports property there? I guess not, as it is PCI.

What I claim here, is that you have exactly the same hardware. Same hardware, same compatible.

Best regards,
Krzysztof

Havalige, Thippeswamy

2024-09-11 04:54 UTC #7 lore raw

RE: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.

Hi Krzysztof,

Quoted 95 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, September 6, 2024 5:49 PM
To: Havalige, Thippeswamy <[email protected]>; [email protected]; [email protected]; linux- [email protected]; [email protected]; linux-arm- [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Gogada, Bharat Kumar <[email protected]>; Simek, Michal <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.
Quoted 13 lines · Thippeswamy
Hi Krzysztof,
Quoted 11 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, September 6, 2024 4:16 PM
To: Havalige, Thippeswamy <[email protected]>; [email protected]; [email protected]; linux- [email protected]; [email protected]; linux-arm- [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Gogada, Bharat Kumar <[email protected]>; Simek, Michal <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible
string
Quoted 37 lines
Quoted 37 lines
for CPM5 controller-1.
Quoted 11 lines · Thippeswamy
Hi Krzysztof
Quoted 9 lines
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, September 6, 2024 3:26 PM
To: Havalige, Thippeswamy <[email protected]>; [email protected]; [email protected]; linux- [email protected]; [email protected]; linux-arm- [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Gogada, Bharat Kumar <[email protected]>; Simek,
Michal
Quoted 22 lines
Quoted 4 lines
<[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH 1/2] dt-bindings: PCI: xilinx-cpm: Add compatible string for CPM5 controller-1.
Quoted 8 lines
Quoted 4 lines · Thippeswamy Havalige
The Xilinx Versal premium series has CPM5 block which supports two typeA Root Port controller functionality at Gen5 speed.
Add compatible string to distinguish between two CPM5 rootport
controller1.
Subjects NEVER end with full stops.
Thanks, Update in the next patch series.
Quoted 9 lines
Quoted 9 lines
Error interrupt register and bits for both the controllers are at different.
Signed-off-by Thippeswamy Havalige <[email protected]>
---
 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml | 1 +
 1 file changed, 1 insertion(+)
Quoted 13 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml index 989fb0fa2577..b63a759ec2d7 100644
--- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml properties: · line 20
Quoted 6 lines
+++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml +1 -0
@@ -17,6 +17,7 @@ properties:
     enum:
       - xlnx,versal-cpm-host-1.00
       - xlnx,versal-cpm5-host
+      - xlnx,versal-cpm5-host1
That's poor naming. "-1.00" and now "1". Get your naming
reasonable...
Quoted 1 line
Quoted 1 line
Quoted 1 line
Here 1.00 represents the IP versioning and host1 represents controller-
1.
Quoted 2 lines
Quoted 2 lines
I understand but you repeating the same is not helping. Make it better
and
Quoted 6 lines
Quoted 4 lines
next time upstream "host1-1" compatible.
Number of ports, BTW, comes from ports, right? So no need for the compatible.
To differentiate between the registers for Controller-0 and Controller-1, I
am utilizing a compatible string in the driver. This approach enables the driver to identify and manage the registers associated with each controller based on the specified compatible string.
Quoted 1 line

Quoted 6 lines

Please don't state the obvious... I know how Linux kernel works. But maybe I wasn't clear - do you have ports property there? I guess not, as it is PCI.
What I claim here, is that you have exactly the same hardware. Same hardware, same compatible.

Apologies for the misunderstanding. You're correct—the ports property is not applicable to PCI devices.
Based on Bjorn's input, I'll follow the recommended process for handling this scenario. Thank you for the clarification.

Quoted 2 lines

Best regards,
Krzysztof

Thippeswamy Havalige

author

2024-09-06 09:32 UTC #8 lore raw

PATCH 2/2 PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1

In reply to Thippeswamy Havalige

In the CPM5, controller-1 has platform-specific error interrupt bits located at different offsets compared to controller-0.

Signed-off-by Thippeswamy Havalige <[email protected]>

---
 drivers/pci/controller/pcie-xilinx-cpm.c | 39 +++++++++++++++++++-----
 1 file changed, 32 insertions(+), 7 deletions(-)

drivers/pci/controller/pcie-xilinx-cpm.c +32 -7

index a0f5e1d67b04..d672f620bc4c 100644
@@ -30,10 +30,13 @@
 #define XILINX_CPM_PCIE_REG_IDRN_MASK	0x00000E3C
 #define XILINX_CPM_PCIE_MISC_IR_STATUS	0x00000340
 #define XILINX_CPM_PCIE_MISC_IR_ENABLE	0x00000348
-#define XILINX_CPM_PCIE_MISC_IR_LOCAL	BIT(1)
+#define XILINX_CPM_PCIE0_MISC_IR_LOCAL	BIT(1)
+#define XILINX_CPM_PCIE1_MISC_IR_LOCAL	BIT(2)
 
-#define XILINX_CPM_PCIE_IR_STATUS       0x000002A0
-#define XILINX_CPM_PCIE_IR_ENABLE       0x000002A8
+#define XILINX_CPM_PCIE0_IR_STATUS       0x000002A0
+#define XILINX_CPM_PCIE1_IR_STATUS       0x000002B4
+#define XILINX_CPM_PCIE0_IR_ENABLE       0x000002A8
+#define XILINX_CPM_PCIE1_IR_ENABLE       0x000002BC
 #define XILINX_CPM_PCIE_IR_LOCAL        BIT(0)
 
 #define IMR(x) BIT(XILINX_PCIE_INTR_ ##x)

@@ -280,10 +283,17 @@ static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc)
 	pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
 
 	if (port->variant->version == CPM5) {
-		val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_IR_STATUS);
+		val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE0_IR_STATUS);
 		if (val)
 			writel_relaxed(val, port->cpm_base +
-					    XILINX_CPM_PCIE_IR_STATUS);
+					    XILINX_CPM_PCIE0_IR_STATUS);
+	}
+
+	else if (port->variant->version == CPM5_HOST1) {
+		val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE1_IR_STATUS);
+		if (val)
+			writel_relaxed(val, port->cpm_base +
+					    XILINX_CPM_PCIE1_IR_STATUS);
 	}
 
 	/*

@@ -483,12 +493,19 @@ static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port)
 	 * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
 	 * CPM SLCR block.
 	 */
-	writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
+	writel(XILINX_CPM_PCIE0_MISC_IR_LOCAL,
 	       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
 
 	if (port->variant->version == CPM5) {
 		writel(XILINX_CPM_PCIE_IR_LOCAL,
-		       port->cpm_base + XILINX_CPM_PCIE_IR_ENABLE);
+		       port->cpm_base + XILINX_CPM_PCIE0_IR_ENABLE);
+	}
+
+	else if (port->variant->version == CPM5_HOST1) {
+		writel(XILINX_CPM_PCIE1_MISC_IR_LOCAL,
+		       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
+		writel(XILINX_CPM_PCIE_IR_LOCAL,
+		       port->cpm_base + XILINX_CPM_PCIE1_IR_ENABLE);
 	}
 
 	/* Enable the Bridge enable bit */

@@ -615,6 +632,10 @@ static const struct xilinx_cpm_variant cpm5_host = {
 	.version = CPM5,
 };
 
+static const struct xilinx_cpm_variant cpm5_host = {
+	.version = CPM5_HOST1,
+};
+
 static const struct of_device_id xilinx_cpm_pcie_of_match[] = {
 	{
 		.compatible = "xlnx,versal-cpm-host-1.00",

@@ -624,6 +645,10 @@ static const struct of_device_id xilinx_cpm_pcie_of_match[] = {
 		.compatible = "xlnx,versal-cpm5-host",
 		.data = &cpm5_host,
 	},
+	{
+		.compatible = "xlnx,versal-cpm5-host1",
+		.data = &cpm5_host1,
+	},
 	{}
 };

Signature · 1 line

2.34.1

Krzysztof Kozlowski

2024-09-06 09:56 UTC #9 lore raw

Re: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1

Quoted 31 lines · Thippeswamy Havalige

In the CPM5, controller-1 has platform-specific error interrupt bits located at different offsets compared to controller-0.

Signed-off-by Thippeswamy Havalige <[email protected]>

---
 drivers/pci/controller/pcie-xilinx-cpm.c | 39 +++++++++++++++++++-----
 1 file changed, 32 insertions(+), 7 deletions(-)

drivers/pci/controller/pcie-xilinx-cpm.c +6 -3

index a0f5e1d67b04..d672f620bc4c 100644
@@ -30,10 +30,13 @@
 #define XILINX_CPM_PCIE_REG_IDRN_MASK	0x00000E3C
 #define XILINX_CPM_PCIE_MISC_IR_STATUS	0x00000340
 #define XILINX_CPM_PCIE_MISC_IR_ENABLE	0x00000348
-#define XILINX_CPM_PCIE_MISC_IR_LOCAL	BIT(1)
+#define XILINX_CPM_PCIE0_MISC_IR_LOCAL	BIT(1)
+#define XILINX_CPM_PCIE1_MISC_IR_LOCAL	BIT(2)
 
-#define XILINX_CPM_PCIE_IR_STATUS       0x000002A0
-#define XILINX_CPM_PCIE_IR_ENABLE       0x000002A8
+#define XILINX_CPM_PCIE0_IR_STATUS       0x000002A0
+#define XILINX_CPM_PCIE1_IR_STATUS       0x000002B4
+#define XILINX_CPM_PCIE0_IR_ENABLE       0x000002A8
+#define XILINX_CPM_PCIE1_IR_ENABLE       0x000002BC
 #define XILINX_CPM_PCIE_IR_LOCAL        BIT(0)
 
 #define IMR(x) BIT(XILINX_PCIE_INTR_ ##x)

@@ -280,10 +283,17 @@ static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc)
 	pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);

drivers/pci/controller/pcie-xilinx-cpm.c static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc) · line 291

Quoted 10 lines

drivers/pci/controller/pcie-xilinx-cpm.c +4 -2

 
 	if (port->variant->version == CPM5) {
-		val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_IR_STATUS);
+		val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE0_IR_STATUS);
 		if (val)
 			writel_relaxed(val, port->cpm_base +
-					    XILINX_CPM_PCIE_IR_STATUS);
+					    XILINX_CPM_PCIE0_IR_STATUS);
+	}
+

There are no blank lines allowed between arms of conditional statements. Please follow coding style. This case is explained there.

Best regards,
Krzysztof

Bjorn Helgaas

2024-09-06 19:19 UTC #10 lore raw

Re: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1

In reply to Thippeswamy Havalige

Quoted 2 lines · Thippeswamy Havalige

In the CPM5, controller-1 has platform-specific error interrupt bits located at different offsets compared to controller-0.

Technically mentions a difference between controller-0 and controller-1, but doesn't say what the patch does.

Quoted 13 lines

Signed-off-by Thippeswamy Havalige <[email protected]>

---
 drivers/pci/controller/pcie-xilinx-cpm.c | 39 +++++++++++++++++++-----
 1 file changed, 32 insertions(+), 7 deletions(-)

drivers/pci/controller/pcie-xilinx-cpm.c +0 -0

index a0f5e1d67b04..d672f620bc4c 100644
@@ -30,10 +30,13 @@
 #define XILINX_CPM_PCIE_REG_IDRN_MASK	0x00000E3C
 #define XILINX_CPM_PCIE_MISC_IR_STATUS	0x00000340
 #define XILINX_CPM_PCIE_MISC_IR_ENABLE	0x00000348

drivers/pci/controller/pcie-xilinx-cpm.c line 39

Quoted 10 lines

drivers/pci/controller/pcie-xilinx-cpm.c +6 -3

-#define XILINX_CPM_PCIE_MISC_IR_LOCAL	BIT(1)
+#define XILINX_CPM_PCIE0_MISC_IR_LOCAL	BIT(1)
+#define XILINX_CPM_PCIE1_MISC_IR_LOCAL	BIT(2)
 
-#define XILINX_CPM_PCIE_IR_STATUS       0x000002A0
-#define XILINX_CPM_PCIE_IR_ENABLE       0x000002A8
+#define XILINX_CPM_PCIE0_IR_STATUS       0x000002A0
+#define XILINX_CPM_PCIE1_IR_STATUS       0x000002B4
+#define XILINX_CPM_PCIE0_IR_ENABLE       0x000002A8
+#define XILINX_CPM_PCIE1_IR_ENABLE       0x000002BC

These are all indented with spaces; should use tabs like the other definitions above.

drivers/pci/controller/pcie-xilinx-cpm.c line 40

Quoted 1 line

drivers/pci/controller/pcie-xilinx-cpm.c +0 -0
 #define XILINX_CPM_PCIE_IR_LOCAL        BIT(0)

Same here (although you didn't change this one).

Quoted 9 lines

drivers/pci/controller/pcie-xilinx-cpm.c +1 -1

 #define IMR(x) BIT(XILINX_PCIE_INTR_ ##x)

@@ -280,10 +283,17 @@ static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc)
 	pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
 
 	if (port->variant->version == CPM5) {
-		val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_IR_STATUS);
+		val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE0_IR_STATUS);
 		if (val)
 			writel_relaxed(val, port->cpm_base +

drivers/pci/controller/pcie-xilinx-cpm.c static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc) · line 297

Quoted 10 lines

drivers/pci/controller/pcie-xilinx-cpm.c +8 -1

-					    XILINX_CPM_PCIE_IR_STATUS);
+					    XILINX_CPM_PCIE0_IR_STATUS);
+	}
+
+	else if (port->variant->version == CPM5_HOST1) {
+		val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE1_IR_STATUS);
+		if (val)
+			writel_relaxed(val, port->cpm_base +
+					    XILINX_CPM_PCIE1_IR_STATUS);
 	}

When possible, I think it would be nicer to encode this difference in the data, i.e., in struct xilinx_cpm_variant, than in the code. For example,

    struct xilinx_cpm_variant {
      enum xilinx_cpm_version version;
 +    u32 ir_status;
    }

    static const struct xilinx_cpm_variant cpm5_host = {
      .version = CPM5,
 +    .ir_status = XILINX_CPM_PCIE0_IR_STATUS,
    };

    static const struct xilinx_cpm_variant cpm5_host = {
      .version = CPM5_HOST1,
 +    .ir_status = XILINX_CPM_PCIE1_IR_STATUS,
    };

Then this code could look like this, where it basically tests a *feature* instead of checking for all the different variants:

  struct xilinx_cpm_variant *variant = port->variant;

  if (variant->ir_status) {
    val = readl_relaxed(port->cpm_base + variant->ir_status);
    if (val)
      writel_relaxed(port->cpm_base + variant->ir_status);
  }

(Apparently the CPM variant doesn't require this at all?)

Quoted 10 lines

drivers/pci/controller/pcie-xilinx-cpm.c +1 -1

/*

@@ -483,12 +493,19 @@ static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port)
 	 * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
 	 * CPM SLCR block.
 	 */
-	writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
+	writel(XILINX_CPM_PCIE0_MISC_IR_LOCAL,
 	       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
 
 	if (port->variant->version == CPM5) {

drivers/pci/controller/pcie-xilinx-cpm.c static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port) · line 508

Quoted 10 lines

drivers/pci/controller/pcie-xilinx-cpm.c +8 -1

 		writel(XILINX_CPM_PCIE_IR_LOCAL,
-		       port->cpm_base + XILINX_CPM_PCIE_IR_ENABLE);
+		       port->cpm_base + XILINX_CPM_PCIE0_IR_ENABLE);
+	}
+
+	else if (port->variant->version == CPM5_HOST1) {
+		writel(XILINX_CPM_PCIE1_MISC_IR_LOCAL,
+		       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
+		writel(XILINX_CPM_PCIE_IR_LOCAL,
+		       port->cpm_base + XILINX_CPM_PCIE1_IR_ENABLE);

This looks questionable and worth a comment if this is what you intend.

CPM
- sets PCIE0_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE

CPM5
- sets PCIE0_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE
- sets PCIE_IR_LOCAL in PCIE0_IR_ENABLE

CPM5_HOST1
- sets PCIE0_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE
- sets PCIE1_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE (overwrite)
- sets PCIE_IR_LOCAL in PCIE1_IR_ENABLE

The CPM5_HOST1 overwrite looks either wrong or like the first write was unnecessary.

You might be able to simplify this by adding .misc_ir_value, .ir_enable, and .ir_local_value:

  struct xilinx_cpm_variant *variant = port->variant;

  writel(variant->misc_ir_value,
         port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
  if (variant->ir_enable)
    writel(variant->ir_local_value, port->cpm_base + ir_enable);

drivers/pci/controller/pcie-xilinx-cpm.c static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port) · line 511

Quoted 1 line

drivers/pci/controller/pcie-xilinx-cpm.c +0 -0
 	/* Enable the Bridge enable bit */

Unrelated to *this* patch except for being in the diff context, but "enable the enable bit" is unclear because "enable" isn't something you can do to a bit; you can *set* or *clear* a bit.

Bjorn

Havalige, Thippeswamy

2024-09-12 09:38 UTC #11 lore raw

RE: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1

Hi Bjorn,

Thanks, for your comments.

Quoted 13 lines

-----Original Message-----
From: Bjorn Helgaas <[email protected]>
Sent: Saturday, September 7, 2024 12:49 AM
To: Havalige, Thippeswamy <[email protected]>
Cc: [email protected]; [email protected]; linux- [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Gogada, Bharat Kumar <[email protected]>; Simek, Michal <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1

Quoted 6 lines

Quoted 2 lines · Thippeswamy Havalige
In the CPM5, controller-1 has platform-specific error interrupt bits located at different offsets compared to controller-0.
Technically mentions a difference between controller-0 and controller-1, but doesn't say what the patch does.

I'll update required detailed description in next patch.

Quoted 19 lines

drivers/pci/controller/pcie-
Quoted 7 lines
Signed-off-by Thippeswamy Havalige <[email protected]>
---
 drivers/pci/controller/pcie-xilinx-cpm.c | 39 +++++++++++++++++++-----
 1 file changed, 32 insertions(+), 7 deletions(-)
drivers/pci/controller/pcie- renamed from drivers/pci/controller/pcie-xilinx-cpm.c +0 -0
xilinx-cpm.c
Quoted 11 lines
index a0f5e1d67b04..d672f620bc4c 100644
drivers/pci/controller/pcie-xilinx-cpm.c +2 -1
@@ -30,10 +30,13 @@
 #define XILINX_CPM_PCIE_REG_IDRN_MASK	0x00000E3C
 #define XILINX_CPM_PCIE_MISC_IR_STATUS	0x00000340
 #define XILINX_CPM_PCIE_MISC_IR_ENABLE	0x00000348
-#define XILINX_CPM_PCIE_MISC_IR_LOCAL	BIT(1)
+#define XILINX_CPM_PCIE0_MISC_IR_LOCAL	BIT(1)
+#define XILINX_CPM_PCIE1_MISC_IR_LOCAL	BIT(2)
 

Quoted 9 lines

drivers/pci/controller/pcie-xilinx-cpm.c line 39
Quoted 6 lines
drivers/pci/controller/pcie-xilinx-cpm.c +4 -2
-#define XILINX_CPM_PCIE_IR_STATUS       0x000002A0
-#define XILINX_CPM_PCIE_IR_ENABLE       0x000002A8
+#define XILINX_CPM_PCIE0_IR_STATUS       0x000002A0
+#define XILINX_CPM_PCIE1_IR_STATUS       0x000002B4
+#define XILINX_CPM_PCIE0_IR_ENABLE       0x000002A8
+#define XILINX_CPM_PCIE1_IR_ENABLE       0x000002BC
These are all indented with spaces; should use tabs like the other definitions above.

Thanks, will add proper tabs in next patch.

Quoted 93 lines

drivers/pci/controller/pcie-xilinx-cpm.c line 40
Quoted 2 lines
drivers/pci/controller/pcie-xilinx-cpm.c +0 -0
 #define XILINX_CPM_PCIE_IR_LOCAL        BIT(0)
Same here (although you didn't change this one).
drivers/pci/controller/pcie-xilinx-cpm.c static void xilinx_cpm_pcie_event_flow(struct
Quoted 2 lines
drivers/pci/controller/pcie-xilinx-cpm.c +0 -0
 #define IMR(x) BIT(XILINX_PCIE_INTR_ ##x)
@@ -280,10 +283,17 @@ static void xilinx_cpm_pcie_event_flow(struct
irq_desc *desc)
drivers/pci/controller/pcie-xilinx-cpm.c static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc)
Quoted 4 lines
drivers/pci/controller/pcie-xilinx-cpm.c +0 -1
 	pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
 
 	if (port->variant->version == CPM5) {
-		val = readl_relaxed(port->cpm_base +
XILINX_CPM_PCIE_IR_STATUS);
drivers/pci/controller/pcie-xilinx-cpm.c static void xilinx_cpm_pcie_event_flow(struct · line 286
Quoted 1 line
drivers/pci/controller/pcie-xilinx-cpm.c +1 -0
+		val = readl_relaxed(port->cpm_base +
XILINX_CPM_PCIE0_IR_STATUS);
drivers/pci/controller/pcie-xilinx-cpm.c static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc)
Quoted 8 lines
drivers/pci/controller/pcie-xilinx-cpm.c +5 -1
 		if (val)
 			writel_relaxed(val, port->cpm_base +
-					    XILINX_CPM_PCIE_IR_STATUS);
+					    XILINX_CPM_PCIE0_IR_STATUS);
+	}
+
+	else if (port->variant->version == CPM5_HOST1) {
+		val = readl_relaxed(port->cpm_base +
XILINX_CPM_PCIE1_IR_STATUS);
drivers/pci/controller/pcie-xilinx-cpm.c static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc) · line 297
Quoted 4 lines
drivers/pci/controller/pcie-xilinx-cpm.c +3 -0
+		if (val)
+			writel_relaxed(val, port->cpm_base +
+					    XILINX_CPM_PCIE1_IR_STATUS);
 	}
When possible, I think it would be nicer to encode this difference in the data, i.e., in struct xilinx_cpm_variant, than in the code. For example,
    struct xilinx_cpm_variant {
      enum xilinx_cpm_version version;
 +    u32 ir_status;
    }

    static const struct xilinx_cpm_variant cpm5_host = {
      .version = CPM5,
 +    .ir_status = XILINX_CPM_PCIE0_IR_STATUS,
    };

    static const struct xilinx_cpm_variant cpm5_host = {
      .version = CPM5_HOST1,
 +    .ir_status = XILINX_CPM_PCIE1_IR_STATUS,
    };
Then this code could look like this, where it basically tests a *feature* instead of checking for all the different variants:
  struct xilinx_cpm_variant *variant = port->variant;

  if (variant->ir_status) {
    val = readl_relaxed(port->cpm_base + variant->ir_status);
    if (val)
      writel_relaxed(port->cpm_base + variant->ir_status);
  }
(Apparently the CPM variant doesn't require this at all?)
drivers/pci/controller/pcie-xilinx-cpm.c static void xilinx_cpm_pcie_init_port(struct
Quoted 2 lines
drivers/pci/controller/pcie-xilinx-cpm.c +0 -0
 	/*
@@ -483,12 +493,19 @@ static void xilinx_cpm_pcie_init_port(struct
xilinx_cpm_pcie *port)
Quoted 8 lines
drivers/pci/controller/pcie-xilinx-cpm.c +1 -1
 	 * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
 	 * CPM SLCR block.
 	 */
-	writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
+	writel(XILINX_CPM_PCIE0_MISC_IR_LOCAL,
 	       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
 
 	if (port->variant->version == CPM5) {
drivers/pci/controller/pcie-xilinx-cpm.c static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port) · line 508
Quoted 10 lines
drivers/pci/controller/pcie-xilinx-cpm.c +8 -1
 		writel(XILINX_CPM_PCIE_IR_LOCAL,
-		       port->cpm_base + XILINX_CPM_PCIE_IR_ENABLE);
+		       port->cpm_base + XILINX_CPM_PCIE0_IR_ENABLE);
+	}
+
+	else if (port->variant->version == CPM5_HOST1) {
+		writel(XILINX_CPM_PCIE1_MISC_IR_LOCAL,
+		       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
+		writel(XILINX_CPM_PCIE_IR_LOCAL,
+		       port->cpm_base + XILINX_CPM_PCIE1_IR_ENABLE);
This looks questionable and worth a comment if this is what you intend.
CPM
- sets PCIE0_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE
CPM5
- sets PCIE0_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE
- sets PCIE_IR_LOCAL in PCIE0_IR_ENABLE

Quoted 7 lines

CPM5_HOST1
- sets PCIE0_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE
- sets PCIE1_MISC_IR_LOCAL in PCIE_MISC_IR_ENABLE (overwrite)
- sets PCIE_IR_LOCAL in PCIE1_IR_ENABLE
The CPM5_HOST1 overwrite looks either wrong or like the first write was unnecessary.

Yes, the initial write is unnecessary here. I will fix this in next patch.

Quoted 18 lines

You might be able to simplify this by adding .misc_ir_value, .ir_enable, and .ir_local_value:
  struct xilinx_cpm_variant *variant = port->variant;

  writel(variant->misc_ir_value,
         port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
  if (variant->ir_enable)
    writel(variant->ir_local_value, port->cpm_base + ir_enable);
drivers/pci/controller/pcie-xilinx-cpm.c static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port) · line 511
Quoted 1 line
drivers/pci/controller/pcie-xilinx-cpm.c +0 -0
 	/* Enable the Bridge enable bit */
Unrelated to *this* patch except for being in the diff context, but "enable the enable bit" is unclear because "enable" isn't something you can do to a bit; you can *set* or *clear* a bit.
Bjorn

kernel test robot

build bot

2024-09-07 06:16 UTC #12 lore raw

Re: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1

In reply to Thippeswamy Havalige

kernel test robot noticed the following build errors:

Hi Thippeswamy,

kernel test robot noticed the following build errors:

[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus mani-mhi/mhi-next robh/for-next linus/master v6.11-rc6 next-20240906] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Thippeswamy-Havalige/dt-bindings-PCI-xilinx-cpm-Add-compatible-string-for-CPM5-controller-1/20240906-173446
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20240906093148.830452-3-thippesw%40amd.com
patch subject: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1
config: alpha-randconfig-r051-20240907 (https://download.01.org/0day-ci/archive/20240907/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240907/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/pci/controller/pcie-xilinx-cpm.c: In function 'xilinx_cpm_pcie_event_flow':

>> drivers/pci/controller/pcie-xilinx-cpm.c:292:44: error: 'CPM5_HOST1' undeclared (first use in this function)
     292 |         else if (port->variant->version == CPM5_HOST1) {
         |                                            ^~~~~~~~~~
   drivers/pci/controller/pcie-xilinx-cpm.c:292:44: note: each undeclared identifier is reported only once for each function it appears in
   drivers/pci/controller/pcie-xilinx-cpm.c: In function 'xilinx_cpm_pcie_init_port':
   drivers/pci/controller/pcie-xilinx-cpm.c:504:44: error: 'CPM5_HOST1' undeclared (first use in this function)
     504 |         else if (port->variant->version == CPM5_HOST1) {
         |                                            ^~~~~~~~~~
   drivers/pci/controller/pcie-xilinx-cpm.c: At top level:
>> drivers/pci/controller/pcie-xilinx-cpm.c:635:40: error: redefinition of 'cpm5_host'
     635 | static const struct xilinx_cpm_variant cpm5_host = {
         |                                        ^~~~~~~~~
   drivers/pci/controller/pcie-xilinx-cpm.c:631:40: note: previous definition of 'cpm5_host' with type 'const struct xilinx_cpm_variant'
     631 | static const struct xilinx_cpm_variant cpm5_host = {
         |                                        ^~~~~~~~~
>> drivers/pci/controller/pcie-xilinx-cpm.c:636:20: error: 'CPM5_HOST1' undeclared here (not in a function)
     636 |         .version = CPM5_HOST1,
         |                    ^~~~~~~~~~
>> drivers/pci/controller/pcie-xilinx-cpm.c:650:26: error: 'cpm5_host1' undeclared here (not in a function); did you mean 'cpm5_host'?
     650 |                 .data = &cpm5_host1,
         |                          ^~~~~~~~~~
         |                          cpm5_host

vim +/CPM5_HOST1 +292 drivers/pci/controller/pcie-xilinx-cpm.c

270
271 static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc)
272 {
273 struct xilinx_cpm_pcie *port = irq_desc_get_handler_data(desc);
274 struct irq_chip *chip = irq_desc_get_chip(desc);
275 unsigned long val;
276 int i;
277
278 chained_irq_enter(chip, desc);
279 val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
280 val &= pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
281 for_each_set_bit(i, &val, 32)
282 generic_handle_domain_irq(port->cpm_domain, i);
283 pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
284
285 if (port->variant->version == CPM5) {
286 val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE0_IR_STATUS);
287 if (val)
288 writel_relaxed(val, port->cpm_base +
289 XILINX_CPM_PCIE0_IR_STATUS);
290 }
291

 > 292		else if (port->variant->version == CPM5_HOST1) {

293 val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE1_IR_STATUS);
294 if (val)
295 writel_relaxed(val, port->cpm_base +
296 XILINX_CPM_PCIE1_IR_STATUS);
297 }
298
299 /*
300 * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to
301 * CPM SLCR block.
302 */
303 val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
304 if (val)
305 writel_relaxed(val,
306 port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
307
308 chained_irq_exit(chip, desc);
309 }
310

Signature · 2 lines

0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

kernel test robot

build bot

2024-09-07 09:00 UTC #13 lore raw

Re: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1

In reply to Thippeswamy Havalige

kernel test robot noticed the following build errors:

Hi Thippeswamy,

kernel test robot noticed the following build errors:

url:    https://github.com/intel-lab-lkp/linux/commits/Thippeswamy-Havalige/dt-bindings-PCI-xilinx-cpm-Add-compatible-string-for-CPM5-controller-1/20240906-173446
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20240906093148.830452-3-thippesw%40amd.com
patch subject: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20240907/[email protected]/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 05f5a91d00b02f4369f46d076411c700755ae041)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240907/[email protected]/reproduce)

All errors (new ones prefixed by >>):

In file included from drivers/pci/controller/pcie-xilinx-cpm.c:10:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/s390/include/asm/io.h:93:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]

     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
         |                                                           ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
     102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
         |                                                      ^
   In file included from drivers/pci/controller/pcie-xilinx-cpm.c:10:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
         |                                                           ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
     115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
         |                                                      ^
   In file included from drivers/pci/controller/pcie-xilinx-cpm.c:10:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     693 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     701 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     709 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     718 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     727 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     736 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   In file included from drivers/pci/controller/pcie-xilinx-cpm.c:10:
   In file included from include/linux/irq.h:591:
   In file included from arch/s390/include/asm/hw_irq.h:6:
   In file included from include/linux/pci.h:37:
   In file included from include/linux/device.h:32:
   In file included from include/linux/device/driver.h:21:
   In file included from include/linux/module.h:19:
   In file included from include/linux/elf.h:6:
   In file included from arch/s390/include/asm/elf.h:181:
   In file included from arch/s390/include/asm/mmu_context.h:11:
   In file included from arch/s390/include/asm/pgalloc.h:18:
   In file included from include/linux/mm.h:2228:
   include/linux/vmstat.h:500:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     500 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     501 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:507:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     507 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     508 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     514 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:519:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     519 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     520 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:528:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     528 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     529 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pci/controller/pcie-xilinx-cpm.c:292:37: error: use of undeclared identifier 'CPM5_HOST1'
     292 |         else if (port->variant->version == CPM5_HOST1) {
         |                                            ^
   drivers/pci/controller/pcie-xilinx-cpm.c:504:37: error: use of undeclared identifier 'CPM5_HOST1'
     504 |         else if (port->variant->version == CPM5_HOST1) {
         |                                            ^
   drivers/pci/controller/pcie-xilinx-cpm.c:636:13: error: use of undeclared identifier 'CPM5_HOST1'
     636 |         .version = CPM5_HOST1,
         |                    ^
>> drivers/pci/controller/pcie-xilinx-cpm.c:650:12: error: use of undeclared identifier 'cpm5_host1'; did you mean 'cpm5_host'?
     650 |                 .data = &cpm5_host1,
         |                          ^~~~~~~~~~
         |                          cpm5_host
   drivers/pci/controller/pcie-xilinx-cpm.c:635:40: note: 'cpm5_host' declared here
     635 | static const struct xilinx_cpm_variant cpm5_host = {
         |                                        ^

17 warnings and 4 errors generated.

vim +/CPM5_HOST1 +292 drivers/pci/controller/pcie-xilinx-cpm.c

 > 292		else if (port->variant->version == CPM5_HOST1) {

Signature · 2 lines

0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

kernel test robot

build bot

2024-09-07 10:02 UTC #14 lore raw

Re: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1

In reply to Thippeswamy Havalige

kernel test robot noticed the following build warnings:

Hi Thippeswamy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus mani-mhi/mhi-next robh/for-next linus/master v6.11-rc6 next-20240906] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Thippeswamy-Havalige/dt-bindings-PCI-xilinx-cpm-Add-compatible-string-for-CPM5-controller-1/20240906-173446
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20240906093148.830452-3-thippesw%40amd.com
patch subject: [PATCH 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root Port controller-1
config: um-allyesconfig (https://download.01.org/0day-ci/archive/20240907/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240907/[email protected]/reproduce)

All warnings (new ones prefixed by >>):

drivers/pci/controller/pcie-xilinx-cpm.c: In function 'xilinx_cpm_pcie_event_flow':
drivers/pci/controller/pcie-xilinx-cpm.c:292:44: error: 'CPM5_HOST1' undeclared (first use in this function)

     292 |         else if (port->variant->version == CPM5_HOST1) {
         |                                            ^~~~~~~~~~
   drivers/pci/controller/pcie-xilinx-cpm.c:292:44: note: each undeclared identifier is reported only once for each function it appears in
   drivers/pci/controller/pcie-xilinx-cpm.c: In function 'xilinx_cpm_pcie_init_port':
   drivers/pci/controller/pcie-xilinx-cpm.c:504:44: error: 'CPM5_HOST1' undeclared (first use in this function)
     504 |         else if (port->variant->version == CPM5_HOST1) {
         |                                            ^~~~~~~~~~
   drivers/pci/controller/pcie-xilinx-cpm.c: At top level:
   drivers/pci/controller/pcie-xilinx-cpm.c:635:40: error: redefinition of 'cpm5_host'
     635 | static const struct xilinx_cpm_variant cpm5_host = {
         |                                        ^~~~~~~~~
   drivers/pci/controller/pcie-xilinx-cpm.c:631:40: note: previous definition of 'cpm5_host' with type 'const struct xilinx_cpm_variant'
     631 | static const struct xilinx_cpm_variant cpm5_host = {
         |                                        ^~~~~~~~~
   drivers/pci/controller/pcie-xilinx-cpm.c:636:20: error: 'CPM5_HOST1' undeclared here (not in a function)
     636 |         .version = CPM5_HOST1,
         |                    ^~~~~~~~~~
   drivers/pci/controller/pcie-xilinx-cpm.c:650:26: error: 'cpm5_host1' undeclared here (not in a function); did you mean 'cpm5_host'?
     650 |                 .data = &cpm5_host1,
         |                          ^~~~~~~~~~
         |                          cpm5_host
>> drivers/pci/controller/pcie-xilinx-cpm.c:631:40: warning: 'cpm5_host' defined but not used [-Wunused-const-variable=]
     631 | static const struct xilinx_cpm_variant cpm5_host = {
         |                                        ^~~~~~~~~

vim +/cpm5_host +631 drivers/pci/controller/pcie-xilinx-cpm.c

51f1ffc00d95e3 Bharat Kumar Gogada 2022-07-05 630

51f1ffc00d95e3 Bharat Kumar Gogada 2022-07-05 @631  static const struct xilinx_cpm_variant cpm5_host = {
51f1ffc00d95e3 Bharat Kumar Gogada 2022-07-05  632  	.version = CPM5,
51f1ffc00d95e3 Bharat Kumar Gogada 2022-07-05  633  };

51f1ffc00d95e3 Bharat Kumar Gogada 2022-07-05 634

Signature · 2 lines

0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki