Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751887Ab3FYRxk (ORCPT ); Tue, 25 Jun 2013 13:53:40 -0400 Received: from mga09.intel.com ([134.134.136.24]:36303 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751101Ab3FYRxi (ORCPT ); Tue, 25 Jun 2013 13:53:38 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.87,938,1363158000"; d="scan'208";a="359406407" From: "Kirill A. Shutemov" To: Andy Shevchenko Cc: Mika Westerberg , Greg Kroah-Hartman , Bjorn Helgaas , "Rafael J. Wysocki" , Jesse Barnes , Yinghai Lu , john.ronciak@intel.com, miles.j.penner@intel.com, bruce.w.allan@intel.com, "Kirill A. Shutemov" , Heikki Krogerus , "linux-kernel@vger.kernel.org" , linux-pci@vger.kernel.org, x86@kernel.org In-Reply-To: References: <1372177330-28013-1-git-send-email-mika.westerberg@linux.intel.com> <1372177330-28013-4-git-send-email-mika.westerberg@linux.intel.com> Subject: Re: [PATCH 3/6] PCI: introduce pci_trim_stale_devices() Content-Transfer-Encoding: 7bit Message-Id: <20130625175625.3F434E0090@blue.fi.intel.com> Date: Tue, 25 Jun 2013 20:56:25 +0300 (EEST) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1654 Lines: 48 Andy Shevchenko wrote: > On Tue, Jun 25, 2013 at 7:22 PM, Mika Westerberg > 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. -- Kirill A. Shutemov -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/