* Re: Odd pci_iounmap() declaration rules.. [not found] ` <CAHk-=wgKc5TY-LiAjog5VKNUQ84CSZyPu+FQekMHDar=kdSW=Q@mail.gmail.com> @ 2021-09-19 21:28 ` Nathan Chancellor 2021-09-19 22:27 ` Linus Torvalds 0 siblings, 1 reply; 6+ messages in thread From: Nathan Chancellor @ 2021-09-19 21:28 UTC (permalink / raw) To: Linus Torvalds Cc: Helge Deller, Arnd Bergmann, Guenter Roeck, Ulrich Teichert, linux-arch, James Bottomley, llvm On Sun, Sep 19, 2021 at 11:05:35AM -0700, Linus Torvalds wrote: > On Sun, Sep 19, 2021 at 10:04 AM Helge Deller <deller@gmx.de> wrote: > > > > Can you test if it fixes your alpha build (with GENERIC_PCI_IOMAP=y) as > > well? > > Yup. With this I can do that "enable GENERIC_PCI_IOMAP > unconditionally" thing on alpha, and the one off EISA/PCI driver now > builds cleanly without PCI. > > I applied it directly (along with the alpha patch to GENERIC_PCI_IOMAP). > > I have now looked at a number of drivers and architectures that I had > happily forgotten _all_ about long long ago. > > It's been kind of fun, but I sure can't claim it has been really _productive_. Commit 9caea0007601 ("parisc: Declare pci_iounmap() parisc version only when CONFIG_PCI enabled") causes the following build error on arm64 with Fedora's config, which CKI initially reported: https://src.fedoraproject.org/rpms/kernel/raw/rawhide/f/kernel-aarch64-fedora.config https://lore.kernel.org/r/cki.E3FB2299E5.UQ0I0LMEXJ@redhat.com/ https://arr-cki-prod-datawarehouse-public.s3.amazonaws.com/datawarehouse-public/2021/09/19/373372721/build_aarch64_redhat%3A1603258426/build.log In file included from ./arch/arm64/include/asm/io.h:185, from ./include/linux/io.h:13, from ./include/acpi/acpi_io.h:5, from ./include/linux/acpi.h:35, from ./include/acpi/apei.h:9, from ./include/acpi/ghes.h:5, from ./include/linux/arm_sdei.h:8, from arch/arm64/kernel/asm-offsets.c:10: ./include/asm-generic/io.h:1059:21: error: static declaration of 'pci_iounmap' follows non-static declaration 1059 | #define pci_iounmap pci_iounmap | ^~~~~~~~~~~ ./include/asm-generic/io.h:1060:20: note: in expansion of macro 'pci_iounmap' 1060 | static inline void pci_iounmap(struct pci_dev *dev, void __iomem *p) | ^~~~~~~~~~~ In file included from ./include/asm-generic/io.h:19, from ./arch/arm64/include/asm/io.h:185, from ./include/linux/io.h:13, from ./include/acpi/acpi_io.h:5, from ./include/linux/acpi.h:35, from ./include/acpi/apei.h:9, from ./include/acpi/ghes.h:5, from ./include/linux/arm_sdei.h:8, from arch/arm64/kernel/asm-offsets.c:10: ./include/asm-generic/pci_iomap.h:21:13: note: previous declaration of 'pci_iounmap' with type 'void(struct pci_dev *, void *)' 21 | extern void pci_iounmap(struct pci_dev *dev, void __iomem *); | ^~~~~~~~~~~ make[2]: *** [scripts/Makefile.build:121: arch/arm64/kernel/asm-offsets.s] Error 1 make[2]: Target '__build' not remade because of errors. make[1]: *** [Makefile:1219: prepare0] Error 2 make[1]: Target 'all' not remade because of errors. make: *** [Makefile:350: __build_one_by_one] Error 2 make: Target 'olddefconfig' not remade because of errors. make: Target 'all' not remade because of errors. Sorry, I do not have time at the moment to look at it (family Sunday and whatnot) but I wanted to be sure you were aware of it. Cheers, Nathan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Odd pci_iounmap() declaration rules.. 2021-09-19 21:28 ` Odd pci_iounmap() declaration rules Nathan Chancellor @ 2021-09-19 22:27 ` Linus Torvalds 2021-09-19 22:44 ` Linus Torvalds 0 siblings, 1 reply; 6+ messages in thread From: Linus Torvalds @ 2021-09-19 22:27 UTC (permalink / raw) To: Nathan Chancellor Cc: Helge Deller, Arnd Bergmann, Guenter Roeck, Ulrich Teichert, linux-arch, James Bottomley, llvm On Sun, Sep 19, 2021 at 2:28 PM Nathan Chancellor <nathan@kernel.org> wrote: > > Commit 9caea0007601 ("parisc: Declare pci_iounmap() parisc version only > when CONFIG_PCI enabled") causes the following build error on arm64 with > Fedora's config, which CKI initially reported: Ok, so I spent a lot of time trying to figure out what the heck is going on. And while this is really *REALLY* confusing code, and nobody should use it, I think the fix is this oneliner: diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h index e93375c710b9..692e964e56b4 100644 --- a/include/asm-generic/io.h +++ b/include/asm-generic/io.h @@ -1047,7 +1047,7 @@ extern void ioport_unmap(void __iomem *p); #endif /* CONFIG_GENERIC_IOMAP */ #endif /* CONFIG_HAS_IOPORT_MAP */ -#ifndef CONFIG_GENERIC_IOMAP +#ifndef CONFIG_GENERIC_PCI_IOMAP struct pci_dev; extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max); let me go test, I do have an arm64 build environment for this all, but for that commit I had only done parisc, alpha and x86-64. Linus ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Odd pci_iounmap() declaration rules.. 2021-09-19 22:27 ` Linus Torvalds @ 2021-09-19 22:44 ` Linus Torvalds 2021-09-20 0:44 ` Linus Torvalds 0 siblings, 1 reply; 6+ messages in thread From: Linus Torvalds @ 2021-09-19 22:44 UTC (permalink / raw) To: Nathan Chancellor Cc: Helge Deller, Arnd Bergmann, Guenter Roeck, Ulrich Teichert, linux-arch, James Bottomley, llvm On Sun, Sep 19, 2021 at 3:27 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > --- a/include/asm-generic/io.h > +++ b/include/asm-generic/io.h > @@ -1047,7 +1047,7 @@ extern void ioport_unmap(void __iomem *p); > -#ifndef CONFIG_GENERIC_IOMAP > +#ifndef CONFIG_GENERIC_PCI_IOMAP > > let me go test, I do have an arm64 build environment for this all, but > for that commit I had only done parisc, alpha and x86-64. Hah. That conditional makes a lot more sense that way, but doing that sensible thing exposes more oddities. In particular, both arm and arm64 do select GENERIC_PCI_IOMAP so they get the bog-standard pci_iomap() from lib/pci_iomap.c. All good and sensible. But is there a pci_iounmap() in lib/pci_iomap.c? No. The default pci_iounmap() is in lib/iomap.c, which arm and arm64 do *not* use, because they don't have GENERIC_IOMAP. So those architectures depended on the header file copy, and with that sane oneliner, the compile part of the build works fine, but then it ends with undefined reference to `pci_iounmap' because that didn't work out at all. The fix seems to be to just move that odd code from the header file to lib/pci_iomap.c, and that should make it all JustWork(tm). That would at least make some of this make more sense. But this is all a maze of random odd architecture things, so I'll have to look some more at it. Linus ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Odd pci_iounmap() declaration rules.. 2021-09-19 22:44 ` Linus Torvalds @ 2021-09-20 0:44 ` Linus Torvalds 2021-09-20 14:30 ` Nathan Chancellor 0 siblings, 1 reply; 6+ messages in thread From: Linus Torvalds @ 2021-09-20 0:44 UTC (permalink / raw) To: Nathan Chancellor Cc: Helge Deller, Arnd Bergmann, Guenter Roeck, Ulrich Teichert, linux-arch, James Bottomley, llvm On Sun, Sep 19, 2021 at 3:44 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > The fix seems to be to just move that odd code from the header file to > lib/pci_iomap.c, and that should make it all JustWork(tm). I'm not 100% happy about the end result, and in particular I think that the new generic pci_iounmap() function for the ARCH_WANTS_GENERIC_PCI_IOUNMAP case should do the "iounmap(p)" thing even if ARCH_HAS_GENERIC_IOPORT_MAP wasn't true, but I tried to keep the old rules, even if they seemed broken. arm and arm64 build for me, as did sparc64 and alpha. At least in the configs I tested. And the code _does_ make a bit more sense than it used to. It still has crazy corners, but moving the pci_iounmap() code out of the header file should make it easier to fix up in the future. Linus ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Odd pci_iounmap() declaration rules.. 2021-09-20 0:44 ` Linus Torvalds @ 2021-09-20 14:30 ` Nathan Chancellor 2021-09-20 15:31 ` Guenter Roeck 0 siblings, 1 reply; 6+ messages in thread From: Nathan Chancellor @ 2021-09-20 14:30 UTC (permalink / raw) To: Linus Torvalds Cc: Helge Deller, Arnd Bergmann, Guenter Roeck, Ulrich Teichert, linux-arch, James Bottomley, llvm On Sun, Sep 19, 2021 at 05:44:03PM -0700, Linus Torvalds wrote: > On Sun, Sep 19, 2021 at 3:44 PM Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > The fix seems to be to just move that odd code from the header file to > > lib/pci_iomap.c, and that should make it all JustWork(tm). > > I'm not 100% happy about the end result, and in particular I think > that the new generic pci_iounmap() function for the > ARCH_WANTS_GENERIC_PCI_IOUNMAP case should do the "iounmap(p)" thing > even if ARCH_HAS_GENERIC_IOPORT_MAP wasn't true, but I tried to keep > the old rules, even if they seemed broken. > > arm and arm64 build for me, as did sparc64 and alpha. At least in the > configs I tested. > > And the code _does_ make a bit more sense than it used to. It still > has crazy corners, but moving the pci_iounmap() code out of the header > file should make it easier to fix up in the future. Thanks, I can confirm that all of my build tests pass without any issues now. Cheers, Nathan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Odd pci_iounmap() declaration rules.. 2021-09-20 14:30 ` Nathan Chancellor @ 2021-09-20 15:31 ` Guenter Roeck 0 siblings, 0 replies; 6+ messages in thread From: Guenter Roeck @ 2021-09-20 15:31 UTC (permalink / raw) To: Nathan Chancellor Cc: Linus Torvalds, Helge Deller, Arnd Bergmann, Ulrich Teichert, linux-arch, James Bottomley, llvm On Mon, Sep 20, 2021 at 07:30:15AM -0700, Nathan Chancellor wrote: > On Sun, Sep 19, 2021 at 05:44:03PM -0700, Linus Torvalds wrote: > > On Sun, Sep 19, 2021 at 3:44 PM Linus Torvalds > > <torvalds@linux-foundation.org> wrote: > > > > > > The fix seems to be to just move that odd code from the header file to > > > lib/pci_iomap.c, and that should make it all JustWork(tm). > > > > I'm not 100% happy about the end result, and in particular I think > > that the new generic pci_iounmap() function for the > > ARCH_WANTS_GENERIC_PCI_IOUNMAP case should do the "iounmap(p)" thing > > even if ARCH_HAS_GENERIC_IOPORT_MAP wasn't true, but I tried to keep > > the old rules, even if they seemed broken. > > > > arm and arm64 build for me, as did sparc64 and alpha. At least in the > > configs I tested. > > > > And the code _does_ make a bit more sense than it used to. It still > > has crazy corners, but moving the pci_iounmap() code out of the header > > file should make it easier to fix up in the future. > > Thanks, I can confirm that all of my build tests pass without any > issues now. > I still see build failures for sparc64:allnoconfig and sparc64:tinyconfig. Guenter ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-20 15:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CAHk-=wjRrh98pZoQ+AzfWmsTZacWxTJKXZ9eKU2X_0+jM=O8nw@mail.gmail.com>
[not found] ` <YUdti08rLzfDZy8S@ls3530>
[not found] ` <CAHk-=wgKc5TY-LiAjog5VKNUQ84CSZyPu+FQekMHDar=kdSW=Q@mail.gmail.com>
2021-09-19 21:28 ` Odd pci_iounmap() declaration rules Nathan Chancellor
2021-09-19 22:27 ` Linus Torvalds
2021-09-19 22:44 ` Linus Torvalds
2021-09-20 0:44 ` Linus Torvalds
2021-09-20 14:30 ` Nathan Chancellor
2021-09-20 15:31 ` Guenter Roeck
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).