* [PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions
@ 2026-04-06 10:56 Hans Zhang
2026-04-06 10:56 ` [PATCH v6 1/3] PCI: Add public pcie_valid_speed() for shared validation Hans Zhang
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Hans Zhang @ 2026-04-06 10:56 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kw, kwilczynski, mani, ilpo.jarvinen, jingoohan1
Cc: robh, linux-pci, linux-kernel, 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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v6 1/3] PCI: Add public pcie_valid_speed() for shared validation 2026-04-06 10:56 [PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions Hans Zhang @ 2026-04-06 10:56 ` Hans Zhang 2026-04-06 10:56 ` [PATCH v6 2/3] PCI: Move pci_bus_speed2lnkctl2() to public header Hans Zhang ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Hans Zhang @ 2026-04-06 10:56 UTC (permalink / raw) To: bhelgaas, lpieralisi, kw, kwilczynski, mani, ilpo.jarvinen, jingoohan1 Cc: robh, linux-pci, linux-kernel, 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v6 2/3] PCI: Move pci_bus_speed2lnkctl2() to public header 2026-04-06 10:56 [PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions Hans Zhang 2026-04-06 10:56 ` [PATCH v6 1/3] PCI: Add public pcie_valid_speed() for shared validation Hans Zhang @ 2026-04-06 10:56 ` Hans Zhang 2026-04-06 10:56 ` [PATCH v6 3/3] PCI: dwc: Use common speed conversion function Hans Zhang 2026-04-07 0:36 ` [PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions Shawn Lin 3 siblings, 0 replies; 9+ messages in thread From: Hans Zhang @ 2026-04-06 10:56 UTC (permalink / raw) To: bhelgaas, lpieralisi, kw, kwilczynski, mani, ilpo.jarvinen, jingoohan1 Cc: robh, linux-pci, linux-kernel, 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v6 3/3] PCI: dwc: Use common speed conversion function 2026-04-06 10:56 [PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions Hans Zhang 2026-04-06 10:56 ` [PATCH v6 1/3] PCI: Add public pcie_valid_speed() for shared validation Hans Zhang 2026-04-06 10:56 ` [PATCH v6 2/3] PCI: Move pci_bus_speed2lnkctl2() to public header Hans Zhang @ 2026-04-06 10:56 ` Hans Zhang 2026-04-07 8:25 ` Ilpo Järvinen 2026-04-07 0:36 ` [PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions Shawn Lin 3 siblings, 1 reply; 9+ messages in thread From: Hans Zhang @ 2026-04-06 10:56 UTC (permalink / raw) To: bhelgaas, lpieralisi, kw, kwilczynski, mani, ilpo.jarvinen, jingoohan1 Cc: robh, linux-pci, linux-kernel, 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6 3/3] PCI: dwc: Use common speed conversion function 2026-04-06 10:56 ` [PATCH v6 3/3] PCI: dwc: Use common speed conversion function Hans Zhang @ 2026-04-07 8:25 ` Ilpo Järvinen 2026-04-07 12:19 ` Hans Zhang 0 siblings, 1 reply; 9+ messages in thread From: Ilpo Järvinen @ 2026-04-07 8:25 UTC (permalink / raw) To: Hans Zhang Cc: bhelgaas, lpieralisi, kw, kwilczynski, mani, jingoohan1, robh, linux-pci, LKML 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6 3/3] PCI: dwc: Use common speed conversion function 2026-04-07 8:25 ` Ilpo Järvinen @ 2026-04-07 12:19 ` Hans Zhang 2026-04-07 12:29 ` Ilpo Järvinen 0 siblings, 1 reply; 9+ messages in thread From: Hans Zhang @ 2026-04-07 12:19 UTC (permalink / raw) To: Ilpo Järvinen Cc: bhelgaas, lpieralisi, kw, kwilczynski, mani, jingoohan1, robh, linux-pci, LKML 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); >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6 3/3] PCI: dwc: Use common speed conversion function 2026-04-07 12:19 ` Hans Zhang @ 2026-04-07 12:29 ` Ilpo Järvinen 2026-04-07 12:32 ` Hans Zhang 0 siblings, 1 reply; 9+ messages in thread From: Ilpo Järvinen @ 2026-04-07 12:29 UTC (permalink / raw) To: Hans Zhang Cc: bhelgaas, lpieralisi, kw, kwilczynski, mani, jingoohan1, robh, linux-pci, LKML [-- 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); > > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6 3/3] PCI: dwc: Use common speed conversion function 2026-04-07 12:29 ` Ilpo Järvinen @ 2026-04-07 12:32 ` Hans Zhang 0 siblings, 0 replies; 9+ messages in thread From: Hans Zhang @ 2026-04-07 12:32 UTC (permalink / raw) To: Ilpo Järvinen Cc: bhelgaas, lpieralisi, kw, kwilczynski, mani, jingoohan1, robh, linux-pci, LKML 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. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions 2026-04-06 10:56 [PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions Hans Zhang ` (2 preceding siblings ...) 2026-04-06 10:56 ` [PATCH v6 3/3] PCI: dwc: Use common speed conversion function Hans Zhang @ 2026-04-07 0:36 ` Shawn Lin 3 siblings, 0 replies; 9+ messages in thread From: Shawn Lin @ 2026-04-07 0:36 UTC (permalink / raw) To: Hans Zhang, bhelgaas, lpieralisi, kw, kwilczynski, mani, ilpo.jarvinen, jingoohan1 Cc: shawn.lin, robh, linux-pci, linux-kernel 在 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-04-07 12:32 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-04-06 10:56 [PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions Hans Zhang 2026-04-06 10:56 ` [PATCH v6 1/3] PCI: Add public pcie_valid_speed() for shared validation Hans Zhang 2026-04-06 10:56 ` [PATCH v6 2/3] PCI: Move pci_bus_speed2lnkctl2() to public header Hans Zhang 2026-04-06 10:56 ` [PATCH v6 3/3] PCI: dwc: Use common speed conversion function Hans Zhang 2026-04-07 8:25 ` Ilpo Järvinen 2026-04-07 12:19 ` Hans Zhang 2026-04-07 12:29 ` Ilpo Järvinen 2026-04-07 12:32 ` Hans Zhang 2026-04-07 0:36 ` [PATCH v6 0/3] PCI: Refactor PCIe speed validation and conversion functions Shawn Lin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®