Re: [PATCH v4 3/7] PCI: cadence: HPA: Add post-link delay

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unchecked error return from `of_pci_get_max_link_speed()` disables the mandatory link training delay when the DT property is missing.
--

commit b05c1e9fe77d90401c0d9f6da4d6922526b3a553
Author: Hans Zhang <18255117159@163.com>

PCI: cadence: HPA: Add post-link delay
    
This commit introduces a mandatory 100 ms post-link training delay for PCIe
speeds > 5.0 GT/s to comply with the PCIe r6.0 specification. It reads the
max-link-speed property from the device tree to determine the appropriate
delay, defaulting to the value from the glue driver if set.

> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
> --- a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c

[ ... ]

> @@ -304,6 +306,8 @@ int cdns_pcie_hpa_host_link_setup(struct cdns_pcie_rc *rc)
>  	ret = cdns_pcie_host_wait_for_link(pcie, cdns_pcie_hpa_link_up);
>  	if (ret)
>  		dev_dbg(dev, "PCIe link never came up\n");
> +	else
> +		pci_host_common_link_train_delay(pcie->max_link_speed);

Will this delay be skipped entirely if the max-link-speed device tree
property is missing?

[ ... ]

> @@ -343,6 +348,9 @@ int cdns_pcie_hpa_host_setup(struct cdns_pcie_rc *rc)
>  		rc->cfg_res = res;
>  	}
>  
> +	if (pcie->max_link_speed < 1)
> +		pcie->max_link_speed = of_pci_get_max_link_speed(np);

If the max-link-speed property is not present in the device tree,
of_pci_get_max_link_speed() returns -EINVAL (-22):

drivers/pci/pci.h:of_pci_get_max_link_speed() {
    ...
    return -EINVAL;
}

This negative value is then assigned to pcie->max_link_speed. Later, when
pci_host_common_link_train_delay() is called, it checks:

drivers/pci/controller/pci-host-common.h:pci_host_common_link_train_delay() {
    if (max_link_speed > 2)
        msleep(PCIE_RESET_CONFIG_WAIT_MS);
}

Since -22 is not greater than 2, the delay would be bypassed. Should the
code check for a negative return value from of_pci_get_max_link_speed()
before assigning it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518004246.1384532-1-18255117159@163.com?part=3