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

Geert Uytterhoeven
2021-11-18T08:01:45Z

Hi Michael,

On Thu, Nov 18, 2021 at 5:58 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> On 18/11/21 03:42, Geert Uytterhoeven wrote:
> > On Mon, Nov 15, 2021 at 12:40 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.
> >>
> >> 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@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>

> >> --- 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@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