mbox series

[v2,0/2] Add APNE PCMCIA 100 Mbit support

Message ID 1623290953-18000-1-git-send-email-schmitzmic@gmail.com
Headers show
Series Add APNE PCMCIA 100 Mbit support | expand

Message

Michael Schmitz June 10, 2021, 2:09 a.m. UTC 
Now that we've sanitzed io_mm.h, revisit support for the 100 Mbit APNE
PCMCIA card variants that didn't find favour with the netdev folks three
years ago.

Add another ISA type for these cards, and switch to that type from the
APNE probe code in response to a module parameter. 

I've addressed review comments received so far, to the best of my ability.
Patch 2 not changed in v2, only comments on patch 1 by Andreas Schwab were
addressed.

Compile tested only - hopefully someone will be encouraged to give this
a try before I submit the apne.c patch to netdev. 

Cheers,

   Michael

Comments

ALeX Kazik June 16, 2021, 9:11 p.m. UTC | #1 
Hi,

I've tested the patches and they work.

At first I got it only to work with the following line removed/commented out:
  if (apne_100_mbit)
And thus enabling the following line always.

I've changed, with the help of Michael, the parameters to:

  static u32 apne_100_mbit = 0;
  module_param_named(100_mbit, apne_100_mbit, uint, 0644);
  MODULE_PARM_DESC(100_mbit, "Enable 100 Mbit support");

And was able to enable it with the kernel option "apne.100_mbit=1".

It's also available as /sys/module/apne/parameters/100_mbit

The 0644 is the permission (root can change it), If it shouldn't be
changed at runtime 0444 or 0 would be used.

(I think there is also a bool option instead of the uint, but I'm glad
it works like this.)

Alex.
Michael Schmitz June 17, 2021, 1:10 a.m. UTC | #2 
Hi Alex,

Thanks for testing!

On 17/06/21 9:11 am, ALeX Kazik wrote:
> Hi,
>
> I've tested the patches and they work.
>
> At first I got it only to work with the following line removed/commented out:
>    if (apne_100_mbit)
> And thus enabling the following line always.
>
> I've changed, with the help of Michael, the parameters to:
>
>    static u32 apne_100_mbit = 0;
>    module_param_named(100_mbit, apne_100_mbit, uint, 0644);
>    MODULE_PARM_DESC(100_mbit, "Enable 100 Mbit support");
>
> And was able to enable it with the kernel option "apne.100_mbit=1".
>
> It's also available as /sys/module/apne/parameters/100_mbit
>
> The 0644 is the permission (root can change it), If it shouldn't be
> changed at runtime 0444 or 0 would be used.
Changing that parameter at runtime won't do anything (it's only tested 
at device probe time). But I'd better change the permission to 444.
> (I think there is also a bool option instead of the uint, but I'm glad
> it works like this.)

True, that would be the better option indeed. I'll change that in the 
next version (which will go to netdev as well).

Cheers,

     Michael


>
> Alex.