netdev - Re: [PATCH net v11 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: <CAMuHMdV5EJB-5F057sAdhAK-jFgyXT9j0UA8trKf+Lsj89K0wQ@mail.gmail.com>
Date:   Thu, 18 Nov 2021 09:01:29 +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 net v11 3/3] net/8390: apne.c - add 100 Mbit support to
 apne.c driver

Hi Michael,

On Thu, Nov 18, 2021 at 5:58 AM Michael Schmitz <schmitzmic@...il.com> wrote:
> On 18/11/21 03:42, Geert Uytterhoeven wrote:
> > On Mon, Nov 15, 2021 at 12:40 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.
> >>
> >> 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. The user must select PCCARD
> >> and PCMCIA in the kernel config to make the necessary support
> >> modules available
> >>
> >> 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>

> >> --- a/drivers/net/ethernet/8390/apne.c
> >> +++ b/drivers/net/ethernet/8390/apne.c
> >> @@ -119,6 +119,48 @@ 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 int apne_100_mbit = -1;
> >> +module_param_named(100_mbit, apne_100_mbit, int, 0444);
> >> +MODULE_PARM_DESC(100_mbit, "Enable 100 Mbit support");
> >> +
> >> +#if IS_ENABLED(CONFIG_PCMCIA)
> >
> > What if CONFIG_PCMIA=m, and CONFIG_APNE=y?
>
> Fails to build (undefined reference to `pcmcia_parse_tuple').
>
> That's what 'select PCMCIA' was avoiding before, but got vetoed. I can
> add a dependency on PCMCIA in the APNE Kconfig entry which does force
> APNE the same as what's selected for PCMCIA, but that means we can't
> build APNE without PCMCIA anymore. Is there a way to express 'constrain
> build type if PCMCIA is enabled, else leave choice to user' ??

#if IS_REACHABLE(CONFIG_PCMIA)

> >> @@ -140,6 +182,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 +216,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()) {
> >> +               pr_info("16-bit PCMCIA card detected!\n");
> >> +               isa_type = ISA_TYPE_AG16;
> >> +               apne_100_mbit = 1;
> >> +       }
> >
> > I think you should reset apne_100_mbit to zero if apne_100_mbit < 0
> > && !pcmcia_is_16bit(), so rmmod + switching card + modprobe
> > has a chance to work.
>
> Good catch - though when switching to another card using this same
> driver, the module parameter can be used again to select IO mode or
> force autoprobe.

The autoprobe won't work if the new card is 8-bit.

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