On 08/14/2018 11:35 AM, Bin Meng wrote: [...]
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?
Just parse the information out of the DT fully and be done with it. That is what I suggest.
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.
This is a technical discussion, we should use arguments, not feelings. I am fine, but thanks for your concern.
- 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.
All those sandbox changes can be done in parallel to this change though, right ?
- 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