arch/m68k/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Hi Geert, This is a follow up in my ongoing effort of making inb()/outb() and similar I/O port accessors compile-time optional. Previously I sent this as a treewide series titled "treewide: Remove I/O port accessors for HAS_IOPORT=n" with the latest being its 5th version[0]. With a significant subset of patches merged I've changed over to per-subsystem series. These series are stand alone and should be merged via the relevant tree such that with all subsystems complete we can follow this up with the final patch that will make the I/O port accessors compile-time optional. The current state of the full series with changes to the remaining subsystems and the aforementioned final patch can be found for your convenience on my git.kernel.org tree in the has_ioport_v6 branch[1] with signed tags. As for compile-time vs runtime see Linus' reply to my first attempt[2]. Thanks, Niklas [0] https://lore.kernel.org/all/20230522105049.1467313-1-schnelle@linux.ibm.com/ [1] https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git/log/?h=has_ioport_v6 [2] https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/ Niklas Schnelle (1): m68k: Let GENERIC_IOMAP depend on HAS_IOPORT arch/m68k/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.40.1
Niklas, how do you propose we handle legacy drivers that do depend on inb()/outb() functions (_not_ actual ISA I/O) on architectures that map inb()/outb() to MMIO functions? (In my case, that's at least ne.c - Geert ought to have a better overview what else does use inb()/outb() on m68k) Cheers, Michael On 4/04/24 01:28, Niklas Schnelle wrote: > Hi Geert, > > This is a follow up in my ongoing effort of making inb()/outb() and > similar I/O port accessors compile-time optional. Previously I sent this > as a treewide series titled "treewide: Remove I/O port accessors for > HAS_IOPORT=n" with the latest being its 5th version[0]. With a significant > subset of patches merged I've changed over to per-subsystem series. These > series are stand alone and should be merged via the relevant tree such > that with all subsystems complete we can follow this up with the final > patch that will make the I/O port accessors compile-time optional. > > The current state of the full series with changes to the remaining > subsystems and the aforementioned final patch can be found for your > convenience on my git.kernel.org tree in the has_ioport_v6 branch[1] with > signed tags. As for compile-time vs runtime see Linus' reply to my first > attempt[2]. > > Thanks, > Niklas > > [0] https://lore.kernel.org/all/20230522105049.1467313-1-schnelle@linux.ibm.com/ > [1] https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git/log/?h=has_ioport_v6 > [2] https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/ > > Niklas Schnelle (1): > m68k: Let GENERIC_IOMAP depend on HAS_IOPORT > > arch/m68k/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) >
On Wed, Apr 3, 2024, at 20:11, Michael Schmitz wrote:
> Niklas,
>
> how do you propose we handle legacy drivers that do depend on
> inb()/outb() functions (_not_ actual ISA I/O) on architectures that map
> inb()/outb() to MMIO functions?
>
> (In my case, that's at least ne.c - Geert ought to have a better
> overview what else does use inb()/outb() on m68k)
If a machine provides an inb()/outb() set of operations that
is actually used, it should set HAS_IOPORT.
For the Q40, it may be better in the long run to change the
drivers to just use MMIO directly though.
Arnd
Hi Arnd,
On Wed, Apr 3, 2024 at 8:35 PM Arnd Bergmann <arnd@kernel.org> wrote:
> On Wed, Apr 3, 2024, at 20:11, Michael Schmitz wrote:
> > how do you propose we handle legacy drivers that do depend on
> > inb()/outb() functions (_not_ actual ISA I/O) on architectures that map
> > inb()/outb() to MMIO functions?
> >
> > (In my case, that's at least ne.c - Geert ought to have a better
> > overview what else does use inb()/outb() on m68k)
>
> If a machine provides an inb()/outb() set of operations that
> is actually used, it should set HAS_IOPORT.
>
> For the Q40, it may be better in the long run to change the
> drivers to just use MMIO directly though.
Q40 uses ISA.
Michael is worried about non-ISA drivers using inb() and friends.
At some point in time (i.e. eons ago), we were told it was better to
use in[bwl]()/read[bwl]() instead of directly dereferencing volatile
pointers...
Anyway, I don't think we have many users of inb() and friends left, and
I assume the bots should have detected any/most remaining users in Niklas'
branch...
arch/m68k/include/asm/floppy.h on Sun-3x might be the only offender?
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
Hi Geert,
Am 05.04.2024 um 23:16 schrieb Geert Uytterhoeven:
> Hi Arnd,
>
> On Wed, Apr 3, 2024 at 8:35 PM Arnd Bergmann <arnd@kernel.org> wrote:
>> On Wed, Apr 3, 2024, at 20:11, Michael Schmitz wrote:
>>> how do you propose we handle legacy drivers that do depend on
>>> inb()/outb() functions (_not_ actual ISA I/O) on architectures that map
>>> inb()/outb() to MMIO functions?
>>>
>>> (In my case, that's at least ne.c - Geert ought to have a better
>>> overview what else does use inb()/outb() on m68k)
>>
>> If a machine provides an inb()/outb() set of operations that
>> is actually used, it should set HAS_IOPORT.
>>
>> For the Q40, it may be better in the long run to change the
>> drivers to just use MMIO directly though.
>
> Q40 uses ISA.
>
> Michael is worried about non-ISA drivers using inb() and friends.
> At some point in time (i.e. eons ago), we were told it was better to
> use in[bwl]()/read[bwl]() instead of directly dereferencing volatile
> pointers...
>
> Anyway, I don't think we have many users of inb() and friends left, and
> I assume the bots should have detected any/most remaining users in Niklas'
> branch...
All the 8390 based ethernet drivers still use inb() and friends.
That is the main reason for the terrible hacks in
arch/m68k/include/asm/io_mm.h ...
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.
Just saying ...
Michael
>
> arch/m68k/include/asm/floppy.h on Sun-3x might be the only offender?
>
> Gr{oetje,eeting}s,
>
> Geert
>
On Fri, Apr 5, 2024 at 8:36 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
> Am 05.04.2024 um 23:16 schrieb Geert Uytterhoeven:
> > On Wed, Apr 3, 2024 at 8:35 PM Arnd Bergmann <arnd@kernel.org> wrote:
> >> On Wed, Apr 3, 2024, at 20:11, Michael Schmitz wrote:
> >>> how do you propose we handle legacy drivers that do depend on
> >>> inb()/outb() functions (_not_ actual ISA I/O) on architectures that map
> >>> inb()/outb() to MMIO functions?
> >>>
> >>> (In my case, that's at least ne.c - Geert ought to have a better
> >>> overview what else does use inb()/outb() on m68k)
> >>
> >> If a machine provides an inb()/outb() set of operations that
> >> is actually used, it should set HAS_IOPORT.
> >>
> >> For the Q40, it may be better in the long run to change the
> >> drivers to just use MMIO directly though.
> >
> > Q40 uses ISA.
> >
> > Michael is worried about non-ISA drivers using inb() and friends.
> > At some point in time (i.e. eons ago), we were told it was better to
> > use in[bwl]()/read[bwl]() instead of directly dereferencing volatile
> > pointers...
> >
> > Anyway, I don't think we have many users of inb() and friends left, and
> > I assume the bots should have detected any/most remaining users in Niklas'
> > branch...
>
> All the 8390 based ethernet drivers still use inb() and friends.
>
> That is the main reason for the terrible hacks in
> arch/m68k/include/asm/io_mm.h ...
>
> 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.
As Arnd already discovered later in this thread, AMIGA_PCMCIA
(sort of) selects ISA, so APNE can be considered a "real" ISA. driver,
so it can keep on using inb() and friends.
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
On Fri, Apr 5, 2024, at 20:36, Michael Schmitz wrote:
> Am 05.04.2024 um 23:16 schrieb Geert Uytterhoeven:
>> On Wed, Apr 3, 2024 at 8:35 PM Arnd Bergmann <arnd@kernel.org> wrote:
>>> On Wed, Apr 3, 2024, at 20:11, Michael Schmitz wrote:
>>>> how do you propose we handle legacy drivers that do depend on
>>>> inb()/outb() functions (_not_ actual ISA I/O) on architectures that map
>>>> inb()/outb() to MMIO functions?
>>>>
>>>> (In my case, that's at least ne.c - Geert ought to have a better
>>>> overview what else does use inb()/outb() on m68k)
>>>
>>> If a machine provides an inb()/outb() set of operations that
>>> is actually used, it should set HAS_IOPORT.
>>>
>>> For the Q40, it may be better in the long run to change the
>>> drivers to just use MMIO directly though.
>>
>> Q40 uses ISA.
>>
>> Michael is worried about non-ISA drivers using inb() and friends.
>> At some point in time (i.e. eons ago), we were told it was better to
>> use in[bwl]()/read[bwl]() instead of directly dereferencing volatile
>> pointers...
>>
>> Anyway, I don't think we have many users of inb() and friends left, and
>> I assume the bots should have detected any/most remaining users in Niklas'
>> branch...
>
> All the 8390 based ethernet drivers still use inb() and friends.
>
> That is the main reason for the terrible hacks in
> arch/m68k/include/asm/io_mm.h ...
>
> 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:
$ 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.
Arnd
8<---
From 5dd43e612a52adf499b1ea3d33e3b2b45894d275 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 5 Apr 2024 21:47:51 +0200
Subject: [PATCH] net: apne: convert to lib8390
The apne driver still uses the ISA-style inb()/outb() wappers through the
8390.c helper module, which won't work in the future.
Change it to include lib8390.c like all the other m68k variants of this
driver do, so it can have custom MMIO abstractions.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/net/ethernet/8390/Makefile b/drivers/net/ethernet/8390/Makefile
index 85c83c566ec6..ec1b325da4e4 100644
--- a/drivers/net/ethernet/8390/Makefile
+++ b/drivers/net/ethernet/8390/Makefile
@@ -4,7 +4,7 @@
#
obj-$(CONFIG_MAC8390) += mac8390.o
-obj-$(CONFIG_APNE) += apne.o 8390.o
+obj-$(CONFIG_APNE) += apne.o
obj-$(CONFIG_ARM_ETHERH) += etherh.o
obj-$(CONFIG_AX88796) += ax88796.o
obj-$(CONFIG_HYDRA) += hydra.o
diff --git a/drivers/net/ethernet/8390/apne.c b/drivers/net/ethernet/8390/apne.c
index 828edca8d30c..ea3747723b3c 100644
--- a/drivers/net/ethernet/8390/apne.c
+++ b/drivers/net/ethernet/8390/apne.c
@@ -41,7 +41,15 @@
#include <asm/amigayle.h>
#include <asm/amipcmcia.h>
-#include "8390.h"
+#define ei_inb(port) in_8(port)
+#define ei_outb(val, port) out_8(port, val)
+#define ei_inb_p(port) in_8(port)
+#define ei_outb_p(val, port) out_8(port, val)
+
+static const char version[] =
+ "apne.c:v1.1 7/10/98 Alain Malek (Alain.Malek@cryogen.ch)\n";
+
+#include "lib8390.c"
/* ---- No user-serviceable parts below ---- */
@@ -105,14 +113,21 @@ static int init_pcmcia(void);
#define MANUAL_HWADDR5 0x9a
*/
-static const char version[] =
- "apne.c:v1.1 7/10/98 Alain Malek (Alain.Malek@cryogen.ch)\n";
-
static int apne_owned; /* signal if card already owned */
-static u32 apne_msg_enable;
-module_param_named(msg_enable, apne_msg_enable, uint, 0444);
-MODULE_PARM_DESC(msg_enable, "Debug message level (see linux/netdevice.h for bitmap)");
+static const struct net_device_ops apne_netdev_ops = {
+ .ndo_open = __ei_open,
+ .ndo_stop = __ei_close,
+ .ndo_start_xmit = __ei_start_xmit,
+ .ndo_tx_timeout = __ei_tx_timeout,
+ .ndo_get_stats = __ei_get_stats,
+ .ndo_set_rx_mode = __ei_set_multicast_list,
+ .ndo_validate_addr = eth_validate_addr,
+ .ndo_set_mac_address = eth_mac_addr,
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ .ndo_poll_controller = __ei_poll,
+#endif
+};
static struct net_device * __init apne_probe(void)
{
@@ -141,11 +156,11 @@ static struct net_device * __init apne_probe(void)
return ERR_PTR(-ENODEV);
}
- dev = alloc_ei_netdev();
+ dev = ____alloc_ei_netdev(0);
if (!dev)
return ERR_PTR(-ENOMEM);
ei_local = netdev_priv(dev);
- ei_local->msg_enable = apne_msg_enable;
+ ei_local->msg_enable = msg_enable;
/* disable pcmcia irq for readtuple */
pcmcia_disable_irq();
@@ -203,7 +218,7 @@ static int __init apne_probe1(struct net_device *dev, int ioaddr)
#endif
static unsigned version_printed;
- if ((apne_msg_enable & NETIF_MSG_DRV) && (version_printed++ == 0))
+ if ((msg_enable & NETIF_MSG_DRV) && (version_printed++ == 0))
netdev_info(dev, version);
netdev_info(dev, "PCMCIA NE*000 ethercard probe");
@@ -309,7 +324,7 @@ static int __init apne_probe1(struct net_device *dev, int ioaddr)
dev->base_addr = ioaddr;
dev->irq = IRQ_AMIGA_PORTS;
- dev->netdev_ops = &ei_netdev_ops;
+ dev->netdev_ops = &apne_netdev_ops;
/* Install the Interrupt handler */
i = request_irq(dev->irq, apne_interrupt, IRQF_SHARED, DRV_NAME, dev);
@@ -333,7 +348,7 @@ static int __init apne_probe1(struct net_device *dev, int ioaddr)
ei_status.block_output = &apne_block_output;
ei_status.get_8390_hdr = &apne_get_8390_hdr;
- NS8390_init(dev, 0);
+ __NS8390_init(dev, 0);
pcmcia_ack_int(pcmcia_get_intreq()); /* ack PCMCIA int req */
pcmcia_enable_irq();
@@ -513,7 +528,7 @@ apne_block_output(struct net_device *dev, int count,
if (time_after(jiffies, dma_start + 2*HZ/100)) { /* 20ms */
netdev_warn(dev, "timeout waiting for Tx RDC.\n");
apne_reset_8390(dev);
- NS8390_init(dev,1);
+ __NS8390_init(dev,1);
break;
}
@@ -534,10 +549,10 @@ static irqreturn_t apne_interrupt(int irq, void *dev_id)
pcmcia_ack_int(pcmcia_intreq);
return IRQ_NONE;
}
- if (apne_msg_enable & NETIF_MSG_INTR)
+ if (msg_enable & NETIF_MSG_INTR)
pr_debug("pcmcia intreq = %x\n", pcmcia_intreq);
pcmcia_disable_irq(); /* to get rid of the sti() within ei_interrupt */
- ei_interrupt(irq, dev_id);
+ __ei_interrupt(irq, dev_id);
pcmcia_ack_int(pcmcia_get_intreq());
pcmcia_enable_irq();
return IRQ_HANDLED;
Hi Arnd,
thanks for your suggestions!
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:
>>> On Wed, Apr 3, 2024 at 8:35 PM Arnd Bergmann <arnd@kernel.org> wrote:
>>>> On Wed, Apr 3, 2024, at 20:11, Michael Schmitz wrote:
>>>>> how do you propose we handle legacy drivers that do depend on
>>>>> inb()/outb() functions (_not_ actual ISA I/O) on architectures that map
>>>>> inb()/outb() to MMIO functions?
>>>>>
>>>>> (In my case, that's at least ne.c - Geert ought to have a better
>>>>> overview what else does use inb()/outb() on m68k)
>>>>
>>>> If a machine provides an inb()/outb() set of operations that
>>>> is actually used, it should set HAS_IOPORT.
>>>>
>>>> For the Q40, it may be better in the long run to change the
>>>> drivers to just use MMIO directly though.
>>>
>>> Q40 uses ISA.
>>>
>>> Michael is worried about non-ISA drivers using inb() and friends.
>>> At some point in time (i.e. eons ago), we were told it was better to
>>> use in[bwl]()/read[bwl]() instead of directly dereferencing volatile
>>> pointers...
>>>
>>> Anyway, I don't think we have many users of inb() and friends left, and
>>> I assume the bots should have detected any/most remaining users in Niklas'
>>> branch...
>>
>> All the 8390 based ethernet drivers still use inb() and friends.
>>
>> That is the main reason for the terrible hacks in
>> arch/m68k/include/asm/io_mm.h ...
>>
>> 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.
>
> $ 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). 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.
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.
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.
You've given me something to work with, thanks again!
Cheers,
Michael
>
> Arnd
>
> 8<---
> From 5dd43e612a52adf499b1ea3d33e3b2b45894d275 Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Fri, 5 Apr 2024 21:47:51 +0200
> Subject: [PATCH] net: apne: convert to lib8390
>
> The apne driver still uses the ISA-style inb()/outb() wappers through the
> 8390.c helper module, which won't work in the future.
>
> Change it to include lib8390.c like all the other m68k variants of this
> driver do, so it can have custom MMIO abstractions.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/net/ethernet/8390/Makefile b/drivers/net/ethernet/8390/Makefile
> index 85c83c566ec6..ec1b325da4e4 100644
> --- a/drivers/net/ethernet/8390/Makefile
> +++ b/drivers/net/ethernet/8390/Makefile
> @@ -4,7 +4,7 @@
> #
>
> obj-$(CONFIG_MAC8390) += mac8390.o
> -obj-$(CONFIG_APNE) += apne.o 8390.o
> +obj-$(CONFIG_APNE) += apne.o
> obj-$(CONFIG_ARM_ETHERH) += etherh.o
> obj-$(CONFIG_AX88796) += ax88796.o
> obj-$(CONFIG_HYDRA) += hydra.o
> diff --git a/drivers/net/ethernet/8390/apne.c b/drivers/net/ethernet/8390/apne.c
> index 828edca8d30c..ea3747723b3c 100644
> --- a/drivers/net/ethernet/8390/apne.c
> +++ b/drivers/net/ethernet/8390/apne.c
> @@ -41,7 +41,15 @@
> #include <asm/amigayle.h>
> #include <asm/amipcmcia.h>
>
> -#include "8390.h"
> +#define ei_inb(port) in_8(port)
> +#define ei_outb(val, port) out_8(port, val)
> +#define ei_inb_p(port) in_8(port)
> +#define ei_outb_p(val, port) out_8(port, val)
> +
> +static const char version[] =
> + "apne.c:v1.1 7/10/98 Alain Malek (Alain.Malek@cryogen.ch)\n";
> +
> +#include "lib8390.c"
>
> /* ---- No user-serviceable parts below ---- */
>
> @@ -105,14 +113,21 @@ static int init_pcmcia(void);
> #define MANUAL_HWADDR5 0x9a
> */
>
> -static const char version[] =
> - "apne.c:v1.1 7/10/98 Alain Malek (Alain.Malek@cryogen.ch)\n";
> -
> static int apne_owned; /* signal if card already owned */
>
> -static u32 apne_msg_enable;
> -module_param_named(msg_enable, apne_msg_enable, uint, 0444);
> -MODULE_PARM_DESC(msg_enable, "Debug message level (see linux/netdevice.h for bitmap)");
> +static const struct net_device_ops apne_netdev_ops = {
> + .ndo_open = __ei_open,
> + .ndo_stop = __ei_close,
> + .ndo_start_xmit = __ei_start_xmit,
> + .ndo_tx_timeout = __ei_tx_timeout,
> + .ndo_get_stats = __ei_get_stats,
> + .ndo_set_rx_mode = __ei_set_multicast_list,
> + .ndo_validate_addr = eth_validate_addr,
> + .ndo_set_mac_address = eth_mac_addr,
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + .ndo_poll_controller = __ei_poll,
> +#endif
> +};
>
> static struct net_device * __init apne_probe(void)
> {
> @@ -141,11 +156,11 @@ static struct net_device * __init apne_probe(void)
> return ERR_PTR(-ENODEV);
> }
>
> - dev = alloc_ei_netdev();
> + dev = ____alloc_ei_netdev(0);
> if (!dev)
> return ERR_PTR(-ENOMEM);
> ei_local = netdev_priv(dev);
> - ei_local->msg_enable = apne_msg_enable;
> + ei_local->msg_enable = msg_enable;
>
> /* disable pcmcia irq for readtuple */
> pcmcia_disable_irq();
> @@ -203,7 +218,7 @@ static int __init apne_probe1(struct net_device *dev, int ioaddr)
> #endif
> static unsigned version_printed;
>
> - if ((apne_msg_enable & NETIF_MSG_DRV) && (version_printed++ == 0))
> + if ((msg_enable & NETIF_MSG_DRV) && (version_printed++ == 0))
> netdev_info(dev, version);
>
> netdev_info(dev, "PCMCIA NE*000 ethercard probe");
> @@ -309,7 +324,7 @@ static int __init apne_probe1(struct net_device *dev, int ioaddr)
>
> dev->base_addr = ioaddr;
> dev->irq = IRQ_AMIGA_PORTS;
> - dev->netdev_ops = &ei_netdev_ops;
> + dev->netdev_ops = &apne_netdev_ops;
>
> /* Install the Interrupt handler */
> i = request_irq(dev->irq, apne_interrupt, IRQF_SHARED, DRV_NAME, dev);
> @@ -333,7 +348,7 @@ static int __init apne_probe1(struct net_device *dev, int ioaddr)
> ei_status.block_output = &apne_block_output;
> ei_status.get_8390_hdr = &apne_get_8390_hdr;
>
> - NS8390_init(dev, 0);
> + __NS8390_init(dev, 0);
>
> pcmcia_ack_int(pcmcia_get_intreq()); /* ack PCMCIA int req */
> pcmcia_enable_irq();
> @@ -513,7 +528,7 @@ apne_block_output(struct net_device *dev, int count,
> if (time_after(jiffies, dma_start + 2*HZ/100)) { /* 20ms */
> netdev_warn(dev, "timeout waiting for Tx RDC.\n");
> apne_reset_8390(dev);
> - NS8390_init(dev,1);
> + __NS8390_init(dev,1);
> break;
> }
>
> @@ -534,10 +549,10 @@ static irqreturn_t apne_interrupt(int irq, void *dev_id)
> pcmcia_ack_int(pcmcia_intreq);
> return IRQ_NONE;
> }
> - if (apne_msg_enable & NETIF_MSG_INTR)
> + if (msg_enable & NETIF_MSG_INTR)
> pr_debug("pcmcia intreq = %x\n", pcmcia_intreq);
> pcmcia_disable_irq(); /* to get rid of the sti() within ei_interrupt */
> - ei_interrupt(irq, dev_id);
> + __ei_interrupt(irq, dev_id);
> pcmcia_ack_int(pcmcia_get_intreq());
> pcmcia_enable_irq();
> return IRQ_HANDLED;
>
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
Hi Arnd, Am 07.04.2024 um 03:27 schrieb Arnd Bergmann: > 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. Thanks for clarifying - I had been left with the impression that inb()/outb() would only be retained for architectures that have not only the ISA bus but inb/outb processor instructions. >> 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. Your patch would not actually be too hard to get right as apne only needs unconditional address translation (unless we want to take this opportunity to introduce 100 Mbit support; I need to dig out my old patches for that). But perhaps leave that for later > 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. We used to have atari_ethernec.c for that. AFAIR the netdev maintainers asked for us to use ne.c instead. They had suggestions around making IO abstractions more flexible for apne.c a while back. I need to revisit that. >> 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. No network drivers that I can remember, but the same ROM I/O is used by the isp116x-hcd USB driver. That one uses the defines from asm/raw_io.h directly though, not inb()/outb(). I don't think any other PCMCIA cards were ever supported on Amiga. Cheers, Michael > > Arnd >
© 2016 - 2026 Red Hat, Inc.