On Wed, Apr 17, 2019 at 6:49 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>
> [+cc Sinan, Luca, Rajat (incidental mention of iwlwifi)]
>
> Hi Scott,
>
> On Tue, Apr 16, 2019 at 04:34:36PM -0700, Scott Branden wrote:
> > I have a possibly simple or may be more involved question:
> >
> > I have a PCI device that the hardware boots up and enumerates in GEN2 mode.
> >
> > Once the linux device driver is loaded we can download new firmware to the
> > card that may reinitialize the PCIe PHY in GEN3 mode and resizing the BARs.
> >
> > We can have the device re-enumerated via user space:
> >
> > echo 1 > /sys/bus/pci/devices/0000\:01\:00.0/remove
> >
> > echo 1 > /sys/bus/pci/rescan
> >
> > But, is it possible to do this inside the kernel device driver after it has
> > downloaded the new firmware instead?
> >
> > Before I start trying to play with pci_stop_and_remove_bus_device followed
> > by pci_rescan_bus I would like to know if what I'm trying to do is a correct
> > approach?
>
> Sinan mentioned the hfi1 driver, which does reinitialize the link to
> transition from gen2 to gen3. This is a little bit messy and I think
> it has a little too much knowledge of link internals that
> theoretically should be managed by the PCI core, but the PCI core
> doesn't offer any good alternatives.
>
> But your situation sounds a little more complicated because you also
> want to resize the BARs, which means you need to remove & re-enumerate
> the device or at least somehow update the whole dev->resource[] and
> resource tree situation. Removing the device of course means
> detaching the driver, and doing that from within the driver itself
> is a little problematic.
>
> The iwlwifi driver does seem to do that (it calls
> pci_stop_and_remove_bus_device() from iwl_trans_pcie_removal_wk(),
> added by 49564a806fc5 ("iwlwifi: pcie: remove non-responsive device"))
> but I'm a little hesitant about copying this design. It's a singleton
> (all other uses are in hotplug drivers), the non-responsive device
> situation is not unique to iwlwifi, and scheduling work to remove the
> current driver seems a little too clever.
Our first choice of solution was to enable hotplug, but Intel had
concerns about enabling it at that time. Also the othe rthing we were
not sure was whether the PCI link actually comes down when the device
stops responding. Thus we did what we did...
>
> In any event, your situation is much different from iwlwifi. Your
> device *is* responding; you merely want to download new firmware,
> re-enumerate the device, and re-attach your driver.
>
> I wonder if we could invent some scheme where you'd have a separate
> driver (possibly implemented in the same source file as your main
> driver but with a different struct pci_driver), where that driver's
> .probe() method would check to see whether a firmware update was
> needed, and if so it would download the new firmware and return an
> indication to the PCI core that the device should be removed, reset,
> and re-enumerated.
+1 to this. I think such a mechanism in the PCI core would find
multiple users. In fact if available, we can possibly also change
iwlwifi to use that.
Thanks,
Rajat
>
> Bjorn
