* [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF @ 2026-09-04 6:18 Hans Zhang 2026-09-04 6:24 ` sashiko-bot 2026-09-04 16:12 ` Bjorn Helgaas 0 siblings, 2 replies; 5+ messages in thread From: Hans Zhang @ 2026-09-04 6:18 UTC (permalink / raw) To: jingoohan1, bhelgaas, lpieralisi, kwilczynski, mani Cc: robh, a-garg7, linux-pci, linux-kernel, Hans Zhang, Feizhou Yu 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. 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@163.com> Signed-off-by: Feizhou Yu <yufeizhou201@163.com> Signed-off-by: Hans Zhang <18255117159@163.com> --- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF 2026-09-04 6:18 [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF Hans Zhang @ 2026-09-04 6:24 ` sashiko-bot 2026-09-04 16:12 ` Bjorn Helgaas 1 sibling, 0 replies; 5+ messages in thread From: sashiko-bot @ 2026-09-04 6:24 UTC (permalink / raw) To: Hans Zhang; +Cc: linux-pci > 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. > > 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@163.com> > Signed-off-by: Feizhou Yu <yufeizhou201@163.com> > Signed-off-by: Hans Zhang <18255117159@163.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.dev/#/patchset/20260904061823.592216-1-18255117159@163.com?part=1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF 2026-09-04 6:18 [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF Hans Zhang 2026-09-04 6:24 ` sashiko-bot @ 2026-09-04 16:12 ` Bjorn Helgaas 2026-09-04 16:32 ` Hans Zhang 1 sibling, 1 reply; 5+ messages in thread From: Bjorn Helgaas @ 2026-09-04 16:12 UTC (permalink / raw) To: Hans Zhang Cc: jingoohan1, bhelgaas, lpieralisi, kwilczynski, mani, robh, a-garg7, linux-pci, linux-kernel, Feizhou Yu 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")? 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. > 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@163.com> > Signed-off-by: Feizhou Yu <yufeizhou201@163.com> > Signed-off-by: Hans Zhang <18255117159@163.com> > --- > 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF 2026-09-04 16:12 ` Bjorn Helgaas @ 2026-09-04 16:32 ` Hans Zhang 2026-09-04 17:13 ` Bjorn Helgaas 0 siblings, 1 reply; 5+ messages in thread From: Hans Zhang @ 2026-09-04 16:32 UTC (permalink / raw) To: Bjorn Helgaas Cc: jingoohan1, bhelgaas, lpieralisi, kwilczynski, mani, robh, a-garg7, linux-pci, linux-kernel, Feizhou Yu 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@163.com> >> Signed-off-by: Feizhou Yu <yufeizhou201@163.com> >> Signed-off-by: Hans Zhang <18255117159@163.com> >> --- >> 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 >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF 2026-09-04 16:32 ` Hans Zhang @ 2026-09-04 17:13 ` Bjorn Helgaas 0 siblings, 0 replies; 5+ messages in thread From: Bjorn Helgaas @ 2026-09-04 17:13 UTC (permalink / raw) To: Hans Zhang Cc: jingoohan1, bhelgaas, lpieralisi, kwilczynski, mani, robh, a-garg7, linux-pci, linux-kernel, Feizhou Yu On Sat, Sep 05, 2026 at 12:32:30AM +0800, Hans Zhang wrote: > 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 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. One possibility is to add a one-line comment at the #define containing the name used in the spec. It's not perfect, but that's basically what's in pci_regs.h: #define PCI_EXP_LNKCAP 0x0c /* Link Capabilities */ #define PCI_EXP_LNKCTL 0x10 /* Link Control */ #define PCI_EXP_LNKSTA 0x12 /* Link Status */ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-04 17:13 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-04 6:18 [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF Hans Zhang 2026-09-04 6:24 ` sashiko-bot 2026-09-04 16:12 ` Bjorn Helgaas 2026-09-04 16:32 ` Hans Zhang 2026-09-04 17:13 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox