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

[Hans Zhang <18255117159@163.com>]

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