[PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation

[PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation

[Hans Zhang]

The current implementation of of_pci_get_max_link_speed() validates
max-link-speed property values to be in the range 1~4 (Gen1~Gen4).
However, this creates maintenance overhead as each new PCIe generation
requires updating this validation logic.

Since device tree binding validation already enforces the allowed
values through the schema, and the callers of this function perform
their own validation checks, this intermediate validation becomes
redundant.

Furthermore, with upcoming SOCs using Synopsys/Cadence IP requiring
Gen5/Gen6 support, removing this hardcoded check enables seamless
support for future PCIe generations without requiring kernel updates
for each new speed grade.

Remove the max-link-speed > 4 validation check while retaining the
property existence and non-zero check. This simplifies maintenance
and aligns with the existing validation architecture where DT binding
and driver-level checks provide sufficient validation.

Signed-off-by: Hans Zhang <18255117159@163.com>
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
---
Changes for v6:
- It'd be good to return the actual errno as of_property_read_u32() can return
  -EINVAL, -ENODATA and -EOVERFLOW. (Mani)

Changes for v5:
https://patchwork.kernel.org/project/linux-pci/patch/20251218125909.305300-1-18255117159@163.com/

- Delete the check for speed. (Mani)

Changes for v4:
https://patchwork.kernel.org/project/linux-pci/patch/20251105134701.182795-1-18255117159@163.com/

- Add pcie_max_supported_link_speed.(Ilpo)

Changes for v3:
https://patchwork.kernel.org/project/linux-pci/patch/20251101164132.14145-1-18255117159@163.com/

- Modify the commit message.
- Add Reviewed-by tag.

Changes for v2:
https://patchwork.kernel.org/project/linux-pci/cover/20250529021026.475861-1-18255117159@163.com/
- The following files have been deleted:
  Documentation/devicetree/bindings/pci/pci.txt

  Update to this file again:
  dtschema/schemas/pci/pci-bus-common.yaml
---
 drivers/pci/of.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 3579265f1198..b56fdbcb3d72 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -888,10 +888,11 @@ bool of_pci_supply_present(struct device_node *np)
 int of_pci_get_max_link_speed(struct device_node *node)
 {
 	u32 max_link_speed;
+	int ret;
 
-	if (of_property_read_u32(node, "max-link-speed", &max_link_speed) ||
-	    max_link_speed == 0 || max_link_speed > 4)
-		return -EINVAL;
+	ret = of_property_read_u32(node, "max-link-speed", &max_link_speed);
+	if (ret)
+		return ret;
 
 	return max_link_speed;
 }
-- 
2.34.1

Re: [PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation

[Hans Zhang]

Hi,

Gentle ping.

Best regards,
Hans

On 2025/12/18 21:20, Hans Zhang wrote:
> The current implementation of of_pci_get_max_link_speed() validates
> max-link-speed property values to be in the range 1~4 (Gen1~Gen4).
> However, this creates maintenance overhead as each new PCIe generation
> requires updating this validation logic.
> 
> Since device tree binding validation already enforces the allowed
> values through the schema, and the callers of this function perform
> their own validation checks, this intermediate validation becomes
> redundant.
> 
> Furthermore, with upcoming SOCs using Synopsys/Cadence IP requiring
> Gen5/Gen6 support, removing this hardcoded check enables seamless
> support for future PCIe generations without requiring kernel updates
> for each new speed grade.
> 
> Remove the max-link-speed > 4 validation check while retaining the
> property existence and non-zero check. This simplifies maintenance
> and aligns with the existing validation architecture where DT binding
> and driver-level checks provide sufficient validation.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
> Changes for v6:
> - It'd be good to return the actual errno as of_property_read_u32() can return
>    -EINVAL, -ENODATA and -EOVERFLOW. (Mani)
> 
> Changes for v5:
> https://patchwork.kernel.org/project/linux-pci/patch/20251218125909.305300-1-18255117159@163.com/
> 
> - Delete the check for speed. (Mani)
> 
> Changes for v4:
> https://patchwork.kernel.org/project/linux-pci/patch/20251105134701.182795-1-18255117159@163.com/
> 
> - Add pcie_max_supported_link_speed.(Ilpo)
> 
> Changes for v3:
> https://patchwork.kernel.org/project/linux-pci/patch/20251101164132.14145-1-18255117159@163.com/
> 
> - Modify the commit message.
> - Add Reviewed-by tag.
> 
> Changes for v2:
> https://patchwork.kernel.org/project/linux-pci/cover/20250529021026.475861-1-18255117159@163.com/
> - The following files have been deleted:
>    Documentation/devicetree/bindings/pci/pci.txt
> 
>    Update to this file again:
>    dtschema/schemas/pci/pci-bus-common.yaml
> ---
>   drivers/pci/of.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 3579265f1198..b56fdbcb3d72 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -888,10 +888,11 @@ bool of_pci_supply_present(struct device_node *np)
>   int of_pci_get_max_link_speed(struct device_node *node)
>   {
>   	u32 max_link_speed;
> +	int ret;
>   
> -	if (of_property_read_u32(node, "max-link-speed", &max_link_speed) ||
> -	    max_link_speed == 0 || max_link_speed > 4)
> -		return -EINVAL;
> +	ret = of_property_read_u32(node, "max-link-speed", &max_link_speed);
> +	if (ret)
> +		return ret;
>   
>   	return max_link_speed;
>   }

Re: [PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation

[Hans Zhang]

Hi,

Gentle ping.

Best regards,
Hans

On 2025/12/18 21:20, Hans Zhang wrote:
> The current implementation of of_pci_get_max_link_speed() validates
> max-link-speed property values to be in the range 1~4 (Gen1~Gen4).
> However, this creates maintenance overhead as each new PCIe generation
> requires updating this validation logic.
> 
> Since device tree binding validation already enforces the allowed
> values through the schema, and the callers of this function perform
> their own validation checks, this intermediate validation becomes
> redundant.
> 
> Furthermore, with upcoming SOCs using Synopsys/Cadence IP requiring
> Gen5/Gen6 support, removing this hardcoded check enables seamless
> support for future PCIe generations without requiring kernel updates
> for each new speed grade.
> 
> Remove the max-link-speed > 4 validation check while retaining the
> property existence and non-zero check. This simplifies maintenance
> and aligns with the existing validation architecture where DT binding
> and driver-level checks provide sufficient validation.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
> Changes for v6:
> - It'd be good to return the actual errno as of_property_read_u32() can return
>    -EINVAL, -ENODATA and -EOVERFLOW. (Mani)
> 
> Changes for v5:
> https://patchwork.kernel.org/project/linux-pci/patch/20251218125909.305300-1-18255117159@163.com/
> 
> - Delete the check for speed. (Mani)
> 
> Changes for v4:
> https://patchwork.kernel.org/project/linux-pci/patch/20251105134701.182795-1-18255117159@163.com/
> 
> - Add pcie_max_supported_link_speed.(Ilpo)
> 
> Changes for v3:
> https://patchwork.kernel.org/project/linux-pci/patch/20251101164132.14145-1-18255117159@163.com/
> 
> - Modify the commit message.
> - Add Reviewed-by tag.
> 
> Changes for v2:
> https://patchwork.kernel.org/project/linux-pci/cover/20250529021026.475861-1-18255117159@163.com/
> - The following files have been deleted:
>    Documentation/devicetree/bindings/pci/pci.txt
> 
>    Update to this file again:
>    dtschema/schemas/pci/pci-bus-common.yaml
> ---
>   drivers/pci/of.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 3579265f1198..b56fdbcb3d72 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -888,10 +888,11 @@ bool of_pci_supply_present(struct device_node *np)
>   int of_pci_get_max_link_speed(struct device_node *node)
>   {
>   	u32 max_link_speed;
> +	int ret;
>   
> -	if (of_property_read_u32(node, "max-link-speed", &max_link_speed) ||
> -	    max_link_speed == 0 || max_link_speed > 4)
> -		return -EINVAL;
> +	ret = of_property_read_u32(node, "max-link-speed", &max_link_speed);
> +	if (ret)
> +		return ret;
>   
>   	return max_link_speed;
>   }

Re: [PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation

[Bjorn Helgaas]

[+cc Rob]

On Thu, Dec 18, 2025 at 09:20:36PM +0800, Hans Zhang wrote:
> The current implementation of of_pci_get_max_link_speed() validates
> max-link-speed property values to be in the range 1~4 (Gen1~Gen4).
> However, this creates maintenance overhead as each new PCIe generation
> requires updating this validation logic.
> 
> Since device tree binding validation already enforces the allowed
> values through the schema, and the callers of this function perform
> their own validation checks, this intermediate validation becomes
> redundant.
> 
> Furthermore, with upcoming SOCs using Synopsys/Cadence IP requiring
> Gen5/Gen6 support, removing this hardcoded check enables seamless
> support for future PCIe generations without requiring kernel updates
> for each new speed grade.

The upcoming SoC info seems like too much detail for the commit log.

> Remove the max-link-speed > 4 validation check while retaining the
> property existence and non-zero check. This simplifies maintenance
> and aligns with the existing validation architecture where DT binding
> and driver-level checks provide sufficient validation.

I don't think it's legit to rely on DT binding checks.  The code
should protect itself regardless of what offline binding checkers
find.

I'm not convinced that callers do their own validation.  For example,
dw_pcie_get_resources() assigns of_pci_get_max_link_speed() to
pci->max_link_speed with no validation at all.

That's in the DWC core, so it's probably OK to expect the actual
controller drivers to validate pci->max_link_speed when they use it,
but dw_pcie_config_presets() uses pcie_link_speed[pci->max_link_speed]
and I don't see an obvious check that prevents an out-of-bounds
reference there.

I think this patch is the right direction -- I don't think there's
anything in drivers/pci/of.c that *uses* the value of
"max-link-speed", so it would be nice if it didn't have to be updated
for every generation.

> Signed-off-by: Hans Zhang <18255117159@163.com>
> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
> Changes for v6:
> - It'd be good to return the actual errno as of_property_read_u32() can return
>   -EINVAL, -ENODATA and -EOVERFLOW. (Mani)
> 
> Changes for v5:
> https://patchwork.kernel.org/project/linux-pci/patch/20251218125909.305300-1-18255117159@163.com/
> 
> - Delete the check for speed. (Mani)
> 
> Changes for v4:
> https://patchwork.kernel.org/project/linux-pci/patch/20251105134701.182795-1-18255117159@163.com/
> 
> - Add pcie_max_supported_link_speed.(Ilpo)
> 
> Changes for v3:
> https://patchwork.kernel.org/project/linux-pci/patch/20251101164132.14145-1-18255117159@163.com/
> 
> - Modify the commit message.
> - Add Reviewed-by tag.
> 
> Changes for v2:
> https://patchwork.kernel.org/project/linux-pci/cover/20250529021026.475861-1-18255117159@163.com/
> - The following files have been deleted:
>   Documentation/devicetree/bindings/pci/pci.txt
> 
>   Update to this file again:
>   dtschema/schemas/pci/pci-bus-common.yaml
> ---
>  drivers/pci/of.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 3579265f1198..b56fdbcb3d72 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -888,10 +888,11 @@ bool of_pci_supply_present(struct device_node *np)
>  int of_pci_get_max_link_speed(struct device_node *node)
>  {
>  	u32 max_link_speed;
> +	int ret;
>  
> -	if (of_property_read_u32(node, "max-link-speed", &max_link_speed) ||
> -	    max_link_speed == 0 || max_link_speed > 4)
> -		return -EINVAL;
> +	ret = of_property_read_u32(node, "max-link-speed", &max_link_speed);
> +	if (ret)
> +		return ret;
>  
>  	return max_link_speed;
>  }
> -- 
> 2.34.1
> 

Re: [PATCH v6 1/1] PCI: of: Remove max-link-speed generation validation

[Hans Zhang]

On 2026/3/7 01:08, Bjorn Helgaas wrote:
> [+cc Rob]
> 
> On Thu, Dec 18, 2025 at 09:20:36PM +0800, Hans Zhang wrote:
>> The current implementation of of_pci_get_max_link_speed() validates
>> max-link-speed property values to be in the range 1~4 (Gen1~Gen4).
>> However, this creates maintenance overhead as each new PCIe generation
>> requires updating this validation logic.
>>
>> Since device tree binding validation already enforces the allowed
>> values through the schema, and the callers of this function perform
>> their own validation checks, this intermediate validation becomes
>> redundant.
>>
>> Furthermore, with upcoming SOCs using Synopsys/Cadence IP requiring
>> Gen5/Gen6 support, removing this hardcoded check enables seamless
>> support for future PCIe generations without requiring kernel updates
>> for each new speed grade.
> 
> The upcoming SoC info seems like too much detail for the commit log.
> 
>> Remove the max-link-speed > 4 validation check while retaining the
>> property existence and non-zero check. This simplifies maintenance
>> and aligns with the existing validation architecture where DT binding
>> and driver-level checks provide sufficient validation.
> 
> I don't think it's legit to rely on DT binding checks.  The code
> should protect itself regardless of what offline binding checkers
> find.
> 
> I'm not convinced that callers do their own validation.  For example,
> dw_pcie_get_resources() assigns of_pci_get_max_link_speed() to
> pci->max_link_speed with no validation at all.
> 
> That's in the DWC core, so it's probably OK to expect the actual
> controller drivers to validate pci->max_link_speed when they use it,
> but dw_pcie_config_presets() uses pcie_link_speed[pci->max_link_speed]
> and I don't see an obvious check that prevents an out-of-bounds
> reference there.
> 
> I think this patch is the right direction -- I don't think there's
> anything in drivers/pci/of.c that *uses* the value of
> "max-link-speed", so it would be nice if it didn't have to be updated
> for every generation.

Hi Bjorn,

Thank you very much for your reply.

I will resend the v7 series patch and check in the dwc code whether 
there is an array out-of-bounds issue with the maximum link speed.

Best regards,
Hans

> 
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
>> ---
>> Changes for v6:
>> - It'd be good to return the actual errno as of_property_read_u32() can return
>>    -EINVAL, -ENODATA and -EOVERFLOW. (Mani)
>>
>> Changes for v5:
>> https://patchwork.kernel.org/project/linux-pci/patch/20251218125909.305300-1-18255117159@163.com/
>>
>> - Delete the check for speed. (Mani)
>>
>> Changes for v4:
>> https://patchwork.kernel.org/project/linux-pci/patch/20251105134701.182795-1-18255117159@163.com/
>>
>> - Add pcie_max_supported_link_speed.(Ilpo)
>>
>> Changes for v3:
>> https://patchwork.kernel.org/project/linux-pci/patch/20251101164132.14145-1-18255117159@163.com/
>>
>> - Modify the commit message.
>> - Add Reviewed-by tag.
>>
>> Changes for v2:
>> https://patchwork.kernel.org/project/linux-pci/cover/20250529021026.475861-1-18255117159@163.com/
>> - The following files have been deleted:
>>    Documentation/devicetree/bindings/pci/pci.txt
>>
>>    Update to this file again:
>>    dtschema/schemas/pci/pci-bus-common.yaml
>> ---
>>   drivers/pci/of.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>> index 3579265f1198..b56fdbcb3d72 100644
>> --- a/drivers/pci/of.c
>> +++ b/drivers/pci/of.c
>> @@ -888,10 +888,11 @@ bool of_pci_supply_present(struct device_node *np)
>>   int of_pci_get_max_link_speed(struct device_node *node)
>>   {
>>   	u32 max_link_speed;
>> +	int ret;
>>   
>> -	if (of_property_read_u32(node, "max-link-speed", &max_link_speed) ||
>> -	    max_link_speed == 0 || max_link_speed > 4)
>> -		return -EINVAL;
>> +	ret = of_property_read_u32(node, "max-link-speed", &max_link_speed);
>> +	if (ret)
>> +		return ret;
>>   
>>   	return max_link_speed;
>>   }
>> -- 
>> 2.34.1
>>