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

Michael Schmitz
2021-11-08T21:22:42Z

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