Message-Id: <a5bca6fc-1720-45cf-8cf3-3f129e699d79@app.fastmail.com>
Date: Sat, 06 Apr 2024 17:27:03 +0200
From: "Arnd Bergmann" <arnd@...nel.org>
To: "Michael Schmitz" <schmitzmic@...il.com>,
 "Geert Uytterhoeven" <geert@...ux-m68k.org>
Cc: "Niklas Schnelle" <schnelle@...ux.ibm.com>,
 linux-m68k@...ts.linux-m68k.org, "Heiko Carstens" <hca@...ux.ibm.com>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/1] m68k: Handle HAS_IOPORT dependencies

On Sat, Apr 6, 2024, at 03:14, Michael Schmitz wrote:
> Am 06.04.2024 um 09:13 schrieb Arnd Bergmann:
>> On Fri, Apr 5, 2024, at 20:36, Michael Schmitz wrote:
>>> Am 05.04.2024 um 23:16 schrieb Geert Uytterhoeven:
>>> The last time I tried to add support for a different PCMCIA ethernet
>>> adapter to apne.c _without_ adding to the hacks in io_mm.h, I wasn't
>>> getting anywhere with the netdev crowd. That was ages ago, and I doubt
>>> their enthusiasm for a rewrite of the 8390 code base to avoid using
>>> inb() on MMIO architectures will be any better now.
>>
>> From what I can see, there is already an abstraction layer in
>> these drivers that is used by all m68k drivers except apne:
>
> As well as ne ... which uses the 8390p.c helper.

Is there any machine using ne.c that doesn't set HAS_IOPORT though?
Q40 and ATARI_ETHERNEC both have custom inb()/outb(), so
those are not affected by the change.

>> $ git grep -w 'define\sei_inb'
>> drivers/net/ethernet/8390/8390.h:#define ei_inb(_p)     inb(_p)
>> drivers/net/ethernet/8390/8390p.c:#define ei_inb(_p)    inb(_p)
>> drivers/net/ethernet/8390/ax88796.c:#define ei_inb(_a) readb(ax_convert_addr(_a))
>> drivers/net/ethernet/8390/etherh.c:#define ei_inb(_p)    readb((void __iomem *)_p)
>> drivers/net/ethernet/8390/hydra.c:#define ei_inb(port)   in_8(port)
>> drivers/net/ethernet/8390/mac8390.c:#define ei_inb(port)        in_8(port)
>> drivers/net/ethernet/8390/mcf8390.c:#define     ei_inb  ei_inb
>> drivers/net/ethernet/8390/xsurf100.c:#define ei_inb(_a) z_readb(ax_convert_addr(_a))
>> drivers/net/ethernet/8390/zorro8390.c:#define ei_inb(port)              in_8(port)
>>
>> Can't apne.c just do the same here? The patch below didn't
>> take that long to come up with, but I may be missing something
>> here of course.
>
> The address translation from ISA IO ports to MMIO addresses needs to be 
> added as well (in_8() does not use address translation on m68k).

Indeed, I totally missed that bit.

> apne.c also uses inw() which does have a different address translation
> yet, but that's only for data transfer from the ring buffers and can
> be handled entirely inside apne.c.

and this as well.

> Now that is all limited to m68k. I might be able to submit a patch, but 
> I cannot test any of this.
>
> ne.c needs the same treatment as far as I can see, and I could actually 
> test that one (on Atari, not actually on a PC ISA card). I'll see what I 
> can come up with.

ATARI_ROM_ISA turns on HAS_IOPORT, so this one doesn't need any
immediate changes as a result of Niklas's series. I see now that
the apne driver doesn't actually need changes either since
AMIGA_PCMCIA turns on ISA.

I don't think there is an easy way to rework ne.c to avoid
inb()/outb(), but you could consider splitting the atari
support out into a separate module the same way as apne.c
to make it use the atari operations directly.

> I might well be missing something else here - as I said, it's been a few 
> years since I worked on the apne driver, and experimented with IO 
> abstractions in that context. The problem has always been making sure 
> drivers shared by different m68k platforms need only be built once and 
> still work on e.g. Q40 and Atari.

Do you know of any other ISA style drivers that are used with the
amiga pcmcia or the atari rom I/O operations, aside from the 8390
family? If this is the only one using it, it does sound like this
could be simplified a lot by just making those two not share the
object code with the ISA-style ne.c.

      Arnd

Powered by blists - more mailing lists