Hi Marek, On 23 August 2018 at 06:58, Marek Vasut <marek.va...@gmail.com> wrote: > On 08/23/2018 12:45 PM, Simon Glass wrote: > [...] >>>> Why is pci_bus_find_devfn() failing? >>> >>> Because this function is a hack to force-bind drivers to PCI devices >>> which are described in the DT with a compatible string. This does not >>> apply to this case. >> >> pci_bus_find_devfn() does not check compatible strings. It checks the >> PCI devfn stuff. If the node is in the DT and has the correct devfn, >> then pci_bus_find_devfn() should find it. >> >> It does require that the node produced a device, which does indeed >> require a compatible string, I think. > > Well, yeah, I skipped the middle step for the sake of brevity. > Ultimately, you need a compat string there. > >> We support either using DT with a compatible string or using >> PCI_DEVICE() without a DT node. But you already know that. > > Right > >>> If this function fails (correctly, no force-binding needs to happen >>> because there is no compatible string, we want to match on PCI IDs), >>> the code will continue and match on PCI ID/class with >>> pci_find_and_bind_driver() . That function is where we need to verify if >>> there isn't a PCI controller subnode in DT with a matching BFD which >>> should be associated with the new driver instance. >> >> So all of this is to avoid having a compatible string in the DT node for PCI? > > Yes, because it is redundant and possibly harmful. And because I want to > parse existing DTs without special U-Boot modifications or > hacking/twisting U-Boot in some obscure way which doesn't scale. > >> Earlier I said: >> >>>> So can you just add a compatible string and everything works? >> >> and you replied: >> >>> No, because that makes no sense. The code can very well match the driver >>> on the PCI IDs or classes, the DT node is supplementary information and >>> it needs to be made available to the matched driver if present in the >>> DT. The DT node is matched on the BFD (the reg property in the DT). >> >> Yes the DT node is matched on the BFD. Yes we can match the node as you say. >> >>> The compatible string would only add redundancy and can in fact be >>> harmful in case the PCI device could be replaced (ie. swapping USB EHCI >>> controller for USB xHCI controller while retaining the USB PHY ; the >>> controller itself could be discovered on the PCI bus, but the PHY can >>> not and must be described in the DT). >> >> This seems a bit odd. Without the compatible string we don't know what >> driver to use, except through the PCI_DEVICE() mechanism. Is that what >> you are using? > > Of course, you can match on PCI IDs or PCI class already, PCI is a > discoverable bus. > >> So is this a reason why we need this 3rd option? For your example are >> you saying that there are two different settings for a device, which >> result in using a different driver? > > What I am saying is the following needs to be supported: > > pci-controller@100 { > ... > usb@1,0 { > reg = <0x800 0 0 0 0>; > phys = <&usb0 0>; > phy-names = "usb"; > }; > ... > }; > > You want to match the PCI device in BFD x.1.0 using PCI ID (ie. the > driver that matches in this case is *hci-pci) and then associate the > node usb@1,0 , which contains PHY information for that *hci-pci driver, > with it. > >> How would DT normally handle this? > > That's how DTs do it, even for other than r8axxxx devices. There are iMX > devices which use the same approach for associating ethernet MAC with a > PCI NIC for example. > >> Overlays? Having two nodes and marking one status = "disabled"? This >> problem does not seem unique to PCI. I could have a similar problem >> with USB or even with a standard SoC having a memory-mapped USB >> controller that can support both protocols (controlled by a setting >> somewhere). > > You don't need overlays at all, the base DT is sufficient as is. >
Everything up to here seems OK. >> If you have both EHCI and a xHCI controller which can occupy the same >> BFD, then how would you supply in the DT options needed by the >> controller itself? Don't you need two nodes in that case? > > For the PHY case, it's controller-type-independent. What do you mean? Your example of why you can't use compatible strings says you might have two different PHYs. But I think you should answer my questions: >> If you have both EHCI and a xHCI controller which can occupy the same >> BFD, then how would you supply in the DT options needed by the >> controller itself? Don't you need two nodes in that case? > >> Also, what if these two devices each had their own PHY and there were >> settings in that PHY DT node? You would need a separate node for each >> one, right? >> >> I suggest that we don't NEED this third way. The question in my mind >> is whether it is necessary and doesn't just add confusion. >> [...] >> In any case, re Bin's list of things that need doing, I worry about >> having different code for sandbox than other archs. It invalidates our >> sandbox testing. Granted, we have to support the PCI emulators, but >> that's OK since that code is not used except in sandbox. We still want >> to support compatible strings in the DT for PCI devices, right? > > We do, since there seems to be usecase for those too. OK, well let's make sure we have some compatible notes too in sandbox, so we retain testing. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot