[U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn()

[Simon Glass]

Hi Stefan,

On Tue, 22 Jan 2019 at 02:36, Stefan Roese <sr at denx.de> wrote:
>
> Hi Simon,
>
> (added Bin, whom I forgot in this PCI patches)
>
> On 21.01.19 19:15, Simon Glass wrote:
> > On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr at denx.de> wrote:
> >>
> >> This function will be used by the Marvell Armada XP/38x PCIe driver,
> >> which is moved to DM right now. It's mostly copied from the Linux
> >> version.
> >>
> >> Signed-off-by: Stefan Roese <sr at denx.de>
> >> Cc: Simon Glass <sjg at chromium.org>
> >> ---
> >>   drivers/core/ofnode.c | 12 ++++++++++++
> >>   include/dm/ofnode.h   | 11 +++++++++++
> >>   2 files changed, 23 insertions(+)
> >
> > The code to do this right now is in pci_uclass_child_post_bind(). Do
> > you think you could break that out into a pci_... function that you
> > can call from your new function?
>
> Sure, I'll give it a try. While working on it, I noticed this difference
> in the current DEVFN usage in this pci_uclass_child_post_bind()
> implementation:
>
>                 pplat->devfn = addr.phys_hi & 0xff00;
>
> So, pplat->devfn uses bits 15-8 for DEVFN. Linux uses this definition
> instead:
>
> include/uapi/linux/pci.h:
> /*
>   * The PCI interface treats multi-function devices as independent
>   * devices.  The slot/function address of each device is encoded
>   * in a single byte as follows:
>   *
>   *     7:3 = slot
>   *     2:0 = function
>   */
> #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
> #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
> #define PCI_FUNC(devfn)         ((devfn) & 0x07)
>
> So here devfn uses bits 7-0 instead, which is what the MVEBU PCIe
> driver also expects. Do you know why there is this different
> implementation for devfn here in pci_uclass_child_post_bind()?
> Is this a bug which I should fix by shifting the bits correspondingly?

Yes I think it should be consistent. I hope this is a simple fix and
does not affect the drivers much.

>
> > Also I had a look at the code you wrote that calls this. Ideally we
> > would have the PCIe nodes have their own driver so that driver model
> > will automatically bind them, but I am not sure that is feasible.
>
> IIRC, this approach of the MISC bind function creating the PCI device
> instances for the child nodes was suggested by you when I wrote the
> Marvell MVPP2 ethernet driver (drivers/net/mvpp2.c) a few years ago.

OK. Can these be added to the device tree, or are they bound dynamically?

Regards,
Simon