[PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF

[PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF

[Hans Zhang]

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

Re: [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF

[sashiko-bot]

> 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

Re: [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF

[Bjorn Helgaas]

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
> 

Re: [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF

[Hans Zhang]

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
>>

Re: [PATCH] pci: dwc: Rename PCIE_PORT_LINK_CONTROL to PORT_LINK_CTRL_OFF

[Bjorn Helgaas]

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 */