Message ID | 20221103164644.70554-5-andriy.shevchenko@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp647213wru; Thu, 3 Nov 2022 09:52:28 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5oQhhyZJWeFUe17NUzqC07Pi47PWMzYqaM2RBcmxzeO39+iJipurRfAoqmS8moMnBPG2Il X-Received: by 2002:a17:902:f64d:b0:178:a963:d400 with SMTP id m13-20020a170902f64d00b00178a963d400mr31647115plg.6.1667494348451; Thu, 03 Nov 2022 09:52:28 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1667494348; cv=none; d=google.com; s=arc-20160816; b=OLPM/I2P6DN8hlio1ONfcB/gtoeA2SWcxh8ce37Gkq26VnC6hdfvtp+yUSUey2b7fw j/nNYEf0Jg1BqVRc28Eyoy53SZo5UHHyqfeQ0pZfHczrAIhFjw5swGaE6z/y1OyIXPjr Dh/QYU21XezY8TnFPU7lZaab07+PCwW8JpZIG0LUdYSi0g985jTZaJbS9qEhXT+qbaJH uxcU1es0ifZHYPi8Dm49A3QRkJ3Q/divKykK8msNTfvDtUEgYFuYkvqmNDD9i/GY+Iku ZqVEqRq6QVLFouj6i0YdOMFgvVwy3TkCthg06W1KHt6VcQQqVOSf9WTWgX0/1P0JhQMr 5usA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=xUJEQ8B+yd3tSJ4o7EXAVL4R3IOmwYEqjSf2LeSEiDo=; b=fUjKcPTB8BqHZxDCtBINhzzNVmmvbUU0CLjJTYBaphMELJV+deTRrHB9hP/kxPtPk0 0wtfuTwqUxKAH7JzGIVS0GiSM66An4fG6IhFtZSdnk/wzO/BypBVLRwSwPcX3e+rsftI BavdIGtiQ/yRcWr1wudqMNeYcl7skkCGcX1pU3/EhdTopZhjNkmMS9ukBJ2f/5gOj/qH xkXF2IogjtSoFq6/twixc9GKVBPu87rhVMSYtZ4bU8oCcuFIaHMpRIh+drK5Mzt6XyRu GNE76TGEM/NhQ3xWDLU14wtic2KjHsayb2nobfTqZVQFtddBSQ7+9FAm/H3SWaBV7lbw 523w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=ZdyafDzS; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id k19-20020aa788d3000000b0056c882d7065si1584648pff.179.2022.11.03.09.52.14; Thu, 03 Nov 2022 09:52:28 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=ZdyafDzS; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232394AbiKCQsC (ORCPT <rfc822;yves.mi.zy@gmail.com> + 99 others); Thu, 3 Nov 2022 12:48:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44154 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232536AbiKCQrL (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 3 Nov 2022 12:47:11 -0400 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EE08119C3D; Thu, 3 Nov 2022 09:46:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1667493998; x=1699029998; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=NhBC+PjScBp2IdaVXcj0juh1+PhjepKM9lxMzcbPhAU=; b=ZdyafDzSEAMw86F6xtdYHYzjq9E3bq9SA2H3+U5Irs+qM9GQNr9YpElX vB2O/9rXJfGoz3UrOINQQ++uimoaPOPj9oxwb2n5A8eE1ekcIwr0p7q/j sxdU4cpBjFZ7e77Tx/HWV0i33aJcIq1Kb1RSXhuvG+YDmsY8vAELGDWJz CSRenxb5bfgWRtm/+7kIrE0IBQJmIFCTRd+DjlY+QDgARwXb1iqAD30/U 3Hxq9QvBp7CJWoSROx+LAmmK61qYl9l80uMgoBFdCg4Z+HEIc3R7dx1LJ /V22rH/t/Xfpk9UMmGn3lIBj/SoPJq2y2LRYD8f1M9saKwzlG6o6x/pBd Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10520"; a="297198943" X-IronPort-AV: E=Sophos;i="5.96,134,1665471600"; d="scan'208";a="297198943" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Nov 2022 09:46:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10520"; a="809735385" X-IronPort-AV: E=Sophos;i="5.96,134,1665471600"; d="scan'208";a="809735385" Received: from black.fi.intel.com ([10.237.72.28]) by orsmga005.jf.intel.com with ESMTP; 03 Nov 2022 09:46:29 -0700 Received: by black.fi.intel.com (Postfix, from userid 1003) id C452A2B7; Thu, 3 Nov 2022 18:46:52 +0200 (EET) From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> To: =?utf-8?q?Micka=C3=ABl_Sala=C3=BCn?= <mic@digikod.net>, Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Mika Westerberg <mika.westerberg@linux.intel.com>, Michael Ellerman <mpe@ellerman.id.au>, Arnd Bergmann <arnd@arndb.de>, Bjorn Helgaas <helgaas@kernel.org>, "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>, Juergen Gross <jgross@suse.com>, Dominik Brodowski <linux@dominikbrodowski.net>, linux-kernel@vger.kernel.org, linux-alpha@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, sparclinux@vger.kernel.org, linux-pci@vger.kernel.org, xen-devel@lists.xenproject.org Cc: Miguel Ojeda <ojeda@kernel.org>, Richard Henderson <richard.henderson@linaro.org>, Ivan Kokshaysky <ink@jurassic.park.msu.ru>, Matt Turner <mattst88@gmail.com>, Russell King <linux@armlinux.org.uk>, Thomas Bogendoerfer <tsbogend@alpha.franken.de>, Nicholas Piggin <npiggin@gmail.com>, Christophe Leroy <christophe.leroy@csgroup.eu>, "David S. Miller" <davem@davemloft.net>, Bjorn Helgaas <bhelgaas@google.com>, Stefano Stabellini <sstabellini@kernel.org>, Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> Subject: [PATCH v2 4/4] pcmcia: Convert to use pci_bus_for_each_resource_p() Date: Thu, 3 Nov 2022 18:46:44 +0200 Message-Id: <20221103164644.70554-5-andriy.shevchenko@linux.intel.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221103164644.70554-1-andriy.shevchenko@linux.intel.com> References: <20221103164644.70554-1-andriy.shevchenko@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1748494553763664486?= X-GMAIL-MSGID: =?utf-8?q?1748494553763664486?= |
Series |
PCI: Add pci_dev_for_each_resource() helper and refactor bus one
|
|
Commit Message
Andy Shevchenko
Nov. 3, 2022, 4:46 p.m. UTC
The pci_bus_for_each_resource_p() hides the iterator loop since
it may be not used otherwise. With this, we may drop that iterator
variable definition.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/pcmcia/rsrc_nonstatic.c | 9 +++------
drivers/pcmcia/yenta_socket.c | 3 +--
2 files changed, 4 insertions(+), 8 deletions(-)
Comments
Am Thu, Nov 03, 2022 at 06:46:44PM +0200 schrieb Andy Shevchenko: > The pci_bus_for_each_resource_p() hides the iterator loop since > it may be not used otherwise. With this, we may drop that iterator > variable definition. Thanks for your patch! > diff --git a/drivers/pcmcia/rsrc_nonstatic.c b/drivers/pcmcia/rsrc_nonstatic.c > index ad1141fddb4c..9d92d4bb6239 100644 > --- a/drivers/pcmcia/rsrc_nonstatic.c > +++ b/drivers/pcmcia/rsrc_nonstatic.c > @@ -934,7 +934,7 @@ static int adjust_io(struct pcmcia_socket *s, unsigned int action, unsigned long > static int nonstatic_autoadd_resources(struct pcmcia_socket *s) > { > struct resource *res; > - int i, done = 0; > + int done = 0; > > if (!s->cb_dev || !s->cb_dev->bus) > return -ENODEV; > @@ -960,12 +960,9 @@ static int nonstatic_autoadd_resources(struct pcmcia_socket *s) > */ > if (s->cb_dev->bus->number == 0) > return -EINVAL; > - > - for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) { > - res = s->cb_dev->bus->resource[i]; > -#else > - pci_bus_for_each_resource(s->cb_dev->bus, res, i) { > #endif > + > + pci_bus_for_each_resource_p(s->cb_dev->bus, res) { > if (!res) > continue; Doesn't this remove the proper iterator for X86? Even if that is the right thing to do, it needs an explict explanation. > > diff --git a/drivers/pcmcia/yenta_socket.c b/drivers/pcmcia/yenta_socket.c > index 3966a6ceb1ac..b200f2b99a7a 100644 > --- a/drivers/pcmcia/yenta_socket.c > +++ b/drivers/pcmcia/yenta_socket.c > @@ -673,9 +673,8 @@ static int yenta_search_res(struct yenta_socket *socket, struct resource *res, > u32 min) > { > struct resource *root; > - int i; > > - pci_bus_for_each_resource(socket->dev->bus, root, i) { > + pci_bus_for_each_resource_p(socket->dev->bus, root) { > if (!root) > continue; > That looks fine! Thanks, Dominik
On Thu, Nov 03, 2022 at 06:03:24PM +0100, Dominik Brodowski wrote: > Am Thu, Nov 03, 2022 at 06:46:44PM +0200 schrieb Andy Shevchenko: ... > > - > > - for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) { > > - res = s->cb_dev->bus->resource[i]; > > -#else > > - pci_bus_for_each_resource(s->cb_dev->bus, res, i) { > > #endif > > + > > + pci_bus_for_each_resource_p(s->cb_dev->bus, res) { > > if (!res) > > continue; > > Doesn't this remove the proper iterator for X86? Even if that is the right > thing to do, it needs an explict explanation. I dunno what was in 2010, but reading code now I have found no differences in the logic on how resources are being iterated in these two pieces of code. But fine, I will add a line to a commit message about this change. Considering this is done, can you issue your conditional tag so I will incorporate it in v3?
Am Thu, Nov 03, 2022 at 07:12:45PM +0200 schrieb Andy Shevchenko: > On Thu, Nov 03, 2022 at 06:03:24PM +0100, Dominik Brodowski wrote: > > Am Thu, Nov 03, 2022 at 06:46:44PM +0200 schrieb Andy Shevchenko: > > ... > > > > - > > > - for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) { > > > - res = s->cb_dev->bus->resource[i]; > > > -#else > > > - pci_bus_for_each_resource(s->cb_dev->bus, res, i) { > > > #endif > > > + > > > + pci_bus_for_each_resource_p(s->cb_dev->bus, res) { > > > if (!res) > > > continue; > > > > Doesn't this remove the proper iterator for X86? Even if that is the right > > thing to do, it needs an explict explanation. > > I dunno what was in 2010, but reading code now I have found no differences in > the logic on how resources are being iterated in these two pieces of code. > > But fine, I will add a line to a commit message about this change. > > Considering this is done, can you issue your conditional tag so I will > incorporate it in v3? Certainly, feel free to add Acked-by: Dominik Brodowski <linux@dominikbrodowski.net> Thanks, Dominik
On Thu, Nov 03, 2022 at 06:25:45PM +0100, Dominik Brodowski wrote: > Am Thu, Nov 03, 2022 at 07:12:45PM +0200 schrieb Andy Shevchenko: > > On Thu, Nov 03, 2022 at 06:03:24PM +0100, Dominik Brodowski wrote: ... > > Considering this is done, can you issue your conditional tag so I will > > incorporate it in v3? > > Certainly, feel free to add > > Acked-by: Dominik Brodowski <linux@dominikbrodowski.net> Thank you for the review!
Hello, [...] > > > - > > > - for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) { > > > - res = s->cb_dev->bus->resource[i]; > > > -#else > > > - pci_bus_for_each_resource(s->cb_dev->bus, res, i) { > > > #endif > > > + > > > + pci_bus_for_each_resource_p(s->cb_dev->bus, res) { > > > if (!res) > > > continue; > > > > Doesn't this remove the proper iterator for X86? Even if that is the right > > thing to do, it needs an explict explanation. > > I dunno what was in 2010, but reading code now I have found no differences in > the logic on how resources are being iterated in these two pieces of code. This code is over a decade old (13 years old to be precise) and there was something odd between Bjorn's and Jesse's patches, as per: 89a74ecccd1f ("PCI: add pci_bus_for_each_resource(), remove direct bus->resource[] refs") cf26e8dc4194 ("pcmcia: do not autoadd root PCI bus resources") > But fine, I will add a line to a commit message about this change. I wouldn't, personally. The change you are proposing is self-explanatory and somewhat in-line with what is there already - unless I am also reading the current implementation wrong. That said, Dominik is the maintainer of PCMCIA driver, so his is the last word, so to speak. :) > Considering this is done, can you issue your conditional tag so I will > incorporate it in v3? No need, really. Again, unless Dominik thinks otherwise. Krzysztof
Am Fri, Nov 04, 2022 at 03:29:44AM +0900 schrieb Krzysztof Wilczyński: > Hello, > > [...] > > > > - > > > > - for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) { > > > > - res = s->cb_dev->bus->resource[i]; > > > > -#else > > > > - pci_bus_for_each_resource(s->cb_dev->bus, res, i) { > > > > #endif > > > > + > > > > + pci_bus_for_each_resource_p(s->cb_dev->bus, res) { > > > > if (!res) > > > > continue; > > > > > > Doesn't this remove the proper iterator for X86? Even if that is the right > > > thing to do, it needs an explict explanation. > > > > I dunno what was in 2010, but reading code now I have found no differences in > > the logic on how resources are being iterated in these two pieces of code. > > This code is over a decade old (13 years old to be precise) and there was > something odd between Bjorn's and Jesse's patches, as per: > > 89a74ecccd1f ("PCI: add pci_bus_for_each_resource(), remove direct bus->resource[] refs") > cf26e8dc4194 ("pcmcia: do not autoadd root PCI bus resources") > > > But fine, I will add a line to a commit message about this change. > > I wouldn't, personally. The change you are proposing is self-explanatory > and somewhat in-line with what is there already - unless I am also reading > the current implementation wrong. > > That said, Dominik is the maintainer of PCMCIA driver, so his is the last > word, so to speak. :) > > > Considering this is done, can you issue your conditional tag so I will > > incorporate it in v3? > > No need, really. Again, unless Dominik thinks otherwise. Ah, thanks for the correction. Then v2 is perfectly fine. Thanks, Dominik
On Fri, Nov 04, 2022 at 03:29:44AM +0900, Krzysztof Wilczyński wrote: > > > > - > > > > - for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) { > > > > - res = s->cb_dev->bus->resource[i]; > > > > -#else > > > > - pci_bus_for_each_resource(s->cb_dev->bus, res, i) { > > > > #endif > > > > + > > > > + pci_bus_for_each_resource_p(s->cb_dev->bus, res) { > > > > if (!res) > > > > continue; > > > > > > Doesn't this remove the proper iterator for X86? Even if that is the right > > > thing to do, it needs an explict explanation. > > > > I dunno what was in 2010, but reading code now I have found no differences in > > the logic on how resources are being iterated in these two pieces of code. > > This code is over a decade old (13 years old to be precise) and there was > something odd between Bjorn's and Jesse's patches, as per: > > 89a74ecccd1f ("PCI: add pci_bus_for_each_resource(), remove direct bus->resource[] refs") > cf26e8dc4194 ("pcmcia: do not autoadd root PCI bus resources") Yeah, thanks for pointing out to the other patch from the same 2010 year. It seems the code was completely identical that time, now it uses more sophisticated way of getting bus resources, but it's kept the same for the resources under PCI_BRIDGE_RESOURCE_NUM threshold. > > But fine, I will add a line to a commit message about this change. > > I wouldn't, personally. The change you are proposing is self-explanatory > and somewhat in-line with what is there already - unless I am also reading > the current implementation wrong. But it wouldn't be harmful either. > That said, Dominik is the maintainer of PCMCIA driver, so his is the last > word, so to speak. :) > > > Considering this is done, can you issue your conditional tag so I will > > incorporate it in v3? > > No need, really. Again, unless Dominik thinks otherwise. I think that what is wanted to have to get his tag. Thanks for review, both of you, guys!
On Thu, Nov 03, 2022 at 07:38:07PM +0100, Dominik Brodowski wrote: > Am Fri, Nov 04, 2022 at 03:29:44AM +0900 schrieb Krzysztof Wilczyński: ... > > That said, Dominik is the maintainer of PCMCIA driver, so his is the last > > word, so to speak. :) > > > > > Considering this is done, can you issue your conditional tag so I will > > > incorporate it in v3? > > > > No need, really. Again, unless Dominik thinks otherwise. > > Ah, thanks for the correction. Then v2 is perfectly fine. I'm fine with either, thanks!
diff --git a/drivers/pcmcia/rsrc_nonstatic.c b/drivers/pcmcia/rsrc_nonstatic.c index ad1141fddb4c..9d92d4bb6239 100644 --- a/drivers/pcmcia/rsrc_nonstatic.c +++ b/drivers/pcmcia/rsrc_nonstatic.c @@ -934,7 +934,7 @@ static int adjust_io(struct pcmcia_socket *s, unsigned int action, unsigned long static int nonstatic_autoadd_resources(struct pcmcia_socket *s) { struct resource *res; - int i, done = 0; + int done = 0; if (!s->cb_dev || !s->cb_dev->bus) return -ENODEV; @@ -960,12 +960,9 @@ static int nonstatic_autoadd_resources(struct pcmcia_socket *s) */ if (s->cb_dev->bus->number == 0) return -EINVAL; - - for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) { - res = s->cb_dev->bus->resource[i]; -#else - pci_bus_for_each_resource(s->cb_dev->bus, res, i) { #endif + + pci_bus_for_each_resource_p(s->cb_dev->bus, res) { if (!res) continue; diff --git a/drivers/pcmcia/yenta_socket.c b/drivers/pcmcia/yenta_socket.c index 3966a6ceb1ac..b200f2b99a7a 100644 --- a/drivers/pcmcia/yenta_socket.c +++ b/drivers/pcmcia/yenta_socket.c @@ -673,9 +673,8 @@ static int yenta_search_res(struct yenta_socket *socket, struct resource *res, u32 min) { struct resource *root; - int i; - pci_bus_for_each_resource(socket->dev->bus, root, i) { + pci_bus_for_each_resource_p(socket->dev->bus, root) { if (!root) continue;