Hi Tim, On Thu, 15 Apr 2021, 12:29 Tim Harvey, <[email protected]> wrote:
> On Wed, Apr 14, 2021 at 12:37 PM Simon Glass <[email protected]> wrote: > > > > Hi Tim, > > > > On Wed, 14 Apr 2021 at 06:32, Tim Harvey <[email protected]> wrote: > > > > > > On Sat, Dec 19, 2020 at 8:43 AM Simon Glass <[email protected]> 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 <[email protected]> > > > > --- > > > > > > > > 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 >
