Re: [PATCH v2 -next 05/11] PCI: brcmstb: Restore CRS in RootCtl after prstn_n

[Stanimir Varbanov <svarbanov@suse.de>]

Hi Florian,

On 9/10/24 19:59, Florian Fainelli wrote:
> On 9/10/24 08:18, Stanimir Varbanov wrote:
>> RootCtl bits might reset by perst_n during probe, re-enable
>> CRS SVE here in pcie_start_link.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> 
> This looks like a bug fix, and we should explain what is the user
> visible effect of that, if any.

It is definitely a bugfix. Otherwise, CRS Software Visibility is
important feature from pcie1.1. Not enabling it on Root Port could lead
to infinite configuration retry cycles when enumerate endpoints which
supports CRS. For more information [1] and [2].

I spent some time debugging it and found that this is not the proper
solution.  I think the issue comes from wrongly implemented .add_bus
pci_ops. Looks like .add_bus op shouldn't call brcm_pcie_start_link()
but invoke before pci_host_probe(), then the issue will fix by itself.

What I observed is that pci_enable_crs() is setting CSR Software
Visibility Enable bit but the controller is ignoring it without error
(reading the Root Control register returns zero). This means that the
controller is not ready to accept configuration write requests at that
time, that's why I tried the following diff which seems to work:

 static struct pci_ops brcm_pcie_ops = {
        .map_bus = brcm_pcie_map_bus,
        .read = pci_generic_config_read,
        .write = pci_generic_config_write,
-       .add_bus = brcm_pcie_add_bus,
-       .remove_bus = brcm_pcie_remove_bus,
 };

 static struct pci_ops brcm7425_pcie_ops = {
@@ -1983,6 +2018,9 @@ static int brcm_pcie_probe(struct platform_device
*pdev)

        platform_set_drvdata(pdev, pcie);

+       //TODO: check for error
+       brcm_pcie_start_link(pcie);
+
        ret = pci_host_probe(bridge);
        if (!ret && !brcm_pcie_link_up(pcie))
                ret = -ENODEV;

Of course this change would work on RPi5 because there are no regulators.

I will drop the patch from the series for now and work on a proper solution.

regards,
~Stan

[1]
https://patchwork.kernel.org/project/linux-pci/patch/53FFA54D.9000907@gmail.com/
[2]
https://blog.linuxplumbersconf.org/2017/ocw/system/presentations/4732/original/crs.pdf