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

[Tom Rini]

On Wed, Aug 08, 2018 at 09:37:25PM +0200, Marek Vasut 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.

DT should but isn't always OS agnostic.  DTS files that reside in the
Linux Kernel are in practice is Linux-centric with the expectation that
even if you could solve a given problem with valid DTS changes you make
whatever is parsing it do additional logic instead.  That,
approximately, is what your patch is doing.  If you added some HW
description information to the dtsi file everything would work as
expected as your DTS is describing the hardware and U-Boot is reading
that description and figuring out what to do with it.

The problem here is that in Linux, something that sees the compatible
renesas,pci-r8a7794 or renesas,pci-rcar-gen2 is doing whatever else
needs to be done.  Or something else?  Please explain how what you want
to have work here in U-Boot is working in the Linux kernel.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180810/0b761643/attachment.sig>