Re: [PATCH net-next v5 2/2] net/8390: apne.c - add 100 Mbit support to apne.c driver

[Michael Schmitz <schmitzmic@gmail.com>]

Hi Geert,

thanks for your review!

Am 19.06.2021 um 21:08 schrieb Geert Uytterhoeven:
> Hi Michael,
>
> On Sat, Jun 19, 2021 at 2:35 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
>> Add Kconfig option, module parameter 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.
>> A module parameter switches Amiga ISA IO accessors to word
>> access by changing isa_type at runtime. Additional code to
>> reset the PCMCIA hardware is also added to the driver probe.
>>
>> 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>
>
> Thanks for your patch!
>
> Note that this patch has a hard dependency on "[PATCH v5 1/2] m68k:
> io_mm.h - add APNE 100 MBit support" in the series, so it must not
> be applied to the netdev tree yet.

Hmm - so we ought to protect the new code by

#ifdef ARCH_HAVE_16BIT_PCMCIA

and set that in the m68k machine Kconfig in the first patch?

(It's almost, but not quite like a config option :-)

>
>> --- a/drivers/net/ethernet/8390/Kconfig
>> +++ b/drivers/net/ethernet/8390/Kconfig
>> @@ -143,6 +143,10 @@ config APNE
>>           To compile this driver as a module, choose M here: the module
>>           will be called apne.
>>
>> +         The driver also supports 10/100Mbit cards (e.g. Netgear FA411,
>> +         CNet Singlepoint). To activate 100 Mbit support at runtime or
>> +         from the kernel command line, use the apne.100mbit module parameter.
>
> According to the recent discussion about that, "at runtime" is not
> really possible?  So that limits it to kernel command line (for the
> builtin case)
> or module parameter (for the modular case).

True - I'll reword that.

Cheers,

	Michael

>
>> +
>>  config PCMCIA_PCNET
>>         tristate "NE2000 compatible PCMCIA support"
>>         depends on PCMCIA
>> diff --git a/drivers/net/ethernet/8390/apne.c b/drivers/net/ethernet/8390/apne.c
>> index fe6c834..8223e15 100644
>> --- a/drivers/net/ethernet/8390/apne.c
>> +++ b/drivers/net/ethernet/8390/apne.c
>> @@ -120,6 +120,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 bool apne_100_mbit;
>> +module_param_named(100_mbit, apne_100_mbit, bool, 0644);
>> +MODULE_PARM_DESC(100_mbit, "Enable 100 Mbit support");
>
> Gr{oetje,eeting}s,
>
>                         Geert
>