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

[Hans Zhang <18255117159@163.com>]

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