Re: [PATCH RFC 2/2] m68k: setup_mm.c: set isa_sex for Atari if ATARI_ROM_ISA not used
From: Finn Thain <fthain@linux-m68k.org>
Date: 2021-06-03 08:29:09
Also in:
linux-m68k
On Wed, 2 Jun 2021, Michael Schmitz wrote:
Hi Geert, Am 02.06.2021 um 19:09 schrieb Geert Uytterhoeven:quoted
Hi Michael, On Wed, Jun 2, 2021 at 7:21 AM Michael Schmitz [off-list ref] wrote:quoted
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!quoted
--- 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.quoted
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, Michaelquoted
quoted
+ } #endif #endifGr{oetje,eeting}s, Geert