Hi Bjoern, On Tue, Aug 11, 2015 at 10:52:02AM -0500, Bjorn Helgaas wrote: > There's no need to BUG() if we enable ATS when it's already enabled. We > don't need to BUG() when disabling ATS on a device that doesn't support ATS > or if it's already disabled. If ATS is enabled, certainly we found an ATS > capability in the past, so it should still be there now. > > Clean up these error paths. > > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> Two comments inline. With these changes: Reviewed-by: Joerg Roedel <jroedel@xxxxxxx> > --- > drivers/pci/ats.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c > index 0b5b0ed..0f05274 100644 > --- a/drivers/pci/ats.c > +++ b/drivers/pci/ats.c > @@ -44,11 +44,13 @@ int pci_enable_ats(struct pci_dev *dev, int ps) > u16 ctrl; > struct pci_dev *pdev; > > - BUG_ON(dev->ats_cap && dev->ats_enabled); > - > if (!dev->ats_cap) > return -EINVAL; > > + WARN_ON(pci_ats_enabled(dev)); > + if (pci_ats_enabled(dev)) > + return -EBUSY; > + Could be written as if (WARN_ON(pci_ats_enabled(dev))) return -EBUSY; > if (ps < PCI_ATS_MIN_STU) > return -EINVAL; > > @@ -83,7 +85,8 @@ void pci_disable_ats(struct pci_dev *dev) > struct pci_dev *pdev; > u16 ctrl; > > - BUG_ON(!dev->ats_cap || !dev->ats_enabled); > + if (!pci_ats_enabled(dev)) > + return; Probably also: if (WARN_ON(!pci_ats_enabled(dev))) ? -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
