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

[Bin Meng]

Hi Marek,

On Thu, Aug 9, 2018 at 3:37 AM, Marek Vasut <marek.vasut at gmail.com> wrote:
> On 08/08/2018 05:32 PM, Bin Meng wrote:
>> Hi Marek,
>>
>> On Wed, Aug 8, 2018 at 10:33 PM, Marek Vasut <marek.vasut at gmail.com> wrote:
>>> On 08/08/2018 03:39 PM, Bin Meng wrote:
>>>> Hi Marek,
>>>>
>>>> On Wed, Aug 8, 2018 at 9:24 PM, Marek Vasut <marek.vasut at gmail.com> wrote:
>>>>> On 08/08/2018 03:14 PM, Bin Meng wrote:
>>>>>> Hi Marek,
>>>>>>
>>>>>> On Wed, Aug 8, 2018 at 9:03 PM, Marek Vasut <marek.vasut at gmail.com> wrote:
>>>>>>> The PCI controller can have DT subnodes describing extra properties
>>>>>>> of particular PCI devices, ie. a PHY attached to an EHCI controller
>>>>>>> on a PCI bus. This patch parses those DT subnodes and assigns a node
>>>>>>> to the PCI device instance, so that the driver can extract details
>>>>>>> from that node and ie. configure the PHY using the PHY subsystem.
>>>>>>>
>>>>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas at gmail.com>
>>>>>>> Cc: Simon Glass <sjg at chromium.org>
>>>>>>> ---
>>>>>>>  drivers/pci/pci-uclass.c | 14 ++++++++++++++
>>>>>>>  1 file changed, 14 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
>>>>>>> index 46e9c71bdf..306bea0dbf 100644
>>>>>>> --- a/drivers/pci/pci-uclass.c
>>>>>>> +++ b/drivers/pci/pci-uclass.c
>>>>>>> @@ -662,6 +662,8 @@ static int pci_find_and_bind_driver(struct udevice *parent,
>>>>>>>                 for (id = entry->match;
>>>>>>>                      id->vendor || id->subvendor || id->class_mask;
>>>>>>>                      id++) {
>>>>>>> +                       ofnode node;
>>>>>>> +
>>>>>>>                         if (!pci_match_one_id(id, find_id))
>>>>>>>                                 continue;
>>>>>>>
>>>>>>> @@ -691,6 +693,18 @@ static int pci_find_and_bind_driver(struct udevice *parent,
>>>>>>>                                 goto error;
>>>>>>>                         debug("%s: Match found: %s\n", __func__, drv->name);
>>>>>>>                         dev->driver_data = find_id->driver_data;
>>>>>>> +
>>>>>>> +                       dev_for_each_subnode(node, parent) {
>>>>>>> +                               phys_addr_t df, size;
>>>>>>> +                               df = ofnode_get_addr_size(node, "reg", &size);
>>>>>>> +
>>>>>>> +                               if (PCI_FUNC(df) == PCI_FUNC(bdf) &&
>>>>>>> +                                   PCI_DEV(df) == PCI_DEV(bdf)) {
>>>>>>> +                                       dev->node = node;
>>>>>>> +                                       break;
>>>>>>> +                               }
>>>>>>
>>>>>> The function pci_find_and_bind_driver() is supposed to bind devices
>>>>>> that are NOT in the device tree. Adding device tree access in this
>>>>>> routine is quite odd. You can add the EHCI controller that need such
>>>>>> PHY subnodes in the device tree and there is no need to modify
>>>>>> anything I believe. If you are looking for an example, please check
>>>>>> pciuart0 in arch/x86/dts/crownbay.dts.
>>>>>
>>>>> Well this does not work for me, the EHCI PCI doesn't get a DT node
>>>>> assigned, check r8a7794.dtsi for the PCI devices I use.
>>>>>
>>>>
>>>> I think that's because you don't specify a "compatible" string for
>>>> these two EHCI PCI nodes.
>>>
>>> That's perfectly fine, why should I specify it ? Linux has no problem
>>> with it either.
>>>
>>
>> Without a "compatible" string, DM does not bind any device in the
>> device tree to a driver, hence no device node created. This is not
>> Linux.
>
> DT is NOT Linux specific, it is OS-agnostic, DT describes hardware and
> hardware only. If U-Boot cannot parse DT correctly, U-Boot is broken and
> must be fixed.
>
> This is a fix. If there is a better fix, I am open to it.
>

Sorry this is a hack to current U-Boot implementation, not fix. The
fix should be adding "ehci-pci" compatible string in the r8a7794.dtsi.

I disagree DT is OS-agnostic. This are lots of stuff in DT that are
OS-specific. eg: there are lots of bindings in DT that requires
Linux's device driver framework to work with. As you said, DT is just
a standard to describe hardware and hardware only. But there are
various methods to describe hardware in DT that's why we have a proper
defined bindings in Linux. How OS parses and utilizes these
information is completely on their own.

Regards,
Bin