Re: [PATCH] arch: m32r: include: asm: add ioread*_rep() and iowrite*_rep()

[Chen Gang <gang.chen@asianux.com>]

On 06/27/2013 04:25 PM, Arnd Bergmann wrote:
> On Thursday 27 June 2013 09:07:29 Geert Uytterhoeven wrote:
>> > On Thu, Jun 27, 2013 at 6:37 AM, Chen Gang <gang.chen@asianux.com> wrote:
>>> > > --- a/arch/m32r/include/asm/io.h
>>> > > +++ b/arch/m32r/include/asm/io.h
>>> > > @@ -169,6 +169,20 @@ static inline void _writel(unsigned long l, unsigned long addr)
>>> > >  #define iowrite16 writew
>>> > >  #define iowrite32 writel
>>> > >
>>> > > +#define ioread8_rep(p, dst, count) \
>>> > > +       insb((unsigned long) (p), (dst), (count))
>> > 
>> > As ioread8() is mapped to readb() (I/O memory space), not inb() (I/O
>> > port space),
>> > ioread8_rep() should map to readsb() (which m32r doesn't have yet
>> > BTW), not insb().
>> > For m32r this does matter, as inb() and readb() use different mechanisms
>> > internally.
>> > 
>> > It seems include/asm-generic/io.h also has this wrong?
> Yes, you are right. I thought we had fixed that long ago.
> 
> Does the patch below look ok to you? Note that none of the architectures
> using asm-generic/io.h supports PIO access and it works by chance
> this way, but we should definitely fix it.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 

Is it better to let the code block (from "#ifndef CONFIG_GENERIC_IOMAP
to #endif) in an individual file (e.g. iomap_inline.h)?

So that many architectures (m32r, tile, ...) can include the individual
file to get these necessary code, and not need consider the confilict
about the other code in generic io.h ?

Thanks.

> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index d5afe96..8af7a64 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -161,108 +161,108 @@ static inline void outl(u32 b, unsigned long addr)
>  #define outw_p(x, addr)	outw((x), (addr))
>  #define outl_p(x, addr)	outl((x), (addr))
>  
> -#ifndef insb
> -static inline void insb(unsigned long addr, void *buffer, int count)
> +#ifndef CONFIG_GENERIC_IOMAP
> +#define ioread8(addr)		readb(addr)
> +#define ioread16(addr)		readw(addr)
> +#define ioread16be(addr)	__be16_to_cpu(__raw_readw(addr))
> +#define ioread32(addr)		readl(addr)
> +#define ioread32be(addr)	__be32_to_cpu(__raw_readl(addr))
> +
> +#define iowrite8(v, addr)	writeb((v), (addr))
> +#define iowrite16(v, addr)	writew((v), (addr))
> +#define iowrite16be(v, addr)	__raw_writew(__cpu_to_be16(v), addr)
> +#define iowrite32(v, addr)	writel((v), (addr))
> +#define iowrite32be(v, addr)	__raw_writel(__cpu_to_be32(v), addr)
> +
> +static inline void ioread8_rep(void __iomem *addr, void *buffer, int count)
>  {
>  	if (count) {
>  		u8 *buf = buffer;
>  		do {
> -			u8 x = __raw_readb(addr + PCI_IOBASE);
> +			u8 x = __raw_readb(addr);
>  			*buf++ = x;
>  		} while (--count);
>  	}
>  }
> -#endif
>  
> -#ifndef insw
> -static inline void insw(unsigned long addr, void *buffer, int count)
> +static inline void ioread16_rep(void __iomem *addr, void *buffer, int count)
>  {
>  	if (count) {
>  		u16 *buf = buffer;
>  		do {
> -			u16 x = __raw_readw(addr + PCI_IOBASE);
> +			u16 x = __raw_readw(addr);
>  			*buf++ = x;
>  		} while (--count);
>  	}
>  }
> -#endif
>  
> -#ifndef insl
> -static inline void insl(unsigned long addr, void *buffer, int count)
> +static inline void ioread32_rep(void __iomem *addr, void *buffer, int count)
>  {
>  	if (count) {
>  		u32 *buf = buffer;
>  		do {
> -			u32 x = __raw_readl(addr + PCI_IOBASE);
> +			u32 x = __raw_readl(addr);
>  			*buf++ = x;
>  		} while (--count);
>  	}
>  }
> -#endif
>  
> -#ifndef outsb
> -static inline void outsb(unsigned long addr, const void *buffer, int count)
> +static inline void iowrite8_rep(void __iomem *addr, const void *buffer, int count)
>  {
>  	if (count) {
>  		const u8 *buf = buffer;
>  		do {
> -			__raw_writeb(*buf++, addr + PCI_IOBASE);
> +			__raw_writeb(*buf++, addr);
>  		} while (--count);
>  	}
>  }
> -#endif
>  
> -#ifndef outsw
> -static inline void outsw(unsigned long addr, const void *buffer, int count)
> +static inline void iowrite16_rep(void __iomem *addr, const void *buffer, int count)
>  {
>  	if (count) {
>  		const u16 *buf = buffer;
>  		do {
> -			__raw_writew(*buf++, addr + PCI_IOBASE);
> +			__raw_writew(*buf++, addr);
>  		} while (--count);
>  	}
>  }
> -#endif
>  
> -#ifndef outsl
> -static inline void outsl(unsigned long addr, const void *buffer, int count)
> +static inline void iowrite32_rep(void __iomem *addr, const void *buffer, int count)
>  {
>  	if (count) {
>  		const u32 *buf = buffer;
>  		do {
> -			__raw_writel(*buf++, addr + PCI_IOBASE);
> +			__raw_writel(*buf++, addr);
>  		} while (--count);
>  	}
>  }
> -#endif
> -
> -#ifndef CONFIG_GENERIC_IOMAP
> -#define ioread8(addr)		readb(addr)
> -#define ioread16(addr)		readw(addr)
> -#define ioread16be(addr)	__be16_to_cpu(__raw_readw(addr))
> -#define ioread32(addr)		readl(addr)
> -#define ioread32be(addr)	__be32_to_cpu(__raw_readl(addr))
> +#endif /* CONFIG_GENERIC_IOMAP */
>  
> -#define iowrite8(v, addr)	writeb((v), (addr))
> -#define iowrite16(v, addr)	writew((v), (addr))
> -#define iowrite16be(v, addr)	__raw_writew(__cpu_to_be16(v), addr)
> -#define iowrite32(v, addr)	writel((v), (addr))
> -#define iowrite32be(v, addr)	__raw_writel(__cpu_to_be32(v), addr)
> +#ifndef insb
> +#define insb(a, dst, count) \
> +	ioread8_rep((a) + PCI_IOBASE, (dst), (count))
> +#endif
> +#ifndef insw
> +#define insw(a, dst, count) \
> +	ioread16_rep((a) + PCI_IOBASE, (dst), (count))
> +#endif
> +#ifndef insl
> +#define insl(a, dst, count) \
> +	ioread32_rep((a) + PCI_IOBASE, (dst), (count))
> +#endif
>  
> -#define ioread8_rep(p, dst, count) \
> -	insb((unsigned long) (p), (dst), (count))
> -#define ioread16_rep(p, dst, count) \
> -	insw((unsigned long) (p), (dst), (count))
> -#define ioread32_rep(p, dst, count) \
> -	insl((unsigned long) (p), (dst), (count))
> -
> -#define iowrite8_rep(p, src, count) \
> -	outsb((unsigned long) (p), (src), (count))
> -#define iowrite16_rep(p, src, count) \
> -	outsw((unsigned long) (p), (src), (count))
> -#define iowrite32_rep(p, src, count) \
> -	outsl((unsigned long) (p), (src), (count))
> -#endif /* CONFIG_GENERIC_IOMAP */
> +#ifndef outsb
> +#define outsb(a, src, count) \
> +	iowrite8_rep((a) + PCI_IOBASE, (src), (count))
> +#endif
> +#ifndef outsw
> +#define outsw(a, src, count) \
> +	iowrite16_rep((a) + PCI_IOBASE, (src), (count))
> +#endif
> +#ifndef outsl
> +#define outsl(a, src, count) \
> +	iowrite32_rep((a) + PCI_IOBASE, (src), (count))
> +#endif
>  
>  #ifndef IO_SPACE_LIMIT
>  #define IO_SPACE_LIMIT 0xffff


-- 
Chen Gang