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

[Hans Zhang <18255117159@163.com>]

On 4/7/26 20:29, Hans Zhang wrote:
> 
> 
> On 4/7/26 20:26, Ilpo Järvinen wrote:
>> 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.
> 
> Hi Ilpo,
> 
> OK, I see.
> 
> 
>>
>>> I think if we want to add a header file, it should be #include
>>> <asm-generic/bug.h> ?
>>
>> Yes
>>
>> #include <asm/bug.h>

Hi Ilpo,

Executing the following check script will trigger a warning. However, 
using #include <asm-generic/bug.h> will not cause any issues. So, should 
we use #include <asm-generic/bug.h> ?

./scripts/checkpatch.pl ...../*patch

0002-PCI-Move-pci_bus_speed2lnkctl2-to-public-header.patch
---------------------------------------------------------------------
WARNING: Use #include <linux/bug.h> instead of <asm/bug.h>
#27: FILE: drivers/pci/pci.h:5:
+#include <asm/bug.h>

total: 0 errors, 1 warnings, 53 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
       mechanically convert to the typical style using --fix or 
--fix-inplace.

Best regards,
Hans