Re: [PATCH v3 5/6] PCI: cadence: Add callback functions for RP and EP controller

Hans Zhang
2025-04-12T16:02:49Z

On 2025/4/12 23:45, Hans Zhang wrote:
>>> +     /*
>>> +      * Clear AXI link-down status
>>> +      */
>>
>> That is an odd thing to do in map_bus. Also, it is completely racy
>> because...
>>
>>> +     regval = cdns_pcie_hpa_readl(pcie, REG_BANK_AXI_SLAVE,
>>> CDNS_PCIE_HPA_AT_LINKDOWN);
>>> +     cdns_pcie_hpa_writel(pcie, REG_BANK_AXI_SLAVE,
>>> CDNS_PCIE_HPA_AT_LINKDOWN,
>>> +                          (regval & GENMASK(0, 0)));
>>> +
>>
>> What if the link goes down again here.
>>
>
> Hi Rob,
>
> Thanks your for reply. Compared to Synopsys PCIe IP, Cadence PCIe IP has
> one more register - CDNS_PCIE_HPA_AT_LINKDOWN.  When the PCIe link
> appears link down, the register -CDNS_PCIE_HPA_AT_LINKDOWN bit0 is set
> to 1.  Then, ECAM cannot access config space. You need to clear
> CDNS_PCIE_HPA_AT_LINKDOWN bit0 to continue the access.
>
> In my opinion, this is where Cadence PCIe IP doesn't make sense.  As
> Cadence users, we had no choice, and the chip had already been posted to
> silicon.

Supplement: Prior to this series of patches, in the current linux master
branch, Cadence first generation PCIe IP has the following code:

void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
int where)
{
......
/* Clear AXI link-down status */
cdns_pcie_writel(pcie, CDNS_PCIE_AT_LINKDOWN, 0x0);
......
}

It seems that all Cadence PCIe IPs have this problem.

>
> Therefore, when we design the second-generation SOC, we will design an
> SPI interrupt. When link down occurs, an SPI interrupt is generated. We
> set CDNS_PCIE_HPA_AT_LINKDOWN bit0 to 0 in the interrupt function.
>
> If there are other reasons, please Manikandan add.
>
>
>
> In addition, by the way, ECAM is not accessible when the link is down.
> For example, if the RP is set to hot reset, the hot reset cannot be
> unreset. In this case, you need to use the APB to unreset. We mentioned
> an RTL bug to Cadence that we currently can't fix with our first or
> second generation chips. Cadence has not released RTL patch to us so far.
>
> This software workaround approach will also later appear in the Cixtech
> PCIe controller series patch.
>
>
> Best regards,
> Hans