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

[Tom Rini <trini@konsulko.com>]

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.

> > 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!

-- 
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/20180813/872492ea/attachment-0001.sig>