mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4 1/1] PCI: of: Relax max-link-speed check to support PCIe Gen5/Gen6
@ 2025-11-05 13:47 Hans Zhang
  2025-12-18  6:31 ` Manivannan Sadhasivam
  0 siblings, 1 reply; 5+ messages in thread
From: Hans Zhang @ 2025-11-05 13:47 UTC (permalink / raw)
  To: bhelgaas, helgaas
  Cc: mani, ilpo.jarvinen, linux-pci, linux-kernel, Hans Zhang

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/1] PCI: of: Relax max-link-speed check to support PCIe Gen5/Gen6
  2025-11-05 13:47 [PATCH v4 1/1] PCI: of: Relax max-link-speed check to support PCIe Gen5/Gen6 Hans Zhang
@ 2025-12-18  6:31 ` Manivannan Sadhasivam
  2025-12-18 11:56   ` Hans Zhang
  0 siblings, 1 reply; 5+ messages in thread
From: Manivannan Sadhasivam @ 2025-12-18  6:31 UTC (permalink / raw)
  To: Hans Zhang; +Cc: bhelgaas, helgaas, ilpo.jarvinen, linux-pci, linux-kernel

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.

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

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/1] PCI: of: Relax max-link-speed check to support PCIe Gen5/Gen6
  2025-12-18  6:31 ` Manivannan Sadhasivam
@ 2025-12-18 11:56   ` Hans Zhang
  2025-12-18 12:44     ` Manivannan Sadhasivam
  0 siblings, 1 reply; 5+ messages in thread
From: Hans Zhang @ 2025-12-18 11:56 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: bhelgaas, helgaas, ilpo.jarvinen, linux-pci, linux-kernel



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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/1] PCI: of: Relax max-link-speed check to support PCIe Gen5/Gen6
  2025-12-18 11:56   ` Hans Zhang
@ 2025-12-18 12:44     ` Manivannan Sadhasivam
  2025-12-18 12:46       ` Hans Zhang
  0 siblings, 1 reply; 5+ messages in thread
From: Manivannan Sadhasivam @ 2025-12-18 12:44 UTC (permalink / raw)
  To: Hans Zhang; +Cc: bhelgaas, helgaas, ilpo.jarvinen, linux-pci, linux-kernel

On Thu, Dec 18, 2025 at 07:56:14PM +0800, Hans Zhang wrote:
> 
> 
> 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;
> 

Yes! But you could remove the 0 check also.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/1] PCI: of: Relax max-link-speed check to support PCIe Gen5/Gen6
  2025-12-18 12:44     ` Manivannan Sadhasivam
@ 2025-12-18 12:46       ` Hans Zhang
  0 siblings, 0 replies; 5+ messages in thread
From: Hans Zhang @ 2025-12-18 12:46 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: bhelgaas, helgaas, ilpo.jarvinen, linux-pci, linux-kernel



On 2025/12/18 20:44, Manivannan Sadhasivam wrote:
> On Thu, Dec 18, 2025 at 07:56:14PM +0800, Hans Zhang wrote:
>>
>>
>> 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;
>>
> 
> Yes! But you could remove the 0 check also.

Hi Mani,

Okay, thank you.

Best regards,
Hans

> 
> - Mani
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-12-18 12:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-05 13:47 [PATCH v4 1/1] PCI: of: Relax max-link-speed check to support PCIe Gen5/Gen6 Hans Zhang
2025-12-18  6:31 ` Manivannan Sadhasivam
2025-12-18 11:56   ` Hans Zhang
2025-12-18 12:44     ` Manivannan Sadhasivam
2025-12-18 12:46       ` Hans Zhang

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®