Hi Tom, On Tue, Aug 14, 2018 at 1:16 AM, Tom Rini <tr...@konsulko.com> wrote: > On Mon, Aug 13, 2018 at 06:07:14PM +0200, Marek Vasut wrote: >> On 08/13/2018 03:39 PM, Tom Rini wrote: >> [...] >> >> >>>> Next step is to upstream the DT >> >>>> changes to Linux kernel, then sync the changes to U-Boot to satisfy >> >>>> this obsession - using exactly the same DT as Linux. >> >>> >> >>> This is not gonna happen. >> >>> >> >>> Sorry, you're really just wasting my time with this foolishness. If >> >>> U-Boot cannot parse valid DT bindings while other OSes can, U-Boot is >> >>> broken and must be fixed. So far I only see you attacking this patch and >> >>> trying to pull in everything you can do avoid accepting this patch or >> >>> providing a better alternative. This is not a constructive discussion, >> >>> so I stop here. >> >> >> >> The fix in this patch is purely hack, period. >> > >> > Lets step back for a moment. >> > >> > First, U-Boot intends to be, in the case where a relevant DTS file >> > exists, the Linux kernel one PLUS additions we require (u-boot,dm-spl, >> > u-boot,dm-pre-reloc, but also sometimes stdout-path or properties that >> > are omitted for various reasons). >> >> Right, which doesn't apply here. None of those u-boot,... props are >> needed in this case. > > Which is why I also mentioned the non-u-boot specific props we also need > sometimes. My point is two-fold: > 1: We can and will _add_ information to the dts files that come from > Linux. > 2: Not all information that we add is U-Boot prefixed. >
It would be better if we document such DT expectation somewhere. >> > Second, I've asked before (both in this thread and on IRC), and not >> > gotten an answer yet, as to how Linux goes "Oh, _this_ PCI device and >> > _this_ DT node need to be matched and populate some data >> > structures". >> >> You did get an answer to that on irc from George. Looks like >> of_pci_find_child_device() in drivers/pci/of.c > > Yeah, George said he thought that might be it but didn't have time to > confirm. > >> > Marek's patch seems to be, in short "here's where U-Boot >> > needs to wire things up". Bin has said that no, the function in >> > question is for other things. >> >> I disagree with this. It's a bind function and assigns other parameters >> of the driver instance too. >> >> > I think knowing where Linux does this >> > would be instructive to figure out where we need to have some additional >> > logic added OR we can make some cost/benefit analysis to see if it makes >> > more sense overall to add compatibles to some nodes rather than add to >> > the binary size. >> >> Adding compatible does not make any sense, the PCI ID provides that >> information. Adding compatible would only add redundancy which could >> possibly be even harmful (ie. if the controller got replaced with >> another one). > > To try and move things along rather than re-argue the same point, you're > saying that our pci_find_and_bind_driver() is the rough equivalent of > of_pci_find_child_device() or at least pci_set_of_node() (which calls > of_pci_find_child_device()). > > So, Bin, if this isn't the right place to start down this path, where > would be? Given that Linux can take a DTB and PCI bus with devices and > get things right, what would it look like for U-Boot to replicate the > same behavior? Instead of having to add explicit compatible nodes for > each PCI device, as I understand (but correct me if I'm wrong) we're > doing today. Thanks! So is this a requirement for all U-Boot driver subsystems to replicate the same Linux behavior? If yes, can we have it officially documented somewhere? Since Marek refused to take the original U-Boot option 1 to support his case, and asked U-Boot to follow Linux's practice on PCI device binding, if we go that way, here is what we can do: * Keep pci-uclass driver's post_bind() and child_post_bind() only for Sandbox configuration * Keep the call to pci_bus_find_devfn() in pci_bind_bus_devices() only for Sandbox configuration * Sandbox is special. We should limit the mechanism of matching PCI emulation device via "compatible" to sandbox only * Assign the DT node to the bound device in pci_find_and_bind_driver() if there is a valid PCI "reg" encoding for a specific PCI device in the device tree * Create DM PCI test case against the DT node assignment * Remove all compatible string in U-Boot's PCI device drivers: eg: ehci_pci_ids[], xhci_pci_ids[], etc. IOW, all PCI device drivers should only use U_BOOT_PCI_DEVICE(), aka the original U-Boot option 2 * Fork a "pci-ns16550" driver to support U_BOOT_PCI_DEVICE(), as currently PCI ns16550 device driver uses "compatible" string to do the matching, and update crownbay.dts and galileo.dts (so far I only know two boards are using PCI ns16550 serial port) * Make sure all DM PCI test cases are not broken * Document all of the above changes in doc/driver-model/pci-info.txt I am not sure if I missed anything. Simon, could you also comment on it? Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot