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

Geert Uytterhoeven
2021-11-09T07:55:51Z

Hi Michael,

On Mon, Nov 8, 2021 at 10:48 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
> On 08/11/21 21:35, Geert Uytterhoeven wrote:
> > On Thu, Nov 4, 2021 at 7:11 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> >> Add the interface definitions for 16 bit card autoprobing (using
> >> the generic PCMCIA cftable entry parser) to amipcmcia.h for use
> >> by apne.c.
> >>
> >> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
> >
> > Thanks for your patch!
> >
> >> --- a/arch/m68k/include/asm/amipcmcia.h
> >> +++ b/arch/m68k/include/asm/amipcmcia.h
> >> @@ -64,6 +64,7 @@ static inline void pcmcia_disable_irq(void)
> >> #define PCMCIA_SPEED_250NS 250
> >> #define PCMCIA_SPEED_720NS 720
> >>
> >> +#ifndef _LINUX_CISTPL_H
> >
> > This #ifndef is here to prevent conflicts with <pcmcia/cistpl.h>?
> > Why can't the definitions below be removed, and <pcmcia/cistpl.h>
> > always included instead?
>
> We'd have to include it everywhere amipcmcia.h is included (because of
> the pcmcia_parse_tuple() external declaration below). Didn't want to do
> that unless absolutely necessary, but if you don't mind that, I'll
> change this part.

That include can be avoided by providing forward declarations for
struct tuple_t and struct cisparse_t, and using the struct types
instead of the typedefs for the pcmcia_parse_tuple() parameters, right?

>
> >
> >> /* PCMCIA Tuple codes */
> >>
> >> #define CISTPL_NULL 0x00
> >> @@ -106,5 +107,8 @@ static inline void pcmcia_disable_irq(void)
> >> #define CISTPL_FUNCID_NETWORK 0x06
> >> #define CISTPL_FUNCID_AIMS 0x07
> >> #define CISTPL_FUNCID_SCSI 0x08
> >> +#else /* defined _LINUX_CISTPL_H */
> >> +extern int pcmcia_parse_tuple(tuple_t *cftuple, cisparse_t *cfparse);
> >
> > This can be unconditional?
>
> Brain fade on my part - this of course needs to be conditional on
> CONFIG_PCMCIA. not on _LINUX_CISTPL_H.

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