Re: [PATCH RFC 2/2] m68k: setup_mm.c: set isa_sex for Atari if ATARI_ROM_ISA not used

[Finn Thain <fthain@linux-m68k.org>]

On Wed, 2 Jun 2021, Michael Schmitz wrote:

> Hi Geert,
> 
> Am 02.06.2021 um 19:09 schrieb Geert Uytterhoeven:
> > Hi Michael,
> > 
> > On Wed, Jun 2, 2021 at 7:21 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> > > For multiplatform kernels where CONFIG_ATARI_ROM_ISA is not set,
> > > at least isa_sex must be set correctly to allow for correct I/O
> > > primitive selection in shared drivers.
> > > 
> > > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
> > 
> > Thanks for your patch!
> > 
> > 
> > > --- a/arch/m68k/kernel/setup_mm.c
> > > +++ b/arch/m68k/kernel/setup_mm.c
> > > @@ -386,6 +386,10 @@ void __init setup_arch(char **cmdline_p)
> > >                 isa_type = ISA_TYPE_ENEC;
> > >                 isa_sex = 0;
> > >         }
> > > +#else
> > > +       if (MACH_IS_ATARI) {
> > > +               isa_sex = 0;
> > 
> > I find it strange that you set isa_sex, but not isa_type?
> 
> Yes, as I said this is only to ensure isa_sex has the correct value (0) so
> isa_readw() resolves to in_le16() for the Atari IDE driver after the change at
> the end of the first patch. Might have been better to rather say
> 
> int isa_sex=0;
> 
> at the start of setup_mm.c ... or rely on that variable being initialized as
> zero anyway.
> 
> > However, this is inside the CONFIG_ISA && MULTI_ISA block, so what 
> > kind of ISA does this correspond to?
> 
> No ISA at all in that case, but since we have to route all readw() calls 
> through isa_readw() for the sake of generality if we want to support Q40 
> ISA and Atari MMIO in the same kernel image, we need to make sure 
> isa_readw() does the right thing on Atari if CONFIG_ISA is set.
> 
> I know io_mm.h says it's about various ISA bridges, but as I found out, 
> drivers using only MMIO are also affected by these definitions.
> 
> 'Maze' does not begin to describe it, 'mess' might be getting closer, 
> but I can't see how we would avoid use of definitions in io_mm.h by 
> non-ISA drivers.
> 

It seems that the need for explicit zero initialization has more to do 
with subtelty over in asm/io_mm.h than anything else. If so, it would be 
better to add commentary there than to add redundant code here.

> Cheers,
> 
> 	Michael
> 
> > 
> > > +       }
> > >  #endif
> > >  #endif
> > 
> > Gr{oetje,eeting}s,
> > 
> >                         Geert
> > 
>