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

[Michael Schmitz <schmitzmic@gmail.com>]

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