On Mon, Nov 25, 2019 at 2:20 PM Alexey Kardashevskiy <a...@ozlabs.ru> wrote: > > > > On 20/11/2019 12:28, Oliver O'Halloran wrote: > > Use the pnv_eeh_find_edev() helper to look up the eeh_dev for a device > > rather than doing it via the pci_dn. > > This is not what the patch does. I struggle to see what is that thing > really.
Hmm, looks like a rebase screw up. This patch and the following one (22/46) used to be one patch, but I thought it was getting a bit large and split them. > > Signed-off-by: Oliver O'Halloran <ooh...@gmail.com> > > --- > > arch/powerpc/platforms/powernv/eeh-powernv.c | 44 ++++++++++++++------ > > 1 file changed, 31 insertions(+), 13 deletions(-) > > > > diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c > > b/arch/powerpc/platforms/powernv/eeh-powernv.c > > index 6ba74836a9f8..1cd80b399995 100644 > > --- a/arch/powerpc/platforms/powernv/eeh-powernv.c > > +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c > > @@ -374,20 +374,40 @@ static struct eeh_dev *pnv_eeh_probe_pdev(struct > > pci_dev *pdev) > > int ret; > > int config_addr = (pdev->bus->number << 8) | (pdev->devfn); > > > > + pci_dbg(pdev, "%s: probing\n", __func__); > > + > > /* > > - * When probing the root bridge, which doesn't have any > > - * subordinate PCI devices. We don't have OF node for > > - * the root bridge. So it's not reasonable to continue > > - * the probing. > > + * EEH keeps the eeh_dev alive over a recovery pass even when the > > + * corresponding pci_dev has been torn down. In that case we need > > + * to find the existing eeh_dev and re-bind the two. > > */ > > - if (!edev || edev->pe) > > - return NULL; > > + edev = pnv_eeh_find_edev(phb, config_addr); > > > What was @edev before this line? 22/46 has the following hunk which should probably be in this patch: diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c index 1cd80b3..7aba18e 100644 --- a/arch/powerpc/platforms/powernv/eeh-powernv.c +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c @@ -366,10 +366,9 @@ static int pnv_eeh_write_config(struct eeh_dev *edev, */ static struct eeh_dev *pnv_eeh_probe_pdev(struct pci_dev *pdev) { - struct pci_dn *pdn = pci_get_pdn(pdev); - struct pci_controller *hose = pdn->phb; - struct pnv_phb *phb = hose->private_data; - struct eeh_dev *edev = pdn_to_eeh_dev(pdn); + struct pnv_phb *phb = pci_bus_to_pnvhb(pdev->bus); + struct pci_controller *hose = phb->hose; + struct eeh_dev *edev; > > + if (edev) { > > + eeh_edev_dbg(edev, "Found existing edev!\n"); > > + > > + /* > > + * XXX: eeh_remove_device() clears pdev so we shouldn't hit > > this > > + * normally. I've found that screwing around with the pci > > probe > > + * path can result in eeh_probe_pdev() being called twice. > > This > > + * is harmless at the moment, but it's pretty strange so emit > > a > > + * warning to be on the safe side. > > + */ > > + if (WARN_ON(edev->pdev)) > > + eeh_edev_dbg(edev, "%s: already bound to a pdev!\n", > > __func__); > > + > > + edev->pdev = pdev; > > + > > + /* should we be doing something with REMOVED too? */ > > + edev->mode &= EEH_DEV_DISCONNECTED; > > + > > + /* update the primary bus if we need to */ > > + // XXX: why do we need to do this? is the pci_bus going away? > > what cleared the flag? > > From just reading this patch alone: if you do not know why we need it, There's a few comments in here that are essentially notes to myself that I thought other people might be able to shed light on. The series is tagged "Very RFC" for a reason ;) > then why did you add it here (it is not cut-n-paste)? Thanks, dunno lol