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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

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

On Tue, 7 Apr 2026, Hans Zhang wrote:

> 
> 
> On 4/7/26 16:11, Ilpo Järvinen wrote:
> > On Mon, 6 Apr 2026, Hans Zhang wrote:
> > 
> > > 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)))
> > 
> > drivers/pci/pci.h doesn't seem to have include for WARN_ON_ONCE() so you
> > should add it.
> 
> Hi Ilpo,
> 
> In the file "drivers/pci/pcie/bwctrl.c", there is no reference to the header
> file of WARN_ON_ONCE(). It seems that no error was reported. I think it might
> be that some other header files have indirectly referenced the header file of
> WARN_ON_ONCE(). However, when I compiled it locally, no errors were reported.

Hi,

Apparently it was missing from there as well.

It might build now (I don't actually even doubt that), but that depends 
on the includes in every file including drivers/pci/pci.h. We should not 
depend on that as it is fragile but ensure a header is self-sufficient 
when it comes to includes necessary for it.

> I think if we want to add a header file, it should be #include
> <asm-generic/bug.h> ?

Yes

#include <asm/bug.h>

-- 
 i.