Regather scattered PCI-Code
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
- v6 Latest4 patches 7 repliesApplied
- v5 posting 4/45 patches 16 repliesSuperseded
- v5 posting 3/45 patches 0 repliesSuperseded
Earlier versions · 6
| Version | Posted | Patches | Replies | Status |
|---|---|---|---|---|
| v5 posting 2/4 | 1 of 5 patches | 0 replies | Superseded | |
| v5 posting 1/4 | 5 patches | 0 replies | Superseded | |
| v4 | 5 patches | 0 replies | Superseded | |
| v3 | 5 patches | 9 replies | Superseded | |
| v2 | 4 patches | 14 replies | Superseded | |
| v1 | 4 patches | 18 replies | Superseded |
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 files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/4 | lib/pci_iomap.c: fix cleanup bug in pci_iounmap()lib/ | +1 | 2 replies | |
| 2/4 | lib: move pci_iomap.c to drivers/pci/Documentation/ Documentation/ MAINTAINERS +5 more | +10 | 0 replies | |
| 3/4 | lib: move pci-specific devres code to drivers/pci/Documentation/ drivers/ drivers/ +1 more | +212 | 0 replies | |
| 4/4 | PCI: Move devres code from pci.c to devres.cdrivers/ drivers/ drivers/ | +267 | 2 replies |
Reviews trailers per patch
2 of 4 patches have a Reviewed-by or Acked-by.
| Patch | Arnd Bergmann |
|---|---|
| 1/4 | R-b |
| 2/4 | R-b |
| 3/4 | |
| 4/4 |
Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.
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