| Message ID | 1446642770-4681-39-git-send-email-gwshan@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Gavin Shan <gwshan@linux.vnet.ibm.com> writes: > When pnv_pci_reset_secondary_bus() is called to issue reset on > the indicated secondary bus, the bus can't be root bus. So we > needn't consider root bus in the function. It took me a while to convince myself that this is correct. For the record, this is why it's correct: pnv_pci_reset_secondary_bus fills the reset_secondary_bus callback in the pci_controller_ops structure, and isn't used elsewhere. In arch/powerpc/kernel/pci.c, that callback is called (if it exists) in pcibios_reset_secondary_bus(). It's not called anywhere else. The PPC pcibios_reset_secondary_bus overrides the weak version in drivers/pci/pci.c. It's called from the same file by pci_reset_bridge_secondary_device() (and nowhere else). pci_reset_bridge_secondary_device() is nicely documented: /** * pci_reset_bridge_secondary_bus - Reset the secondary bus on a PCI bridge. * @dev: Bridge device * * Use the bridge control register to assert reset on the secondary bus. * Devices on the secondary bus are left in power-on state. */ Therefore, by the definiton of pci_reset_bridge_secondary_bus, pnv_pci_reset_secondary_bus() can only be called with a bridge device. As such, a bridge reset only is appropriate. If this breaks anything, the caller is broken. It might be worth including a condensed version of this in the commit message. Reviewed-by: Daniel Axtens <dja@axtens.net> Regards, Daniel Axtens > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > --- > arch/powerpc/platforms/powernv/eeh-powernv.c | 12 ++---------- > 1 file changed, 2 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c > index a7d84a4..c69b6a1 100644 > --- a/arch/powerpc/platforms/powernv/eeh-powernv.c > +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c > @@ -880,16 +880,8 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option) > > void pnv_pci_reset_secondary_bus(struct pci_dev *dev) > { > - struct pci_controller *hose; > - > - if (pci_is_root_bus(dev->bus)) { > - hose = pci_bus_to_host(dev->bus); > - pnv_eeh_root_reset(hose, EEH_RESET_HOT); > - pnv_eeh_root_reset(hose, EEH_RESET_DEACTIVATE); > - } else { > - pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); > - pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); > - } > + pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); > + 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
On Fri, Nov 13, 2015 at 09:59:27AM +1100, Daniel Axtens wrote: >Gavin Shan <gwshan@linux.vnet.ibm.com> writes: > >> When pnv_pci_reset_secondary_bus() is called to issue reset on >> the indicated secondary bus, the bus can't be root bus. So we >> needn't consider root bus in the function. > >It took me a while to convince myself that this is correct. For the >record, this is why it's correct: > >pnv_pci_reset_secondary_bus fills the reset_secondary_bus callback in >the pci_controller_ops structure, and isn't used elsewhere. > >In arch/powerpc/kernel/pci.c, that callback is called (if it exists) in >pcibios_reset_secondary_bus(). It's not called anywhere else. > >The PPC pcibios_reset_secondary_bus overrides the weak version in >drivers/pci/pci.c. It's called from the same file by >pci_reset_bridge_secondary_device() (and nowhere else). > >pci_reset_bridge_secondary_device() is nicely documented: > >/** > * pci_reset_bridge_secondary_bus - Reset the secondary bus on a PCI bridge. > * @dev: Bridge device > * > * Use the bridge control register to assert reset on the secondary bus. > * Devices on the secondary bus are left in power-on state. > */ > >Therefore, by the definiton of pci_reset_bridge_secondary_bus, >pnv_pci_reset_secondary_bus() can only be called with a bridge >device. As such, a bridge reset only is appropriate. If this breaks >anything, the caller is broken. > >It might be worth including a condensed version of this in the commit >message. > Right. I'll add more description to the changelog in next revision. >Reviewed-by: Daniel Axtens <dja@axtens.net> > Thanks, Gavin >Regards, >Daniel Axtens > >> >> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >> --- >> arch/powerpc/platforms/powernv/eeh-powernv.c | 12 ++---------- >> 1 file changed, 2 insertions(+), 10 deletions(-) >> >> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c >> index a7d84a4..c69b6a1 100644 >> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c >> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c >> @@ -880,16 +880,8 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option) >> >> void pnv_pci_reset_secondary_bus(struct pci_dev *dev) >> { >> - struct pci_controller *hose; >> - >> - if (pci_is_root_bus(dev->bus)) { >> - hose = pci_bus_to_host(dev->bus); >> - pnv_eeh_root_reset(hose, EEH_RESET_HOT); >> - pnv_eeh_root_reset(hose, EEH_RESET_DEACTIVATE); >> - } else { >> - pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); >> - pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); >> - } >> + pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); >> + 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
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c index a7d84a4..c69b6a1 100644 --- a/arch/powerpc/platforms/powernv/eeh-powernv.c +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c @@ -880,16 +880,8 @@ static int pnv_eeh_bridge_reset(struct pci_dev *dev, int option) void pnv_pci_reset_secondary_bus(struct pci_dev *dev) { - struct pci_controller *hose; - - if (pci_is_root_bus(dev->bus)) { - hose = pci_bus_to_host(dev->bus); - pnv_eeh_root_reset(hose, EEH_RESET_HOT); - pnv_eeh_root_reset(hose, EEH_RESET_DEACTIVATE); - } else { - pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); - pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); - } + pnv_eeh_bridge_reset(dev, EEH_RESET_HOT); + pnv_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE); } /**
When pnv_pci_reset_secondary_bus() is called to issue reset on the indicated secondary bus, the bus can't be root bus. So we needn't consider root bus in the function. Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> --- arch/powerpc/platforms/powernv/eeh-powernv.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)