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

Geert Uytterhoeven
2021-06-01T08:43:16Z

Hi Michael,

On Tue, Jun 1, 2021 at 9:55 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> 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

Bogus hunk?

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

Sorry, being an Amiga MMIO user, I don't know my way through the
maze of ISA I/O accessors...

> than addresses < 1024 still mangled by wrong translation and IO
> primitive on non-Atari platforms)?

Hence q40ide is still broken on Q40 in multi-platform kernels?

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