netdev - Re: [PATCH net v12 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] [day] [month] [year] [list]

Message-ID: <555cb10e-4613-f839-7fc8-7c23c2257f17@gmail.com>
Date:   Sat, 20 Nov 2021 08:31:26 +1300
From:   Michael Schmitz <schmitzmic@...il.com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Linux/m68k <linux-m68k@...r.kernel.org>,
        ALeX Kazik <alex@...ik.de>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net v12 3/3] net/8390: apne.c - add 100 Mbit support to
 apne.c driver

Hi Geert,

thanks for your review!

On 19/11/21 20:59, Geert Uytterhoeven wrote:
> Hi Michael,
>
> On Fri, Nov 19, 2021 at 7:06 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>
>>
>> --
>>
>> Changes from v11:
>>
>> Geert Uytterhoeven:
>> - use IS_REACHABLE() for PCMCIA dependent code
>> - use container_of() instead of cast in pcmcia_parse_tuple()
>>   call
>> - set isa_type and apne_100_mbit correctly if autoprobe fails
>> - reset isa_type and apne_100_mbit on module exit
>>
>> Joe Perches:
>> - use pr_debug and co. to avoid #ifdef DEBUG
>
> Thanks for the update!
>
>> --- a/drivers/net/ethernet/8390/Kconfig
>> +++ b/drivers/net/ethernet/8390/Kconfig
>> @@ -144,6 +144,14 @@ 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, use the kernel
>> +         option apne.100mbit=1 (builtin) at boot time, or the apne.100mbit
>> +         module parameter. The driver will attempt to autoprobe 100 Mbit
>> +         mode if the PCCARD and PCMCIA kernel configuration options are
>> +         selected, so this option may not be necessary. Use apne.100mbit=0
>> +         should autoprobe mis-detect a 100 Mbit card.
>
> 10 Mbit?

Mis-detect a 10 Mbit card as 100 Mbit would be more accurate. I'll add 
that.

>
> Sorry for reporting it only now. I had noticed in v11, but forgot to
> mention it during my review.

No matter - I'll resend with your reviewed-by added directly.

Cheers,

	Michael

> For the code:
> Reviewed-by: Geert Uytterhoeven <geert@...ux-m68k.org>
>
> 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
>