| Message ID | 1446642770-4681-40-git-send-email-gwshan@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thu, Nov 05, 2015 at 12:12:39AM +1100, Gavin Shan wrote: >In pnv_pci_reset_secondary_bus(), we should issue fundamental >reset if any one subordinate device of the specified is requesting ^^^^^^^^^^^^^^ the specified bus I put the note reminding me to admend the changelog in next revision. >that. Otherwise, the device might not come up after the reset. > >Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >--- > arch/powerpc/platforms/powernv/eeh-powernv.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > >diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c >index c69b6a1..ab8b93e 100644 >--- a/arch/powerpc/platforms/powernv/eeh-powernv.c >+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c >@@ -878,9 +878,28 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option) > return 0; > } > >+static int pnv_pci_dev_reset_type(struct pci_dev *pdev, void *data) >+{ >+ int *freset = data; >+ >+ /* >+ * Stop the iteration immediately if there has any one >+ * PCI device requesting fundamental reset. >+ */ >+ *freset |= pdev->needs_freset; >+ return *freset; >+} >+ > void pnv_pci_reset_secondary_bus(struct pci_dev *dev) > { >- pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); >+ int option, freset = 0; >+ >+ if (dev->subordinate) >+ pci_walk_bus(dev->subordinate, >+ pnv_pci_dev_reset_type, &freset); >+ >+ option = freset ? EEH_RESET_FUNDAMENTAL : EEH_RESET_HOT; >+ pnv_eeh_bridge_reset(dev, option); > pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); > } > >-- >2.1.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Gavin Shan <gwshan@linux.vnet.ibm.com> writes: > void pnv_pci_reset_secondary_bus(struct pci_dev *dev) > { > - pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); > + int option, freset = 0; > + > + if (dev->subordinate) > + pci_walk_bus(dev->subordinate, > + pnv_pci_dev_reset_type, &freset); > + > + option = freset ? EEH_RESET_FUNDAMENTAL : EEH_RESET_HOT; > + pnv_eeh_bridge_reset(dev, option); According to the skiboot sources, fundamental reset isn't supported on p5ioc2. As far as I can tell from your corresponding skiboot patches, this is still the case after they are applied. Do we need a fallback to EEH_RESET_HOT in this case? Otherwise there will be no reset performed at all. Likewise, if the FUNDAMENTAL reset fails for any reason, should we fall back to a HOT reset? Regards, Daniel
On Fri, Nov 13, 2015 at 11:08:29AM +1100, Daniel Axtens wrote: >Gavin Shan <gwshan@linux.vnet.ibm.com> writes: > >> void pnv_pci_reset_secondary_bus(struct pci_dev *dev) >> { >> - pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); >> + int option, freset = 0; >> + >> + if (dev->subordinate) >> + pci_walk_bus(dev->subordinate, >> + pnv_pci_dev_reset_type, &freset); >> + >> + option = freset ? EEH_RESET_FUNDAMENTAL : EEH_RESET_HOT; >> + pnv_eeh_bridge_reset(dev, option); > >According to the skiboot sources, fundamental reset isn't supported on >p5ioc2. As far as I can tell from your corresponding skiboot patches, >this is still the case after they are applied. Do we need a fallback to >EEH_RESET_HOT in this case? Otherwise there will be no reset performed >at all. > >Likewise, if the FUNDAMENTAL reset fails for any reason, should we fall >back to a HOT reset? > P5IOC2 won't export any PCI slots. So kernel won't issue fundamental reset to PCI buses on P5IOC2. We had the failback: hot reset is picked if fundamental reset can't be supported on the target PCI bus. In case fundamental reset fails, we shouldn't go ahead try hot reset. Thanks, Gavin >Regards, >Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2015-11-13 at 11:08 +1100, Daniel Axtens wrote: > Gavin Shan <gwshan@linux.vnet.ibm.com> writes: > > > void pnv_pci_reset_secondary_bus(struct pci_dev *dev) > > { > > -> > > > pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); > > +> > > > int option, freset = 0; > > + > > +> > > > if (dev->subordinate) > > +> > > > > > pci_walk_bus(dev->subordinate, > > +> > > > > > > > pnv_pci_dev_reset_type, &freset); > > + > > +> > > > option = freset ? EEH_RESET_FUNDAMENTAL : EEH_RESET_HOT; > > +> > > > pnv_eeh_bridge_reset(dev, option); > > According to the skiboot sources, fundamental reset isn't supported on > p5ioc2. As far as I can tell from your corresponding skiboot patches, > this is still the case after they are applied. Do we need a fallback to > EEH_RESET_HOT in this case? Otherwise there will be no reset performed > at all. We don't really care that much about what happens on p5ioc2 :-) > Likewise, if the FUNDAMENTAL reset fails for any reason, should we fall > back to a HOT reset? Probably. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Following some discussion on IRC, it looks like there are roughly 2 machines on the planet with skiboot and p5ioc2, so I'm not worried about that any more. I am still vaguely concerned about a failing fundamental reset. Regards, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c index c69b6a1..ab8b93e 100644 --- a/arch/powerpc/platforms/powernv/eeh-powernv.c +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c @@ -878,9 +878,28 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option) return 0; } +static int pnv_pci_dev_reset_type(struct pci_dev *pdev, void *data) +{ + int *freset = data; + + /* + * Stop the iteration immediately if there has any one + * PCI device requesting fundamental reset. + */ + *freset |= pdev->needs_freset; + return *freset; +} + void pnv_pci_reset_secondary_bus(struct pci_dev *dev) { - pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); + int option, freset = 0; + + if (dev->subordinate) + pci_walk_bus(dev->subordinate, + pnv_pci_dev_reset_type, &freset); + + option = freset ? EEH_RESET_FUNDAMENTAL : EEH_RESET_HOT; + pnv_eeh_bridge_reset(dev, option); pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); }
In pnv_pci_reset_secondary_bus(), we should issue fundamental reset if any one subordinate device of the specified is requesting that. Otherwise, the device might not come up after the reset. Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> --- arch/powerpc/platforms/powernv/eeh-powernv.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)