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

[Hans Zhang <18255117159@163.com>]

On 5/18/26 10:16, Manikandan Karunakaran Pillai wrote:
> 
> 
>> EXTERNAL MAIL
>>
>>
>> The Cadence HPA (High Performance Architecture IP) specific link setup
>> function cdns_pcie_hpa_host_link_setup() waits for the link to come up
>> but does not implement the required 100 ms delay after link training
>> completes for speeds > 5.0 GT/s (PCIe r6.0 sec 6.6.1).
>>
>> Add a call to pci_host_common_link_train_delay() immediately after the
>> link is confirmed to be up, using the max_link_speed field. Also, in the
>> HPA host setup function, read the device tree property "max-link-speed"
>> to initialize max_link_speed if not already set by a glue driver.
>>
>> This ensures compliance for HPA-based platforms.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>> drivers/pci/controller/cadence/pcie-cadence-host-hpa.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
>> b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
>> index 0f540bed58e8..8ef58ed01daa 100644
>> --- a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
>> +++ b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
>> @@ -15,6 +15,8 @@
>>
>> #include "pcie-cadence.h"
>> #include "pcie-cadence-host-common.h"
>> +#include "../pci-host-common.h"
>> +#include "../../pci.h"
>>
>> static u8 bar_aperture_mask[] = {
>> 	[RP_BAR0] = 0x3F,
>> @@ -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);
>>
>> 	return ret;
>> }
>> @@ -313,6 +317,7 @@ int cdns_pcie_hpa_host_setup(struct cdns_pcie_rc *rc)
>> {
>> 	struct device *dev = rc->pcie.dev;
>> 	struct platform_device *pdev = to_platform_device(dev);
>> +	struct device_node *np = dev->of_node;
>> 	struct pci_host_bridge *bridge;
>> 	enum   cdns_pcie_rp_bar bar;
>> 	struct cdns_pcie *pcie;
>> @@ -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);
>> +
> 
> Similar queries as for Cadence LGA controllers. Why do you need the max_link_speed check for "<1" and
> What would be the consequences of not defining the max-link-speed in dts ?

Hi Manikandan,

It has been replied in patch 0002.

Best regards,
Hans

> 
>> 	/* Put EROM Bar aperture to 0 */
>> 	cdns_pcie_hpa_writel(pcie, REG_BANK_IP_CFG_CTRL_REG,
>> CDNS_PCIE_EROM, 0x0);
>>
>> --
>> 2.43.0