[U-Boot] [PATCH] pci: Support parsing PCI controller DT subnodes

[Marek Vasut]

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
> 


-- 
Best regards,
Marek Vasut