Re: [PATCH v11 1/3] m68k: amiga/pcmcia - add 16 bit detection interface to amipcmcia.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Michael,

On Thu, Nov 18, 2021 at 5:58 AM Michael Schmitz <schmitzmic@xxxxxxxxx> wrote:
On 18/11/21 03:35, Geert Uytterhoeven wrote:
On Wed, Nov 17, 2021 at 8:15 AM Michael Schmitz <schmitzmic@xxxxxxxxx> wrote:
On 15/11/21 22:59, Geert Uytterhoeven wrote:
On Mon, Nov 15, 2021 at 12:40 AM Michael Schmitz <schmitzmic@xxxxxxxxx> wrote:
Add the interface definition for 16 bit card autoprobing (using
the generic PCMCIA cftable entry parser) to amipcmcia.h for use
by apne.c. Include necessary cftable definitions from cistpl.h,
remove locally duplicate definitions from amipcmcia.h.

Signed-off-by: Michael Schmitz <schmitzmic@xxxxxxxxx>

Reviewed-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>

I may have to give this another spin, in order to address a review
comment by Denis about making the apne.c 16 bit IO autoprobe module
parameter ('100mbit') a sysfs parameter.

I suggest to move that parameter to arch/m68k/amiga/pcmcia.c to ensure
it's always visible, and rename it to better reflect its actual function
(pcmcia_16bit comes to mind). apne.c would then use that parameter to
autoprobe 16 bit IO support, or set isa_type according to the parameter.

Would that satisfy your concerns, Denis?

What's the point of the sysfs parameter, given the driver cannot be
unbound and rebound, as it does not use the driver model yet?

Pre-setting the desired IO type before loading the module - though
actual setting of isa_type still has to wait until module load (or
driver probe). I'd have to write a callback that validates user input if
we want to set isa_type directly in pcmcia.c ...

IMHO the sysfs parameter would only make sense if all of the 8/16-bit
handling (and the special reset) is done in arch/m68k/amiga/pcmcia.c,
so it can be shared for multiple cards.

The only thing you can (try to) do is unload and reload the module,
at which point you can pass the module parameter again?

I take that as 'don't do that', then ... ?

I meant you can do the above without the sysfs parameter.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux