* [PATCH v3 1/3] PCI: dw-rockchip: Remove unused PCIE_CLIENT_GENERAL_DEBUG
2025-04-23 15:32 [PATCH v3 0/3] PCI: dw-rockchip: Reorganize register and bitfield definitions Hans Zhang
@ 2025-04-23 15:32 ` Hans Zhang
2025-04-23 15:32 ` [PATCH v3 2/3] PCI: dw-rockchip: Reorganize register and bitfield definitions Hans Zhang
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Hans Zhang @ 2025-04-23 15:32 UTC (permalink / raw)
To: lpieralisi, kw, bhelgaas, heiko
Cc: manivannan.sadhasivam, robh, jingoohan1, shawn.lin, linux-pci,
linux-kernel, linux-arm-kernel, linux-rockchip, Hans Zhang,
Niklas Cassel
The PCIE_CLIENT_GENERAL_DEBUG register offset is defined but never
used in the driver. Its presence adds noise to the register map and
may mislead future developers.
Remove this redundant definition to keep the register list minimal
and aligned with actual hardware usage.
Signed-off-by: Hans Zhang <18255117159@163.com>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 0e0c09bafd63..fd5827bbfae3 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -54,7 +54,6 @@
#define PCIE_CLIENT_GENERAL_CONTROL 0x0
#define PCIE_CLIENT_INTR_STATUS_LEGACY 0x8
#define PCIE_CLIENT_INTR_MASK_LEGACY 0x1c
-#define PCIE_CLIENT_GENERAL_DEBUG 0x104
#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
#define PCIE_CLIENT_LTSSM_STATUS 0x300
#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 2/3] PCI: dw-rockchip: Reorganize register and bitfield definitions
2025-04-23 15:32 [PATCH v3 0/3] PCI: dw-rockchip: Reorganize register and bitfield definitions Hans Zhang
2025-04-23 15:32 ` [PATCH v3 1/3] PCI: dw-rockchip: Remove unused PCIE_CLIENT_GENERAL_DEBUG Hans Zhang
@ 2025-04-23 15:32 ` Hans Zhang
2025-04-23 17:50 ` Niklas Cassel
2025-04-23 15:32 ` [PATCH v3 3/3] PCI: dw-rockchip: Unify link status checks with FIELD_GET Hans Zhang
` (2 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Hans Zhang @ 2025-04-23 15:32 UTC (permalink / raw)
To: lpieralisi, kw, bhelgaas, heiko
Cc: manivannan.sadhasivam, robh, jingoohan1, shawn.lin, linux-pci,
linux-kernel, linux-arm-kernel, linux-rockchip, Hans Zhang
Register definitions were scattered with ambiguous names (e.g.,
PCIE_RDLH_LINK_UP_CHGED in PCIE_CLIENT_INTR_STATUS_MISC) and lacked
hierarchical grouping. Magic values for bit operations reduced code
clarity.
Group registers and their associated bitfields logically. This improves
maintainability and aligns the code with hardware documentation.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 71 ++++++++++++-------
1 file changed, 45 insertions(+), 26 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index fd5827bbfae3..96ca394da42c 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -34,30 +34,49 @@
#define to_rockchip_pcie(x) dev_get_drvdata((x)->dev)
-#define PCIE_CLIENT_RC_MODE HIWORD_UPDATE_BIT(0x40)
-#define PCIE_CLIENT_EP_MODE HIWORD_UPDATE(0xf0, 0x0)
-#define PCIE_CLIENT_ENABLE_LTSSM HIWORD_UPDATE_BIT(0xc)
-#define PCIE_CLIENT_DISABLE_LTSSM HIWORD_UPDATE(0x0c, 0x8)
-#define PCIE_CLIENT_INTR_STATUS_MSG_RX 0x04
-#define PCIE_CLIENT_INTR_STATUS_MISC 0x10
-#define PCIE_CLIENT_INTR_MASK_MISC 0x24
-#define PCIE_CLIENT_POWER 0x2c
-#define PCIE_CLIENT_MSG_GEN 0x34
-#define PME_READY_ENTER_L23 BIT(3)
-#define PME_TURN_OFF (BIT(4) | BIT(20))
-#define PME_TO_ACK (BIT(9) | BIT(25))
-#define PCIE_SMLH_LINKUP BIT(16)
-#define PCIE_RDLH_LINKUP BIT(17)
-#define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP)
-#define PCIE_RDLH_LINK_UP_CHGED BIT(1)
-#define PCIE_LINK_REQ_RST_NOT_INT BIT(2)
-#define PCIE_CLIENT_GENERAL_CONTROL 0x0
+/* General Control Register */
+#define PCIE_CLIENT_GENERAL_CON 0x0
+#define PCIE_CLIENT_RC_MODE HIWORD_UPDATE_BIT(0x40)
+#define PCIE_CLIENT_EP_MODE HIWORD_UPDATE(0xf0, 0x0)
+#define PCIE_CLIENT_ENABLE_LTSSM HIWORD_UPDATE_BIT(0xc)
+#define PCIE_CLIENT_DISABLE_LTSSM HIWORD_UPDATE(0x0c, 0x8)
+
+/* Interrupt Status Register Related to Message Reception */
+#define PCIE_CLIENT_INTR_STATUS_MSG_RX 0x4
+
+/* Interrupt Status Register Related to Legacy Interrupt */
#define PCIE_CLIENT_INTR_STATUS_LEGACY 0x8
+
+/* Interrupt Status Register Related to Miscellaneous Operation */
+#define PCIE_CLIENT_INTR_STATUS_MISC 0x10
+#define PCIE_RDLH_LINK_UP_CHGED BIT(1)
+#define PCIE_LINK_REQ_RST_NOT_INT BIT(2)
+
+/* Interrupt Mask Register Related to Legacy Interrupt */
#define PCIE_CLIENT_INTR_MASK_LEGACY 0x1c
+
+/* Interrupt Mask Register Related to Miscellaneous Operation */
+#define PCIE_CLIENT_INTR_MASK_MISC 0x24
+
+/* Power Management Control Register */
+#define PCIE_CLIENT_POWER_CON 0x2c
+#define PME_READY_ENTER_L23 BIT(3)
+
+/* Message Generation Control Register */
+#define PCIE_CLIENT_MSG_GEN_CON 0x34
+#define PME_TURN_OFF HIWORD_UPDATE_BIT(BIT(4))
+#define PME_TO_ACK HIWORD_UPDATE_BIT(BIT(9))
+
+/* Hot Reset Control Register */
#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
+#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
+
+/* LTSSM Status Register */
#define PCIE_CLIENT_LTSSM_STATUS 0x300
-#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
-#define PCIE_LTSSM_STATUS_MASK GENMASK(5, 0)
+#define PCIE_SMLH_LINKUP BIT(16)
+#define PCIE_RDLH_LINKUP BIT(17)
+#define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP)
+#define PCIE_LTSSM_STATUS_MASK GENMASK(5, 0)
struct rockchip_pcie {
struct dw_pcie pci;
@@ -176,13 +195,13 @@ static u32 rockchip_pcie_get_pure_ltssm(struct dw_pcie *pci)
static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip)
{
rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM,
- PCIE_CLIENT_GENERAL_CONTROL);
+ PCIE_CLIENT_GENERAL_CON);
}
static void rockchip_pcie_disable_ltssm(struct rockchip_pcie *rockchip)
{
rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_DISABLE_LTSSM,
- PCIE_CLIENT_GENERAL_CONTROL);
+ PCIE_CLIENT_GENERAL_CON);
}
static int rockchip_pcie_link_up(struct dw_pcie *pci)
@@ -274,8 +293,8 @@ static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
u32 status;
/* 1. Broadcast PME_Turn_Off Message, bit 4 self-clear once done */
- rockchip_pcie_writel_apb(rockchip, PME_TURN_OFF, PCIE_CLIENT_MSG_GEN);
- ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_MSG_GEN,
+ rockchip_pcie_writel_apb(rockchip, PME_TURN_OFF, PCIE_CLIENT_MSG_GEN_CON);
+ ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_MSG_GEN_CON,
status, !(status & BIT(4)), PCIE_PME_TO_L2_TIMEOUT_US / 10,
PCIE_PME_TO_L2_TIMEOUT_US);
if (ret) {
@@ -294,7 +313,7 @@ static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
/* 3. Clear PME_TO_Ack and Wait for ready to enter L23 message */
rockchip_pcie_writel_apb(rockchip, PME_TO_ACK, PCIE_CLIENT_INTR_STATUS_MSG_RX);
- ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_POWER,
+ ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_POWER_CON,
status, status & PME_READY_ENTER_L23,
PCIE_PME_TO_L2_TIMEOUT_US / 10,
PCIE_PME_TO_L2_TIMEOUT_US);
@@ -552,7 +571,7 @@ static void rockchip_pcie_ltssm_enable_control_mode(struct rockchip_pcie *rockch
val = HIWORD_UPDATE_BIT(PCIE_LTSSM_ENABLE_ENHANCE);
rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
- rockchip_pcie_writel_apb(rockchip, mode, PCIE_CLIENT_GENERAL_CONTROL);
+ rockchip_pcie_writel_apb(rockchip, mode, PCIE_CLIENT_GENERAL_CON);
}
static void rockchip_pcie_unmask_dll_indicator(struct rockchip_pcie *rockchip)
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 2/3] PCI: dw-rockchip: Reorganize register and bitfield definitions
2025-04-23 15:32 ` [PATCH v3 2/3] PCI: dw-rockchip: Reorganize register and bitfield definitions Hans Zhang
@ 2025-04-23 17:50 ` Niklas Cassel
0 siblings, 0 replies; 8+ messages in thread
From: Niklas Cassel @ 2025-04-23 17:50 UTC (permalink / raw)
To: Hans Zhang
Cc: lpieralisi, kw, bhelgaas, heiko, manivannan.sadhasivam, robh,
jingoohan1, shawn.lin, linux-pci, linux-kernel, linux-arm-kernel,
linux-rockchip
On Wed, Apr 23, 2025 at 11:32:13PM +0800, Hans Zhang wrote:
> Register definitions were scattered with ambiguous names (e.g.,
> PCIE_RDLH_LINK_UP_CHGED in PCIE_CLIENT_INTR_STATUS_MISC) and lacked
> hierarchical grouping. Magic values for bit operations reduced code
> clarity.
>
> Group registers and their associated bitfields logically. This improves
> maintainability and aligns the code with hardware documentation.
>
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
Reviewed-by: Niklas Cassel <cassel@kernel.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 3/3] PCI: dw-rockchip: Unify link status checks with FIELD_GET
2025-04-23 15:32 [PATCH v3 0/3] PCI: dw-rockchip: Reorganize register and bitfield definitions Hans Zhang
2025-04-23 15:32 ` [PATCH v3 1/3] PCI: dw-rockchip: Remove unused PCIE_CLIENT_GENERAL_DEBUG Hans Zhang
2025-04-23 15:32 ` [PATCH v3 2/3] PCI: dw-rockchip: Reorganize register and bitfield definitions Hans Zhang
@ 2025-04-23 15:32 ` Hans Zhang
2025-04-26 16:40 ` [PATCH v3 0/3] PCI: dw-rockchip: Reorganize register and bitfield definitions Manivannan Sadhasivam
2025-04-27 11:10 ` Manivannan Sadhasivam
4 siblings, 0 replies; 8+ messages in thread
From: Hans Zhang @ 2025-04-23 15:32 UTC (permalink / raw)
To: lpieralisi, kw, bhelgaas, heiko
Cc: manivannan.sadhasivam, robh, jingoohan1, shawn.lin, linux-pci,
linux-kernel, linux-arm-kernel, linux-rockchip, Hans Zhang,
Niklas Cassel
Link-up detection manually checked PCIE_LINKUP bits across RC/EP modes,
leading to code duplication. Centralize the logic using FIELD_GET. This
removes redundancy and abstracts hardware-specific bit masking, ensuring
consistent link state handling.
Signed-off-by: Hans Zhang <18255117159@163.com>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 21 +++++++------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 96ca394da42c..f2f8d8ada7a5 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -8,6 +8,7 @@
* Author: Simon Xue <xxm@rock-chips.com>
*/
+#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/gpio/consumer.h>
#include <linux/irqchip/chained_irq.h>
@@ -73,9 +74,8 @@
/* LTSSM Status Register */
#define PCIE_CLIENT_LTSSM_STATUS 0x300
-#define PCIE_SMLH_LINKUP BIT(16)
-#define PCIE_RDLH_LINKUP BIT(17)
-#define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP)
+#define PCIE_LINKUP 0x3
+#define PCIE_LINKUP_MASK GENMASK(17, 16)
#define PCIE_LTSSM_STATUS_MASK GENMASK(5, 0)
struct rockchip_pcie {
@@ -209,10 +209,7 @@ static int rockchip_pcie_link_up(struct dw_pcie *pci)
struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
u32 val = rockchip_pcie_get_ltssm(rockchip);
- if ((val & PCIE_LINKUP) == PCIE_LINKUP)
- return 1;
-
- return 0;
+ return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
}
static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
@@ -512,7 +509,7 @@ static irqreturn_t rockchip_pcie_rc_sys_irq_thread(int irq, void *arg)
struct dw_pcie *pci = &rockchip->pci;
struct dw_pcie_rp *pp = &pci->pp;
struct device *dev = pci->dev;
- u32 reg, val;
+ u32 reg;
reg = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_STATUS_MISC);
rockchip_pcie_writel_apb(rockchip, reg, PCIE_CLIENT_INTR_STATUS_MISC);
@@ -521,8 +518,7 @@ static irqreturn_t rockchip_pcie_rc_sys_irq_thread(int irq, void *arg)
dev_dbg(dev, "LTSSM_STATUS: %#x\n", rockchip_pcie_get_ltssm(rockchip));
if (reg & PCIE_RDLH_LINK_UP_CHGED) {
- val = rockchip_pcie_get_ltssm(rockchip);
- if ((val & PCIE_LINKUP) == PCIE_LINKUP) {
+ if (rockchip_pcie_link_up(pci)) {
dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
/* Rescan the bus to enumerate endpoint devices */
pci_lock_rescan_remove();
@@ -539,7 +535,7 @@ static irqreturn_t rockchip_pcie_ep_sys_irq_thread(int irq, void *arg)
struct rockchip_pcie *rockchip = arg;
struct dw_pcie *pci = &rockchip->pci;
struct device *dev = pci->dev;
- u32 reg, val;
+ u32 reg;
reg = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_STATUS_MISC);
rockchip_pcie_writel_apb(rockchip, reg, PCIE_CLIENT_INTR_STATUS_MISC);
@@ -553,8 +549,7 @@ static irqreturn_t rockchip_pcie_ep_sys_irq_thread(int irq, void *arg)
}
if (reg & PCIE_RDLH_LINK_UP_CHGED) {
- val = rockchip_pcie_get_ltssm(rockchip);
- if ((val & PCIE_LINKUP) == PCIE_LINKUP) {
+ if (rockchip_pcie_link_up(pci)) {
dev_dbg(dev, "link up\n");
dw_pcie_ep_linkup(&pci->ep);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 0/3] PCI: dw-rockchip: Reorganize register and bitfield definitions
2025-04-23 15:32 [PATCH v3 0/3] PCI: dw-rockchip: Reorganize register and bitfield definitions Hans Zhang
` (2 preceding siblings ...)
2025-04-23 15:32 ` [PATCH v3 3/3] PCI: dw-rockchip: Unify link status checks with FIELD_GET Hans Zhang
@ 2025-04-26 16:40 ` Manivannan Sadhasivam
2025-04-27 11:10 ` Manivannan Sadhasivam
4 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2025-04-26 16:40 UTC (permalink / raw)
To: Hans Zhang
Cc: lpieralisi, kw, bhelgaas, heiko, robh, jingoohan1, shawn.lin,
linux-pci, linux-kernel, linux-arm-kernel, linux-rockchip
On Wed, Apr 23, 2025 at 11:32:11PM +0800, Hans Zhang wrote:
> 1. PCI: dw-rockchip: Remove unused PCIE_CLIENT_GENERAL_DEBUG
> 2. PCI: dw-rockchip: Reorganize register and bitfield definitions
> 3. PCI: dw-rockchip: Unify link status checks with FIELD_GET
>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
- Mani
> ---
> Changes for v3:
> - Delete the redundant Spaces in the comments of patch 2/3.
>
> Changes for v2:
> - Add register annotations to enhance readability.
> - Use macro definitions instead of magic numbers.
>
> https://patchwork.kernel.org/project/linux-pci/patch/20250416151926.140202-1-18255117159@163.com/
>
> Bjorn Helgaas:
> These would be material for a separate patch:
>
> - The #defines for register offsets and bits are kind of a mess,
> e.g., PCIE_SMLH_LINKUP, PCIE_RDLH_LINKUP, PCIE_LINKUP,
> PCIE_L0S_ENTRY, and PCIE_LTSSM_STATUS_MASK are in
> PCIE_CLIENT_LTSSM_STATUS, but you couldn't tell that from the
> names, and they're not even defined together.
>
> - Same for PCIE_RDLH_LINK_UP_CHGED, PCIE_LINK_REQ_RST_NOT_INT,
> PCIE_RDLH_LINK_UP_CHGED, which are in PCIE_CLIENT_INTR_STATUS_MISC.
>
> - PCIE_LTSSM_ENABLE_ENHANCE is apparently in PCIE_CLIENT_HOT_RESET_CTRL?
> Sure wouldn't guess that from the names or the order of #defines.
>
> - PCIE_CLIENT_GENERAL_DEBUG isn't used at all.
>
> - Submissions based on the following v5 patches:
> https://patchwork.kernel.org/project/linux-pci/patch/1744850111-236269-1-git-send-email-shawn.lin@rock-chips.com/
> https://patchwork.kernel.org/project/linux-pci/patch/1744850111-236269-2-git-send-email-shawn.lin@rock-chips.com/
> https://patchwork.kernel.org/project/linux-pci/patch/1744850111-236269-3-git-send-email-shawn.lin@rock-chips.com/
> https://patchwork.kernel.org/project/linux-pci/patch/1744940759-23823-1-git-send-email-shawn.lin@rock-chips.com/
> ---
>
> Hans Zhang (3):
> PCI: dw-rockchip: Remove unused PCIE_CLIENT_GENERAL_DEBUG
> PCI: dw-rockchip: Reorganize register and bitfield definitions
> PCI: dw-rockchip: Unify link status checks with FIELD_GET
>
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 87 +++++++++++--------
> 1 file changed, 50 insertions(+), 37 deletions(-)
>
>
> base-commit: 9d7a0577c9db35c4cc52db90bc415ea248446472
> prerequisite-patch-id: 5d9f110f238212cde763b841f1337d0045d93f5b
> prerequisite-patch-id: b63975b89227a41b9b6d701c9130ee342848c8b6
> prerequisite-patch-id: 46f02da0db4737b46cd06cd0d25ba69b8d789f90
> prerequisite-patch-id: d06e25de3658b73ad85d148728ed3948bfcec731
> --
> 2.25.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3 0/3] PCI: dw-rockchip: Reorganize register and bitfield definitions
2025-04-23 15:32 [PATCH v3 0/3] PCI: dw-rockchip: Reorganize register and bitfield definitions Hans Zhang
` (3 preceding siblings ...)
2025-04-26 16:40 ` [PATCH v3 0/3] PCI: dw-rockchip: Reorganize register and bitfield definitions Manivannan Sadhasivam
@ 2025-04-27 11:10 ` Manivannan Sadhasivam
2025-04-27 12:59 ` Hans Zhang
4 siblings, 1 reply; 8+ messages in thread
From: Manivannan Sadhasivam @ 2025-04-27 11:10 UTC (permalink / raw)
To: Hans Zhang
Cc: lpieralisi, kw, bhelgaas, heiko, robh, jingoohan1, shawn.lin,
linux-pci, linux-kernel, linux-arm-kernel, linux-rockchip
On Wed, Apr 23, 2025 at 11:32:11PM +0800, Hans Zhang wrote:
> 1. PCI: dw-rockchip: Remove unused PCIE_CLIENT_GENERAL_DEBUG
> 2. PCI: dw-rockchip: Reorganize register and bitfield definitions
> 3. PCI: dw-rockchip: Unify link status checks with FIELD_GET
>
> ---
> Changes for v3:
> - Delete the redundant Spaces in the comments of patch 2/3.
>
> Changes for v2:
> - Add register annotations to enhance readability.
> - Use macro definitions instead of magic numbers.
>
> https://patchwork.kernel.org/project/linux-pci/patch/20250416151926.140202-1-18255117159@163.com/
>
> Bjorn Helgaas:
> These would be material for a separate patch:
>
> - The #defines for register offsets and bits are kind of a mess,
> e.g., PCIE_SMLH_LINKUP, PCIE_RDLH_LINKUP, PCIE_LINKUP,
> PCIE_L0S_ENTRY, and PCIE_LTSSM_STATUS_MASK are in
> PCIE_CLIENT_LTSSM_STATUS, but you couldn't tell that from the
> names, and they're not even defined together.
>
> - Same for PCIE_RDLH_LINK_UP_CHGED, PCIE_LINK_REQ_RST_NOT_INT,
> PCIE_RDLH_LINK_UP_CHGED, which are in PCIE_CLIENT_INTR_STATUS_MISC.
>
> - PCIE_LTSSM_ENABLE_ENHANCE is apparently in PCIE_CLIENT_HOT_RESET_CTRL?
> Sure wouldn't guess that from the names or the order of #defines.
>
> - PCIE_CLIENT_GENERAL_DEBUG isn't used at all.
>
> - Submissions based on the following v5 patches:
> https://patchwork.kernel.org/project/linux-pci/patch/1744850111-236269-1-git-send-email-shawn.lin@rock-chips.com/
> https://patchwork.kernel.org/project/linux-pci/patch/1744850111-236269-2-git-send-email-shawn.lin@rock-chips.com/
> https://patchwork.kernel.org/project/linux-pci/patch/1744850111-236269-3-git-send-email-shawn.lin@rock-chips.com/
> https://patchwork.kernel.org/project/linux-pci/patch/1744940759-23823-1-git-send-email-shawn.lin@rock-chips.com/
Because of *this* dependency, I couldn't apply this series. I'd suggest to
respin this series avoiding the above mentioned patch and just rebase on top of
controller/dw-rockchip branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/dw-rockchip
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3 0/3] PCI: dw-rockchip: Reorganize register and bitfield definitions
2025-04-27 11:10 ` Manivannan Sadhasivam
@ 2025-04-27 12:59 ` Hans Zhang
0 siblings, 0 replies; 8+ messages in thread
From: Hans Zhang @ 2025-04-27 12:59 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: lpieralisi, kw, bhelgaas, heiko, robh, jingoohan1, shawn.lin,
linux-pci, linux-kernel, linux-arm-kernel, linux-rockchip
On 2025/4/27 19:10, Manivannan Sadhasivam wrote:
> On Wed, Apr 23, 2025 at 11:32:11PM +0800, Hans Zhang wrote:
>> 1. PCI: dw-rockchip: Remove unused PCIE_CLIENT_GENERAL_DEBUG
>> 2. PCI: dw-rockchip: Reorganize register and bitfield definitions
>> 3. PCI: dw-rockchip: Unify link status checks with FIELD_GET
>>
>> ---
>> Changes for v3:
>> - Delete the redundant Spaces in the comments of patch 2/3.
>>
>> Changes for v2:
>> - Add register annotations to enhance readability.
>> - Use macro definitions instead of magic numbers.
>>
>> https://patchwork.kernel.org/project/linux-pci/patch/20250416151926.140202-1-18255117159@163.com/
>>
>> Bjorn Helgaas:
>> These would be material for a separate patch:
>>
>> - The #defines for register offsets and bits are kind of a mess,
>> e.g., PCIE_SMLH_LINKUP, PCIE_RDLH_LINKUP, PCIE_LINKUP,
>> PCIE_L0S_ENTRY, and PCIE_LTSSM_STATUS_MASK are in
>> PCIE_CLIENT_LTSSM_STATUS, but you couldn't tell that from the
>> names, and they're not even defined together.
>>
>> - Same for PCIE_RDLH_LINK_UP_CHGED, PCIE_LINK_REQ_RST_NOT_INT,
>> PCIE_RDLH_LINK_UP_CHGED, which are in PCIE_CLIENT_INTR_STATUS_MISC.
>>
>> - PCIE_LTSSM_ENABLE_ENHANCE is apparently in PCIE_CLIENT_HOT_RESET_CTRL?
>> Sure wouldn't guess that from the names or the order of #defines.
>>
>> - PCIE_CLIENT_GENERAL_DEBUG isn't used at all.
>>
>> - Submissions based on the following v5 patches:
>> https://patchwork.kernel.org/project/linux-pci/patch/1744850111-236269-1-git-send-email-shawn.lin@rock-chips.com/
>> https://patchwork.kernel.org/project/linux-pci/patch/1744850111-236269-2-git-send-email-shawn.lin@rock-chips.com/
>> https://patchwork.kernel.org/project/linux-pci/patch/1744850111-236269-3-git-send-email-shawn.lin@rock-chips.com/
>
>> https://patchwork.kernel.org/project/linux-pci/patch/1744940759-23823-1-git-send-email-shawn.lin@rock-chips.com/
>
> Because of *this* dependency, I couldn't apply this series. I'd suggest to
> respin this series avoiding the above mentioned patch and just rebase on top of
> controller/dw-rockchip branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/dw-rockchip
Dear Mani,
Thank you very much for your reply and suggestions. I have submitted the
V4 patch based on the controller/dw-rockchip branch. Please help merge
it. :)
V4:
https://patchwork.kernel.org/project/linux-pci/cover/20250427125316.99627-1-18255117159@163.com/
Best regards,
Hans
^ permalink raw reply [flat|nested] 8+ messages in thread