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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdVMG8x-s-1_a2vGw3cqP=1OfDjePL+knsLR-=zjEDzN1g@mail.gmail.com>
Date:   Mon, 8 Nov 2021 09:38:18 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Michael Schmitz <schmitzmic@...il.com>
Cc:     "Linux/m68k" <linux-m68k@...r.kernel.org>,
        ALeX Kazik <alex@...ik.de>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v8 3/3] net/8390: apne.c - add 100 Mbit support to apne.c driver

Hi Michael,

On Thu, Nov 4, 2021 at 7:11 AM Michael Schmitz <schmitzmic@...il.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@...ik.de>.
>
> CC: netdev@...r.kernel.org
> Link: https://lore.kernel.org/r/1622958877-2026-1-git-send-email-schmitzmic@gmail.com
> Tested-by: Alex Kazik <alex@...ik.de>
> Signed-off-by: Michael Schmitz <schmitzmic@...il.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@...ux-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