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

[Michael Schmitz <schmitzmic@gmail.com>]

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.

Cheers,

	Michael

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