| Message ID | 20130801165539.16145.79655.stgit@bling.home (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 08/01/2013 12:55 PM, Alex Williamson wrote: > Only cosmetic changes to existing paths. > > Signed-off-by: Alex Williamson<alex.williamson@redhat.com> > --- > drivers/pci/pci.c | 52 +++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 35 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index db90692..f6c757a 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -3378,22 +3378,46 @@ done: > return rc; > } > > +static void pci_dev_lock(struct pci_dev *dev) > +{ > + pci_cfg_access_lock(dev); > + /* block PM suspend, driver probe, etc. */ > + device_lock(&dev->dev); > +} > + > +static void pci_dev_unlock(struct pci_dev *dev) > +{ > + device_unlock(&dev->dev); > + pci_cfg_access_unlock(dev); > +} > + > +static void pci_dev_save_and_disable(struct pci_dev *dev) > +{ > + pci_save_state(dev); > + /* > + * both INTx and MSI are disabled after the Interrupt Disable bit > + * is set and the Bus Master bit is cleared. > + */ > + pci_write_config_word(dev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE); > +} although the above is just a shuffling of the previous code, does the comment need to point out that BARs are disabled by this write as well ? > + > +static void pci_dev_restore(struct pci_dev *dev) > +{ > + pci_restore_state(dev); > +} > + > static int pci_dev_reset(struct pci_dev *dev, int probe) > { > int rc; > > - if (!probe) { > - pci_cfg_access_lock(dev); > - /* block PM suspend, driver probe, etc. */ > - device_lock(&dev->dev); > - } > + if (!probe) > + pci_dev_lock(dev); > > rc = __pci_dev_reset(dev, probe); > > - if (!probe) { > - device_unlock(&dev->dev); > - pci_cfg_access_unlock(dev); > - } > + if (!probe) > + pci_dev_unlock(dev); > + > return rc; > } > /** > @@ -3484,17 +3508,11 @@ int pci_reset_function(struct pci_dev *dev) > if (rc) > return rc; > > - pci_save_state(dev); > - > - /* > - * both INTx and MSI are disabled after the Interrupt Disable bit > - * is set and the Bus Master bit is cleared. > - */ > - pci_write_config_word(dev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE); > + pci_dev_save_and_disable(dev); > > rc = pci_dev_reset(dev, 0); > > - pci_restore_state(dev); > + pci_dev_restore(dev); > > return rc; > } > > -- > 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 -- 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 Thu, 2013-08-01 at 16:59 -0400, Don Dutile wrote: > On 08/01/2013 12:55 PM, Alex Williamson wrote: > > Only cosmetic changes to existing paths. > > > > Signed-off-by: Alex Williamson<alex.williamson@redhat.com> > > --- > > drivers/pci/pci.c | 52 +++++++++++++++++++++++++++++++++++----------------- > > 1 file changed, 35 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index db90692..f6c757a 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -3378,22 +3378,46 @@ done: > > return rc; > > } > > > > +static void pci_dev_lock(struct pci_dev *dev) > > +{ > > + pci_cfg_access_lock(dev); > > + /* block PM suspend, driver probe, etc. */ > > + device_lock(&dev->dev); > > +} > > + > > +static void pci_dev_unlock(struct pci_dev *dev) > > +{ > > + device_unlock(&dev->dev); > > + pci_cfg_access_unlock(dev); > > +} > > + > > +static void pci_dev_save_and_disable(struct pci_dev *dev) > > +{ > > + pci_save_state(dev); > > + /* > > + * both INTx and MSI are disabled after the Interrupt Disable bit > > + * is set and the Bus Master bit is cleared. > > + */ > > + pci_write_config_word(dev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE); > > +} > although the above is just a shuffling of the previous code, > does the comment need to point out that BARs are disabled by this write as well ? I could certainly add such a comment. That's also why I added the _and_disable to this function name instead of just pci_dev_save(). Thanks, Alex > > + > > +static void pci_dev_restore(struct pci_dev *dev) > > +{ > > + pci_restore_state(dev); > > +} > > + > > static int pci_dev_reset(struct pci_dev *dev, int probe) > > { > > int rc; > > > > - if (!probe) { > > - pci_cfg_access_lock(dev); > > - /* block PM suspend, driver probe, etc. */ > > - device_lock(&dev->dev); > > - } > > + if (!probe) > > + pci_dev_lock(dev); > > > > rc = __pci_dev_reset(dev, probe); > > > > - if (!probe) { > > - device_unlock(&dev->dev); > > - pci_cfg_access_unlock(dev); > > - } > > + if (!probe) > > + pci_dev_unlock(dev); > > + > > return rc; > > } > > /** > > @@ -3484,17 +3508,11 @@ int pci_reset_function(struct pci_dev *dev) > > if (rc) > > return rc; > > > > - pci_save_state(dev); > > - > > - /* > > - * both INTx and MSI are disabled after the Interrupt Disable bit > > - * is set and the Bus Master bit is cleared. > > - */ > > - pci_write_config_word(dev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE); > > + pci_dev_save_and_disable(dev); > > > > rc = pci_dev_reset(dev, 0); > > > > - pci_restore_state(dev); > > + pci_dev_restore(dev); > > > > return rc; > > } > > > > -- > > 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 > -- 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/drivers/pci/pci.c b/drivers/pci/pci.c index db90692..f6c757a 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3378,22 +3378,46 @@ done: return rc; } +static void pci_dev_lock(struct pci_dev *dev) +{ + pci_cfg_access_lock(dev); + /* block PM suspend, driver probe, etc. */ + device_lock(&dev->dev); +} + +static void pci_dev_unlock(struct pci_dev *dev) +{ + device_unlock(&dev->dev); + pci_cfg_access_unlock(dev); +} + +static void pci_dev_save_and_disable(struct pci_dev *dev) +{ + pci_save_state(dev); + /* + * both INTx and MSI are disabled after the Interrupt Disable bit + * is set and the Bus Master bit is cleared. + */ + pci_write_config_word(dev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE); +} + +static void pci_dev_restore(struct pci_dev *dev) +{ + pci_restore_state(dev); +} + static int pci_dev_reset(struct pci_dev *dev, int probe) { int rc; - if (!probe) { - pci_cfg_access_lock(dev); - /* block PM suspend, driver probe, etc. */ - device_lock(&dev->dev); - } + if (!probe) + pci_dev_lock(dev); rc = __pci_dev_reset(dev, probe); - if (!probe) { - device_unlock(&dev->dev); - pci_cfg_access_unlock(dev); - } + if (!probe) + pci_dev_unlock(dev); + return rc; } /** @@ -3484,17 +3508,11 @@ int pci_reset_function(struct pci_dev *dev) if (rc) return rc; - pci_save_state(dev); - - /* - * both INTx and MSI are disabled after the Interrupt Disable bit - * is set and the Bus Master bit is cleared. - */ - pci_write_config_word(dev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE); + pci_dev_save_and_disable(dev); rc = pci_dev_reset(dev, 0); - pci_restore_state(dev); + pci_dev_restore(dev); return rc; }
Only cosmetic changes to existing paths. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- drivers/pci/pci.c | 52 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 17 deletions(-) -- 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