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

Michael Schmitz
2021-11-09T19:00:17Z

Hi Geert,

On 09/11/21 20:55, Geert Uytterhoeven wrote:
>>>> --- 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?

Probably, but we'd still have the conflicts from including both
pcmcia/cistpl.h and asm/amipcmcia.h in apne.c.

>
>>
>>>
>>>> /* 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.
>

Got your meaning now - this can be unconditional now.

Cheers,

Michael

> 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
>