Hi Jiaxun,
On Tue, Jan 14, 2025 at 12:32 AM Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> wrote:
> 在2025年1月13日一月 下午10:16，Mateusz Jończyk写道：
> > On Linux 6.13-rc6 for mipsel in QEMU on the Malta platform, the RTC CMOS
> > driver does not load and /sys/class/rtc is empty. I have tested this with
> > "make malta_defconfig", which compiles this driver into the kernel
> > (CONFIG_RTC_DRV_CMOS=y).
>
> Hi Mateusz,
>
> Thanks for tracking it down, this is indeed a huge effort.
>
> >
> > I have bisected this down to:
> >
> > commit 4bfb53e7d317c01f296b2feb2fae7c421c1d52dc
> > Author: Jiaxun Yang<jiaxun.yang@xxxxxxxxxxx>
> > Date: Thu Sep 21 19:04:22 2023 +0800
> >
> > mips: add <asm-generic/io.h> including
> > With the adding, some default ioremap_xx methods defined in
> > asm-generic/io.h can be used. E.g the default ioremap_uc() returning
> > NULL.
> > We also massaged various headers to avoid nested includes.
>
> #regzbot introduced: 4bfb53e7d317c01f296b2feb2fae7c421c1d52dc
>
> >
> > I have tried to debug this.
> >
> > The fallout is apparently limited to the CMOS RTC driver, other
> > drivers that access IO ports seem to function correctly (e.g. the
> > PATA driver). Also, the read_persistent_clock64 function in
> > arch/mips/mti-malta/malta-time.c, which accesses the same hardware
> > works correctly.
> >
> > The CMOS RTC driver is likely special because this device is defined
> > in a devicetree (arch/mips/boot/dts/mti/malta.dts) and there it is
> > the only defined device on the ISA bus.
> >
> > That driver fails to load because the call to
> >
> > platform_get_resource(pdev, IORESOURCE_IO, 0);
> >
> > in cmos_platform_probe in drivers/rtc/rtc-cmos.c returns NULL.
> >
> > The mediator seems to be that this bad commit causes
> > arch/mips/include/asm/io.h
> > to #include <asm-generic/io.h> at the end. As a side effect, this causes
> > the PCI_IOBASE macro to be defined:
> >
> > #ifndef PCI_IOBASE
> > #define PCI_IOBASE ((void __iomem *)0)
> > #endif
> >
> > That PCI_IOBASE value above is AFAIK incorrect for MIPS (it should be
> > defined to mips_io_port_base as far as I can tell), but this does not
> > matter much here.
>
> You are right, this is what should be done.
>
> A quick fix would be #undef PCI_IOBASE in arch/mips/include/asm/io.h
> just after including #include <asm-generic/io.h>, with ralink and loongson64
> as exception.
Shouldn't arch/mips/include/asm/io.h do
#define PCI_IOBASE mips_io_port_base
unconditionally, _before_ including <asm-generic/io.h>?
> In the long term, we should scrutinize platform usage of mips_io_base
> following ralink's approach.
Currently ralink handles that in a mach-specific include:
arch/mips/include/asm/mach-ralink/spaces.h:#define PCI_IOBASE
mips_io_port_base
Loongson does it differently:
arch/mips/include/asm/mach-loongson64/spaces.h:#define PCI_IOBASE
_AC(0xc000000000000000 + SZ_128K, UL)
But still sets mips_io_port_base in prom_init():
arch/mips/loongson64/init.c: set_io_port_base(PCI_IOBASE);
so defining PCI_IOBASE to mips_io_port_base in the main <asm/io.h>
should work.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
