From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pw0-f46.google.com ([209.85.160.46]:44108 "EHLO mail-pw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751382Ab2CIHRU convert rfc822-to-8bit (ORCPT ); Fri, 9 Mar 2012 02:17:20 -0500 MIME-Version: 1.0 In-Reply-To: References: <1331018040-30725-1-git-send-email-yinghai@kernel.org> <1331018040-30725-15-git-send-email-yinghai@kernel.org> Date: Thu, 8 Mar 2012 23:17:20 -0800 Message-ID: Subject: Re: [PATCH 14/23] PCI: add __pci_remove_bus_devices() From: Yinghai Lu To: Bjorn Helgaas Cc: Jesse Barnes , x86 , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-pci-owner@vger.kernel.org List-ID: On Thu, Mar 8, 2012 at 5:11 PM, Bjorn Helgaas wrote: > On Tue, Mar 6, 2012 at 12:13 AM, Yinghai Lu wrote: >> will use it with pci_stop_and_remove_bus later. >> >> also remove __pci_remove_behind_bridge and pci_stop_behind_bridge. >> >> they are same except one take bridge and one take bus. >> >> and we already have pci_stop_bus_devices() >> >> Signed-off-by: Yinghai Lu >> --- >>  drivers/pci/remove.c |   28 +++++++++++----------------- >>  1 files changed, 11 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c >> index 243d59b..62c348c 100644 >> --- a/drivers/pci/remove.c >> +++ b/drivers/pci/remove.c >> @@ -78,7 +78,7 @@ void pci_remove_bus(struct pci_bus *pci_bus) >>  } >>  EXPORT_SYMBOL(pci_remove_bus); >> >> -static void __pci_remove_behind_bridge(struct pci_dev *dev); >> +static void __pci_remove_bus_devices(struct pci_bus *bus); >>  /** >>  * pci_stop_and_remove_bus_device - remove a PCI device and any children >>  * @dev: the device to remove >> @@ -96,7 +96,7 @@ void __pci_remove_bus_device(struct pci_dev *dev) >>        if (dev->subordinate) { >>                struct pci_bus *b = dev->subordinate; >> >> -               __pci_remove_behind_bridge(dev); >> +               __pci_remove_bus_devices(b); >>                pci_remove_bus(b); >>                dev->subordinate = NULL; >>        } >> @@ -111,22 +111,12 @@ void pci_stop_and_remove_bus_device(struct pci_dev *dev) >>        __pci_remove_bus_device(dev); >>  } >> >> -static void __pci_remove_behind_bridge(struct pci_dev *dev) >> +static void __pci_remove_bus_devices(struct pci_bus *bus) >>  { >>        struct list_head *l, *n; >> >> -       if (dev->subordinate) >> -               list_for_each_safe(l, n, &dev->subordinate->devices) >> -                       __pci_remove_bus_device(pci_dev_b(l)); >> -} >> - >> -static void pci_stop_behind_bridge(struct pci_dev *dev) >> -{ >> -       struct list_head *l, *n; >> - >> -       if (dev->subordinate) >> -               list_for_each_safe(l, n, &dev->subordinate->devices) >> -                       pci_stop_bus_device(pci_dev_b(l)); >> +       list_for_each_safe(l, n, &bus->devices) >> +               __pci_remove_bus_device(pci_dev_b(l)); > > Use list_for_each_entry_safe() so you don't need pci_dev_b(). just want to keep the patch to simple, and looks like just name renaming. also use list_for_each_safe instead of list_for_each_entry_safe could have less conversion. > >>  } >> >>  static void pci_stop_bus_devices(struct pci_bus *bus) >> @@ -158,8 +148,12 @@ static void pci_stop_bus_devices(struct pci_bus *bus) >>  */ >>  void pci_stop_and_remove_behind_bridge(struct pci_dev *dev) >>  { >> -       pci_stop_behind_bridge(dev); >> -       __pci_remove_behind_bridge(dev); >> +       struct pci_bus *bus = dev->subordinate; >> + >> +       if (bus) { > > Don't check "bus" here.  If the caller screws up and passes a > non-bridge pointer, I want to learn about it rather than ignore it. old code have that if (dev->subordinate) checking. Yinghai