Re: [PATCH] m68k/mac: Replace macide driver with generic platform driver

Michael Schmitz
2021-06-01T07:55:36Z

Hi Geert,

Am 01.06.2021 um 15:32 schrieb Michael Schmitz:
>> That's supposed to work, else our multi_defconfig is broken.
>
> Took a while to get back to this, but I believe unless this
> configuration sets CONFIG_ISA && !CONFIG_ATARI_ROM_ISA, Q40 ISA support
> will indeed be broken, due to
>
> #define inb(port) ((port) < 1024 ? isa_rom_inb(port) : in_8(port))
>
> using incorrect address translation on Q40 (either that for the Atari
> ROM port, or none)? Not entirely sure what I had been smoking when
> writing that ...
>
> Conversely, if that combination _is_ set, neither ROM port access nor
> IDE access on Atari will work (again due to incorrect address
> translation - indiscriminately applying the one required for the ROM
> port in that case, which is inappropriate for Atari IDE).
>
> I'm currently considering a fix that does conditional address
> translation in isa_itb() and its buddys, and replaces in_8() by
> isa_inb() in the definition above (and equivalent changes elsewhere).

Untested (and mangled formatting, sorry):

> diff --git a/arch/m68k/include/asm/io_mm.h b/arch/m68k/include/asm/io_mm.h
> index d41fa48..aaae12f 100644
> --- a/arch/m68k/include/asm/io_mm.h
> +++ b/arch/m68k/include/asm/io_mm.h
> @@ -135,7 +135,10 @@ static inline u8 __iomem *isa_itb(unsigned long addr)
> case ISA_TYPE_AG: return (u8 __iomem *)AG_ISA_IO_B(addr);
> #endif
> #ifdef CONFIG_ATARI_ROM_ISA
> - case ISA_TYPE_ENEC: return (u8 __iomem *)ENEC_ISA_IO_B(addr);
> #endif
> #ifdef CONFIG_ATARI_ROM_ISA
> - case ISA_TYPE_ENEC: return (u8 __iomem *)ENEC_ISA_IO_B(addr);
> + case ISA_TYPE_ENEC: if (addr < 1024)
> + return (u8 __iomem *)ENEC_ISA_IO_B(addr);
> + else
> + return (u8 __iomem *)(addr);
> #endif
> default: return NULL; /* avoid warnings, just in case */
> }
> @@ -151,7 +154,10 @@ static inline u16 __iomem *isa_itw(unsigned long addr)
> case ISA_TYPE_AG: return (u16 __iomem *)AG_ISA_IO_W(addr);
> #endif
> #ifdef CONFIG_ATARI_ROM_ISA
> - case ISA_TYPE_ENEC: return (u16 __iomem *)ENEC_ISA_IO_W(addr);
> + case ISA_TYPE_ENEC: if (addr < 1024)
> + return (u16 __iomem *)ENEC_ISA_IO_W(addr);
> + else
> + return (u16 __iomem *)(addr);
> #endif
> default: return NULL; /* avoid warnings, just in case */
> }
> @@ -177,7 +183,10 @@ static inline u8 __iomem *isa_mtb(unsigned long addr)
> case ISA_TYPE_AG: return (u8 __iomem *)addr;
> #endif
> #ifdef CONFIG_ATARI_ROM_ISA
> - case ISA_TYPE_ENEC: return (u8 __iomem *)ENEC_ISA_MEM_B(addr);
> + case ISA_TYPE_ENEC: if (addr < 1024)
> + return (u8 __iomem *)ENEC_ISA_MEM_B(addr);
> + else
> + return (u8 __iomem *)(addr);
> #endif
> default: return NULL; /* avoid warnings, just in case */
> }
> @@ -193,7 +202,10 @@ static inline u16 __iomem *isa_mtw(unsigned long addr)
> case ISA_TYPE_AG: return (u16 __iomem *)addr;
> #endif
> #ifdef CONFIG_ATARI_ROM_ISA
> - case ISA_TYPE_ENEC: return (u16 __iomem *)ENEC_ISA_MEM_W(addr);
> + case ISA_TYPE_ENEC: if (addr < 1024)
> + return (u16 __iomem *)ENEC_ISA_MEM_W(addr);
> + else
> + return (u16 __iomem *)(addr);
> #endif
> default: return NULL; /* avoid warnings, just in case */
> }
> @@ -344,17 +356,17 @@ static inline void isa_delay(void)
> * ROM port ISA and everything else regular ISA for IDE. read,write defined
> * below.
> */
> -#define inb(port) ((port) < 1024 ? isa_rom_inb(port) : in_8(port))
> -#define inb_p(port) ((port) < 1024 ? isa_rom_inb_p(port) : in_8(port))
> -#define inw(port) ((port) < 1024 ? isa_rom_inw(port) : in_le16(port))
> -#define inw_p(port) ((port) < 1024 ? isa_rom_inw_p(port) : in_le16(port))
> +#define inb(port) ((port) < 1024 ? isa_rom_inb(port) : isa_inb(port))
> +#define inb_p(port) ((port) < 1024 ? isa_rom_inb_p(port) : isa_inb_p(port))
> +#define inw(port) ((port) < 1024 ? isa_rom_inw(port) : isa_inw(port))
> +#define inw_p(port) ((port) < 1024 ? isa_rom_inw_p(port) : isa_inw_p(port))
> #define inl isa_inl
> #define inl_p isa_inl_p
>
> -#define outb(val, port) ((port) < 1024 ? isa_rom_outb((val), (port)) : out_8((port), (val)))
> -#define outb_p(val, port) ((port) < 1024 ? isa_rom_outb_p((val), (port)) : out_8((port), (val)))
> -#define outw(val, port) ((port) < 1024 ? isa_rom_outw((val), (port)) : out_le16((port), (val)))
> -#define outw_p(val, port) ((port) < 1024 ? isa_rom_outw_p((val), (port)) : out_le16((port), (val)))
> +#define outb(val, port) ((port) < 1024 ? isa_rom_outb((val), (port)) : isa_outb((val), (port)))
> +#define outb_p(val, port) ((port) < 1024 ? isa_rom_outb_p((val), (port)) : isa_outb_p((val), (port)))
> +#define outw(val, port) ((port) < 1024 ? isa_rom_outw((val), (port)) : isa_outw((val), (port)))
> +#define outw_p(val, port) ((port) < 1024 ? isa_rom_outw_p((val), (port)) : isa_outw_p((val), (port)))
> #define outl isa_outl
> #define outl_p isa_outl_p
>

Could that have any new side effects on multiplatform kernels (other
than addresses < 1024 still mangled by wrong translation and IO
primitive on non-Atari platforms)?

Cheers,

Michael