Re: [PATCH v8 3/3] net/8390: apne.c - add 100 Mbit support to apne.c driver

Geert Uytterhoeven
2021-11-08T08:38:36Z

Hi Michael,

On Thu, Nov 4, 2021 at 7:11 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> Add module parameter, IO mode autoprobe and PCMCIA reset code
> required to support 100 Mbit PCMCIA ethernet cards on Amiga.
>
> Select core PCMCIA support modules for use by APNE driver.
>
> 10 Mbit and 100 Mbit mode are supported by the same module.
> Use the core PCMCIA cftable parser to detect 16 bit cards,
> and automatically enable 16 bit ISA IO access for those cards
> by changing isa_type at runtime. Code to reset the PCMCIA
> hardware required for 16 bit cards is also added to the driver
> probe.
>
> An optional module parameter switches Amiga ISA IO accessors
> to 8 or 16 bit access in case autoprobe fails.
>
> Patch modified after patch "[PATCH RFC net-next] Amiga PCMCIA
> 100 MBit card support" submitted to netdev 2018/09/16 by Alex
> Kazik <alex@kazik.de>.
>
> CC: netdev@vger.kernel.org
> Link: https://lore.kernel.org/r/1622958877-2026-1-git-send-email-schmitzmic@gmail.com
> Tested-by: Alex Kazik <alex@kazik.de>
> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>

> Changes from v7:
>
> - move 'select' for PCCARD and PCMCIA to 8390 Kconfig, so
> Amiga pcmcia.c may remain built-in while core PCMCIA
> code can be built as modules if APNE driver is a module.
> - move 16 bit mode autoprobe code from amiga/pcmcia.c to this
> driver, to allow the core PCMCIA code we depend on to be
> built as modules.
> - change module parameter type from bool to int to allow for
> tri-state semantics (autoprobe, 8 bit, 16 bit).

Thanks for the update!

> --- a/drivers/net/ethernet/8390/apne.c
> +++ b/drivers/net/ethernet/8390/apne.c

> @@ -119,6 +121,10 @@ 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 u32 apne_100_mbit = -1;

The changelog said you changed this to int, not u32...

> +module_param_named(100_mbit, apne_100_mbit, uint, 0444);
> +MODULE_PARM_DESC(100_mbit, "Enable 100 Mbit support");
> +
> static struct net_device * __init apne_probe(void)
> {
> struct net_device *dev;
> @@ -140,6 +146,13 @@ static struct net_device * __init apne_probe(void)
>
> pr_info("Looking for PCMCIA ethernet card : ");
>
> + if (apne_100_mbit == 1)
> + isa_type = ISA_TYPE_AG16;
> + else if (apne_100_mbit == 0)
> + isa_type = ISA_TYPE_AG;
> + else
> + pr_cont(" (autoprobing 16 bit mode) ");
> +
> /* check if a card is inserted */
> if (!(PCMCIA_INSERTED)) {
> pr_cont("NO PCMCIA card inserted\n");
> @@ -167,6 +180,14 @@ static struct net_device * __init apne_probe(void)
>
> pr_cont("ethernet PCMCIA card inserted\n");
>
> +#if IS_ENABLED(CONFIG_PCMCIA)
> + if (apne_100_mbit < 0 && pcmcia_is_16bit()) {

apne_100_mbit is u32, hence can never be negative.

> + pr_info("16-bit PCMCIA card detected!\n");
> + isa_type = ISA_TYPE_AG16;
> + apne_100_mbit = 1;
> + }
> +#endif
> +
> if (!init_pcmcia()) {
> /* XXX: shouldn't we re-enable irq here? */
> free_netdev(dev);

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