Re: [PATCH v4 6/7] PCI: mediatek-gen3: Add 100 ms delay after link up

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The required 100ms link training delay is silently bypassed when the `max-link-speed` property is omitted from the device tree.
--

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

PCI: mediatek-gen3: Add 100 ms delay after link up

This commit adds a 100 ms delay after link training completes for speeds
> 5.0 GT/s to comply with the PCIe r6.0 specification. It calls
pci_host_common_link_train_delay() using the max_link_speed value from the
device tree.

> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> index b0accd8285892..5abddec4e9bec 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
[ ... ]
> @@ -570,6 +571,8 @@ static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
>  		goto err_power_down_device;
>  	}
>  
> +	pci_host_common_link_train_delay(pcie->max_link_speed);

Is the 100ms link training delay silently bypassed here when the
max-link-speed property is omitted from the device tree?

Since pcie->max_link_speed defaults to 0 and is only populated if the
max-link-speed property is explicitly defined in the device tree, a missing
property leaves pcie->max_link_speed as 0. The controller then trains at
its native maximum capability (Gen3), but
pci_host_common_link_train_delay(0) will evaluate 0 > 2 as false,
completely skipping the required 100ms delay.

Additionally, due to a pre-existing inverted check in mtk_pcie_setup()
(if (max_speed >= 0 && max_speed <= err)), even if the link speed is
limited via the device tree, the override is usually ignored, leaving
pcie->max_link_speed as 0.

Does the driver need to determine the actual maximum link speed of the
port (for example, via mtk_pcie_get_controller_max_link_speed()) or read
the negotiated link status, rather than relying exclusively on the
optional device tree override variable?

> +
>  	return 0;
>  
>  err_power_down_device:

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