Re: [PATCH v4 2/7] PCI: cadence: Add post-link delay for LGA and j721e glue driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unhandled error code and failure to enforce mandatory PCIe link delay
--

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

PCI: cadence: Add post-link delay for LGA and j721e glue driver

The Cadence LGA PCIe host controller is missing the mandatory 100 ms delay
after link training for speeds > 5.0 GT/s. This patch adds a max_link_speed
field to struct cdns_pcie and inserts the required delay after link
establishment.

> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
> index 0bc9e6e90e0e0..058e4e619654c 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
[ ... ]
> @@ -397,6 +398,9 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
>  	rc->device_id = 0xffff;
>  	of_property_read_u32(np, "device-id", &rc->device_id);
>  
> +	if (pcie->max_link_speed < 1)
> +		pcie->max_link_speed = of_pci_get_max_link_speed(np);
> +

If the max-link-speed property is missing from the device tree,
of_pci_get_max_link_speed() returns -EINVAL. Does this code handle that
error correctly?

It looks like the negative error code (-22) will be assigned directly to
pcie->max_link_speed.

> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
> index 2b0211870f02a..18e4b6c760b51 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
[ ... ]
> @@ -115,6 +116,9 @@ int cdns_pcie_host_start_link(struct cdns_pcie_rc *rc,
>  	if (!ret && rc->quirk_retrain_flag)
>  		ret = cdns_pcie_retrain(pcie, pcie_link_up);
>  
> +	if (!ret)
> +		pci_host_common_link_train_delay(pcie->max_link_speed);
> +
>  	return ret;

Since pci_host_common_link_train_delay() checks if max_link_speed > 2,
would a negative value silently bypass the required 100ms delay on
platforms where the device tree property is missing?

Could we instead fall back to querying the hardware's actual link capability
from the PCI_EXP_LNKCAP register when the device tree property is absent,
so the hardware trains correctly and the delay is enforced without requiring
device tree updates?

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