| Message ID | 20160209120032.GB19840@leverpostej |
|---|---|
| State | New |
| Headers | show |
On Tue, Feb 9, 2016 at 1:00 PM, Mark Rutland <mark.rutland@arm.com> wrote: > On Tue, Feb 09, 2016 at 01:04:28PM +0530, Sudip Mukherjee wrote: >> On Tue, Feb 09, 2016 at 04:41:04PM +1100, Stephen Rothwell wrote: >> > Changes since 20160208: >> >> tilepro, tilegx, mips defconfig build fails with the error: >> ../include/asm-generic/fixmap.h: In function '__set_fixmap_offset': >> ../include/asm-generic/fixmap.h:77:2: error: implicit declaration of >> function '__set_fixmap' [-Werror=implicit-function-declaration] >> >> caused by: >> commit ac4c0ac73485 ("asm-generic: make __set_fixmap_offset a static inline") >> >> Reverting the commit fixes the issue. > > Sorry about this. > > Is seems any arch without its own __set_fixmap may be adversely > affected. > > I can't easily stub __set_fixmap as it's not implemented as a macro. But you can add a forward declaration? BTW, it seems the following drivers rely on <asm/fixmap.h>, which is not available on all architectures: drivers/firewire/init_ohci1394_dma.c:#include <asm/fixmap.h> drivers/tty/serial/earlycon.c:#include <asm/fixmap.h> drivers/usb/early/ehci-dbgp.c:#include <asm/fixmap.h> > I think we can stick with a macro and remove 'addr', by returning the > result of the expression directly. As fix_to_virt gave us an unsigned > long I think the types should line up (i.e. the result will be at least > unsigned long wide). > > Arnd, would you be happy with the below patch instead? > --- a/include/asm-generic/fixmap.h > +++ b/include/asm-generic/fixmap.h > @@ -72,10 +72,8 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr) > /* Return a pointer with offset calculated */ > #define __set_fixmap_offset(idx, phys, flags) \ > ({ \ > - unsigned long addr; \ > __set_fixmap(idx, phys, flags); \ Missing parentheses: __set_fixmap((idx), (phys), (flags)); > - addr = fix_to_virt(idx) + ((phys) & (PAGE_SIZE - 1)); \ > - addr; \ > + fix_to_virt(idx) + ((phys) & (PAGE_SIZE - 1)); \ > }) "idx" and "phys" are evaluated multiple times, which can lead to subtle bugs. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Feb 09, 2016 at 02:48:26PM +0100, Geert Uytterhoeven wrote: > On Tue, Feb 9, 2016 at 1:00 PM, Mark Rutland <mark.rutland@arm.com> wrote: > > On Tue, Feb 09, 2016 at 01:04:28PM +0530, Sudip Mukherjee wrote: > >> On Tue, Feb 09, 2016 at 04:41:04PM +1100, Stephen Rothwell wrote: > >> > Changes since 20160208: > >> > >> tilepro, tilegx, mips defconfig build fails with the error: > >> ../include/asm-generic/fixmap.h: In function '__set_fixmap_offset': > >> ../include/asm-generic/fixmap.h:77:2: error: implicit declaration of > >> function '__set_fixmap' [-Werror=implicit-function-declaration] > >> > >> caused by: > >> commit ac4c0ac73485 ("asm-generic: make __set_fixmap_offset a static inline") > >> > >> Reverting the commit fixes the issue. > > > > Sorry about this. > > > > Is seems any arch without its own __set_fixmap may be adversely > > affected. > > > > I can't easily stub __set_fixmap as it's not implemented as a macro. > > But you can add a forward declaration? Good point. That seems to work (tested on arm64, mips, tilegx). Arnd, Catalin, happy with the bloew fixup to the original patch? Mark. ---->8---- From e53a0fa34689cca13846475fb5a7cb4c7da87384 Mon Sep 17 00:00:00 2001 From: Mark Rutland <mark.rutland@arm.com> Date: Tue, 9 Feb 2016 14:27:36 +0000 Subject: [PATCH] asm-generic: Fix build when __set_fixmap is absent Commit ac4c0ac73485 ("asm-generic: make __set_fixmap_offset a static inline") relied on the architecture code to define a __set_fixmap function, even if unused. This broke the build for architectures which do not implement __set_fixmap: ../include/asm-generic/fixmap.h:Infunction'__set_fixmap_offset': ../include/asm-generic/fixmap.h:77:2:error:implicitdeclarationof function'__set_fixmap'[-Werror=implicit-function-declaration] As we only require the prototype to be present, add this to asm-generic/fixmap.h, preventing build failures on these architectures. Signed-off-by: Mark Rutland <mark.rutland@arm.com> Reported-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com> Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Jeremy Linton <jeremy.linton@arm.com> Cc: Laura Abbott <labbott@fedoraproject.org> --- include/asm-generic/fixmap.h | 2 ++ 1 file changed, 2 insertions(+) -- 1.9.1diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h index f9c27b6..e5255ff 100644 --- a/include/asm-generic/fixmap.h +++ b/include/asm-generic/fixmap.h @@ -69,6 +69,8 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr) __set_fixmap(idx, 0, FIXMAP_PAGE_CLEAR) #endif +void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot); + /* Return a pointer with offset calculated */ static inline unsigned long __set_fixmap_offset(enum fixed_addresses idx, phys_addr_t phys,
diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h index 1cbb833..ae2cc43 100644 --- a/include/asm-generic/fixmap.h +++ b/include/asm-generic/fixmap.h @@ -72,10 +72,8 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr) /* Return a pointer with offset calculated */ #define __set_fixmap_offset(idx, phys, flags) \ ({ \ - unsigned long addr; \ __set_fixmap(idx, phys, flags); \ - addr = fix_to_virt(idx) + ((phys) & (PAGE_SIZE - 1)); \ - addr; \ + fix_to_virt(idx) + ((phys) & (PAGE_SIZE - 1)); \ }) #define set_fixmap_offset(idx, phys) \