All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Schmitz <schmitzmic@gmail.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Finn Thain <fthain@linux-m68k.org>,
	Linux/m68k <linux-m68k@vger.kernel.org>,
	ALeX Kazik <alex@kazik.de>, netdev <netdev@vger.kernel.org>
Subject: Re: [PATCH net-next v3 2/2] net/8390: apne.c - add 100 Mbit support to apne.c driver
Date: Fri, 18 Jun 2021 20:28:42 +1200	[thread overview]
Message-ID: <9faae8bf-74c2-52e8-ce3d-9abef34412e4@gmail.com> (raw)
In-Reply-To: <CAMuHMdV5Yd2w+maSn-dQ=NOrVyVc8JjV38miKRc-pvnzBcKSig@mail.gmail.com>

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
>

      reply	other threads:[~2021-06-18  8:28 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-17  5:28 [PATCH v3 0/2] Add APNE PCMCIA 100 Mbit support Michael Schmitz
2021-06-17  5:28 ` [PATCH v3 1/2] m68k: io_mm.h - add APNE 100 MBit support Michael Schmitz
2021-06-17  6:58   ` Finn Thain
2021-06-17 21:42     ` Michael Schmitz
2021-06-17  5:28 ` [PATCH net-next v3 2/2] net/8390: apne.c - add 100 Mbit support to apne.c driver Michael Schmitz
2021-06-17  6:51   ` Finn Thain
2021-06-17 19:33     ` Michael Schmitz
2021-06-18  7:16       ` Geert Uytterhoeven
2021-06-18  8:06         ` Michael Schmitz
2021-06-18  8:13           ` Geert Uytterhoeven
2021-06-18  8:28             ` Michael Schmitz [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9faae8bf-74c2-52e8-ce3d-9abef34412e4@gmail.com \
    --to=schmitzmic@gmail.com \
    --cc=alex@kazik.de \
    --cc=fthain@linux-m68k.org \
    --cc=geert@linux-m68k.org \
    --cc=linux-m68k@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.