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@xxxxxxx>
> > Acked-by: Manivannan Sadhasivam <mani@xxxxxxxxxx>
> > ---
> > 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@xxxxxxx/
> >
> > - Delete the check for speed. (Mani)
> >
> > Changes for v4:
> > https://patchwork.kernel.org/project/linux-pci/patch/20251105134701.182795-1-18255117159@xxxxxxx/
> >
> > - Add pcie_max_supported_link_speed.(Ilpo)
> >
> > Changes for v3:
> > https://patchwork.kernel.org/project/linux-pci/patch/20251101164132.14145-1-18255117159@xxxxxxx/
> >
> > - Modify the commit message.
> > - Add Reviewed-by tag.
> >
> > Changes for v2:
> > https://patchwork.kernel.org/project/linux-pci/cover/20250529021026.475861-1-18255117159@xxxxxxx/
> > - 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