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] [day] [month] [year] [list]

Message-ID: <7f11aeef-f3f5-d00c-d220-4417bd1959d0@gmail.com>
Date:   Tue, 9 Nov 2021 10:22:24 +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>,
        Denis Kirjanov <dkirjanov@...e.de>
Subject: Re: [PATCH v8 3/3] net/8390: apne.c - add 100 Mbit support to apne.c
 driver

Hi Geert,

On 08/11/21 21:38, Geert Uytterhoeven wrote:
>> --- 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...

Yes, and I had certainly done that, for the express purpose of allowing 
the negative ...

No idea how that one snuck back in, I'll fix that.

Denis suggested this be changed to a sysfs parameter, but neither this 
driver nor the Amiga PCMCIA support have been converted to the driver 
framework. I couldn't see a way to add a sysfs parameter, so I'd rather 
keep this as kernel command line or module parameter.

Cheers,

	Michael


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