Re: [PATCH v2 02/22] asm-generic/io.h: add ioremap_nopost remap interface

[Russell King - ARM Linux]

On Thu, Apr 06, 2017 at 05:21:56PM +0100, Lorenzo Pieralisi wrote:
> Ok, so:
> 
> (1) I can do asm-generic/ioremap-nopost.h, which I assume you want to
>     contain something like
> 
> static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
> {
> 	return ioremap_nocache(offset, size);
> }
> 
> Funny bit is that it has to be included by asm*/io.h files _after_
> ioremap_nocache has been #defined (that's the reason my approach was
> failing miserably even on arches like eg powerpc (see [1] below) that
> does have ioremap_nocache),

PowerPC does have ioremap_nocache() though:

/**
 * ioremap     -   map bus memory into CPU space
...
 * * ioremap_nocache is identical to ioremap
extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
#define ioremap_nocache(addr, size)     ioremap((addr), (size))

and this include file is included very early on in linux/io.h.  I don't
see anything that conditionalises it on anything except __KERNEL__.  So,
the report from 0-day really doesn't make any sense to me.

Do we know how we're ending up in linux/io.h line 169 without having
picked up the ioremap_nocache() definition provided by PowerPC's
asm/io.h ?

> not sure it is going to be very nice to have
> an include in the middle of asm*/io.h include files (and I have to patch
> all arches which I can do).

You mean like we already have to do with this asm-generic/io.h thing in
the ARM io.h header file, because we need to define all the accessors
first, to prevent the asm-generic/io.h thing defining them for us?
Given how asm-generic has headed in this regard, having include files
at all sorts of strange locations within the architecture asm/*.h
header files has become quite normal, unfortunately.

> (2) I add:
> 
> #ifndef ioremap_nopost
> static inline void __iomem *ioremap_nopost(phys_addr_t offset, size_t size)
> {
> 	return NULL; <-(making it return ioremap_nocache() does not
> 			work, see [1] for the reason)
> }
> #endif
> 
> in linux/io.h

... which breaks the kernel if ioremap_nopost is missing from the arch
header, and one of the drivers that you're modifying to use this new
ioremap variant happens to be built and used on such an architecture.

> (3) ioremap_nopost follows Luis' ioremap_uc approach

Same problem as (2), as I understand correctly.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.