Re: [PATCH v3 1/2] m68k: io_mm.h - add APNE 100 MBit support

Michael Schmitz
2021-06-17T21:42:21Z

Hi Finn,

thanks for your comments!

On 17/06/21 6:58 pm, Finn Thain wrote:
>> -#define isa_inb(port) in_8(isa_itb(port))
>> #define isa_inw(port) (ISA_SEX ? in_be16(isa_itw(port)) : in_le16(isa_itw(port)))
>> #define isa_inl(port) (ISA_SEX ? in_be32(isa_itl(port)) : in_le32(isa_itl(port)))
>> #define isa_outb(val,port) out_8(isa_itb(port),(val))
>> #define isa_outw(val,port) (ISA_SEX ? out_be16(isa_itw(port),(val)) : out_le16(isa_itw(port),(val)))
>> #define isa_outl(val,port) (ISA_SEX ? out_be32(isa_itl(port),(val)) : out_le32(isa_itl(port),(val)))
>>
>> +#define isa_inb(port) ((ISA_TYPE == ISA_TYPE_AG16) ? ((port) & 1 ? isa_inw((port) - 1) & 0xff : isa_inw(port) >> 8) : in_8(isa_itb(port)))
>> +
>> #define isa_readb(p) in_8(isa_mtb((unsigned long)(p)))
>> #define isa_readw(p) \
>> (ISA_SEX ? in_be16(isa_mtw((unsigned long)(p))) \
> Was the re-ordering of definitions deliberate?

Yes, it was. I remembered looking at a bug report for curl on the m68k
kernel that was due to ordering of #defines.

On second thought, there is no prior definition of isa_inw in our case,
so even this won't really be an issue.

>
>
>> @@ -270,6 +291,9 @@ static inline void isa_delay(void)
>> case ISA_TYPE_Q40: isa_outb(0,0x80); break;
>> #endif
>> #ifdef CONFIG_AMIGA_PCMCIA
>> +#ifdef CONFIG_APNE100MBIT
>> + case ISA_TYPE_AG16: break;
>> +#endif
>> case ISA_TYPE_AG: break;
>> #endif
>> #ifdef CONFIG_ATARI_ROM_ISA
>>
> I think most of these "#ifdef CONFIG_APNE100MBIT" conditionals are
> redundant. case ISA_TYPE_AG16 should be optimized away as dead code in the
> MULTI_ISA == 0 configuration. And in the MULTI_ISA == 1 configuration, the
> logic used to assign isa_type already depends on
> defined(CONFIG_APNE100MBIT).

You're right there, too. I'll drop those.

Cheers,

Michael