[PATCH v3 13/28] pci: Update to use new sequence numbers

[Tim Harvey]

On Wed, Apr 14, 2021 at 12:37 PM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Tim,
>
> On Wed, 14 Apr 2021 at 06:32, Tim Harvey <tharvey at gateworks.com> wrote:
> >
> > On Sat, Dec 19, 2020 at 8:43 AM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Now that we know the sequence number at bind time, there is no need for
> > > special-case code in dm_pci_hose_probe_bus().
> > >
> > > Note: the PCI_CAP_ID_EA code may need a look, but there are no test
> > > failures so I have left it as is.
> > >
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > ---
> > >
> > > Changes in v3:
> > > - Update PCI to use manual sequence numbering
> > >
> > > Changes in v2:
> > > - Use the sequence number directly instead of max bus
> > >
> > >  drivers/pci/pci-uclass.c | 45 ++++++++++++++++++++++++----------------
> > >  drivers/pci/pci_auto.c   | 10 ++++-----
> > >  2 files changed, 32 insertions(+), 23 deletions(-)
> > >
> > > Applied to u-boot-dm/next, thanks!
> >
> > Hi Simon,
> >
> > I have a (not yet submitted) pending patch to migrate the gwventana
> > board to DM_PCI / DM_ETH that this particular patch broke and I'm
> > trying to understand why.
> >
> > The Gateworks Ventana boards have a PCIe switch where the downstream
> > PERST#'s are GPIO's off the switch and a e1000 PCIe GBe device is on
> > one of those downstream ports. For non-dm PCI I have a
> > 'board_pci_fixup_dev' function that allows board support to configure
> > the switch's GPIO and toggle downstream PERST# during enumeration.
> > When I add this function to dm_pci I end up getting a data abort when
> > the e1000 driver tries to initialize (after PCI enumeration).
>
> Firstly, I think I did the PCI conversion about 6 years ago so it is
> not fresh in my memory.
>
> >
> > Any idea what is causing this and what I need to do to work around it?
> > Nothing in my patch deals with device sequence numbers.
>
> An abort presumably indicates that the memory is not there, so perhaps
> the e1000 is still in reset, or has been set up to appear at a
> different address?
>

Simon,

It does appear that everything downstream from the switch is
inaccessible 'after' enumeration and I've verified that PERST# both
upstream and downstream is in the correct state and never gets
asserted again.

The issue appears to be related to the PCI_CAP_ID_EA code and per your
commit log it sounds like you were not sure if that was correct.
Adding back in the following resolves the issue for me:

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 9abdbc54b0..fcfe520e5e 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -646,6 +646,15 @@ int dm_pci_hose_probe_bus(struct udevice *bus)
                return log_msg_ret("probe", ret);
        }

+       if (!ea_pos) {
+               if (sub_bus != dev_seq(bus)) {
+                       debug("%s: Internal error, bus '%s' got seq
%d, expected %d\n",
+                                       __func__, bus->name,
dev_seq(bus), sub_bus);
+                       return -EPIPE;
+               }
+               sub_bus = pci_get_bus_max();
+       }
+
        dm_pciauto_postscan_setup_bridge(bus, sub_bus);

        return sub_bus;

Adding some debugging it appears that the devices I have downstream
from the bridge have 'ea_os = 0'.

Does the above let you know what's going on?

Tim





> You should be abe to use 'pci hdr 0.4.0' or whatever to look at the
> BARs and make sure they are value with 'md', etc.
>
> > Additionally I feel like the best way to add support for the custom
> > downstream PCI switch reset requirements is to add a UCLASS_PCI driver
> > for the switch in my board support but when I attempted that solution
> > I run into an issue where pci read/write's cause a prefetch abort
> > because I need to use imx_pcie_dm_{read,write}_config instead of the
> > default ops. I'm not quite sure how to get hold of the ops for the imx
>
> If you write a PCI driver then you can provide the access operations
> yourself....see drivers/pci for some examples. I suggest you add
> subnodes to the devicetree and specify the compatible string of your
> PCI switch so it picks up the right driver.
>
> > controller to set this up properly. Furthermore if I do end up with
> > that solution I later need to be able to walk the PCI bus to perform
> > various dt fixups on pci device nodes - do you have an example of how
> > I could walk the bus using DM_PCI?
>
> Do you mean a breadth-first search? It is something like:
>
> struct udevice *dev;
>
> static void do_something(struct udevice *parent)
> {
>    device_foreach_child_probe(dev, parent) {
>        do_something((dev);
>    }
>
> ...
>
> struct udevice *bus;
> int  ret;
>
> ret = uclass_first_device_err(UCLASS_PCI, &bus);
> if (ret)
>    return log_msg_ret("pci", ret);
> do_something(bus);
>
> >
> > Best regards,
> >
> > Tim
>
> Regards,
> Simon