| Message ID | 1372177330-28013-4-git-send-email-mika.westerberg@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Jun 25, 2013 at 7:22 PM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > The new function stops and removes the device if it doesn't respond. > If the device responds and it's a bus we apply the function to all > subdevices recursively. > > It's useful for hotplug bus check case, when you need drop all unplugged > devices before looking for new ones. One comment below. > +void pci_trim_stale_devices(struct pci_dev *dev) > +{ > + struct pci_bus *bus = dev->subordinate; > + struct pci_dev *child, *tmp; > + bool alive; > + u32 l; > + > + /* check if the device responds */ > + alive = pci_bus_read_dev_vendor_id(dev->bus, dev->devfn, &l, 0); > + if (!alive) > + pci_stop_and_remove_bus_device(dev); > + > + if (alive && bus) > + list_for_each_entry_safe(child, tmp, &bus->devices, bus_list) > + pci_trim_stale_devices(child); It's not a tail call anyway, so, what about if (!pci_bus_read_dev_vendor_id(dev->bus, dev->devfn, &l, 0)) { pci_stop_and_remove_bus_device(dev); } else if (bus) { list_for_each_entry_safe(child, tmp, &bus->devices, bus_list) pci_trim_stale_devices(child); } > +} -- With Best Regards, Andy Shevchenko -- 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
Andy Shevchenko wrote: > On Tue, Jun 25, 2013 at 7:22 PM, Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > > The new function stops and removes the device if it doesn't respond. > > If the device responds and it's a bus we apply the function to all > > subdevices recursively. > > > > It's useful for hotplug bus check case, when you need drop all unplugged > > devices before looking for new ones. > > One comment below. > > > +void pci_trim_stale_devices(struct pci_dev *dev) > > +{ > > + struct pci_bus *bus = dev->subordinate; > > + struct pci_dev *child, *tmp; > > + bool alive; > > + u32 l; > > + > > + /* check if the device responds */ > > + alive = pci_bus_read_dev_vendor_id(dev->bus, dev->devfn, &l, 0); > > + if (!alive) > > + pci_stop_and_remove_bus_device(dev); > > + > > + if (alive && bus) > > + list_for_each_entry_safe(child, tmp, &bus->devices, bus_list) > > + pci_trim_stale_devices(child); > > It's not a tail call anyway, so, what about > > if (!pci_bus_read_dev_vendor_id(dev->bus, dev->devfn, &l, 0)) { > pci_stop_and_remove_bus_device(dev); > } else if (bus) { > list_for_each_entry_safe(child, tmp, &bus->devices, bus_list) > pci_trim_stale_devices(child); > } Why? It looks uglier.
On Tuesday, June 25, 2013 07:22:07 PM Mika Westerberg wrote: > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > The new function stops and removes the device if it doesn't respond. > If the device responds and it's a bus we apply the function to all > subdevices recursively. > > It's useful for hotplug bus check case, when you need drop all unplugged > devices before looking for new ones. > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> I think it'd be better to fold this into [4/6]. This way it would be clearer what that patch did. Thanks, Rafael > --- > drivers/pci/remove.c | 20 ++++++++++++++++++++ > include/linux/pci.h | 1 + > 2 files changed, 21 insertions(+) > > diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c > index 8fc54b7..77b7a64 100644 > --- a/drivers/pci/remove.c > +++ b/drivers/pci/remove.c > @@ -112,6 +112,26 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev) > } > EXPORT_SYMBOL(pci_stop_and_remove_bus_device); > > +/** > + * pci_trim_stale_devices - remove stale device or any stale child > + */ > +void pci_trim_stale_devices(struct pci_dev *dev) > +{ > + struct pci_bus *bus = dev->subordinate; > + struct pci_dev *child, *tmp; > + bool alive; > + u32 l; > + > + /* check if the device responds */ > + alive = pci_bus_read_dev_vendor_id(dev->bus, dev->devfn, &l, 0); > + if (!alive) > + pci_stop_and_remove_bus_device(dev); > + > + if (alive && bus) > + list_for_each_entry_safe(child, tmp, &bus->devices, bus_list) > + pci_trim_stale_devices(child); > +} > + > void pci_stop_root_bus(struct pci_bus *bus) > { > struct pci_dev *child, *tmp; > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 3a24e4f..8f6e7a0 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -754,6 +754,7 @@ struct pci_dev *pci_dev_get(struct pci_dev *dev); > void pci_dev_put(struct pci_dev *dev); > void pci_remove_bus(struct pci_bus *b); > void pci_stop_and_remove_bus_device(struct pci_dev *dev); > +void pci_trim_stale_devices(struct pci_dev *dev); > void pci_stop_root_bus(struct pci_bus *bus); > void pci_remove_root_bus(struct pci_bus *bus); > void pci_setup_cardbus(struct pci_bus *bus); >
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c index 8fc54b7..77b7a64 100644 --- a/drivers/pci/remove.c +++ b/drivers/pci/remove.c @@ -112,6 +112,26 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev) } EXPORT_SYMBOL(pci_stop_and_remove_bus_device); +/** + * pci_trim_stale_devices - remove stale device or any stale child + */ +void pci_trim_stale_devices(struct pci_dev *dev) +{ + struct pci_bus *bus = dev->subordinate; + struct pci_dev *child, *tmp; + bool alive; + u32 l; + + /* check if the device responds */ + alive = pci_bus_read_dev_vendor_id(dev->bus, dev->devfn, &l, 0); + if (!alive) + pci_stop_and_remove_bus_device(dev); + + if (alive && bus) + list_for_each_entry_safe(child, tmp, &bus->devices, bus_list) + pci_trim_stale_devices(child); +} + void pci_stop_root_bus(struct pci_bus *bus) { struct pci_dev *child, *tmp; diff --git a/include/linux/pci.h b/include/linux/pci.h index 3a24e4f..8f6e7a0 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -754,6 +754,7 @@ struct pci_dev *pci_dev_get(struct pci_dev *dev); void pci_dev_put(struct pci_dev *dev); void pci_remove_bus(struct pci_bus *b); void pci_stop_and_remove_bus_device(struct pci_dev *dev); +void pci_trim_stale_devices(struct pci_dev *dev); void pci_stop_root_bus(struct pci_bus *bus); void pci_remove_root_bus(struct pci_bus *bus); void pci_setup_cardbus(struct pci_bus *bus);