Skip to content

Regather scattered PCI-Code

Philipp Stanner linux-arch linux-pci lkml oe-kbuild-all stable 9 versions – Applied

Status from this message: Applied to pci/devres for v6.9, thanks!

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v6 Latest4 patches 7 repliesApplied
  2. v5 posting 4/45 patches 16 repliesSuperseded
  3. v5 posting 3/45 patches 0 repliesSuperseded
Earlier versions · 6
VersionPostedPatchesRepliesStatus
v5 posting 2/41 of 5 patches0 repliesSuperseded
v5 posting 1/45 patches0 repliesSuperseded
v45 patches0 repliesSuperseded
v35 patches9 repliesSuperseded
v24 patches14 repliesSuperseded
v14 patches18 repliesSuperseded

Changes in v6 from the cover letter

- Remove the addition of ioport_unmap() from patch #1, since this is not really a bug, as explained by the comment above pci_iounmap. (Bjorn)
- Drop the patch unifying the two versions of pci_iounmap(). (Bjorn)
- Make patch #4's style congruent with PCI style.
- Drop (in any case empty) ioport_unmap() again from pci_iounmap()
- Add forgotten updates to Documentation/ when moving files from lib/ to drivers/pci/

Patches in v6 Applied

#Subject and filesSizeRepliesApplied
1/4lib/pci_iomap.c: fix cleanup bug in pci_iounmap()lib/pci_iomap.c+1 -12 replies
2/4lib: move pci_iomap.c to drivers/pci/Documentation/driver-api/device-io.rst Documentation/driver-api/pci/pci.rst MAINTAINERS +5 more+10 -90 replies
3/4lib: move pci-specific devres code to drivers/pci/Documentation/driver-api/pci/pci.rst drivers/pci/Makefile drivers/pci/devres.c +1 more+212 -2080 replies
4/4PCI: Move devres code from pci.c to devres.cdrivers/pci/devres.c drivers/pci/pci.c drivers/pci/pci.h+267 -2492 replies

Reviews trailers per patch

2 of 4 patches have a Reviewed-by or Acked-by.

PatchArnd Bergmann
1/4R-b
2/4R-b
3/4
4/4

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v6 thread

Cover letter [PATCH v6 0/4] Regather scattered PCI-Code

@Stable-Kernel:
You receive this patch series because its first patch fixes a leak in PCI.

@Bjorn:
I decided that it's now actually possible to just embed the docu updates to the respective patches, instead of a separate patch.
Also dropped the ioport_unmap() for now.

Changes in v6:
- Remove the addition of ioport_unmap() from patch #1, since this is not really a bug, as explained by the comment above pci_iounmap. (Bjorn)
- Drop the patch unifying the two versions of pci_iounmap(). (Bjorn)
- Make patch #4's style congruent with PCI style.
- Drop (in any case empty) ioport_unmap() again from pci_iounmap()
- Add forgotten updates to Documentation/ when moving files from lib/ to drivers/pci/

Changes in v5:
- Add forgotten update to MAINTAINERS file.

Changes in v4:
- Apply Arnd's Reviewed-by's
- Add ifdef CONFIG_HAS_IOPORT_MAP guard in drivers/pci/iomap.c (build error on openrisc)
- Fix typo in patch no.5

Changes in v3:
- Create a separate patch for the leaks in lib/iomap.c. Make it the series' first patch. (Arnd)
- Turns out the aforementioned bug wasn't just accidentally removing iounmap() with the ifdef, it was also missing ioport_unmap() to begin with. Add it.
- Move the ARCH_WANTS_GENERIC_IOMEM_IS_IOPORT-mechanism from asm-generic/io.h to asm-generic/ioport.h. (Arnd)
- Adjust the implementation of iomem_is_ioport() in asm-generic/io.h so that it matches exactly what pci_iounmap() previously did in lib/pci_iomap.c. (Arnd)
- Move the CONFIG_HAS_IOPORT guard in asm-generic/io.h so that iomem_is_ioport() will always be compiled and just returns false if there are no ports.
- Add TODOs to several places informing about the generic iomem_is_ioport() in lib/iomap.c not being generic.
- Add TODO about the followup work to make drivers/pci/iomap.c's pci_iounmap() actually generic.

Changes in v2:
- Replace patch 4, previously extending the comment about pci_iounmap() in lib/iomap.c, with a patch that moves pci_iounmap() from that file to drivers/pci/iomap.c, creating a unified version there. (Arnd)
- Implement iomem_is_ioport() as a new helper in asm-generic/io.h and lib/iomap.c. (Arnd)
- Move the build rule in drivers/pci/Makefile for iomap.o under the guard of #if PCI. This had to be done because when just checking for GENERIC_PCI_IOMAP being defined, the functions don't disappear, which was the case previously in lib/pci_iomap.c, where the entire file was made empty if PCI was not set by the guard #ifdef PCI. (Intel's Bots)
- Rephares all patches' commit messages a little bit.

Sooooooooo. I reworked v1.

Please review this carefully, the IO-Ranges are obviously a bit tricky, as is the build-system / ifdef-ery.

Arnd has suggested that architectures defining a custom inb() need their own iomem_is_ioport(), as well. I've grepped for inb() and found the following list of archs that define their own:
- alpha
- arm
- m68k <--
- parisc
- powerpc
- sh
- sparc
- x86 <--

All of those have their own definitons of pci_iounmap(). Therefore, they don't need our generic version in the first place and, thus, also need no iomem_is_ioport().
The two exceptions are x86 and m68k. The former uses lib/iomap.c through CONFIG_GENERIC_IOMAP, as Arnd pointed out in the previous discussion (thus, CONFIG_GENERIC_IOMAP is not really generic in this regard).

So as I see it, only m68k WOULD need its own custom definition of iomem_is_ioport(). But as I understand it it doesn't because it uses the one from asm-generic/pci_iomap.h ??

I wasn't entirely sure how to deal with the address ranges for the generic implementation in asm-generic/io.h. It's marked with a TODO. Input appreciated.

I removed the guard around define pci_iounmap in asm-generic/io.h. An alternative would be to have it be guarded by CONFIG_GENERIC_IOMAP and CONFIG_GENERIC_PCI_IOMAP, both. Without such a guard, there is no collision however, because generic pci_iounmap() from drivers/pci/iomap.c will only get pulled in when CONFIG_GENERIC_PCI_IOMAP is actually set.

I cross-built this for a variety of architectures, including the usual suspects (s390, m68k). So far successfully. But let's see what Intel's robots say :O

P.

Original cover letter:

Hi!

So it seems that since ca. 2007 the PCI code has been scattered a bit. PCI's devres code, which is only ever used by users of the entire PCI-subsystem anyways, resides in lib/devres.c and is guarded by an ifdef PCI, just as the content of lib/pci_iomap.c is.

It, thus, seems reasonable to move all of that.

As I were at it, I moved as much of the devres-specific code from pci.c to devres.c, too. The only exceptions are four functions that are currently difficult to move. More information about that can be read here [1].

I noticed these scattered files while working on (new) PCI-specific devres functions. If we can get this here merged, I'll soon send another patch series that addresses some API-inconsistencies and could move the devres-part of the four remaining functions.

I don't want to do that in this series as this here is only about moving code, whereas the next series would have to actually change API behavior.

I successfully (cross-)built this for x86, x86_64, AARCH64 and ARM (allyesconfig). I booted a kernel with it on x86_64, with a Fedora desktop environment as payload. The OS came up fine

I hope this is OK. If we can get it in, we'd soon have a very consistent PCI API again.

Regards,
P.

Philipp Stanner (4):
lib/pci_iomap.c: fix cleanup bug in pci_iounmap()
lib: move pci_iomap.c to drivers/pci/
lib: move pci-specific devres code to drivers/pci/
PCI: Move devres code from pci.c to devres.c

 Documentation/driver-api/device-io.rst |   2 +-
 Documentation/driver-api/pci/pci.rst   |   6 +
 MAINTAINERS                            |   1 -
 drivers/pci/Kconfig                    |   5 +
 drivers/pci/Makefile                   |   3 +-
 drivers/pci/devres.c                   | 450 +++++++++++++++++++++++++
 lib/pci_iomap.c => drivers/pci/iomap.c |   5 +-
 drivers/pci/pci.c                      | 249 --------------
 drivers/pci/pci.h                      |  24 ++
 lib/Kconfig                            |   3 -
 lib/Makefile                           |   1 -
 lib/devres.c                           | 208 +-----------
 12 files changed, 490 insertions(+), 467 deletions(-)
 create mode 100644 drivers/pci/devres.c
 rename lib/pci_iomap.c => drivers/pci/iomap.c (99%)
Signature · 1 line
2.43.0