Hi Marek, On Tue, Aug 14, 2018 at 4:54 PM, Marek Vasut <marek.va...@gmail.com> wrote: > On 08/14/2018 04:34 AM, Bin Meng wrote: >> 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? > > No, because we are not replicating Linux behavior. >
But you kept mentioning you wanted U-Boot to use exactly the same DT from Linux. And I pointed out that FreeBSD's DT files are not the same as Linux. Here you are saying you don't want U-Boot to replicate Linux behavior, if not the Linux behavior, what do you want to suggest then? >> 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: > > You are inserting words into my mouth and I dislike that. I never said > anything about Linux. I said DT is OS agnostic and U-Boot should be able > to parse DT as fully as possible. See above comment. I might have used some words that made you feel uncomfortable, and for me I felt the same way. Part of the reason is that I am not a native English speaker and I may mis-use/interpret words. If that's the case I apologize. Anyway I won't quarrel against this over and over again. This does not help to move things forward. > >> * 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 > > I don't think this is limited to sandbox, although I don't see a > real-world usecase right now. > You are welcome to send patches against sandbox PCI codes to remove such limitation, to make it behave like a real-world bus device. >> * 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) > > I cannot test such changes, but I believe there is > PCI_CLASS_COMMUNICATION_SERIAL and matching on that would suffice ? > Maybe, but I need check the datasheet to confirm. This can be a good start though. Note I can test the changes, and I can do the whole bunch of such proposed design changes if you don't mind, but let's wait for Simon's comments. >> * 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
