From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ot1-f50.google.com (mail-ot1-f50.google.com [209.85.210.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9C76BBE58 for ; Sat, 6 Apr 2024 01:14:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712366087; cv=none; b=sZ4FCH7NKysFI4ZgYLsktjap37G9mnTHAZ2ZW8/nYFaIEhpEVoupyCS1B80H4/36H7+VMOr4fOI6fC8pPJ/oATh5CJVIEz2ZH+vQaM9CDCPXpdAuKHgYGY2AKQdhKgSt8xJ6r42Zi41suwZ+/nni3hJSGFYzvGnogW9jfsOdEkc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712366087; c=relaxed/simple; bh=r/gYE/Zz9tJKKilM7m8Zai2JPeh/htkd2LIkdAiTKNc=; h=Subject:To:References:Cc:From:Message-ID:Date:MIME-Version: In-Reply-To:Content-Type; b=BYzUyAEb5Zh00uWgw+k4/fnK4sWBTMZAd2ZN/dV//eFhTDYaWPqy9Mb6hVgOeObofpKksbZjI9EOSDB7/+4GzFgtISdku8QRmi17D+9OJ/vsPi8ZRyABkBUMTXPnuWJxxlgwaGhPsAc2hsiPaJVqtWANQOtI5JRZCCZtX7hluoE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=VL0mS5XJ; arc=none smtp.client-ip=209.85.210.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="VL0mS5XJ" Received: by mail-ot1-f50.google.com with SMTP id 46e09a7af769-6e6d089f603so1614946a34.1 for ; Fri, 05 Apr 2024 18:14:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712366084; x=1712970884; darn=lists.linux-m68k.org; h=content-transfer-encoding:in-reply-to:mime-version:user-agent:date :message-id:from:cc:references:to:subject:from:to:cc:subject:date :message-id:reply-to; bh=OUOCIQFB2R9168eqGn8/GvggwQbCuKvMKomVQmoNhkI=; b=VL0mS5XJ4yd38xIpDpI4q3136n3fjZbMgSV7IZPZPA6vL5tEQuDNBN0PzgvfLlmhT0 jo9u65bUjBDBy1QuAptceHNpYp5Nqug8fww5qjsp648eQfqyHTenNcNayXBJL5GAmb+a INX5cKwm2HWL2YSlCyaHgf0ucFI88gBtICW28bFKzcM0Zq2R1XHkVq53gGgC0XxE4biF m1PtZi7hjQVHYKZ2RkTpJ6V78HkZfGcHBAbThqIbPCHOqw9SRLxZeMPQX2AZXx+3iAVf ICesbzKpFDEGp/9Jm4HSoSWCFpk6VQif+ZN4zVw7h7/lyZXQtiu/8RcaPyl6OmG0wIEb v3Kw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712366084; x=1712970884; h=content-transfer-encoding:in-reply-to:mime-version:user-agent:date :message-id:from:cc:references:to:subject:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=OUOCIQFB2R9168eqGn8/GvggwQbCuKvMKomVQmoNhkI=; b=rtmS27kt4vPZtrIBu2rRdSOCr9fvtlp9FE4VSKSCv0Wc4pogRmMnJokaJ7GeyRrqxc DQRtrevo3KprfTfbQ5Uf+oCe0eJsoRq7F9+mZLONES4+cmmNpuH7xN/GGsAKo9w2MUSO c4fYPn6uRHM6x3GoznVwatVvUE3hjMV73vwnG1TYgtTjQRrTI+c5oS5ImsKN5vrnvkpW SjsHXQhKDnt65aJgpiROaxg20ktIt8vDS45ak3vDs9M3SayTSK+FCdMibetMDd+6WRbg 9wvoaDPGfxqvMCv0GFYy2umRVq8SdPkC8Kq4WY7Oa/2Ss8bhaL3cp6kaeFtOh5kIo5MK eMzA== X-Forwarded-Encrypted: i=1; AJvYcCUeBUVW/2ZJpOZBr2UslffHM2rmCItqn3r8bn/UI9uLCPJx5Hh/KxqOWDOpW0+QnyxarOdWpUpd+N6bMypRjukbOfvRLsx0PVFAyffR/DlH X-Gm-Message-State: AOJu0Yw11Uk44iXGjAuWKU2pWukfMVGoSy9GFLWcZzL210iEhbpUF74T YMDoERUPwjGquX02fe3M0+XLiueEHAtF7UjwmRjVz8SL+ACZk06K X-Google-Smtp-Source: AGHT+IEe6Ulz+W731tqsz/LUln5enWgNx5JLI+x3ByqhMGEdtA74wHX/xIdqny7RQXWQMsBRDh8qVA== X-Received: by 2002:a05:6870:f286:b0:22e:c504:74af with SMTP id u6-20020a056870f28600b0022ec50474afmr3577208oap.20.1712366084300; Fri, 05 Apr 2024 18:14:44 -0700 (PDT) Received: from [10.1.1.24] (222-152-175-63-fibre.sparkbb.co.nz. [222.152.175.63]) by smtp.gmail.com with ESMTPSA id w3-20020aa78583000000b006ecd942161fsm2119552pfn.190.2024.04.05.18.14.39 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 05 Apr 2024 18:14:43 -0700 (PDT) Subject: Re: [PATCH 0/1] m68k: Handle HAS_IOPORT dependencies To: Arnd Bergmann , Geert Uytterhoeven References: <20240403122851.38808-1-schnelle@linux.ibm.com> <6b308ab0-dda7-4052-98b3-cb03e17e3e12@app.fastmail.com> Cc: Niklas Schnelle , linux-m68k@lists.linux-m68k.org, Heiko Carstens , linux-kernel@vger.kernel.org From: Michael Schmitz Message-ID: Date: Sat, 6 Apr 2024 14:14:35 +1300 User-Agent: Mozilla/5.0 (X11; Linux ppc; rv:45.0) Gecko/20100101 Icedove/45.4.0 Precedence: bulk X-Mailing-List: linux-m68k@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 In-Reply-To: <6b308ab0-dda7-4052-98b3-cb03e17e3e12@app.fastmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit 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 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 > 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 > > 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 > #include > > -#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; >