Re: [PATCH v4 1/1] PCI: of: Relax max-link-speed check to support PCIe Gen5/Gen6

[Hans Zhang <18255117159@163.com>]

On 2025/12/18 14:31, Manivannan Sadhasivam wrote:
> On Wed, Nov 05, 2025 at 09:47:01PM +0800, Hans Zhang wrote:
>> The existing code restricted max-link-speed to values 1~4 (Gen1~Gen4),
>> but current SOCs using Synopsys/Cadence IP may require Gen5/Gen6 support.
>> While DT binding validation already checks this property, the code-level
>> validation in of_pci_get_max_link_speed still lags behind, needing an
>> update to accommodate newer PCIe generations.
>>
>> Hardcoded literals in such validation logic create maintainability
>> challenges, as they are difficult to track and update when adding
>> support for future PCIe link speeds.  To address this, a helper function
>> pcie_max_supported_link_speed() is added in drivers/pci/pci.h, which
>> calculates the maximum supported link speed generation using existing
>> PCIe capability macros (PCI_EXP_LNKCAP_SLS_*). This ensures alignment
>> with the kernel's generic PCIe link speed definitions and avoids
>> standalone hardcoded values.
>>
>> The previous hardcoded "4" in the validation check is replaced with a
>> call to this helper function, eliminating the need to modify this specific
>> code path when extending support for future PCIe generations.
> 
> How can you not modify this function when PCIe 7.0 gets added? It still requires
> an update.
> 
> I'd prefer to just drop the check altogether as the callers already have checks
> on their own.

Hi Mani,


Thank you very much for your reply. Do you mean the following modification?

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 3579265f1198..9d3980e425b4 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -890,7 +890,7 @@ int of_pci_get_max_link_speed(struct device_node *node)
  	u32 max_link_speed;

  	if (of_property_read_u32(node, "max-link-speed", &max_link_speed) ||
-	    max_link_speed == 0 || max_link_speed > 4)
+	    max_link_speed == 0)
  		return -EINVAL;

  	return max_link_speed;

Best regards,
Hans


> 
> - Mani
> 
>> The
>> implementation maintains full backward compatibility with existing
>> configurations, while enabling seamless extension for newer link
>> speeds, future updates will only require updating the relevant PCI
>> capability macros without changing the validation logic here.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>>   drivers/pci/of.c  | 3 ++-
>>   drivers/pci/pci.h | 5 +++++
>>   2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>> index 3579265f1198..de1fe6b9ba6a 100644
>> --- a/drivers/pci/of.c
>> +++ b/drivers/pci/of.c
>> @@ -890,7 +890,8 @@ int of_pci_get_max_link_speed(struct device_node *node)
>>   	u32 max_link_speed;
>>   
>>   	if (of_property_read_u32(node, "max-link-speed", &max_link_speed) ||
>> -	    max_link_speed == 0 || max_link_speed > 4)
>> +	    max_link_speed == 0 ||
>> +	    max_link_speed > pcie_max_supported_link_speed())
>>   		return -EINVAL;
>>   
>>   	return max_link_speed;
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 4492b809094b..2f0f319e80ce 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -548,6 +548,11 @@ static inline int pcie_dev_speed_mbps(enum pci_bus_speed speed)
>>   	return -EINVAL;
>>   }
>>   
>> +static inline int pcie_max_supported_link_speed(void)
>> +{
>> +	return PCI_EXP_LNKCAP_SLS_64_0GB - PCI_EXP_LNKCAP_SLS_2_5GB + 1;
>> +}
>> +
>>   u8 pcie_get_supported_speeds(struct pci_dev *dev);
>>   const char *pci_speed_string(enum pci_bus_speed speed);
>>   void __pcie_print_link_status(struct pci_dev *dev, bool verbose);
>> -- 
>> 2.34.1
>>
>