Re: [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF
From: Hans Zhang
Date: Fri Sep 04 2026 - 13:25:07 EST
On 9/5/26 00:12, Bjorn Helgaas wrote:
On Fri, Sep 04, 2026 at 02:18:23PM +0800, Hans Zhang wrote:
The register at offset 0x710 is documented in the Synopsys DesignWare
PCIe Controller Reference Manual as "PORT_LINK_CTRL_OFF" (Port Link
Control Register). The current macro name PCIE_PORT_LINK_CONTROL does
not match the documentation, making it difficult to cross-reference
with the spec when debugging or maintaining the code.
I'm a little skeptical about this change. I agree it's good to match
the spec, and the leading "PCIE_" seems superfluous. But I think it's
pointless to add "_OFF" to every register offset. "PORT_LINK_CTRL"
seems like enough, and it's pretty close to the spec's name.
Adding "_OFF" also invites confusion: does it mean "offset" or "off"
(the opposite of "on")?
Hi Bjorn,
I checked the official documentation of Synopsys. It seems that the suffix "_OFF" is added to each register, and it appears to mean "offset".
It's also nice if names of fields within a register include the
register name, e.g., PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_ASPMC, etc.
For long register names like PCIE_PORT_MULTI_LANE_CTRL, with a
PORT_MLTI_UPCFG_SUPPORT field, you have squint pretty hard to see the
connection between the register and the field, but a name like
"PCIE_PORT_MULTI_LANE_CTRL_UPCFG_SUPPORT" would be just ridiculous.
I agree with what you said. The official documentation of Synopsys currently defines it this way. If the naming is simplified, it would make it harder to search for where the registers are located. This is one of the purposes of this modification.
I will send a document to you and Mani. Or there might be a better way that I would be very happy to accept. I will also make the necessary modifications according to your preferences.
Best regards,
Hans
Rename the macro to PORT_LINK_CTRL_OFF to align with the Synopsys
documentation, and update all usage sites accordingly. No functional
change is intended.
This improves code maintainability and eases future reference to the
controller manual.
Co-developed-by: Feizhou Yu <yufeizhou201@xxxxxxx>
Signed-off-by: Feizhou Yu <yufeizhou201@xxxxxxx>
Signed-off-by: Hans Zhang <18255117159@xxxxxxx>
---
drivers/pci/controller/dwc/pcie-designware.c | 8 ++++----
drivers/pci/controller/dwc/pcie-designware.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 593388f29bdd..9a0d8f0be792 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -894,7 +894,7 @@ static void dw_pcie_link_set_max_link_width(struct dw_pcie *pci, u32 num_lanes)
return;
/* Set the number of lanes */
- plc = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
+ plc = dw_pcie_readl_dbi(pci, PORT_LINK_CTRL_OFF);
plc &= ~PORT_LINK_FAST_LINK_MODE;
plc &= ~PORT_LINK_MODE_MASK;
@@ -922,7 +922,7 @@ static void dw_pcie_link_set_max_link_width(struct dw_pcie *pci, u32 num_lanes)
dev_err(pci->dev, "num-lanes %u: invalid value\n", num_lanes);
return;
}
- dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, plc);
+ dw_pcie_writel_dbi(pci, PORT_LINK_CTRL_OFF, plc);
dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, lwsc);
cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
@@ -1295,10 +1295,10 @@ void dw_pcie_setup(struct dw_pcie *pci)
dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, val);
}
- val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
+ val = dw_pcie_readl_dbi(pci, PORT_LINK_CTRL_OFF);
val &= ~PORT_LINK_FAST_LINK_MODE;
val |= PORT_LINK_DLL_LINK_EN;
- dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
+ dw_pcie_writel_dbi(pci, PORT_LINK_CTRL_OFF, val);
dw_pcie_link_set_max_link_width(pci, pci->num_lanes);
}
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 0735ae940924..77420230433b 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -87,7 +87,7 @@
#define PORT_AFR_L1_ENTRANCE_LAT_SHIFT 27
#define PORT_AFR_L1_ENTRANCE_LAT_MASK GENMASK(29, 27)
-#define PCIE_PORT_LINK_CONTROL 0x710
+#define PORT_LINK_CTRL_OFF 0x710
#define PORT_LINK_DLL_LINK_EN BIT(5)
#define PORT_LINK_FAST_LINK_MODE BIT(7)
#define PORT_LINK_MODE_MASK GENMASK(21, 16)
--
2.43.0