Re: [PATCH] arch: m68k: include: asm: the 3rd parameter of 'insl' and 'outsl' need '<< 2'

schmitz
2013-06-01T00:26:45Z

Geert ,
>>
>> The related git number:
>> for parport.h: "4914802 m68k,m68knommu: merge header files" in 2009
>> for io_mm.h: "84b16b7 m68k/atari: ROM port ISA adapter support" in Apr 6 2013
>>
>> The related warning (make EXTRA_CFLAG=-W ARCH=m68k allmodconfig):
>> arch/m68k/include/asm/parport.h:14:0: warning: "insl" redefined [enabled by default]
>> arch/m68k/include/asm/io_mm.h:403:0: note: this is the location of the previous definition
>> arch/m68k/include/asm/parport.h:15:0: warning: "outsl" redefined [enabled by default]
>> arch/m68k/include/asm/io_mm.h:406:0: note: this is the location of the previous definition
>>
Is that the same problem Thorsten reported recently? parport.h should
either use what the arch io.h include defined, or (in the case of Q40 on
m68k) undef and redefine as needed.
>>
>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>> ---
>> arch/m68k/include/asm/io_mm.h | 5 +++--
>> arch/m68k/include/asm/parport.h | 9 +++++++--
>> 2 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/m68k/include/asm/io_mm.h b/arch/m68k/include/asm/io_mm.h
>> index ffdf54f4..66be3b2 100644
>> --- a/arch/m68k/include/asm/io_mm.h
>> +++ b/arch/m68k/include/asm/io_mm.h
>> @@ -400,10 +400,11 @@ static inline void isa_delay(void)
>>
>> #define insb(port, buf, nr) ((port) < 1024 ? isa_rom_insb((port), (buf), (nr)) : isa_insb((port), (buf), (nr)))
>> #define insw(port, buf, nr) ((port) < 1024 ? isa_rom_insw((port), (buf), (nr)) : isa_insw((port), (buf), (nr)))
>> -#define insl isa_insl
>> +#define insl(port, buf, len) isa_insb((port), (buf), (len) << 2)
>>
>
> Oops, changes from 32-bit accesses to byte accesses?
>

That's in the Atari specific branch - please explain why you think this
needs to be done. Has this patch been tested by running on ARAnyM, at least?

Unless this has been properly tested on Atari (hardware), please leave
as-is.

>
>> #define outsb(port, buf, nr) ((port) < 1024 ? isa_rom_outsb((port), (buf), (nr)) : isa_outsb((port), (buf), (nr)))
>> #define outsw(port, buf, nr) ((port) < 1024 ? isa_rom_outsw((port), (buf), (nr)) : isa_outsw((port), (buf), (nr)))
>> -#define outsl isa_outsl
>> +#define outsl(port, buf, len) isa_outsb((port), (buf), (len) << 2)
>>
>> #define readb(addr) in_8(addr)
>> #define writeb(val, addr) out_8((addr), (val))
>> diff --git a/arch/m68k/include/asm/parport.h b/arch/m68k/include/asm/parport.h
>> index 5ea75e6..e8e4a2a 100644
>> --- a/arch/m68k/include/asm/parport.h
>> +++ b/arch/m68k/include/asm/parport.h
>> @@ -11,8 +11,13 @@
>> #ifndef _ASM_M68K_PARPORT_H
>> #define _ASM_M68K_PARPORT_H 1
>>
>> -#define insl(port,buf,len) isa_insb(port,buf,(len)<<2)
>> -#define outsl(port,buf,len) isa_outsb(port,buf,(len)<<2)
>> +#ifndef insl
>> +#define insl(port, buf, len) isa_insb((port), (buf), (len) << 2)
>> +#endif
>> +
>> +#ifndef outsl
>> +#define outsl(port, buf, len) isa_outsb((port), (buf), (len) << 2)
>> +#endif
>>
I think that should read
#undef insl
#define insl(port,buf,len) isa_insb(port,buf,(len)<<2)

instead. I distinctly remember this brought up a few weeks ago.
> Now the (re)definitions are identical to the originals, so they can just
> be removed. But the ones in <asm/io.h> are not correct anymore, IMHO.
>
Seconded.

Cheers,

Michael