Re: [PATCH] m68k: Atari io.h fixes for EtherNAT driver

[Geert Uytterhoeven <geert@linux-m68k.org>]

On Mon, 21 Jul 2008, Michael Schmitz wrote:
> Here goes ...
> 
> > > On Wed, Jul 16, 2008 at 11:59:53PM +0200, Michael Schmitz wrote:
> > > > We should close it with the next kernel upload... with any luck, I'll
> > > > have the SMC91C111 driver working by then (it did finally own up to
> > > > detecting the proper hardware, instead of stuffing up the ROM-port card
> > > > or just hanging the kernel).
> > > 
> > > Does Geert have this in his queue yet?
> > 
> > No. Where's the patch?
> 
> This patch fixes errors I introduced in preparing the io.h code for ROM port
> ISA access (for the EtherNEC driver, originally).
> 
> My assumption was that insb/outsb and friends should be mapped to the ROM port
> accessors unconditionally, and the read*/write* functions should behave like
> regular ISA accesses. As it turned out, it's just the other way 'round.
> "Regular" ISA readb would apply the ROM port address mapping without actually
> using the correct access function.
> 
> ins*/outs* are conditionalized now (port < 1024 goes to the ROM port, all else
> via MMIO), read*/write* use straight MMIO no questions asked.
> 
> This fixes register access problems for the EtherNAT (SMC91C111) when used
> together with the EtherNEC (ROM port NE2k clone). Tested on my Falcon.
> 
> Applies after Geert's linux-m68k-patches-2.6/atari-rom-isa.diff .... should
> integrate accordingly in the Debian kernel source.

Sorry, it took a while...

The patch idea looks fine to me.

> --- a/include/asm-m68k/io.h
> +++ b/include/asm-m68k/io.h
> @@ -429,8 +429,8 @@
>  /*
>   * kernel with both ROM port ISA and IDE compiled in, those have
>   * conflicting defs for in/out. Simply consider port < 1024
> - * ROM port ISA and everything else regular ISA for IDE. read,write not
> defined

But it's seriously whitespace-challenged. Time to tame alpine?

> + * Originally, insb/outsb and insw/outsw were set to the ROM variants in
> 2.6.19

Do we need this explanation in the comments? It's already in the patch
comment.

> + * insl/outsl not used. isa_ins* and isa_outs* do the Right Thing so this is
> OK.
> + */
> +#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(port, buf, nr)	((port) < 1024 ? isa_rom_insl((port), (buf),
> (nr)) : isa_insl((port), (buf), (nr)))
> +#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(port, buf, nr)	((port) < 1024 ? isa_rom_outsl((port), (buf),
> (nr)) : isa_outsl((port), (buf), (nr)))
> +
> +/*
> + * Previously, read* / write* used the isa_ variants here.

Same here.

> + * This is absolutely deadly: with CONFIG_ATARI_ROM_ISA defined, isa_readb +
> * will use the ROM port space mtb translation, but at the same time, uses + *
> plain old in_8() instead of the ROM port access code. So that'll bang + * on
> the ROM port using invalid addresses, actually. Mayhem ensues.
> + *
> + * Using in_* / out_*, both EtherNEC and EtherNAT work. If things get more +
> * complex than that, I'll have to make the ENEC_ISA_* mappings conditional + *
> on the 'port' addresses, perhaps. + * First try to make read[bw]/write[bw]
> conditional, though. + */
> +#define readb(addr)      in_8(addr)
> +#define writeb(val,addr) out_8((addr),(val))
> +#define readw(addr)      in_le16(addr)
> +#define writew(val,addr) out_le16((addr),(val))
> +
> +#define readsw  raw_insw
> +#define writesw raw_outsw
>  #define readsl  raw_insl
>  #define writesl raw_outsl
>  #endif

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