[PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions

[PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions

[Hans Zhang]

This series refactors PCIe speed validation and conversion logic to
shared functions in the public header, eliminating code duplication
and ensuring consistency across drivers.

---
Changes for v6:
- missing one line of code:
  link_speed = pcie_get_link_speed(pci->max_link_speed); 

Changes for v5:
https://patchwork.kernel.org/project/linux-pci/patch/20260406104708.1218648-1-18255117159@163.com/

- Rebase to v7.0-rc1. (pci/next tree)

Changes for v4:
https://patchwork.kernel.org/project/linux-pci/patch/20251102143206.111347-1-18255117159@163.com/

- Maintain O(1) array-based lookup for speed conversion (addressing
  performance concerns from v3 feedback)
- Move pcie_valid_speed() and pci_bus_speed2lnkctl2() to pci.h
- Update dwc driver to use the shared functions
- Rebase to v6.18-rc3.

This addresses the feedback from Lukas Wunner and Manivannan Sadhasivam
on the v3 submission, ensuring no runtime performance regression while
achieving code reuse.

Changes for v3:
https://patchwork.kernel.org/project/linux-pci/patch/20250816154633.338653-1-18255117159@163.com/

- Rebase to v6.17-rc1.
- Gentle ping.

Changes for v2:
- s/PCIE_SPEED2LNKCTL2_TLS_ENC/PCIE_SPEED2LNKCTL2_TLS
- The patch commit message were modified.
---

Hans Zhang (3):
  PCI: Add public pcie_valid_speed() for shared validation
  PCI: Move pci_bus_speed2lnkctl2() to public header
  PCI: dwc: Use common speed conversion function

 drivers/pci/controller/dwc/pcie-designware.c | 18 +++-------------
 drivers/pci/pci.h                            | 22 ++++++++++++++++++++
 drivers/pci/pcie/bwctrl.c                    | 22 --------------------
 3 files changed, 25 insertions(+), 37 deletions(-)


base-commit: 525e91d84dc085492b36d4b87abb7c1cc93fcb44
-- 
2.34.1

[PATCH v6 1/3] PCI: Add public pcie_valid_speed() for shared validation

[Hans Zhang]

Extract the PCIe speed validation logic from bwctrl.c's static
pcie_valid_speed() into a public static inline function in pci.h.

This allows consistent speed range checks (2.5GT/s to 64.0GT/s) across
multiple drivers and functions, avoiding duplicate code and ensuring
validation consistency as per PCIe specifications.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/pci.h         | 5 +++++
 drivers/pci/pcie/bwctrl.c | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4a14f88e543a..f0a082bfd6f1 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -606,6 +606,11 @@ void pci_bus_put(struct pci_bus *bus);
 	 (speed) == PCIE_SPEED_2_5GT  ?  2500*8/10 : \
 	 0)
 
+static inline bool pcie_valid_speed(enum pci_bus_speed speed)
+{
+	return (speed >= PCIE_SPEED_2_5GT) && (speed <= PCIE_SPEED_64_0GT);
+}
+
 static inline int pcie_dev_speed_mbps(enum pci_bus_speed speed)
 {
 	switch (speed) {
diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c
index c4c8d260bf96..ea82e326f164 100644
--- a/drivers/pci/pcie/bwctrl.c
+++ b/drivers/pci/pcie/bwctrl.c
@@ -48,11 +48,6 @@ struct pcie_bwctrl_data {
 /* Prevent port removal during Link Speed changes. */
 static DECLARE_RWSEM(pcie_bwctrl_setspeed_rwsem);
 
-static bool pcie_valid_speed(enum pci_bus_speed speed)
-{
-	return (speed >= PCIE_SPEED_2_5GT) && (speed <= PCIE_SPEED_64_0GT);
-}
-
 static u16 pci_bus_speed2lnkctl2(enum pci_bus_speed speed)
 {
 	static const u8 speed_conv[] = {
-- 
2.34.1

[PATCH v6 2/3] PCI: Move pci_bus_speed2lnkctl2() to public header

[Hans Zhang]

Move the static array-based pci_bus_speed2lnkctl2() function from
bwctrl.c to pci.h as a public inline function.

This provides efficient O(1) speed-to-LNKCTL2 value conversion using
static array lookup, maintaining optimal performance while enabling
code reuse by other PCIe drivers.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/pci.h         | 17 +++++++++++++++++
 drivers/pci/pcie/bwctrl.c | 17 -----------------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index f0a082bfd6f1..db91878a86ac 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -611,6 +611,23 @@ static inline bool pcie_valid_speed(enum pci_bus_speed speed)
 	return (speed >= PCIE_SPEED_2_5GT) && (speed <= PCIE_SPEED_64_0GT);
 }
 
+static inline u16 pci_bus_speed2lnkctl2(enum pci_bus_speed speed)
+{
+	static const u8 speed_conv[] = {
+		[PCIE_SPEED_2_5GT] = PCI_EXP_LNKCTL2_TLS_2_5GT,
+		[PCIE_SPEED_5_0GT] = PCI_EXP_LNKCTL2_TLS_5_0GT,
+		[PCIE_SPEED_8_0GT] = PCI_EXP_LNKCTL2_TLS_8_0GT,
+		[PCIE_SPEED_16_0GT] = PCI_EXP_LNKCTL2_TLS_16_0GT,
+		[PCIE_SPEED_32_0GT] = PCI_EXP_LNKCTL2_TLS_32_0GT,
+		[PCIE_SPEED_64_0GT] = PCI_EXP_LNKCTL2_TLS_64_0GT,
+	};
+
+	if (WARN_ON_ONCE(!pcie_valid_speed(speed)))
+		return 0;
+
+	return speed_conv[speed];
+}
+
 static inline int pcie_dev_speed_mbps(enum pci_bus_speed speed)
 {
 	switch (speed) {
diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c
index ea82e326f164..d48021bfd844 100644
--- a/drivers/pci/pcie/bwctrl.c
+++ b/drivers/pci/pcie/bwctrl.c
@@ -48,23 +48,6 @@ struct pcie_bwctrl_data {
 /* Prevent port removal during Link Speed changes. */
 static DECLARE_RWSEM(pcie_bwctrl_setspeed_rwsem);
 
-static u16 pci_bus_speed2lnkctl2(enum pci_bus_speed speed)
-{
-	static const u8 speed_conv[] = {
-		[PCIE_SPEED_2_5GT] = PCI_EXP_LNKCTL2_TLS_2_5GT,
-		[PCIE_SPEED_5_0GT] = PCI_EXP_LNKCTL2_TLS_5_0GT,
-		[PCIE_SPEED_8_0GT] = PCI_EXP_LNKCTL2_TLS_8_0GT,
-		[PCIE_SPEED_16_0GT] = PCI_EXP_LNKCTL2_TLS_16_0GT,
-		[PCIE_SPEED_32_0GT] = PCI_EXP_LNKCTL2_TLS_32_0GT,
-		[PCIE_SPEED_64_0GT] = PCI_EXP_LNKCTL2_TLS_64_0GT,
-	};
-
-	if (WARN_ON_ONCE(!pcie_valid_speed(speed)))
-		return 0;
-
-	return speed_conv[speed];
-}
-
 static inline u16 pcie_supported_speeds2target_speed(u8 supported_speeds)
 {
 	return __fls(supported_speeds);
-- 
2.34.1

[PATCH v6 3/3] PCI: dwc: Use common speed conversion function

[Hans Zhang]

Replace the private switch-based speed conversion in
dw_pcie_link_set_max_speed() with the public pci_bus_speed2lnkctl2()
function.

This eliminates duplicate conversion logic and ensures consistency with
other PCIe drivers, while handling invalid speeds by falling back to
hardware capabilities.

Signed-off-by: Hans Zhang <18255117159@163.com>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
---
 drivers/pci/controller/dwc/pcie-designware.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 06792ba92aa7..ab8dee5d6c7a 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -861,24 +861,12 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci)
 	ctrl2 = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
 	ctrl2 &= ~PCI_EXP_LNKCTL2_TLS;
 
-	switch (pcie_get_link_speed(pci->max_link_speed)) {
-	case PCIE_SPEED_2_5GT:
-		link_speed = PCI_EXP_LNKCTL2_TLS_2_5GT;
-		break;
-	case PCIE_SPEED_5_0GT:
-		link_speed = PCI_EXP_LNKCTL2_TLS_5_0GT;
-		break;
-	case PCIE_SPEED_8_0GT:
-		link_speed = PCI_EXP_LNKCTL2_TLS_8_0GT;
-		break;
-	case PCIE_SPEED_16_0GT:
-		link_speed = PCI_EXP_LNKCTL2_TLS_16_0GT;
-		break;
-	default:
+	link_speed = pcie_get_link_speed(pci->max_link_speed);
+	link_speed = pci_bus_speed2lnkctl2(link_speed);
+	if (link_speed == 0) {
 		/* Use hardware capability */
 		link_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, cap);
 		ctrl2 &= ~PCI_EXP_LNKCTL2_HASD;
-		break;
 	}
 
 	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, ctrl2 | link_speed);
-- 
2.34.1

Re: [PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions

[Shawn Lin]

在 2026/04/06 星期一 18:56, Hans Zhang 写道:
> This series refactors PCIe speed validation and conversion logic to
> shared functions in the public header, eliminating code duplication
> and ensuring consistency across drivers.
> 

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> ---
> Changes for v6:
> - missing one line of code:
>    link_speed = pcie_get_link_speed(pci->max_link_speed);
> 
> Changes for v5:
> https://patchwork.kernel.org/project/linux-pci/patch/20260406104708.1218648-1-18255117159@163.com/
> 
> - Rebase to v7.0-rc1. (pci/next tree)
> 
> Changes for v4:
> https://patchwork.kernel.org/project/linux-pci/patch/20251102143206.111347-1-18255117159@163.com/
> 
> - Maintain O(1) array-based lookup for speed conversion (addressing
>    performance concerns from v3 feedback)
> - Move pcie_valid_speed() and pci_bus_speed2lnkctl2() to pci.h
> - Update dwc driver to use the shared functions
> - Rebase to v6.18-rc3.
> 
> This addresses the feedback from Lukas Wunner and Manivannan Sadhasivam
> on the v3 submission, ensuring no runtime performance regression while
> achieving code reuse.
> 
> Changes for v3:
> https://patchwork.kernel.org/project/linux-pci/patch/20250816154633.338653-1-18255117159@163.com/
> 
> - Rebase to v6.17-rc1.
> - Gentle ping.
> 
> Changes for v2:
> - s/PCIE_SPEED2LNKCTL2_TLS_ENC/PCIE_SPEED2LNKCTL2_TLS
> - The patch commit message were modified.
> ---
> 
> Hans Zhang (3):
>    PCI: Add public pcie_valid_speed() for shared validation
>    PCI: Move pci_bus_speed2lnkctl2() to public header
>    PCI: dwc: Use common speed conversion function
> 
>   drivers/pci/controller/dwc/pcie-designware.c | 18 +++-------------
>   drivers/pci/pci.h                            | 22 ++++++++++++++++++++
>   drivers/pci/pcie/bwctrl.c                    | 22 --------------------
>   3 files changed, 25 insertions(+), 37 deletions(-)
> 
> 
> base-commit: 525e91d84dc085492b36d4b87abb7c1cc93fcb44

Re: [PATCH v6 3/3] PCI: dwc: Use common speed conversion function

[Ilpo Järvinen]

On Mon, 6 Apr 2026, Hans Zhang wrote:

> Replace the private switch-based speed conversion in
> dw_pcie_link_set_max_speed() with the public pci_bus_speed2lnkctl2()
> function.
> 
> This eliminates duplicate conversion logic and ensures consistency with
> other PCIe drivers, while handling invalid speeds by falling back to
> hardware capabilities.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 06792ba92aa7..ab8dee5d6c7a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -861,24 +861,12 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci)
>  	ctrl2 = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
>  	ctrl2 &= ~PCI_EXP_LNKCTL2_TLS;
>  
> -	switch (pcie_get_link_speed(pci->max_link_speed)) {
> -	case PCIE_SPEED_2_5GT:
> -		link_speed = PCI_EXP_LNKCTL2_TLS_2_5GT;
> -		break;
> -	case PCIE_SPEED_5_0GT:
> -		link_speed = PCI_EXP_LNKCTL2_TLS_5_0GT;
> -		break;
> -	case PCIE_SPEED_8_0GT:
> -		link_speed = PCI_EXP_LNKCTL2_TLS_8_0GT;
> -		break;
> -	case PCIE_SPEED_16_0GT:
> -		link_speed = PCI_EXP_LNKCTL2_TLS_16_0GT;
> -		break;
> -	default:
> +	link_speed = pcie_get_link_speed(pci->max_link_speed);
> +	link_speed = pci_bus_speed2lnkctl2(link_speed);

Its signature is:

u16 pci_bus_speed2lnkctl2(enum pci_bus_speed speed)

Using link_speed variable both for in and out does contradict with the
expected typing.

Maybe it would be beneficial to rename the current 'link_speed' to 
'ctrl2_speed' (or something along those lines) to differentiate Link 
Control 2 speed representation from PCI core's internal link speed 
representation. And then reintroduce link_speed variable as the 
pci_bus_speed. ...But this is just my suggestion, there may be better ways 
to untangle this type abuse.

> +	if (link_speed == 0) {
>  		/* Use hardware capability */
>  		link_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, cap);
>  		ctrl2 &= ~PCI_EXP_LNKCTL2_HASD;
> -		break;
>  	}
>  
>  	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, ctrl2 | link_speed);
> 

-- 
 i.

Re: [PATCH v6 3/3] PCI: dwc: Use common speed conversion function

[Hans Zhang]

On 4/7/26 16:25, Ilpo Järvinen wrote:
> On Mon, 6 Apr 2026, Hans Zhang wrote:
> 
>> Replace the private switch-based speed conversion in
>> dw_pcie_link_set_max_speed() with the public pci_bus_speed2lnkctl2()
>> function.
>>
>> This eliminates duplicate conversion logic and ensures consistency with
>> other PCIe drivers, while handling invalid speeds by falling back to
>> hardware capabilities.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
>> ---
>>   drivers/pci/controller/dwc/pcie-designware.c | 18 +++---------------
>>   1 file changed, 3 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>> index 06792ba92aa7..ab8dee5d6c7a 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>> @@ -861,24 +861,12 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie *pci)
>>   	ctrl2 = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
>>   	ctrl2 &= ~PCI_EXP_LNKCTL2_TLS;
>>   
>> -	switch (pcie_get_link_speed(pci->max_link_speed)) {
>> -	case PCIE_SPEED_2_5GT:
>> -		link_speed = PCI_EXP_LNKCTL2_TLS_2_5GT;
>> -		break;
>> -	case PCIE_SPEED_5_0GT:
>> -		link_speed = PCI_EXP_LNKCTL2_TLS_5_0GT;
>> -		break;
>> -	case PCIE_SPEED_8_0GT:
>> -		link_speed = PCI_EXP_LNKCTL2_TLS_8_0GT;
>> -		break;
>> -	case PCIE_SPEED_16_0GT:
>> -		link_speed = PCI_EXP_LNKCTL2_TLS_16_0GT;
>> -		break;
>> -	default:
>> +	link_speed = pcie_get_link_speed(pci->max_link_speed);
>> +	link_speed = pci_bus_speed2lnkctl2(link_speed);
> 
> Its signature is:
> 
> u16 pci_bus_speed2lnkctl2(enum pci_bus_speed speed)
> 
> Using link_speed variable both for in and out does contradict with the
> expected typing.
> 
> Maybe it would be beneficial to rename the current 'link_speed' to
> 'ctrl2_speed' (or something along those lines) to differentiate Link
> Control 2 speed representation from PCI core's internal link speed
> representation. And then reintroduce link_speed variable as the
> pci_bus_speed. ...But this is just my suggestion, there may be better ways
> to untangle this type abuse.

Hi Ilpo,


Thank you for your review comments. This way it will be clearer. Here 
are my revisions. If there are no issues, I will send the next version.


diff --git a/drivers/pci/controller/dwc/pcie-designware.c 
b/drivers/pci/controller/dwc/pcie-designware.c
index 06792ba92aa7..21a709a47e82 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -843,8 +843,9 @@ EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);

  static void dw_pcie_link_set_max_speed(struct dw_pcie *pci)
  {
-       u32 cap, ctrl2, link_speed;
+       u32 cap, ctrl2, pci_bus_speed;
         u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+       u16 ctrl2_speed;

         cap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);

@@ -861,30 +862,18 @@ static void dw_pcie_link_set_max_speed(struct 
dw_pcie *pci)
         ctrl2 = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
         ctrl2 &= ~PCI_EXP_LNKCTL2_TLS;

-       switch (pcie_get_link_speed(pci->max_link_speed)) {
-       case PCIE_SPEED_2_5GT:
-               link_speed = PCI_EXP_LNKCTL2_TLS_2_5GT;
-               break;
-       case PCIE_SPEED_5_0GT:
-               link_speed = PCI_EXP_LNKCTL2_TLS_5_0GT;
-               break;
-       case PCIE_SPEED_8_0GT:
-               link_speed = PCI_EXP_LNKCTL2_TLS_8_0GT;
-               break;
-       case PCIE_SPEED_16_0GT:
-               link_speed = PCI_EXP_LNKCTL2_TLS_16_0GT;
-               break;
-       default:
+       pci_bus_speed = pcie_get_link_speed(pci->max_link_speed);
+       ctrl2_speed = pci_bus_speed2lnkctl2(pci_bus_speed);
+       if (ctrl2_speed == 0) {
                 /* Use hardware capability */
-               link_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, cap);
+               ctrl2_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, cap);
                 ctrl2 &= ~PCI_EXP_LNKCTL2_HASD;
-               break;
         }

-       dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, ctrl2 | 
link_speed);
+       dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, ctrl2 | 
ctrl2_speed);

         cap &= ~((u32)PCI_EXP_LNKCAP_SLS);
-       dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, cap | link_speed);
+       dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, cap | ctrl2_speed);


Best regards,
Hans

> 
>> +	if (link_speed == 0) {
>>   		/* Use hardware capability */
>>   		link_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, cap);
>>   		ctrl2 &= ~PCI_EXP_LNKCTL2_HASD;
>> -		break;
>>   	}
>>   
>>   	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, ctrl2 | link_speed);
>>
> 

Re: [PATCH v6 3/3] PCI: dwc: Use common speed conversion function

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 5398 bytes --]

On Tue, 7 Apr 2026, Hans Zhang wrote:
> On 4/7/26 16:25, Ilpo Järvinen wrote:
> > On Mon, 6 Apr 2026, Hans Zhang wrote:
> > 
> > > Replace the private switch-based speed conversion in
> > > dw_pcie_link_set_max_speed() with the public pci_bus_speed2lnkctl2()
> > > function.
> > > 
> > > This eliminates duplicate conversion logic and ensures consistency with
> > > other PCIe drivers, while handling invalid speeds by falling back to
> > > hardware capabilities.
> > > 
> > > Signed-off-by: Hans Zhang <18255117159@163.com>
> > > Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> > > ---
> > >   drivers/pci/controller/dwc/pcie-designware.c | 18 +++---------------
> > >   1 file changed, 3 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > > b/drivers/pci/controller/dwc/pcie-designware.c
> > > index 06792ba92aa7..ab8dee5d6c7a 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -861,24 +861,12 @@ static void dw_pcie_link_set_max_speed(struct
> > > dw_pcie *pci)
> > >   	ctrl2 = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
> > >   	ctrl2 &= ~PCI_EXP_LNKCTL2_TLS;
> > >   -	switch (pcie_get_link_speed(pci->max_link_speed)) {
> > > -	case PCIE_SPEED_2_5GT:
> > > -		link_speed = PCI_EXP_LNKCTL2_TLS_2_5GT;
> > > -		break;
> > > -	case PCIE_SPEED_5_0GT:
> > > -		link_speed = PCI_EXP_LNKCTL2_TLS_5_0GT;
> > > -		break;
> > > -	case PCIE_SPEED_8_0GT:
> > > -		link_speed = PCI_EXP_LNKCTL2_TLS_8_0GT;
> > > -		break;
> > > -	case PCIE_SPEED_16_0GT:
> > > -		link_speed = PCI_EXP_LNKCTL2_TLS_16_0GT;
> > > -		break;
> > > -	default:
> > > +	link_speed = pcie_get_link_speed(pci->max_link_speed);
> > > +	link_speed = pci_bus_speed2lnkctl2(link_speed);
> > 
> > Its signature is:
> > 
> > u16 pci_bus_speed2lnkctl2(enum pci_bus_speed speed)
> > 
> > Using link_speed variable both for in and out does contradict with the
> > expected typing.
> > 
> > Maybe it would be beneficial to rename the current 'link_speed' to
> > 'ctrl2_speed' (or something along those lines) to differentiate Link
> > Control 2 speed representation from PCI core's internal link speed
> > representation. And then reintroduce link_speed variable as the
> > pci_bus_speed. ...But this is just my suggestion, there may be better ways
> > to untangle this type abuse.
> 
> Hi Ilpo,
> 
> 
> Thank you for your review comments. This way it will be clearer. Here are my
> revisions. If there are no issues, I will send the next version.
> 
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> b/drivers/pci/controller/dwc/pcie-designware.c
> index 06792ba92aa7..21a709a47e82 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -843,8 +843,9 @@ EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);
> 
>  static void dw_pcie_link_set_max_speed(struct dw_pcie *pci)
>  {
> -       u32 cap, ctrl2, link_speed;
> +       u32 cap, ctrl2, pci_bus_speed;

I meant something like this (instead of introducing variable named 
"pci_bus_speed"):

	enum pci_bus_speed link_speed;

>         u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +       u16 ctrl2_speed;

...And keep this like you have it now.

-- 
 i.

>         cap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> 
> @@ -861,30 +862,18 @@ static void dw_pcie_link_set_max_speed(struct dw_pcie
> *pci)
>         ctrl2 = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
>         ctrl2 &= ~PCI_EXP_LNKCTL2_TLS;
> 
> -       switch (pcie_get_link_speed(pci->max_link_speed)) {
> -       case PCIE_SPEED_2_5GT:
> -               link_speed = PCI_EXP_LNKCTL2_TLS_2_5GT;
> -               break;
> -       case PCIE_SPEED_5_0GT:
> -               link_speed = PCI_EXP_LNKCTL2_TLS_5_0GT;
> -               break;
> -       case PCIE_SPEED_8_0GT:
> -               link_speed = PCI_EXP_LNKCTL2_TLS_8_0GT;
> -               break;
> -       case PCIE_SPEED_16_0GT:
> -               link_speed = PCI_EXP_LNKCTL2_TLS_16_0GT;
> -               break;
> -       default:
> +       pci_bus_speed = pcie_get_link_speed(pci->max_link_speed);
> +       ctrl2_speed = pci_bus_speed2lnkctl2(pci_bus_speed);
> +       if (ctrl2_speed == 0) {
>                 /* Use hardware capability */
> -               link_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, cap);
> +               ctrl2_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, cap);
>                 ctrl2 &= ~PCI_EXP_LNKCTL2_HASD;
> -               break;
>         }
> 
> -       dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, ctrl2 | link_speed);
> +       dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, ctrl2 |
> ctrl2_speed);
> 
>         cap &= ~((u32)PCI_EXP_LNKCAP_SLS);
> -       dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, cap | link_speed);
> +       dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, cap | ctrl2_speed);
> 
> 
> Best regards,
> Hans
> 
> > 
> > > +	if (link_speed == 0) {
> > >   		/* Use hardware capability */
> > >   		link_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, cap);
> > >   		ctrl2 &= ~PCI_EXP_LNKCTL2_HASD;
> > > -		break;
> > >   	}
> > >     	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, ctrl2 |
> > > link_speed);
> > > 
> > 
> 

Re: [PATCH v6 3/3] PCI: dwc: Use common speed conversion function

[Hans Zhang]

On 4/7/26 20:29, Ilpo Järvinen wrote:
> On Tue, 7 Apr 2026, Hans Zhang wrote:
>> On 4/7/26 16:25, Ilpo Järvinen wrote:
>>> On Mon, 6 Apr 2026, Hans Zhang wrote:
>>>
>>>> Replace the private switch-based speed conversion in
>>>> dw_pcie_link_set_max_speed() with the public pci_bus_speed2lnkctl2()
>>>> function.
>>>>
>>>> This eliminates duplicate conversion logic and ensures consistency with
>>>> other PCIe drivers, while handling invalid speeds by falling back to
>>>> hardware capabilities.
>>>>
>>>> Signed-off-by: Hans Zhang <18255117159@163.com>
>>>> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
>>>> ---
>>>>    drivers/pci/controller/dwc/pcie-designware.c | 18 +++---------------
>>>>    1 file changed, 3 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c
>>>> b/drivers/pci/controller/dwc/pcie-designware.c
>>>> index 06792ba92aa7..ab8dee5d6c7a 100644
>>>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>>>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>>>> @@ -861,24 +861,12 @@ static void dw_pcie_link_set_max_speed(struct
>>>> dw_pcie *pci)
>>>>    	ctrl2 = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
>>>>    	ctrl2 &= ~PCI_EXP_LNKCTL2_TLS;
>>>>    -	switch (pcie_get_link_speed(pci->max_link_speed)) {
>>>> -	case PCIE_SPEED_2_5GT:
>>>> -		link_speed = PCI_EXP_LNKCTL2_TLS_2_5GT;
>>>> -		break;
>>>> -	case PCIE_SPEED_5_0GT:
>>>> -		link_speed = PCI_EXP_LNKCTL2_TLS_5_0GT;
>>>> -		break;
>>>> -	case PCIE_SPEED_8_0GT:
>>>> -		link_speed = PCI_EXP_LNKCTL2_TLS_8_0GT;
>>>> -		break;
>>>> -	case PCIE_SPEED_16_0GT:
>>>> -		link_speed = PCI_EXP_LNKCTL2_TLS_16_0GT;
>>>> -		break;
>>>> -	default:
>>>> +	link_speed = pcie_get_link_speed(pci->max_link_speed);
>>>> +	link_speed = pci_bus_speed2lnkctl2(link_speed);
>>>
>>> Its signature is:
>>>
>>> u16 pci_bus_speed2lnkctl2(enum pci_bus_speed speed)
>>>
>>> Using link_speed variable both for in and out does contradict with the
>>> expected typing.
>>>
>>> Maybe it would be beneficial to rename the current 'link_speed' to
>>> 'ctrl2_speed' (or something along those lines) to differentiate Link
>>> Control 2 speed representation from PCI core's internal link speed
>>> representation. And then reintroduce link_speed variable as the
>>> pci_bus_speed. ...But this is just my suggestion, there may be better ways
>>> to untangle this type abuse.
>>
>> Hi Ilpo,
>>
>>
>> Thank you for your review comments. This way it will be clearer. Here are my
>> revisions. If there are no issues, I will send the next version.
>>
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c
>> b/drivers/pci/controller/dwc/pcie-designware.c
>> index 06792ba92aa7..21a709a47e82 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>> @@ -843,8 +843,9 @@ EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);
>>
>>   static void dw_pcie_link_set_max_speed(struct dw_pcie *pci)
>>   {
>> -       u32 cap, ctrl2, link_speed;
>> +       u32 cap, ctrl2, pci_bus_speed;
> 
> I meant something like this (instead of introducing variable named
> "pci_bus_speed"):

Hi Ilpo,

Thank you. Will change.

Best regards,
Hans

> 
> 	enum pci_bus_speed link_speed;
> 
>>          u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>> +       u16 ctrl2_speed;
> 
> ...And keep this like you have it now.
>