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

Michael Schmitz
2021-06-18T08:28:50Z

Hi Geert,

Am 18.06.2021 um 20:13 schrieb Geert Uytterhoeven:
>>
>> How does re-probing after a card change for a builtin driver work?
>> Changing the permission bits is a minor issue.
>
> Oh right, this driver predates the driver framework, and doesn't support
> PCMCIA hotplug. So auto-unregister on removal doesn't work.
> Even using unbind/bind in sysfs won't work.
>
> So rmmod/modprobe is the only thing that has a chance to work...
>
>>>> The comment there says isa_type must be set as early as possible, so I'd
>>>> rather leave that alone, and add an 'else' clause here.
>>>>
>>>> This of course raise the question whether we ought to move the entire
>>>> isa_type handling into arch code instead - make it a generic
>>>> amiga_pcmcia_16bit option settable via sysfs. There may be other 16 bit
>>>> cards that require the same treatment, and duplicating PCMCIA mode
>>>> switching all over the place could be avoided. Opinions?
>>>
>>> Indeed.
>>
>> The only downside I can see is that setting isa_type needs to be done
>> ahead of modprobe, through sysfs. That might be a little error prone.
>>
>>> Still, can we autodetect in the driver?
>>
>> Guess we'll have to find out how the 16 bit cards behave if first poked
>> in 8 bit mode, attempting to force a reset of the 8390 chip, and
>> switching to 16 bit mode if this fails. That's normally done in
>> apne_probe1() which runs after init_pcmcia(), so we can't rely on the
>> result of a 8390 reset autoprobe to do the PCMCIA software reset there.
>>
>> The 8390 reset part does not rely on anything else in apne_probe1(), so
>> that code can be lifted out of apne_probe1() and run early in
>> apne_probe() (after the check for an inserted PCMCIA card). I'll try and
>> prepare a patch for Alex to test that method.

I just realized that might not work - ínit_pcmcia() enables the PCMCIA
interface for us, so the early 8390 reset may not go through at all...
The module parameter may have to stay as a fallback option at least.

>>
>>> I'm wondering how this is handled on PCs with PCMCIA, or if there
>>> really is something special about Amiga PCMCIA hardware...
>>
>> What's special about Amiga PCMCIA hardware is that the card reset isn't
>> connected for those 16 bit cards, so pcmcia_reset() does not work.
>
> I was mostly thinking about the difference between 8-bit and 16-bit
> accesses.

No idea. I've never even seen an 8 bit PCMCIA card - I have a few old
16/32 bit ones around that were great for crashing my PowerBook, nothing
else...

>>> And I'd really like to get rid of the CONFIG_APNE100MBIT option,
>>> i.e. always include the support, if possible.
>>
>> I can't see why that wouldn't be possible - the only downside is that we
>> force MULTI_ISA=1 always for Amiga, and lose the optimizations done for
>> MUTLI_ISA=0 in io_mm.h. Unless we autoprobe, we can use isa_type to
>> guard against running a software reset on 8 bit cards ...
>
> The latter sounds like a neat trick...

Yes, we can at least get rid of the APNE100MBIT option that way. I'll
have to think about the autoprobe a bit more.

Cheers,

Michael

> Gr{oetje,eeting}s,
>
> Geert
>