Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH 0/4] Fix interrupt log message
@ 2025-05-09 15:53 Hans Zhang
  2025-05-09 15:53 ` [PATCH 1/4] PCI: rockchip-host: Fix "Unexpected Completion" " Hans Zhang
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Hans Zhang @ 2025-05-09 15:53 UTC (permalink / raw)
  To: shawn.lin, lpieralisi, kw, bhelgaas, heiko, manivannan.sadhasivam
  Cc: robh, linux-pci, linux-kernel, linux-arm-kernel, linux-rockchip,
	Hans Zhang

1. PCI: rockchip-host: Fix "Unexpected Completion" log message
2. PCI: rockchip-host: Correct non-fatal error log message
3. PCI: rockchip-host: Refactor IRQ handling with info arrays
4. PCI: rockchip-host: Remove unused header includes

---
Dear Maintainers,

Detailed descriptions of interrupts can be seen from RK3399 TRM doc.
I found two errors, optimized the code and cleaned up the driver by the way.
---

Hans Zhang (4):
  PCI: rockchip-host: Fix "Unexpected Completion" log message
  PCI: rockchip-host: Correct non-fatal error log message
  PCI: rockchip-host: Refactor IRQ handling with info arrays
  PCI: rockchip-host: Remove unused header includes

 drivers/pci/controller/pcie-rockchip-host.c | 119 ++++++++------------
 1 file changed, 48 insertions(+), 71 deletions(-)


base-commit: 01f95500a162fca88cefab9ed64ceded5afabc12
-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/4] PCI: rockchip-host: Fix "Unexpected Completion" log message
  2025-05-09 15:53 [PATCH 0/4] Fix interrupt log message Hans Zhang
@ 2025-05-09 15:53 ` Hans Zhang
  2025-05-09 15:54 ` [PATCH 2/4] PCI: rockchip-host: Correct non-fatal error " Hans Zhang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Hans Zhang @ 2025-05-09 15:53 UTC (permalink / raw)
  To: shawn.lin, lpieralisi, kw, bhelgaas, heiko, manivannan.sadhasivam
  Cc: robh, linux-pci, linux-kernel, linux-arm-kernel, linux-rockchip,
	Hans Zhang

Fix the debug message for the PCIE_CORE_INT_UCR interrupt to clearly
indicate "Unexpected Completion" instead of a duplicate "malformed TLP"
message. This improves error log accuracy.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/pcie-rockchip-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index 6a46be17aa91..2804980bab86 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -439,7 +439,7 @@ static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
 			dev_dbg(dev, "malformed TLP received from the link\n");
 
 		if (sub_reg & PCIE_CORE_INT_UCR)
-			dev_dbg(dev, "malformed TLP received from the link\n");
+			dev_dbg(dev, "Unexpected Completion received from the link\n");
 
 		if (sub_reg & PCIE_CORE_INT_FCE)
 			dev_dbg(dev, "an error was observed in the flow control advertisements from the other side\n");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/4] PCI: rockchip-host: Correct non-fatal error log message
  2025-05-09 15:53 [PATCH 0/4] Fix interrupt log message Hans Zhang
  2025-05-09 15:53 ` [PATCH 1/4] PCI: rockchip-host: Fix "Unexpected Completion" " Hans Zhang
@ 2025-05-09 15:54 ` Hans Zhang
  2025-05-09 15:54 ` [PATCH 3/4] PCI: rockchip-host: Refactor IRQ handling with info arrays Hans Zhang
  2025-05-09 15:54 ` [PATCH 4/4] PCI: rockchip-host: Remove unused header includes Hans Zhang
  3 siblings, 0 replies; 7+ messages in thread
From: Hans Zhang @ 2025-05-09 15:54 UTC (permalink / raw)
  To: shawn.lin, lpieralisi, kw, bhelgaas, heiko, manivannan.sadhasivam
  Cc: robh, linux-pci, linux-kernel, linux-arm-kernel, linux-rockchip,
	Hans Zhang

Correct the debug message for PCIE_CLIENT_INT_NFATAL_ERR from
"no fatal error" to "non fatal error interrupt received" to match the
actual interrupt semantics. This avoids confusion in log interpretation.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/pcie-rockchip-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index 2804980bab86..209eb94ece1b 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -489,7 +489,7 @@ static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg)
 		dev_dbg(dev, "fatal error interrupt received\n");
 
 	if (reg & PCIE_CLIENT_INT_NFATAL_ERR)
-		dev_dbg(dev, "no fatal error interrupt received\n");
+		dev_dbg(dev, "non fatal error interrupt received\n");
 
 	if (reg & PCIE_CLIENT_INT_CORR_ERR)
 		dev_dbg(dev, "correctable error interrupt received\n");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/4] PCI: rockchip-host: Refactor IRQ handling with info arrays
  2025-05-09 15:53 [PATCH 0/4] Fix interrupt log message Hans Zhang
  2025-05-09 15:53 ` [PATCH 1/4] PCI: rockchip-host: Fix "Unexpected Completion" " Hans Zhang
  2025-05-09 15:54 ` [PATCH 2/4] PCI: rockchip-host: Correct non-fatal error " Hans Zhang
@ 2025-05-09 15:54 ` Hans Zhang
  2025-05-13 10:40   ` Krzysztof Wilczyński
  2025-05-09 15:54 ` [PATCH 4/4] PCI: rockchip-host: Remove unused header includes Hans Zhang
  3 siblings, 1 reply; 7+ messages in thread
From: Hans Zhang @ 2025-05-09 15:54 UTC (permalink / raw)
  To: shawn.lin, lpieralisi, kw, bhelgaas, heiko, manivannan.sadhasivam
  Cc: robh, linux-pci, linux-kernel, linux-arm-kernel, linux-rockchip,
	Hans Zhang

Replace repetitive if-conditions for IRQ status checks with structured
arrays (`pcie_subsys_irq_info` and `pcie_client_irq_info`) and loop-based
logging. This simplifies maintenance and reduces code duplication.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/pcie-rockchip-host.c | 110 +++++++++-----------
 1 file changed, 48 insertions(+), 62 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index 209eb94ece1b..ba1f7832bda5 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -36,6 +36,45 @@
 #include "../pci.h"
 #include "pcie-rockchip.h"
 
+struct rockchip_irq_info {
+	u32 bit;
+	const char *msg;
+};
+
+static const struct rockchip_irq_info pcie_subsys_irq_info[] = {
+	{ PCIE_CORE_INT_PRFPE,
+	  "parity error detected while reading from the PNP receive FIFO RAM" },
+	{ PCIE_CORE_INT_CRFPE,
+	  "parity error detected while reading from the Completion Receive FIFO RAM" },
+	{ PCIE_CORE_INT_RRPE,
+	  "parity error detected while reading from replay buffer RAM" },
+	{ PCIE_CORE_INT_PRFO, "overflow occurred in the PNP receive FIFO" },
+	{ PCIE_CORE_INT_CRFO,
+	  "overflow occurred in the completion receive FIFO" },
+	{ PCIE_CORE_INT_RT, "replay timer timed out" },
+	{ PCIE_CORE_INT_RTR,
+	  "replay timer rolled over after 4 transmissions of the same TLP" },
+	{ PCIE_CORE_INT_PE, "phy error detected on receive side" },
+	{ PCIE_CORE_INT_MTR, "malformed TLP received from the link" },
+	{ PCIE_CORE_INT_UCR, "Unexpected Completion received from the link" },
+	{ PCIE_CORE_INT_FCE,
+	  "an error was observed in the flow control advertisements from the other side" },
+	{ PCIE_CORE_INT_CT, "a request timed out waiting for completion" },
+	{ PCIE_CORE_INT_UTC, "unmapped TC error" },
+	{ PCIE_CORE_INT_MMVC, "MSI mask register changes" },
+};
+
+static const struct rockchip_irq_info pcie_client_irq_info[] = {
+	{ PCIE_CLIENT_INT_LEGACY_DONE, "legacy done" },
+	{ PCIE_CLIENT_INT_MSG, "message done" },
+	{ PCIE_CLIENT_INT_HOT_RST, "hot reset" },
+	{ PCIE_CLIENT_INT_DPA, "dpa" },
+	{ PCIE_CLIENT_INT_FATAL_ERR, "fatal error" },
+	{ PCIE_CLIENT_INT_NFATAL_ERR, "Non fatal error" },
+	{ PCIE_CLIENT_INT_CORR_ERR, "correctable error" },
+	{ PCIE_CLIENT_INT_PHY, "phy" },
+};
+
 static void rockchip_pcie_enable_bw_int(struct rockchip_pcie *rockchip)
 {
 	u32 status;
@@ -411,47 +450,11 @@ static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
 	if (reg & PCIE_CLIENT_INT_LOCAL) {
 		dev_dbg(dev, "local interrupt received\n");
 		sub_reg = rockchip_pcie_read(rockchip, PCIE_CORE_INT_STATUS);
-		if (sub_reg & PCIE_CORE_INT_PRFPE)
-			dev_dbg(dev, "parity error detected while reading from the PNP receive FIFO RAM\n");
-
-		if (sub_reg & PCIE_CORE_INT_CRFPE)
-			dev_dbg(dev, "parity error detected while reading from the Completion Receive FIFO RAM\n");
-
-		if (sub_reg & PCIE_CORE_INT_RRPE)
-			dev_dbg(dev, "parity error detected while reading from replay buffer RAM\n");
-
-		if (sub_reg & PCIE_CORE_INT_PRFO)
-			dev_dbg(dev, "overflow occurred in the PNP receive FIFO\n");
-
-		if (sub_reg & PCIE_CORE_INT_CRFO)
-			dev_dbg(dev, "overflow occurred in the completion receive FIFO\n");
-
-		if (sub_reg & PCIE_CORE_INT_RT)
-			dev_dbg(dev, "replay timer timed out\n");
-
-		if (sub_reg & PCIE_CORE_INT_RTR)
-			dev_dbg(dev, "replay timer rolled over after 4 transmissions of the same TLP\n");
-
-		if (sub_reg & PCIE_CORE_INT_PE)
-			dev_dbg(dev, "phy error detected on receive side\n");
 
-		if (sub_reg & PCIE_CORE_INT_MTR)
-			dev_dbg(dev, "malformed TLP received from the link\n");
-
-		if (sub_reg & PCIE_CORE_INT_UCR)
-			dev_dbg(dev, "Unexpected Completion received from the link\n");
-
-		if (sub_reg & PCIE_CORE_INT_FCE)
-			dev_dbg(dev, "an error was observed in the flow control advertisements from the other side\n");
-
-		if (sub_reg & PCIE_CORE_INT_CT)
-			dev_dbg(dev, "a request timed out waiting for completion\n");
-
-		if (sub_reg & PCIE_CORE_INT_UTC)
-			dev_dbg(dev, "unmapped TC error\n");
-
-		if (sub_reg & PCIE_CORE_INT_MMVC)
-			dev_dbg(dev, "MSI mask register changes\n");
+		for (int i = 0; i < ARRAY_SIZE(pcie_subsys_irq_info); i++) {
+			if (sub_reg & pcie_subsys_irq_info[i].bit)
+				dev_dbg(dev, "%s\n", pcie_subsys_irq_info[i].msg);
+		}
 
 		rockchip_pcie_write(rockchip, sub_reg, PCIE_CORE_INT_STATUS);
 	} else if (reg & PCIE_CLIENT_INT_PHY) {
@@ -473,29 +476,12 @@ static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg)
 	u32 reg;
 
 	reg = rockchip_pcie_read(rockchip, PCIE_CLIENT_INT_STATUS);
-	if (reg & PCIE_CLIENT_INT_LEGACY_DONE)
-		dev_dbg(dev, "legacy done interrupt received\n");
-
-	if (reg & PCIE_CLIENT_INT_MSG)
-		dev_dbg(dev, "message done interrupt received\n");
 
-	if (reg & PCIE_CLIENT_INT_HOT_RST)
-		dev_dbg(dev, "hot reset interrupt received\n");
-
-	if (reg & PCIE_CLIENT_INT_DPA)
-		dev_dbg(dev, "dpa interrupt received\n");
-
-	if (reg & PCIE_CLIENT_INT_FATAL_ERR)
-		dev_dbg(dev, "fatal error interrupt received\n");
-
-	if (reg & PCIE_CLIENT_INT_NFATAL_ERR)
-		dev_dbg(dev, "non fatal error interrupt received\n");
-
-	if (reg & PCIE_CLIENT_INT_CORR_ERR)
-		dev_dbg(dev, "correctable error interrupt received\n");
-
-	if (reg & PCIE_CLIENT_INT_PHY)
-		dev_dbg(dev, "phy interrupt received\n");
+	for (int i = 0; i < ARRAY_SIZE(pcie_client_irq_info); i++) {
+		if (reg & pcie_client_irq_info[i].bit)
+			dev_dbg(dev, "%s interrupt received\n",
+				pcie_client_irq_info[i].msg);
+	}
 
 	rockchip_pcie_write(rockchip, reg & (PCIE_CLIENT_INT_LEGACY_DONE |
 			      PCIE_CLIENT_INT_MSG | PCIE_CLIENT_INT_HOT_RST |
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/4] PCI: rockchip-host: Remove unused header includes
  2025-05-09 15:53 [PATCH 0/4] Fix interrupt log message Hans Zhang
                   ` (2 preceding siblings ...)
  2025-05-09 15:54 ` [PATCH 3/4] PCI: rockchip-host: Refactor IRQ handling with info arrays Hans Zhang
@ 2025-05-09 15:54 ` Hans Zhang
  3 siblings, 0 replies; 7+ messages in thread
From: Hans Zhang @ 2025-05-09 15:54 UTC (permalink / raw)
  To: shawn.lin, lpieralisi, kw, bhelgaas, heiko, manivannan.sadhasivam
  Cc: robh, linux-pci, linux-kernel, linux-arm-kernel, linux-rockchip,
	Hans Zhang

Clean up the driver by removing unnecessary header includes
(e.g., <linux/clk.h>, <linux/reset.h>) that are no longer referenced
after refactoring. This improves code readability and build efficiency.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/pcie-rockchip-host.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index ba1f7832bda5..e2c603634bda 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -12,26 +12,17 @@
  */
 
 #include <linux/bitrev.h>
-#include <linux/clk.h>
-#include <linux/delay.h>
 #include <linux/gpio/consumer.h>
-#include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/iopoll.h>
 #include <linux/irq.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
-#include <linux/kernel.h>
-#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_pci.h>
-#include <linux/pci.h>
-#include <linux/pci_ids.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
-#include <linux/reset.h>
-#include <linux/regmap.h>
 
 #include "../pci.h"
 #include "pcie-rockchip.h"
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/4] PCI: rockchip-host: Refactor IRQ handling with info arrays
  2025-05-09 15:54 ` [PATCH 3/4] PCI: rockchip-host: Refactor IRQ handling with info arrays Hans Zhang
@ 2025-05-13 10:40   ` Krzysztof Wilczyński
  2025-05-13 15:00     ` Hans Zhang
  0 siblings, 1 reply; 7+ messages in thread
From: Krzysztof Wilczyński @ 2025-05-13 10:40 UTC (permalink / raw)
  To: Hans Zhang
  Cc: shawn.lin, lpieralisi, bhelgaas, heiko, manivannan.sadhasivam,
	robh, linux-pci, linux-kernel, linux-arm-kernel, linux-rockchip

Hello,

Thank you for the patch and the proposed changes.

> Replace repetitive if-conditions for IRQ status checks with structured
> arrays (`pcie_subsys_irq_info` and `pcie_client_irq_info`) and loop-based
> logging. This simplifies maintenance and reduces code duplication.
[...]
> +static const struct rockchip_irq_info pcie_subsys_irq_info[] = {
> +	{ PCIE_CORE_INT_PRFPE,
> +	  "parity error detected while reading from the PNP receive FIFO RAM" },
> +	{ PCIE_CORE_INT_CRFPE,
> +	  "parity error detected while reading from the Completion Receive FIFO RAM" },
> +	{ PCIE_CORE_INT_RRPE,
> +	  "parity error detected while reading from replay buffer RAM" },
> +	{ PCIE_CORE_INT_PRFO, "overflow occurred in the PNP receive FIFO" },
> +	{ PCIE_CORE_INT_CRFO,
> +	  "overflow occurred in the completion receive FIFO" },
> +	{ PCIE_CORE_INT_RT, "replay timer timed out" },
> +	{ PCIE_CORE_INT_RTR,
> +	  "replay timer rolled over after 4 transmissions of the same TLP" },
> +	{ PCIE_CORE_INT_PE, "phy error detected on receive side" },
> +	{ PCIE_CORE_INT_MTR, "malformed TLP received from the link" },
> +	{ PCIE_CORE_INT_UCR, "Unexpected Completion received from the link" },
> +	{ PCIE_CORE_INT_FCE,
> +	  "an error was observed in the flow control advertisements from the other side" },
> +	{ PCIE_CORE_INT_CT, "a request timed out waiting for completion" },
> +	{ PCIE_CORE_INT_UTC, "unmapped TC error" },
> +	{ PCIE_CORE_INT_MMVC, "MSI mask register changes" },
> +};
> +
> +static const struct rockchip_irq_info pcie_client_irq_info[] = {
> +	{ PCIE_CLIENT_INT_LEGACY_DONE, "legacy done" },
> +	{ PCIE_CLIENT_INT_MSG, "message done" },
> +	{ PCIE_CLIENT_INT_HOT_RST, "hot reset" },
> +	{ PCIE_CLIENT_INT_DPA, "dpa" },
> +	{ PCIE_CLIENT_INT_FATAL_ERR, "fatal error" },
> +	{ PCIE_CLIENT_INT_NFATAL_ERR, "Non fatal error" },
> +	{ PCIE_CLIENT_INT_CORR_ERR, "correctable error" },
> +	{ PCIE_CLIENT_INT_PHY, "phy" },
> +};
> +
>  static void rockchip_pcie_enable_bw_int(struct rockchip_pcie *rockchip)
>  {
>  	u32 status;
> @@ -411,47 +450,11 @@ static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
>  	if (reg & PCIE_CLIENT_INT_LOCAL) {
>  		dev_dbg(dev, "local interrupt received\n");
>  		sub_reg = rockchip_pcie_read(rockchip, PCIE_CORE_INT_STATUS);
> -		if (sub_reg & PCIE_CORE_INT_PRFPE)
> -			dev_dbg(dev, "parity error detected while reading from the PNP receive FIFO RAM\n");
> -
> -		if (sub_reg & PCIE_CORE_INT_CRFPE)
> -			dev_dbg(dev, "parity error detected while reading from the Completion Receive FIFO RAM\n");
> -
> -		if (sub_reg & PCIE_CORE_INT_RRPE)
> -			dev_dbg(dev, "parity error detected while reading from replay buffer RAM\n");
> -
> -		if (sub_reg & PCIE_CORE_INT_PRFO)
> -			dev_dbg(dev, "overflow occurred in the PNP receive FIFO\n");
> -
> -		if (sub_reg & PCIE_CORE_INT_CRFO)
> -			dev_dbg(dev, "overflow occurred in the completion receive FIFO\n");
> -
> -		if (sub_reg & PCIE_CORE_INT_RT)
> -			dev_dbg(dev, "replay timer timed out\n");
> -
> -		if (sub_reg & PCIE_CORE_INT_RTR)
> -			dev_dbg(dev, "replay timer rolled over after 4 transmissions of the same TLP\n");
> -
> -		if (sub_reg & PCIE_CORE_INT_PE)
> -			dev_dbg(dev, "phy error detected on receive side\n");
>  
> -		if (sub_reg & PCIE_CORE_INT_MTR)
> -			dev_dbg(dev, "malformed TLP received from the link\n");
> -
> -		if (sub_reg & PCIE_CORE_INT_UCR)
> -			dev_dbg(dev, "Unexpected Completion received from the link\n");
> -
> -		if (sub_reg & PCIE_CORE_INT_FCE)
> -			dev_dbg(dev, "an error was observed in the flow control advertisements from the other side\n");
> -
> -		if (sub_reg & PCIE_CORE_INT_CT)
> -			dev_dbg(dev, "a request timed out waiting for completion\n");
> -
> -		if (sub_reg & PCIE_CORE_INT_UTC)
> -			dev_dbg(dev, "unmapped TC error\n");
> -
> -		if (sub_reg & PCIE_CORE_INT_MMVC)
> -			dev_dbg(dev, "MSI mask register changes\n");
> +		for (int i = 0; i < ARRAY_SIZE(pcie_subsys_irq_info); i++) {
> +			if (sub_reg & pcie_subsys_irq_info[i].bit)
> +				dev_dbg(dev, "%s\n", pcie_subsys_irq_info[i].msg);
> +		}
>  
>  		rockchip_pcie_write(rockchip, sub_reg, PCIE_CORE_INT_STATUS);
>  	} else if (reg & PCIE_CLIENT_INT_PHY) {
> @@ -473,29 +476,12 @@ static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg)
>  	u32 reg;
>  
>  	reg = rockchip_pcie_read(rockchip, PCIE_CLIENT_INT_STATUS);
> -	if (reg & PCIE_CLIENT_INT_LEGACY_DONE)
> -		dev_dbg(dev, "legacy done interrupt received\n");
> -
> -	if (reg & PCIE_CLIENT_INT_MSG)
> -		dev_dbg(dev, "message done interrupt received\n");
>  
> -	if (reg & PCIE_CLIENT_INT_HOT_RST)
> -		dev_dbg(dev, "hot reset interrupt received\n");
> -
> -	if (reg & PCIE_CLIENT_INT_DPA)
> -		dev_dbg(dev, "dpa interrupt received\n");
> -
> -	if (reg & PCIE_CLIENT_INT_FATAL_ERR)
> -		dev_dbg(dev, "fatal error interrupt received\n");
> -
> -	if (reg & PCIE_CLIENT_INT_NFATAL_ERR)
> -		dev_dbg(dev, "non fatal error interrupt received\n");
> -
> -	if (reg & PCIE_CLIENT_INT_CORR_ERR)
> -		dev_dbg(dev, "correctable error interrupt received\n");
> -
> -	if (reg & PCIE_CLIENT_INT_PHY)
> -		dev_dbg(dev, "phy interrupt received\n");
> +	for (int i = 0; i < ARRAY_SIZE(pcie_client_irq_info); i++) {
> +		if (reg & pcie_client_irq_info[i].bit)
> +			dev_dbg(dev, "%s interrupt received\n",
> +				pcie_client_irq_info[i].msg);

Why do you think that this is better?

Other patches in this series seem sensible, but this one does not stands
out as something that needs to be changed.

Thank you!

	Krzysztof

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/4] PCI: rockchip-host: Refactor IRQ handling with info arrays
  2025-05-13 10:40   ` Krzysztof Wilczyński
@ 2025-05-13 15:00     ` Hans Zhang
  0 siblings, 0 replies; 7+ messages in thread
From: Hans Zhang @ 2025-05-13 15:00 UTC (permalink / raw)
  To: Krzysztof Wilczyński
  Cc: shawn.lin, lpieralisi, bhelgaas, heiko, manivannan.sadhasivam,
	robh, linux-pci, linux-kernel, linux-arm-kernel, linux-rockchip



On 2025/5/13 18:40, Krzysztof Wilczyński wrote:
> Hello,
> 
> Thank you for the patch and the proposed changes.
> 
>> Replace repetitive if-conditions for IRQ status checks with structured
>> arrays (`pcie_subsys_irq_info` and `pcie_client_irq_info`) and loop-based
>> logging. This simplifies maintenance and reduces code duplication.
> [...]
>> +static const struct rockchip_irq_info pcie_subsys_irq_info[] = {
>> +	{ PCIE_CORE_INT_PRFPE,
>> +	  "parity error detected while reading from the PNP receive FIFO RAM" },
>> +	{ PCIE_CORE_INT_CRFPE,
>> +	  "parity error detected while reading from the Completion Receive FIFO RAM" },
>> +	{ PCIE_CORE_INT_RRPE,
>> +	  "parity error detected while reading from replay buffer RAM" },
>> +	{ PCIE_CORE_INT_PRFO, "overflow occurred in the PNP receive FIFO" },
>> +	{ PCIE_CORE_INT_CRFO,
>> +	  "overflow occurred in the completion receive FIFO" },
>> +	{ PCIE_CORE_INT_RT, "replay timer timed out" },
>> +	{ PCIE_CORE_INT_RTR,
>> +	  "replay timer rolled over after 4 transmissions of the same TLP" },
>> +	{ PCIE_CORE_INT_PE, "phy error detected on receive side" },
>> +	{ PCIE_CORE_INT_MTR, "malformed TLP received from the link" },
>> +	{ PCIE_CORE_INT_UCR, "Unexpected Completion received from the link" },
>> +	{ PCIE_CORE_INT_FCE,
>> +	  "an error was observed in the flow control advertisements from the other side" },
>> +	{ PCIE_CORE_INT_CT, "a request timed out waiting for completion" },
>> +	{ PCIE_CORE_INT_UTC, "unmapped TC error" },
>> +	{ PCIE_CORE_INT_MMVC, "MSI mask register changes" },
>> +};
>> +
>> +static const struct rockchip_irq_info pcie_client_irq_info[] = {
>> +	{ PCIE_CLIENT_INT_LEGACY_DONE, "legacy done" },
>> +	{ PCIE_CLIENT_INT_MSG, "message done" },
>> +	{ PCIE_CLIENT_INT_HOT_RST, "hot reset" },
>> +	{ PCIE_CLIENT_INT_DPA, "dpa" },
>> +	{ PCIE_CLIENT_INT_FATAL_ERR, "fatal error" },
>> +	{ PCIE_CLIENT_INT_NFATAL_ERR, "Non fatal error" },
>> +	{ PCIE_CLIENT_INT_CORR_ERR, "correctable error" },
>> +	{ PCIE_CLIENT_INT_PHY, "phy" },
>> +};
>> +
>>   static void rockchip_pcie_enable_bw_int(struct rockchip_pcie *rockchip)
>>   {
>>   	u32 status;
>> @@ -411,47 +450,11 @@ static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
>>   	if (reg & PCIE_CLIENT_INT_LOCAL) {
>>   		dev_dbg(dev, "local interrupt received\n");
>>   		sub_reg = rockchip_pcie_read(rockchip, PCIE_CORE_INT_STATUS);
>> -		if (sub_reg & PCIE_CORE_INT_PRFPE)
>> -			dev_dbg(dev, "parity error detected while reading from the PNP receive FIFO RAM\n");
>> -
>> -		if (sub_reg & PCIE_CORE_INT_CRFPE)
>> -			dev_dbg(dev, "parity error detected while reading from the Completion Receive FIFO RAM\n");
>> -
>> -		if (sub_reg & PCIE_CORE_INT_RRPE)
>> -			dev_dbg(dev, "parity error detected while reading from replay buffer RAM\n");
>> -
>> -		if (sub_reg & PCIE_CORE_INT_PRFO)
>> -			dev_dbg(dev, "overflow occurred in the PNP receive FIFO\n");
>> -
>> -		if (sub_reg & PCIE_CORE_INT_CRFO)
>> -			dev_dbg(dev, "overflow occurred in the completion receive FIFO\n");
>> -
>> -		if (sub_reg & PCIE_CORE_INT_RT)
>> -			dev_dbg(dev, "replay timer timed out\n");
>> -
>> -		if (sub_reg & PCIE_CORE_INT_RTR)
>> -			dev_dbg(dev, "replay timer rolled over after 4 transmissions of the same TLP\n");
>> -
>> -		if (sub_reg & PCIE_CORE_INT_PE)
>> -			dev_dbg(dev, "phy error detected on receive side\n");
>>   
>> -		if (sub_reg & PCIE_CORE_INT_MTR)
>> -			dev_dbg(dev, "malformed TLP received from the link\n");
>> -
>> -		if (sub_reg & PCIE_CORE_INT_UCR)
>> -			dev_dbg(dev, "Unexpected Completion received from the link\n");
>> -
>> -		if (sub_reg & PCIE_CORE_INT_FCE)
>> -			dev_dbg(dev, "an error was observed in the flow control advertisements from the other side\n");
>> -
>> -		if (sub_reg & PCIE_CORE_INT_CT)
>> -			dev_dbg(dev, "a request timed out waiting for completion\n");
>> -
>> -		if (sub_reg & PCIE_CORE_INT_UTC)
>> -			dev_dbg(dev, "unmapped TC error\n");
>> -
>> -		if (sub_reg & PCIE_CORE_INT_MMVC)
>> -			dev_dbg(dev, "MSI mask register changes\n");
>> +		for (int i = 0; i < ARRAY_SIZE(pcie_subsys_irq_info); i++) {
>> +			if (sub_reg & pcie_subsys_irq_info[i].bit)
>> +				dev_dbg(dev, "%s\n", pcie_subsys_irq_info[i].msg);
>> +		}
>>   
>>   		rockchip_pcie_write(rockchip, sub_reg, PCIE_CORE_INT_STATUS);
>>   	} else if (reg & PCIE_CLIENT_INT_PHY) {
>> @@ -473,29 +476,12 @@ static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg)
>>   	u32 reg;
>>   
>>   	reg = rockchip_pcie_read(rockchip, PCIE_CLIENT_INT_STATUS);
>> -	if (reg & PCIE_CLIENT_INT_LEGACY_DONE)
>> -		dev_dbg(dev, "legacy done interrupt received\n");
>> -
>> -	if (reg & PCIE_CLIENT_INT_MSG)
>> -		dev_dbg(dev, "message done interrupt received\n");
>>   
>> -	if (reg & PCIE_CLIENT_INT_HOT_RST)
>> -		dev_dbg(dev, "hot reset interrupt received\n");
>> -
>> -	if (reg & PCIE_CLIENT_INT_DPA)
>> -		dev_dbg(dev, "dpa interrupt received\n");
>> -
>> -	if (reg & PCIE_CLIENT_INT_FATAL_ERR)
>> -		dev_dbg(dev, "fatal error interrupt received\n");
>> -
>> -	if (reg & PCIE_CLIENT_INT_NFATAL_ERR)
>> -		dev_dbg(dev, "non fatal error interrupt received\n");
>> -
>> -	if (reg & PCIE_CLIENT_INT_CORR_ERR)
>> -		dev_dbg(dev, "correctable error interrupt received\n");
>> -
>> -	if (reg & PCIE_CLIENT_INT_PHY)
>> -		dev_dbg(dev, "phy interrupt received\n");
>> +	for (int i = 0; i < ARRAY_SIZE(pcie_client_irq_info); i++) {
>> +		if (reg & pcie_client_irq_info[i].bit)
>> +			dev_dbg(dev, "%s interrupt received\n",
>> +				pcie_client_irq_info[i].msg);
> 
> Why do you think that this is better?
> 
> Other patches in this series seem sensible, but this one does not stands
> out as something that needs to be changed.
> 

Dear Krzysztof

Thank you very much for your reply.

In the interrupt handling function: rockchip_pcie_subsys_irq_handler, 
personally, I think the amount of code can be simplified and it looks 
more concise.

There are many repetitive "interrupt received" print messages in the 
interrupt handling function: rockchip_pcie_client_irq_handler. My 
original intention was to simplify it.

If you think it's not necessary, I will drop this patch.

Best regards,
Hans


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-05-13 15:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-09 15:53 [PATCH 0/4] Fix interrupt log message Hans Zhang
2025-05-09 15:53 ` [PATCH 1/4] PCI: rockchip-host: Fix "Unexpected Completion" " Hans Zhang
2025-05-09 15:54 ` [PATCH 2/4] PCI: rockchip-host: Correct non-fatal error " Hans Zhang
2025-05-09 15:54 ` [PATCH 3/4] PCI: rockchip-host: Refactor IRQ handling with info arrays Hans Zhang
2025-05-13 10:40   ` Krzysztof Wilczyński
2025-05-13 15:00     ` Hans Zhang
2025-05-09 15:54 ` [PATCH 4/4] PCI: rockchip-host: Remove unused header includes Hans Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox