[+cc Rob for DT platform device dependency question]
On Thu, Apr 02, 2020 at 04:27:23PM +0200, Nicolas Saenz Julienne wrote:
> On Wed, 2020-04-01 at 15:41 -0500, Bjorn Helgaas wrote:
> > On Tue, Mar 24, 2020 at 07:28:11PM +0100, Nicolas Saenz Julienne wrote:
> > > xHCI's PCI fixup, run at the end of pcie-brcmstb's probe, depends on
> > > RPi4's VideoCore firmware interface to be up and running. It's possible
> > > for both initializations to race, so make sure it's available prior to
> > > starting.
> > >
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx>
> > > Reviewed-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
> > > ---
> > > drivers/pci/controller/pcie-brcmstb.c | 15 +++++++++++++++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/pcie-brcmstb.c
> > > b/drivers/pci/controller/pcie-brcmstb.c
> > > index 3a10e678c7f4..a3d3070a5832 100644
> > > --- a/drivers/pci/controller/pcie-brcmstb.c
> > > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > > @@ -28,6 +28,8 @@
> > > #include <linux/string.h>
> > > #include <linux/types.h>
> > >
> > > +#include <soc/bcm2835/raspberrypi-firmware.h>
> > > +
> > > #include "../pci.h"
> > >
> > > /* BRCM_PCIE_CAP_REGS - Offset for the mandatory capability config regs */
> > > @@ -917,11 +919,24 @@ static int brcm_pcie_probe(struct platform_device
> > > *pdev)
> > > {
> > > struct device_node *np = pdev->dev.of_node, *msi_np;
> > > struct pci_host_bridge *bridge;
> > > + struct device_node *fw_np;
> > > struct brcm_pcie *pcie;
> > > struct pci_bus *child;
> > > struct resource *res;
> > > int ret;
> > >
> > > + /*
> > > + * We have to wait for the Raspberry Pi's firmware interface to be up
> > > + * as some PCI fixups depend on it.
> >
> > It'd be nice to know the nature of this dependency between the
> > firmware interface and the fixups. This may be useful for future
> > maintenance. E.g., if PCI config access doesn't work until the
> > firmware interface is up, that would affect almost everything. But
> > you say "some PCI fixups", so I suppose the actual dependency is
> > probably something else.
>
> Sorry it wasn't clear enough, I'll redo this comment. Also note that
> the PCIe bus and the XHCI chip are hardwired, so that's the only
> device that'll ever be available on the bus.
>
> VIA805's XHCI firmware has to be loaded trough RPi's firmware
> mailbox in between the PCIe bus probe and the subsequent USB probe.
> Note that a PCI reset clears the firmware. The only mechanism
> available in between the two operations are PCI Fixups. These are
> limited in their own way, as I can't return -EPROBE_DEFER if the
> firmware interface isn't available yet. Hence the need for an
> explicit dependency between pcie-brcmstb and raspberrypi's firmware
> mailbox device.
>
> Your concern here showcases this series' limitations. From a high
> level perspective it's not clear to me who should be responsible for
> downloading the firmware.
I think it's fairly common for drivers to download firmware to their
devices. I guess there's not really any need to download the firmware
until a driver wants to use the device, right?
> And I get the feeling I'm abusing PCI fixups. I haven't found any
> smart way to deal with this three way dependency of
> platform/non-platform devices.
So IIUC, the three-way dependency involves:
1) brcm_pcie_probe(), which initialize the PCI host controller
platform device, enumerates PCI devices, and makes them available
for driver binding,
2) the firmware mailbox initialization (maybe
rpi_firmware_probe()?),
3) quirk_usb_early_handoff(), which downloads firmware to the VL805
PCI USB adapter via rpi_firmware_init_vl805(), which uses the
firmware mailbox?
Is there some way to express a dependency between
"raspberrypi,bcm2835-firmware" (the platform device claimed by
rpi_firmware_probe() and "brcm,bcm2711-pcie"? If we could ensure that
rpi_firmware_probe() runs before brcm_pcie_probe(), would that solve
part of this?
Bjorn
