| Message ID | 1430460188-31343-11-git-send-email-gwshan@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 05/01/2015 04:02 PM, Gavin Shan wrote: > Function pnv_pci_reset_secondary_bus() is used to reset specified > PCI bus, which is leaded by root complex or PCI bridge. That means > the function shouldn't be called on PCI root bus and the patch > removes the logic for that case. > > Also, some adapters beneath the indicated PCI bus may require > fundamental reset in order to successfully reload their firmwares > after the reset. The patch translates hot reset to fundamental reset > for that case. > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > --- > arch/powerpc/platforms/powernv/eeh-powernv.c | 35 +++++++++++++++++++++------- > 1 file changed, 26 insertions(+), 9 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c > index 3c01095..58e4dcf 100644 > --- a/arch/powerpc/platforms/powernv/eeh-powernv.c > +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c > @@ -888,18 +888,35 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option) > return (rc == OPAL_SUCCESS) ? 0 : -EIO; > } > > -void pnv_pci_reset_secondary_bus(struct pci_dev *dev) Why changing dev to pdev? Keeping "dev" could make the patch simpler. > +static int pnv_pci_dev_reset_type(struct pci_dev *pdev, void *data) > { > - struct pci_controller *hose; > + int *freset = data; > > - if (pci_is_root_bus(dev->bus)) { > - hose = pci_bus_to_host(dev->bus); > - pnv_eeh_phb_reset(hose, EEH_RESET_HOT); > - pnv_eeh_phb_reset(hose, EEH_RESET_DEACTIVATE); > - } else { > - pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); > - pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); > + /* > + * Stop the iteration immediately if there is any > + * one PCI device requesting fundamental reset > + */ > + *freset |= pdev->needs_freset; > + return *freset; > +} > + > +void pnv_pci_reset_secondary_bus(struct pci_dev *pdev) > +{ > + int option = EEH_RESET_HOT; > + int freset = 0; > + > + /* Check if there're any PCI devices asking for fundamental reset */ > + if (pdev->subordinate) { > + pci_walk_bus(pdev->subordinate, > + pnv_pci_dev_reset_type, > + &freset); > + if (freset) > + option = EEH_RESET_FUNDAMENTAL; > } > + > + /* Issue the requested type of reset */ > + pnv_eeh_bridge_reset(pdev, option); > + pnv_eeh_bridge_reset(pdev, EEH_RESET_DEACTIVATE); > } > > /** >
On Sun, May 10, 2015 at 12:12:18AM +1000, Alexey Kardashevskiy wrote: >On 05/01/2015 04:02 PM, Gavin Shan wrote: >>Function pnv_pci_reset_secondary_bus() is used to reset specified >>PCI bus, which is leaded by root complex or PCI bridge. That means >>the function shouldn't be called on PCI root bus and the patch >>removes the logic for that case. >> >>Also, some adapters beneath the indicated PCI bus may require >>fundamental reset in order to successfully reload their firmwares >>after the reset. The patch translates hot reset to fundamental reset >>for that case. >> >>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >>--- >> arch/powerpc/platforms/powernv/eeh-powernv.c | 35 +++++++++++++++++++++------- >> 1 file changed, 26 insertions(+), 9 deletions(-) >> >>diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c >>index 3c01095..58e4dcf 100644 >>--- a/arch/powerpc/platforms/powernv/eeh-powernv.c >>+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c >>@@ -888,18 +888,35 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option) >> return (rc == OPAL_SUCCESS) ? 0 : -EIO; >> } >> >>-void pnv_pci_reset_secondary_bus(struct pci_dev *dev) > > >Why changing dev to pdev? Keeping "dev" could make the patch simpler. > In the early stage when I wrote the EEH code, I had "dev" to refer PCI device, which isn't precisely enough. Actually, "dev" means "struct device" while "pdev" stands for "struct pci_dev". That's why I changed it. >>+static int pnv_pci_dev_reset_type(struct pci_dev *pdev, void *data) >> { >>- struct pci_controller *hose; >>+ int *freset = data; >> >>- if (pci_is_root_bus(dev->bus)) { >>- hose = pci_bus_to_host(dev->bus); >>- pnv_eeh_phb_reset(hose, EEH_RESET_HOT); >>- pnv_eeh_phb_reset(hose, EEH_RESET_DEACTIVATE); >>- } else { >>- pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); >>- pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); >>+ /* >>+ * Stop the iteration immediately if there is any >>+ * one PCI device requesting fundamental reset >>+ */ >>+ *freset |= pdev->needs_freset; >>+ return *freset; >>+} >>+ >>+void pnv_pci_reset_secondary_bus(struct pci_dev *pdev) >>+{ >>+ int option = EEH_RESET_HOT; >>+ int freset = 0; >>+ >>+ /* Check if there're any PCI devices asking for fundamental reset */ >>+ if (pdev->subordinate) { >>+ pci_walk_bus(pdev->subordinate, >>+ pnv_pci_dev_reset_type, >>+ &freset); >>+ if (freset) >>+ option = EEH_RESET_FUNDAMENTAL; >> } >>+ >>+ /* Issue the requested type of reset */ >>+ pnv_eeh_bridge_reset(pdev, option); >>+ pnv_eeh_bridge_reset(pdev, EEH_RESET_DEACTIVATE); >> } >> >> /** >> Thanks, Gavin -- 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 05/11/2015 04:47 PM, Gavin Shan wrote: > On Sun, May 10, 2015 at 12:12:18AM +1000, Alexey Kardashevskiy wrote: >> On 05/01/2015 04:02 PM, Gavin Shan wrote: >>> Function pnv_pci_reset_secondary_bus() is used to reset specified >>> PCI bus, which is leaded by root complex or PCI bridge. That means >>> the function shouldn't be called on PCI root bus and the patch >>> removes the logic for that case. >>> >>> Also, some adapters beneath the indicated PCI bus may require >>> fundamental reset in order to successfully reload their firmwares >>> after the reset. The patch translates hot reset to fundamental reset >>> for that case. >>> >>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >>> --- >>> arch/powerpc/platforms/powernv/eeh-powernv.c | 35 +++++++++++++++++++++------- >>> 1 file changed, 26 insertions(+), 9 deletions(-) >>> >>> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c >>> index 3c01095..58e4dcf 100644 >>> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c >>> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c >>> @@ -888,18 +888,35 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option) >>> return (rc == OPAL_SUCCESS) ? 0 : -EIO; >>> } >>> >>> -void pnv_pci_reset_secondary_bus(struct pci_dev *dev) >> >> >> Why changing dev to pdev? Keeping "dev" could make the patch simpler. >> > > In the early stage when I wrote the EEH code, I had "dev" to refer PCI > device, which isn't precisely enough. Actually, "dev" means "struct device" > while "pdev" stands for "struct pci_dev". That's why I changed it. The rest of the file and the kernel overall use "dev" for pci_dev just fine. I would not bother. >>> +static int pnv_pci_dev_reset_type(struct pci_dev *pdev, void *data) >>> { >>> - struct pci_controller *hose; >>> + int *freset = data; >>> >>> - if (pci_is_root_bus(dev->bus)) { >>> - hose = pci_bus_to_host(dev->bus); >>> - pnv_eeh_phb_reset(hose, EEH_RESET_HOT); >>> - pnv_eeh_phb_reset(hose, EEH_RESET_DEACTIVATE); >>> - } else { >>> - pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); >>> - pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); >>> + /* >>> + * Stop the iteration immediately if there is any >>> + * one PCI device requesting fundamental reset >>> + */ >>> + *freset |= pdev->needs_freset; >>> + return *freset; >>> +} >>> + >>> +void pnv_pci_reset_secondary_bus(struct pci_dev *pdev) >>> +{ >>> + int option = EEH_RESET_HOT; >>> + int freset = 0; >>> + >>> + /* Check if there're any PCI devices asking for fundamental reset */ >>> + if (pdev->subordinate) { >>> + pci_walk_bus(pdev->subordinate, >>> + pnv_pci_dev_reset_type, >>> + &freset); >>> + if (freset) >>> + option = EEH_RESET_FUNDAMENTAL; >>> } >>> + >>> + /* Issue the requested type of reset */ >>> + pnv_eeh_bridge_reset(pdev, option); >>> + pnv_eeh_bridge_reset(pdev, EEH_RESET_DEACTIVATE); >>> } >>> >>> /** >>> > > Thanks, > Gavin >
On Mon, May 11, 2015 at 05:17:42PM +1000, Alexey Kardashevskiy wrote: >On 05/11/2015 04:47 PM, Gavin Shan wrote: >>On Sun, May 10, 2015 at 12:12:18AM +1000, Alexey Kardashevskiy wrote: >>>On 05/01/2015 04:02 PM, Gavin Shan wrote: >>>>Function pnv_pci_reset_secondary_bus() is used to reset specified >>>>PCI bus, which is leaded by root complex or PCI bridge. That means >>>>the function shouldn't be called on PCI root bus and the patch >>>>removes the logic for that case. >>>> >>>>Also, some adapters beneath the indicated PCI bus may require >>>>fundamental reset in order to successfully reload their firmwares >>>>after the reset. The patch translates hot reset to fundamental reset >>>>for that case. >>>> >>>>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >>>>--- >>>> arch/powerpc/platforms/powernv/eeh-powernv.c | 35 +++++++++++++++++++++------- >>>> 1 file changed, 26 insertions(+), 9 deletions(-) >>>> >>>>diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c >>>>index 3c01095..58e4dcf 100644 >>>>--- a/arch/powerpc/platforms/powernv/eeh-powernv.c >>>>+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c >>>>@@ -888,18 +888,35 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option) >>>> return (rc == OPAL_SUCCESS) ? 0 : -EIO; >>>> } >>>> >>>>-void pnv_pci_reset_secondary_bus(struct pci_dev *dev) >>> >>> >>>Why changing dev to pdev? Keeping "dev" could make the patch simpler. >>> >> >>In the early stage when I wrote the EEH code, I had "dev" to refer PCI >>device, which isn't precisely enough. Actually, "dev" means "struct device" >>while "pdev" stands for "struct pci_dev". That's why I changed it. > > >The rest of the file and the kernel overall use "dev" for pci_dev just fine. >I would not bother. > Ok. I'll keep it. >>>>+static int pnv_pci_dev_reset_type(struct pci_dev *pdev, void *data) >>>> { >>>>- struct pci_controller *hose; >>>>+ int *freset = data; >>>> >>>>- if (pci_is_root_bus(dev->bus)) { >>>>- hose = pci_bus_to_host(dev->bus); >>>>- pnv_eeh_phb_reset(hose, EEH_RESET_HOT); >>>>- pnv_eeh_phb_reset(hose, EEH_RESET_DEACTIVATE); >>>>- } else { >>>>- pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); >>>>- pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); >>>>+ /* >>>>+ * Stop the iteration immediately if there is any >>>>+ * one PCI device requesting fundamental reset >>>>+ */ >>>>+ *freset |= pdev->needs_freset; >>>>+ return *freset; >>>>+} >>>>+ >>>>+void pnv_pci_reset_secondary_bus(struct pci_dev *pdev) >>>>+{ >>>>+ int option = EEH_RESET_HOT; >>>>+ int freset = 0; >>>>+ >>>>+ /* Check if there're any PCI devices asking for fundamental reset */ >>>>+ if (pdev->subordinate) { >>>>+ pci_walk_bus(pdev->subordinate, >>>>+ pnv_pci_dev_reset_type, >>>>+ &freset); >>>>+ if (freset) >>>>+ option = EEH_RESET_FUNDAMENTAL; >>>> } >>>>+ >>>>+ /* Issue the requested type of reset */ >>>>+ pnv_eeh_bridge_reset(pdev, option); >>>>+ pnv_eeh_bridge_reset(pdev, EEH_RESET_DEACTIVATE); >>>> } >>>> >>>> /** >>>> Thanks, Gavin -- 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 3c01095..58e4dcf 100644 --- a/arch/powerpc/platforms/powernv/eeh-powernv.c +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c @@ -888,18 +888,35 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option) return (rc == OPAL_SUCCESS) ? 0 : -EIO; } -void pnv_pci_reset_secondary_bus(struct pci_dev *dev) +static int pnv_pci_dev_reset_type(struct pci_dev *pdev, void *data) { - struct pci_controller *hose; + int *freset = data; - if (pci_is_root_bus(dev->bus)) { - hose = pci_bus_to_host(dev->bus); - pnv_eeh_phb_reset(hose, EEH_RESET_HOT); - pnv_eeh_phb_reset(hose, EEH_RESET_DEACTIVATE); - } else { - pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); - pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); + /* + * Stop the iteration immediately if there is any + * one PCI device requesting fundamental reset + */ + *freset |= pdev->needs_freset; + return *freset; +} + +void pnv_pci_reset_secondary_bus(struct pci_dev *pdev) +{ + int option = EEH_RESET_HOT; + int freset = 0; + + /* Check if there're any PCI devices asking for fundamental reset */ + if (pdev->subordinate) { + pci_walk_bus(pdev->subordinate, + pnv_pci_dev_reset_type, + &freset); + if (freset) + option = EEH_RESET_FUNDAMENTAL; } + + /* Issue the requested type of reset */ + pnv_eeh_bridge_reset(pdev, option); + pnv_eeh_bridge_reset(pdev, EEH_RESET_DEACTIVATE); } /**
Function pnv_pci_reset_secondary_bus() is used to reset specified PCI bus, which is leaded by root complex or PCI bridge. That means the function shouldn't be called on PCI root bus and the patch removes the logic for that case. Also, some adapters beneath the indicated PCI bus may require fundamental reset in order to successfully reload their firmwares after the reset. The patch translates hot reset to fundamental reset for that case. Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> --- arch/powerpc/platforms/powernv/eeh-powernv.c | 35 +++++++++++++++++++++------- 1 file changed, 26 insertions(+), 9 deletions(-)