Hi Stefan,
On Thu, 31 Jan 2019 at 03:45, Stefan Roese sr@denx.de wrote:
Hi Simon,
On 31.01.19 11:04, Simon Glass wrote:
Hi Stefan,
On Tue, 22 Jan 2019 at 02:36, Stefan Roese sr@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@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@denx.de Cc: Simon Glass sjg@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 = slot2: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.
As you might have spotted in my later patch version (e.g. v3), I've moved my patch to use the U-Boot "devfn" usage in bits 15-8. I've commented this in the driver (ported from Linux) and also added a comment about this in the function header of pci_get_devfn():
OK.
+/**
- pci_get_devfn() - Extract the devfn from fdt_pci_addr of the device
- Get devfn from fdt_pci_addr of the specifified device
- @dev: PCI device
- @return devfn in bits 15...8 if found, -ENODEV if not found
This seemed to be the least intrusive option. I hesitate to completely move to the Linux "devfn" usage in bits 7-0, as this might have serious problems with the current U-Boot implementation in its drivers and interfaces.
Yes that sounds best..
One thing we might do though, is to add a comment about this difference in the U-Boot PCI_DEVFN macro definition. Should I generate a patch for this?
Yes that's a good idea.
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?
The child nodes are already in the device tree. Each child represents a PCIe root port with its own register base. But the compatible property resides in the parent node. Please note that I don't want to make any changes to the DT, as this is the original Linux version.
OK, fair enough.
Regards, Simon