* Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users [not found] <ZF6YIezraETr9iNM@bhelgaas> @ 2023-05-30 21:24 ` Bjorn Helgaas [not found] ` <CAOiHx==5YWhDiZP2PyHZiJrmtqRzvqCqoSO59RwuYuR85BezBg@mail.gmail.com> 0 siblings, 1 reply; 11+ messages in thread From: Bjorn Helgaas @ 2023-05-30 21:24 UTC (permalink / raw) To: Andy Shevchenko Cc: Krzysztof Wilczyński, Rich Felker, linux-sh, linux-pci, Dominik Brodowski, linux-kernel, Mickaël Salaün, Andrew Lunn, sparclinux, Stefano Stabellini, Yoshinori Sato, Gregory Clement, Rafael J. Wysocki, Russell King, linux-acpi, Miguel Ojeda, xen-devel, Matt Turner, Anatolij Gustschin, Sebastian Hesselbarth, Arnd Bergmann, Niklas Schnelle, Richard Henderson, Nicholas Piggin, Ivan Kokshaysky, John Paul Adrian Glaubitz, Bjorn Helgaas, Mika Westerberg, linux-arm-kernel, Juergen Gross, Thomas Bogendoerfer, Philippe Mathieu-Daudé, linuxppc-dev, Randy Dunlap, linux-mips, Oleksandr Tyshchenko, linux-alpha, Pali Rohár, David S. Miller, Maciej W. Rozycki On Fri, May 12, 2023 at 02:48:51PM -0500, Bjorn Helgaas wrote: > On Fri, May 12, 2023 at 01:56:29PM +0300, Andy Shevchenko wrote: > > On Tue, May 09, 2023 at 01:21:22PM -0500, Bjorn Helgaas wrote: > > > On Tue, Apr 04, 2023 at 11:11:01AM -0500, Bjorn Helgaas wrote: > > > > On Thu, Mar 30, 2023 at 07:24:27PM +0300, Andy Shevchenko wrote: > > > > > Provide two new helper macros to iterate over PCI device resources and > > > > > convert users. > > > > > > > Applied 2-7 to pci/resource for v6.4, thanks, I really like this! > > > > > > This is 09cc90063240 ("PCI: Introduce pci_dev_for_each_resource()") > > > upstream now. > > > > > > Coverity complains about each use, > > > > It needs more clarification here. Use of reduced variant of the > > macro or all of them? If the former one, then I can speculate that > > Coverity (famous for false positives) simply doesn't understand `for > > (type var; var ...)` code. > > True, Coverity finds false positives. It flagged every use in > drivers/pci and drivers/pnp. It didn't mention the arch/alpha, arm, > mips, powerpc, sh, or sparc uses, but I think it just didn't look at > those. > > It flagged both: > > pbus_size_io pci_dev_for_each_resource(dev, r) > pbus_size_mem pci_dev_for_each_resource(dev, r, i) > > Here's a spreadsheet with a few more details (unfortunately I don't > know how to make it dump the actual line numbers or analysis like I > pasted below, so "pci_dev_for_each_resource" doesn't appear). These > are mostly in the "Drivers-PCI" component. > > https://docs.google.com/spreadsheets/d/1ohOJwxqXXoDUA0gwopgk-z-6ArLvhN7AZn4mIlDkHhQ/edit?usp=sharing > > These particular reports are in the "High Impact Outstanding" tab. Where are we at? Are we going to ignore this because some Coverity reports are false positives? Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <CAOiHx==5YWhDiZP2PyHZiJrmtqRzvqCqoSO59RwuYuR85BezBg@mail.gmail.com>]
* Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users [not found] ` <CAOiHx==5YWhDiZP2PyHZiJrmtqRzvqCqoSO59RwuYuR85BezBg@mail.gmail.com> @ 2023-05-31 21:30 ` Bjorn Helgaas 2023-06-01 11:17 ` Jonas Gorski 2023-06-05 14:04 ` Andy Shevchenko 2023-06-01 16:25 ` Andy Shevchenko 1 sibling, 2 replies; 11+ messages in thread From: Bjorn Helgaas @ 2023-05-31 21:30 UTC (permalink / raw) To: Jonas Gorski Cc: Andy Shevchenko, Krzysztof Wilczyński, linux-pci, linux-kernel, Mickaël Salaün, Rich Felker, linux-sh, Dominik Brodowski, Andrew Lunn, sparclinux, Stefano Stabellini, Yoshinori Sato, Gregory Clement, Rafael J. Wysocki, Russell King, linux-acpi, Miguel Ojeda, xen-devel, Matt Turner, Anatolij Gustschin, Sebastian Hesselbarth, Arnd Bergmann, Niklas Schnelle, Richard Henderson, Nicholas Piggin, Ivan Kokshaysky, John Paul Adrian Glaubitz, Bjorn Helgaas, Mika Westerberg, linux-arm-kernel, Juergen Gross, Thomas Bogendoerfer, Philippe Mathieu-Daudé, linuxppc-dev, Randy Dunlap, linux-mips, Oleksandr Tyshchenko, linux-alpha, Pali Rohár, David S. Miller, Maciej W. Rozycki On Wed, May 31, 2023 at 08:48:35PM +0200, Jonas Gorski wrote: > ... > Looking at the code I understand where coverity is coming from: > > #define __pci_dev_for_each_res0(dev, res, ...) \ > for (unsigned int __b = 0; \ > res = pci_resource_n(dev, __b), __b < PCI_NUM_RESOURCES; \ > __b++) > > res will be assigned before __b is checked for being less than > PCI_NUM_RESOURCES, making it point to behind the array at the end of > the last loop iteration. > > Rewriting the test expression as > > __b < PCI_NUM_RESOURCES && (res = pci_resource_n(dev, __b)); > > should avoid the (coverity) warning by making use of lazy evaluation. > > It probably makes the code slightly less performant as res will now be > checked for being not NULL (which will always be true), but I doubt it > will be significant (or in any hot paths). Thanks a lot for looking into this! I think you're right, and I think the rewritten expression is more logical as well. Do you want to post a patch for it? Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users 2023-05-31 21:30 ` Bjorn Helgaas @ 2023-06-01 11:17 ` Jonas Gorski 2023-06-05 14:04 ` Andy Shevchenko 1 sibling, 0 replies; 11+ messages in thread From: Jonas Gorski @ 2023-06-01 11:17 UTC (permalink / raw) To: Bjorn Helgaas Cc: Andy Shevchenko, Krzysztof Wilczyński, linux-pci, linux-kernel, Mickaël Salaün, Rich Felker, linux-sh, Dominik Brodowski, Andrew Lunn, sparclinux, Stefano Stabellini, Yoshinori Sato, Gregory Clement, Rafael J. Wysocki, Russell King, linux-acpi, Miguel Ojeda, xen-devel, Matt Turner, Anatolij Gustschin, Sebastian Hesselbarth, Arnd Bergmann, Niklas Schnelle, Richard Henderson, Nicholas Piggin, Ivan Kokshaysky, John Paul Adrian Glaubitz, Bjorn Helgaas, Mika Westerberg, linux-arm-kernel, Juergen Gross, Thomas Bogendoerfer, Philippe Mathieu-Daudé, linuxppc-dev, Randy Dunlap, linux-mips, Oleksandr Tyshchenko, linux-alpha, Pali Rohár, David S. Miller, Maciej W. Rozycki On Wed, 31 May 2023 at 23:30, Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Wed, May 31, 2023 at 08:48:35PM +0200, Jonas Gorski wrote: > > ... > > > Looking at the code I understand where coverity is coming from: > > > > #define __pci_dev_for_each_res0(dev, res, ...) \ > > for (unsigned int __b = 0; \ > > res = pci_resource_n(dev, __b), __b < PCI_NUM_RESOURCES; \ > > __b++) > > > > res will be assigned before __b is checked for being less than > > PCI_NUM_RESOURCES, making it point to behind the array at the end of > > the last loop iteration. > > > > Rewriting the test expression as > > > > __b < PCI_NUM_RESOURCES && (res = pci_resource_n(dev, __b)); > > > > should avoid the (coverity) warning by making use of lazy evaluation. > > > > It probably makes the code slightly less performant as res will now be > > checked for being not NULL (which will always be true), but I doubt it > > will be significant (or in any hot paths). > > Thanks a lot for looking into this! I think you're right, and I think > the rewritten expression is more logical as well. Do you want to post > a patch for it? Not sure when I'll come around to, so I have no strong feeling here. So feel free to just borrow my suggestion, especially since I won't be able to test it (don't have a kernel tree ready I can build and boot). Also looking more closely at the Coverity output, I think it might not handle the comma operator well in the loop condition: > 11. incr: Incrementing __b. The value of __b may now be up to 17. > 12. alias: Assigning: r = &pdev->resource[__b]. r may now point to as high as element 17 of pdev->resource (which consists of 17 64-byte elements). > 13. Condition __b < PCI_NUM_RESOURCES, taking true branch. > 14. Condition (r = &pdev->resource[__b]) , (__b < PCI_NUM_RESOURCES), taking true branch. 13 If __b is 17 ( = PCI_NUM_RESOURCES) we wouldn't taking the true branch, but somehow Coverity thinks that we do. No idea if it is worth reporting to Coverity. The changed condition statement should hopefully silence the warning though. Regards Jonas _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users 2023-05-31 21:30 ` Bjorn Helgaas 2023-06-01 11:17 ` Jonas Gorski @ 2023-06-05 14:04 ` Andy Shevchenko 1 sibling, 0 replies; 11+ messages in thread From: Andy Shevchenko @ 2023-06-05 14:04 UTC (permalink / raw) To: Bjorn Helgaas Cc: Jonas Gorski, Krzysztof Wilczyński, linux-pci, linux-kernel, Mickaël Salaün, Rich Felker, linux-sh, Dominik Brodowski, Andrew Lunn, sparclinux, Stefano Stabellini, Yoshinori Sato, Gregory Clement, Rafael J. Wysocki, Russell King, linux-acpi, Miguel Ojeda, xen-devel, Matt Turner, Anatolij Gustschin, Sebastian Hesselbarth, Arnd Bergmann, Niklas Schnelle, Richard Henderson, Nicholas Piggin, Ivan Kokshaysky, John Paul Adrian Glaubitz, Bjorn Helgaas, Mika Westerberg, linux-arm-kernel, Juergen Gross, Thomas Bogendoerfer, Philippe Mathieu-Daudé, linuxppc-dev, Randy Dunlap, linux-mips, Oleksandr Tyshchenko, linux-alpha, Pali Rohár, David S. Miller, Maciej W. Rozycki On Wed, May 31, 2023 at 04:30:28PM -0500, Bjorn Helgaas wrote: > On Wed, May 31, 2023 at 08:48:35PM +0200, Jonas Gorski wrote: ... > > Looking at the code I understand where coverity is coming from: > > > > #define __pci_dev_for_each_res0(dev, res, ...) \ > > for (unsigned int __b = 0; \ > > res = pci_resource_n(dev, __b), __b < PCI_NUM_RESOURCES; \ > > __b++) > > > > res will be assigned before __b is checked for being less than > > PCI_NUM_RESOURCES, making it point to behind the array at the end of > > the last loop iteration. > > > > Rewriting the test expression as > > > > __b < PCI_NUM_RESOURCES && (res = pci_resource_n(dev, __b)); > > > > should avoid the (coverity) warning by making use of lazy evaluation. > > > > It probably makes the code slightly less performant as res will now be > > checked for being not NULL (which will always be true), but I doubt it > > will be significant (or in any hot paths). > > Thanks a lot for looking into this! I think you're right, and I think > the rewritten expression is more logical as well. Do you want to post > a patch for it? Gimme some time, I was on a long leave and now it's a pile to handle. -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users [not found] ` <CAOiHx==5YWhDiZP2PyHZiJrmtqRzvqCqoSO59RwuYuR85BezBg@mail.gmail.com> 2023-05-31 21:30 ` Bjorn Helgaas @ 2023-06-01 16:25 ` Andy Shevchenko 2023-06-01 16:27 ` Andy Shevchenko 1 sibling, 1 reply; 11+ messages in thread From: Andy Shevchenko @ 2023-06-01 16:25 UTC (permalink / raw) To: Jonas Gorski Cc: Bjorn Helgaas, Krzysztof Wilczyński, Rich Felker, linux-sh, linux-pci, Dominik Brodowski, linux-kernel, Mickaël Salaün, Andrew Lunn, sparclinux, Stefano Stabellini, Yoshinori Sato, Gregory Clement, Rafael J. Wysocki, Russell King, linux-acpi, Miguel Ojeda, xen-devel, Matt Turner, Anatolij Gustschin, Sebastian Hesselbarth, Arnd Bergmann, Niklas Schnelle, Richard Henderson, Nicholas Piggin, Ivan Kokshaysky, John Paul Adrian Glaubitz, Bjorn Helgaas, Mika Westerberg, linux-arm-kernel, Juergen Gross, Thomas Bogendoerfer, Philippe Mathieu-Daudé, linuxppc-dev, Randy Dunlap, linux-mips, Oleksandr Tyshchenko, linux-alpha, Pali Rohár, David S. Miller, Maciej W. Rozycki On Wed, May 31, 2023 at 08:48:35PM +0200, Jonas Gorski wrote: > On Tue, 30 May 2023 at 23:34, Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Fri, May 12, 2023 at 02:48:51PM -0500, Bjorn Helgaas wrote: > > > On Fri, May 12, 2023 at 01:56:29PM +0300, Andy Shevchenko wrote: > > > > On Tue, May 09, 2023 at 01:21:22PM -0500, Bjorn Helgaas wrote: > > > > > On Tue, Apr 04, 2023 at 11:11:01AM -0500, Bjorn Helgaas wrote: > > > > > > On Thu, Mar 30, 2023 at 07:24:27PM +0300, Andy Shevchenko wrote: > > > > > > > Provide two new helper macros to iterate over PCI device resources and > > > > > > > convert users. > > > > > > > > > > > Applied 2-7 to pci/resource for v6.4, thanks, I really like this! > > > > > > > > > > This is 09cc90063240 ("PCI: Introduce pci_dev_for_each_resource()") > > > > > upstream now. > > > > > > > > > > Coverity complains about each use, > > > > > > > > It needs more clarification here. Use of reduced variant of the > > > > macro or all of them? If the former one, then I can speculate that > > > > Coverity (famous for false positives) simply doesn't understand `for > > > > (type var; var ...)` code. > > > > > > True, Coverity finds false positives. It flagged every use in > > > drivers/pci and drivers/pnp. It didn't mention the arch/alpha, arm, > > > mips, powerpc, sh, or sparc uses, but I think it just didn't look at > > > those. > > > > > > It flagged both: > > > > > > pbus_size_io pci_dev_for_each_resource(dev, r) > > > pbus_size_mem pci_dev_for_each_resource(dev, r, i) > > > > > > Here's a spreadsheet with a few more details (unfortunately I don't > > > know how to make it dump the actual line numbers or analysis like I > > > pasted below, so "pci_dev_for_each_resource" doesn't appear). These > > > are mostly in the "Drivers-PCI" component. > > > > > > https://docs.google.com/spreadsheets/d/1ohOJwxqXXoDUA0gwopgk-z-6ArLvhN7AZn4mIlDkHhQ/edit?usp=sharing > > > > > > These particular reports are in the "High Impact Outstanding" tab. > > > > Where are we at? Are we going to ignore this because some Coverity > > reports are false positives? > > Looking at the code I understand where coverity is coming from: > > #define __pci_dev_for_each_res0(dev, res, ...) \ > for (unsigned int __b = 0; \ > res = pci_resource_n(dev, __b), __b < PCI_NUM_RESOURCES; \ > __b++) > > res will be assigned before __b is checked for being less than > PCI_NUM_RESOURCES, making it point to behind the array at the end of > the last loop iteration. Which is fine and you stumbled over the same mistake I made, that's why the documentation has been added to describe why the heck this macro is written the way it's written. Coverity sucks. > Rewriting the test expression as > > __b < PCI_NUM_RESOURCES && (res = pci_resource_n(dev, __b)); > > should avoid the (coverity) warning by making use of lazy evaluation. Obviously NAK. > It probably makes the code slightly less performant as res will now be > checked for being not NULL (which will always be true), but I doubt it > will be significant (or in any hot paths). -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users 2023-06-01 16:25 ` Andy Shevchenko @ 2023-06-01 16:27 ` Andy Shevchenko 0 siblings, 0 replies; 11+ messages in thread From: Andy Shevchenko @ 2023-06-01 16:27 UTC (permalink / raw) To: Jonas Gorski Cc: Bjorn Helgaas, Krzysztof Wilczyński, Rich Felker, linux-sh, linux-pci, Dominik Brodowski, linux-kernel, Mickaël Salaün, Andrew Lunn, sparclinux, Stefano Stabellini, Yoshinori Sato, Gregory Clement, Rafael J. Wysocki, Russell King, linux-acpi, Miguel Ojeda, xen-devel, Matt Turner, Anatolij Gustschin, Sebastian Hesselbarth, Arnd Bergmann, Niklas Schnelle, Richard Henderson, Nicholas Piggin, Ivan Kokshaysky, John Paul Adrian Glaubitz, Bjorn Helgaas, Mika Westerberg, linux-arm-kernel, Juergen Gross, Thomas Bogendoerfer, Philippe Mathieu-Daudé, linuxppc-dev, Randy Dunlap, linux-mips, Oleksandr Tyshchenko, linux-alpha, Pali Rohár, David S. Miller, Maciej W. Rozycki On Thu, Jun 01, 2023 at 07:25:46PM +0300, Andy Shevchenko wrote: > On Wed, May 31, 2023 at 08:48:35PM +0200, Jonas Gorski wrote: > > On Tue, 30 May 2023 at 23:34, Bjorn Helgaas <helgaas@kernel.org> wrote: > > > On Fri, May 12, 2023 at 02:48:51PM -0500, Bjorn Helgaas wrote: ... > > > Where are we at? Are we going to ignore this because some Coverity > > > reports are false positives? > > > > Looking at the code I understand where coverity is coming from: > > > > #define __pci_dev_for_each_res0(dev, res, ...) \ > > for (unsigned int __b = 0; \ > > res = pci_resource_n(dev, __b), __b < PCI_NUM_RESOURCES; \ > > __b++) > > > > res will be assigned before __b is checked for being less than > > PCI_NUM_RESOURCES, making it point to behind the array at the end of > > the last loop iteration. > > Which is fine and you stumbled over the same mistake I made, that's why the > documentation has been added to describe why the heck this macro is written > the way it's written. > > Coverity sucks. > > > Rewriting the test expression as > > > > __b < PCI_NUM_RESOURCES && (res = pci_resource_n(dev, __b)); > > > > should avoid the (coverity) warning by making use of lazy evaluation. > > Obviously NAK. > > > It probably makes the code slightly less performant as res will now be > > checked for being not NULL (which will always be true), but I doubt it > > will be significant (or in any hot paths). Oh my god, I mistakenly read this as bus macro, sorry for my rant, it's simply wrong. -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users @ 2023-03-30 16:24 Andy Shevchenko 2023-04-04 16:11 ` Bjorn Helgaas 0 siblings, 1 reply; 11+ messages in thread From: Andy Shevchenko @ 2023-03-30 16:24 UTC (permalink / raw) To: Mickaël Salaün, Andy Shevchenko, Krzysztof Wilczyński, Mika Westerberg, Michael Ellerman, Randy Dunlap, Arnd Bergmann, Philippe Mathieu-Daudé, Niklas Schnelle, Bjorn Helgaas, Rafael J. Wysocki, Pali Rohár, Maciej W. Rozycki, Juergen Gross, Dominik Brodowski, linux-kernel, linux-alpha, linux-arm-kernel, linux-mips, linuxppc-dev, linux-sh, sparclinux, linux-pci, xen-devel, linux-acpi Cc: 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 Provide two new helper macros to iterate over PCI device resources and convert users. Looking at it, refactor existing pci_bus_for_each_resource() and convert users accordingly. Note, the amount of lines grew due to the documentation update. Changelog v8: - fixed issue with pci_bus_for_each_resource() macro (LKP) - due to above added a new patch to document how it works - moved the last patch to be #2 (Philippe) - added tags (Philippe) Changelog v7: - made both macros to share same name (Bjorn) - split out the pci_resource_n() conversion (Bjorn) Changelog v6: - dropped unused variable in PPC code (LKP) Changelog v5: - renamed loop variable to minimize the clash (Keith) - addressed smatch warning (Dan) - addressed 0-day bot findings (LKP) Changelog v4: - rebased on top of v6.3-rc1 - added tag (Krzysztof) Changelog v3: - rebased on top of v2 by Mika, see above - added tag to pcmcia patch (Dominik) Changelog v2: - refactor to have two macros - refactor existing pci_bus_for_each_resource() in the same way and convert users Andy Shevchenko (6): kernel.h: Split out COUNT_ARGS() and CONCATENATE() PCI: Introduce pci_resource_n() PCI: Document pci_bus_for_each_resource() to avoid confusion PCI: Allow pci_bus_for_each_resource() to take less arguments EISA: Convert to use less arguments in pci_bus_for_each_resource() pcmcia: Convert to use less arguments in pci_bus_for_each_resource() Mika Westerberg (1): PCI: Introduce pci_dev_for_each_resource() .clang-format | 1 + arch/alpha/kernel/pci.c | 5 +- arch/arm/kernel/bios32.c | 16 +++-- arch/arm/mach-dove/pcie.c | 10 ++-- arch/arm/mach-mv78xx0/pcie.c | 10 ++-- arch/arm/mach-orion5x/pci.c | 10 ++-- arch/mips/pci/ops-bcm63xx.c | 8 +-- arch/mips/pci/pci-legacy.c | 3 +- arch/powerpc/kernel/pci-common.c | 21 +++---- arch/powerpc/platforms/4xx/pci.c | 8 +-- arch/powerpc/platforms/52xx/mpc52xx_pci.c | 5 +- arch/powerpc/platforms/pseries/pci.c | 16 ++--- arch/sh/drivers/pci/pcie-sh7786.c | 10 ++-- arch/sparc/kernel/leon_pci.c | 5 +- arch/sparc/kernel/pci.c | 10 ++-- arch/sparc/kernel/pcic.c | 5 +- drivers/eisa/pci_eisa.c | 4 +- drivers/pci/bus.c | 7 +-- drivers/pci/hotplug/shpchp_sysfs.c | 8 +-- drivers/pci/pci.c | 3 +- drivers/pci/probe.c | 2 +- drivers/pci/remove.c | 5 +- drivers/pci/setup-bus.c | 37 +++++------- drivers/pci/setup-res.c | 4 +- drivers/pci/vgaarb.c | 17 ++---- drivers/pci/xen-pcifront.c | 4 +- drivers/pcmcia/rsrc_nonstatic.c | 9 +-- drivers/pcmcia/yenta_socket.c | 3 +- drivers/pnp/quirks.c | 29 ++++----- include/linux/args.h | 13 ++++ include/linux/kernel.h | 8 +-- include/linux/pci.h | 72 +++++++++++++++++++---- 32 files changed, 190 insertions(+), 178 deletions(-) create mode 100644 include/linux/args.h -- 2.40.0.1.gaa8946217a0b _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users 2023-03-30 16:24 Andy Shevchenko @ 2023-04-04 16:11 ` Bjorn Helgaas 2023-04-05 8:28 ` Andy Shevchenko 0 siblings, 1 reply; 11+ messages in thread From: Bjorn Helgaas @ 2023-04-04 16:11 UTC (permalink / raw) To: Andy Shevchenko Cc: Mickaël Salaün, Krzysztof Wilczyński, Mika Westerberg, Michael Ellerman, Randy Dunlap, Arnd Bergmann, Philippe Mathieu-Daudé, Niklas Schnelle, Rafael J. Wysocki, Pali Rohár, Maciej W. Rozycki, Juergen Gross, Dominik Brodowski, linux-kernel, linux-alpha, linux-arm-kernel, linux-mips, linuxppc-dev, linux-sh, sparclinux, linux-pci, xen-devel, linux-acpi, 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 On Thu, Mar 30, 2023 at 07:24:27PM +0300, Andy Shevchenko wrote: > Provide two new helper macros to iterate over PCI device resources and > convert users. > > Looking at it, refactor existing pci_bus_for_each_resource() and convert > users accordingly. > > Note, the amount of lines grew due to the documentation update. > > Changelog v8: > - fixed issue with pci_bus_for_each_resource() macro (LKP) > - due to above added a new patch to document how it works > - moved the last patch to be #2 (Philippe) > - added tags (Philippe) > > Changelog v7: > - made both macros to share same name (Bjorn) I didn't actually request the same name for both; I would have had no idea how to even do that :) v6 had: pci_dev_for_each_resource_p(dev, res) pci_dev_for_each_resource(dev, res, i) and I suggested: pci_dev_for_each_resource(dev, res) pci_dev_for_each_resource_idx(dev, res, i) because that pattern is used elsewhere. But you figured out how to do it, and having one name is even better, so thanks for that extra work! > - split out the pci_resource_n() conversion (Bjorn) > > Changelog v6: > - dropped unused variable in PPC code (LKP) > > Changelog v5: > - renamed loop variable to minimize the clash (Keith) > - addressed smatch warning (Dan) > - addressed 0-day bot findings (LKP) > > Changelog v4: > - rebased on top of v6.3-rc1 > - added tag (Krzysztof) > > Changelog v3: > - rebased on top of v2 by Mika, see above > - added tag to pcmcia patch (Dominik) > > Changelog v2: > - refactor to have two macros > - refactor existing pci_bus_for_each_resource() in the same way and > convert users > > Andy Shevchenko (6): > kernel.h: Split out COUNT_ARGS() and CONCATENATE() > PCI: Introduce pci_resource_n() > PCI: Document pci_bus_for_each_resource() to avoid confusion > PCI: Allow pci_bus_for_each_resource() to take less arguments > EISA: Convert to use less arguments in pci_bus_for_each_resource() > pcmcia: Convert to use less arguments in pci_bus_for_each_resource() > > Mika Westerberg (1): > PCI: Introduce pci_dev_for_each_resource() > > .clang-format | 1 + > arch/alpha/kernel/pci.c | 5 +- > arch/arm/kernel/bios32.c | 16 +++-- > arch/arm/mach-dove/pcie.c | 10 ++-- > arch/arm/mach-mv78xx0/pcie.c | 10 ++-- > arch/arm/mach-orion5x/pci.c | 10 ++-- > arch/mips/pci/ops-bcm63xx.c | 8 +-- > arch/mips/pci/pci-legacy.c | 3 +- > arch/powerpc/kernel/pci-common.c | 21 +++---- > arch/powerpc/platforms/4xx/pci.c | 8 +-- > arch/powerpc/platforms/52xx/mpc52xx_pci.c | 5 +- > arch/powerpc/platforms/pseries/pci.c | 16 ++--- > arch/sh/drivers/pci/pcie-sh7786.c | 10 ++-- > arch/sparc/kernel/leon_pci.c | 5 +- > arch/sparc/kernel/pci.c | 10 ++-- > arch/sparc/kernel/pcic.c | 5 +- > drivers/eisa/pci_eisa.c | 4 +- > drivers/pci/bus.c | 7 +-- > drivers/pci/hotplug/shpchp_sysfs.c | 8 +-- > drivers/pci/pci.c | 3 +- > drivers/pci/probe.c | 2 +- > drivers/pci/remove.c | 5 +- > drivers/pci/setup-bus.c | 37 +++++------- > drivers/pci/setup-res.c | 4 +- > drivers/pci/vgaarb.c | 17 ++---- > drivers/pci/xen-pcifront.c | 4 +- > drivers/pcmcia/rsrc_nonstatic.c | 9 +-- > drivers/pcmcia/yenta_socket.c | 3 +- > drivers/pnp/quirks.c | 29 ++++----- > include/linux/args.h | 13 ++++ > include/linux/kernel.h | 8 +-- > include/linux/pci.h | 72 +++++++++++++++++++---- > 32 files changed, 190 insertions(+), 178 deletions(-) > create mode 100644 include/linux/args.h Applied 2-7 to pci/resource for v6.4, thanks, I really like this! I omitted [1/7] kernel.h: Split out COUNT_ARGS() and CONCATENATE()" only because it's not essential to this series and has only a trivial one-line impact on include/linux/pci.h. Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users 2023-04-04 16:11 ` Bjorn Helgaas @ 2023-04-05 8:28 ` Andy Shevchenko 2023-04-05 20:18 ` Bjorn Helgaas 0 siblings, 1 reply; 11+ messages in thread From: Andy Shevchenko @ 2023-04-05 8:28 UTC (permalink / raw) To: Bjorn Helgaas Cc: Mickaël Salaün, Krzysztof Wilczyński, Mika Westerberg, Michael Ellerman, Randy Dunlap, Arnd Bergmann, Philippe Mathieu-Daudé, Niklas Schnelle, Rafael J. Wysocki, Pali Rohár, Maciej W. Rozycki, Juergen Gross, Dominik Brodowski, linux-kernel, linux-alpha, linux-arm-kernel, linux-mips, linuxppc-dev, linux-sh, sparclinux, linux-pci, xen-devel, linux-acpi, 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 On Tue, Apr 04, 2023 at 11:11:01AM -0500, Bjorn Helgaas wrote: > On Thu, Mar 30, 2023 at 07:24:27PM +0300, Andy Shevchenko wrote: > > Provide two new helper macros to iterate over PCI device resources and > > convert users. > > > > Looking at it, refactor existing pci_bus_for_each_resource() and convert > > users accordingly. > > > > Note, the amount of lines grew due to the documentation update. > > > > Changelog v8: > > - fixed issue with pci_bus_for_each_resource() macro (LKP) > > - due to above added a new patch to document how it works > > - moved the last patch to be #2 (Philippe) > > - added tags (Philippe) > > > > Changelog v7: > > - made both macros to share same name (Bjorn) > > I didn't actually request the same name for both; I would have had no > idea how to even do that :) > > v6 had: > > pci_dev_for_each_resource_p(dev, res) > pci_dev_for_each_resource(dev, res, i) > > and I suggested: > > pci_dev_for_each_resource(dev, res) > pci_dev_for_each_resource_idx(dev, res, i) > > because that pattern is used elsewhere. Ah, sorry I misinterpreted your suggestion (I thought that at the end of the day you wanted the macro to be less intrusive, so we change less code, that's why I interpreted it the way described in the Changelog). > But you figured out how to do > it, and having one name is even better, so thanks for that extra work! You are welcome! > > - split out the pci_resource_n() conversion (Bjorn) > > > > Changelog v6: > > - dropped unused variable in PPC code (LKP) > > > > Changelog v5: > > - renamed loop variable to minimize the clash (Keith) > > - addressed smatch warning (Dan) > > - addressed 0-day bot findings (LKP) > > > > Changelog v4: > > - rebased on top of v6.3-rc1 > > - added tag (Krzysztof) > > > > Changelog v3: > > - rebased on top of v2 by Mika, see above > > - added tag to pcmcia patch (Dominik) > > > > Changelog v2: > > - refactor to have two macros > > - refactor existing pci_bus_for_each_resource() in the same way and > > convert users > > > > Andy Shevchenko (6): > > kernel.h: Split out COUNT_ARGS() and CONCATENATE() > > PCI: Introduce pci_resource_n() > > PCI: Document pci_bus_for_each_resource() to avoid confusion > > PCI: Allow pci_bus_for_each_resource() to take less arguments > > EISA: Convert to use less arguments in pci_bus_for_each_resource() > > pcmcia: Convert to use less arguments in pci_bus_for_each_resource() ... > Applied 2-7 to pci/resource for v6.4, thanks, I really like this! Btw, can you actually drop patch 7, please? After I have updated the documentation I have realised that why the first chunk is invalid. It needs mode careful check and rework. > I omitted > > [1/7] kernel.h: Split out COUNT_ARGS() and CONCATENATE()" > > only because it's not essential to this series and has only a trivial > one-line impact on include/linux/pci.h. I'm not sure I understood what exactly "essentiality" means to you, but I included that because it makes the split which can be used later by others and not including kernel.h in the header is the objective I want to achieve. Without this patch the achievement is going to be deferred. Yet, this, as you have noticed, allows to compile and use the macros in the rest of the patches. P.S. Thank you for the review and application of the rest! -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users 2023-04-05 8:28 ` Andy Shevchenko @ 2023-04-05 20:18 ` Bjorn Helgaas 2023-04-06 10:31 ` Andy Shevchenko 0 siblings, 1 reply; 11+ messages in thread From: Bjorn Helgaas @ 2023-04-05 20:18 UTC (permalink / raw) To: Andy Shevchenko Cc: Mickaël Salaün, Krzysztof Wilczyński, Mika Westerberg, Michael Ellerman, Randy Dunlap, Arnd Bergmann, Philippe Mathieu-Daudé, Niklas Schnelle, Rafael J. Wysocki, Pali Rohár, Maciej W. Rozycki, Juergen Gross, Dominik Brodowski, linux-kernel, linux-alpha, linux-arm-kernel, linux-mips, linuxppc-dev, linux-sh, sparclinux, linux-pci, xen-devel, linux-acpi, 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 On Wed, Apr 05, 2023 at 11:28:27AM +0300, Andy Shevchenko wrote: > On Tue, Apr 04, 2023 at 11:11:01AM -0500, Bjorn Helgaas wrote: > > On Thu, Mar 30, 2023 at 07:24:27PM +0300, Andy Shevchenko wrote: > > > Provide two new helper macros to iterate over PCI device resources and > > > convert users. > > > > > > Looking at it, refactor existing pci_bus_for_each_resource() and convert > > > users accordingly. > > Applied 2-7 to pci/resource for v6.4, thanks, I really like this! > > Btw, can you actually drop patch 7, please? Done. > > I omitted > > > > [1/7] kernel.h: Split out COUNT_ARGS() and CONCATENATE()" > > > > only because it's not essential to this series and has only a trivial > > one-line impact on include/linux/pci.h. > > I'm not sure I understood what exactly "essentiality" means to you, but > I included that because it makes the split which can be used later by > others and not including kernel.h in the header is the objective I want > to achieve. Without this patch the achievement is going to be deferred. > Yet, this, as you have noticed, allows to compile and use the macros in > the rest of the patches. I haven't followed the kernel.h splitting, and I try to avoid incidental changes outside of the files I maintain, so I just wanted to keep this series purely PCI and avoid any possible objections to a new include file or discussion about how it should be done. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users 2023-04-05 20:18 ` Bjorn Helgaas @ 2023-04-06 10:31 ` Andy Shevchenko 0 siblings, 0 replies; 11+ messages in thread From: Andy Shevchenko @ 2023-04-06 10:31 UTC (permalink / raw) To: Bjorn Helgaas Cc: Mickaël Salaün, Krzysztof Wilczyński, Mika Westerberg, Michael Ellerman, Randy Dunlap, Arnd Bergmann, Philippe Mathieu-Daudé, Niklas Schnelle, Rafael J. Wysocki, Pali Rohár, Maciej W. Rozycki, Juergen Gross, Dominik Brodowski, linux-kernel, linux-alpha, linux-arm-kernel, linux-mips, linuxppc-dev, linux-sh, sparclinux, linux-pci, xen-devel, linux-acpi, 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 On Wed, Apr 05, 2023 at 03:18:32PM -0500, Bjorn Helgaas wrote: > On Wed, Apr 05, 2023 at 11:28:27AM +0300, Andy Shevchenko wrote: > > On Tue, Apr 04, 2023 at 11:11:01AM -0500, Bjorn Helgaas wrote: > > > On Thu, Mar 30, 2023 at 07:24:27PM +0300, Andy Shevchenko wrote: ... > > > I omitted > > > > > > [1/7] kernel.h: Split out COUNT_ARGS() and CONCATENATE()" > > > > > > only because it's not essential to this series and has only a trivial > > > one-line impact on include/linux/pci.h. > > > > I'm not sure I understood what exactly "essentiality" means to you, but > > I included that because it makes the split which can be used later by > > others and not including kernel.h in the header is the objective I want > > to achieve. Without this patch the achievement is going to be deferred. > > Yet, this, as you have noticed, allows to compile and use the macros in > > the rest of the patches. > > I haven't followed the kernel.h splitting, and I try to avoid > incidental changes outside of the files I maintain, so I just wanted > to keep this series purely PCI and avoid any possible objections to a > new include file or discussion about how it should be done. Okay, fair enough :-) Thank you for elaboration, I will send the new version of patch 7 separately. -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-06-05 14:08 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <ZF6YIezraETr9iNM@bhelgaas>
2023-05-30 21:24 ` [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users Bjorn Helgaas
[not found] ` <CAOiHx==5YWhDiZP2PyHZiJrmtqRzvqCqoSO59RwuYuR85BezBg@mail.gmail.com>
2023-05-31 21:30 ` Bjorn Helgaas
2023-06-01 11:17 ` Jonas Gorski
2023-06-05 14:04 ` Andy Shevchenko
2023-06-01 16:25 ` Andy Shevchenko
2023-06-01 16:27 ` Andy Shevchenko
2023-03-30 16:24 Andy Shevchenko
2023-04-04 16:11 ` Bjorn Helgaas
2023-04-05 8:28 ` Andy Shevchenko
2023-04-05 20:18 ` Bjorn Helgaas
2023-04-06 10:31 ` Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).