From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8C51DC76195 for ; Wed, 22 Mar 2023 19:29:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=mYf6HSElcPqOSvVekjLz9mnb404IoTFlqSmr9YuOwrw=; b=aHE9RLwhYRESOi iUbMsTaoqIhWd3JXfozEwBQB9/keWVGqG0jqhox8dpJmL2khS2h91cmbTgBfxLZvYzYJTmUNv810A w2ZbhuOChe0hEAIVAzwz57kWQrMwQ0bxiP2z3SPSLICETFbyWKWX5wXQRyoAEZ/KM0Rm0yv7p23Ex qfd8MBZ9dmWmK+3n1WVyCY6Yy7QJe9fY3toMqdLXwEg6ViY16TvTDN3xS0s7vwv255QN6dEhwJME5 TGITciTygVlnXHFJ3E6E03OLIEar3MW4S8AfLywUO8bimFDE3cepYr7vJ8hsUd4Hnp6RJiDvobBvx A5KdPbPOYV1m03zNFcbQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pf483-00HBaT-0R; Wed, 22 Mar 2023 19:28:11 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pf47z-00HBZV-1b for linux-arm-kernel@lists.infradead.org; Wed, 22 Mar 2023 19:28:08 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D6684621BD; Wed, 22 Mar 2023 19:28:06 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 03E40C433EF; Wed, 22 Mar 2023 19:28:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1679513286; bh=QIwrcxSinbm192yIC5GYq8pmRiEZXatEfhuZCKvaZVA=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=kI7PD7oKOFUaqAGxpAEV1J4pqcq5sYqOlE6tZdI8XvLDftVUW7xBLrIm39EDMCeew YbY7ekOsQ8pk2imeaozqedO08eUYORwgATq+PR72VZKK9Z+xSe875Jwime72E5sujt OKX3MZ05HrUnh6VUWqiK4KPTgw44b5/A4+dvjU5eUJO6DS6BfQZ05ojXQDveuaCfbR 6kmoRGkL8zdl0AgEDHh0jZJq5WemnXINfkZV2PRGtY6aFlS4evcy50ZkNqbrgQ5Ywt FkmgtOO8ISwsbbhE86dRqqKL+QyXR15+6PMXkOeNiqH/SJXrUqlZdQCXic7R1fgBC0 vZjopyk7ipwkQ== Date: Wed, 22 Mar 2023 14:28:04 -0500 From: Bjorn Helgaas To: Andy Shevchenko Cc: =?iso-8859-1?Q?Micka=EBl_Sala=FCn?= , Mika Westerberg , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Michael Ellerman , Randy Dunlap , Arnd Bergmann , Niklas Schnelle , "Rafael J. Wysocki" , Pali =?iso-8859-1?Q?Roh=E1r?= , "Maciej W. Rozycki" , Juergen Gross , Dominik Brodowski , 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, linux-sh@vger.kernel.org, sparclinux@vger.kernel.org, linux-pci@vger.kernel.org, xen-devel@lists.xenproject.org, linux-acpi@vger.kernel.org, Miguel Ojeda , Richard Henderson , Ivan Kokshaysky , Matt Turner , Russell King , Andrew Lunn , Sebastian Hesselbarth , Gregory Clement , Thomas Bogendoerfer , Nicholas Piggin , Christophe Leroy , Anatolij Gustschin , Yoshinori Sato , Rich Felker , John Paul Adrian Glaubitz , "David S. Miller" , Bjorn Helgaas , Stefano Stabellini , Oleksandr Tyshchenko Subject: Re: [PATCH v6 1/4] PCI: Introduce pci_dev_for_each_resource() Message-ID: <20230322192804.GA2485349@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230320131633.61680-2-andriy.shevchenko@linux.intel.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230322_122807_608798_3E26BF1A X-CRM114-Status: GOOD ( 22.08 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Andy and Mika, I really like the improvements here. They make the code read much better. On Mon, Mar 20, 2023 at 03:16:30PM +0200, Andy Shevchenko wrote: > From: Mika Westerberg > ... > static void fixup_winbond_82c105(struct pci_dev* dev) > { > - int i; > + struct resource *r; > unsigned int reg; > > if (!machine_is(pseries)) > @@ -251,14 +251,14 @@ static void fixup_winbond_82c105(struct pci_dev* dev) > /* Enable LEGIRQ to use INTC instead of ISA interrupts */ > pci_write_config_dword(dev, 0x40, reg | (1<<11)); > > - for (i = 0; i < DEVICE_COUNT_RESOURCE; ++i) { > + pci_dev_for_each_resource_p(dev, r) { > /* zap the 2nd function of the winbond chip */ > - if (dev->resource[i].flags & IORESOURCE_IO > - && dev->bus->number == 0 && dev->devfn == 0x81) > - dev->resource[i].flags &= ~IORESOURCE_IO; > - if (dev->resource[i].start == 0 && dev->resource[i].end) { > - dev->resource[i].flags = 0; > - dev->resource[i].end = 0; > + if (dev->bus->number == 0 && dev->devfn == 0x81 && > + r->flags & IORESOURCE_IO) This is a nice literal conversion, but it's kind of lame to test bus->number and devfn *inside* the loop here, since they can't change inside the loop. > + r->flags &= ~IORESOURCE_IO; > + if (r->start == 0 && r->end) { > + r->flags = 0; > + r->end = 0; > } > } > #define pci_resource_len(dev,bar) \ > ((pci_resource_end((dev), (bar)) == 0) ? 0 : \ > \ > - (pci_resource_end((dev), (bar)) - \ > - pci_resource_start((dev), (bar)) + 1)) > + resource_size(pci_resource_n((dev), (bar)))) I like this change, but it's unrelated to pci_dev_for_each_resource() and unmentioned in the commit log. > +#define __pci_dev_for_each_resource(dev, res, __i, vartype) \ > + for (vartype __i = 0; \ > + res = pci_resource_n(dev, __i), __i < PCI_NUM_RESOURCES; \ > + __i++) > + > +#define pci_dev_for_each_resource(dev, res, i) \ > + __pci_dev_for_each_resource(dev, res, i, ) > + > +#define pci_dev_for_each_resource_p(dev, res) \ > + __pci_dev_for_each_resource(dev, res, __i, unsigned int) This series converts many cases to drop the iterator variable ("i"), which is fantastic. Several of the remaining places need the iterator variable only to call pci_claim_resource(), which could be converted to take a "struct resource *" directly without much trouble. We don't have to do that pci_claim_resource() conversion now, but since we're converging on the "(dev, res)" style, I think we should reverse the names so we have something like: pci_dev_for_each_resource(dev, res) pci_dev_for_each_resource_idx(dev, res, i) Not sure __pci_dev_for_each_resource() is worthwhile since it only avoids repeating that single "for" statement, and passing in "vartype" (sometimes empty to implicitly avoid the declaration) is a little complicated to read. I think it'd be easier to read like this: #define pci_dev_for_each_resource(dev, res) \ for (unsigned int __i = 0; \ res = pci_resource_n(dev, __i), __i < PCI_NUM_RESOURCES; \ __i++) #define pci_dev_for_each_resource_idx(dev, res, idx) \ for (idx = 0; \ res = pci_resource_n(dev, idx), idx < PCI_NUM_RESOURCES; \ idx++) Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel