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

[Simon Glass]

Hi Tim,

On Thu, 15 Apr 2021, 12:29 Tim Harvey, <tharvey at gateworks.com> wrote:

> 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?
>

Maybe,but I think someone just sent a patch for it.can you check the
mailing list?

Regards,
Simon


> 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
>