Re: linux-next: Tree for Feb 9

Geert Uytterhoeven
2016-02-09T13:48:31Z

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