Re: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

On 14/08/2022 12:41, Conor.Dooley@microchip.com wrote:

quoted

Doesn't really matter since thats long enough to get past the switch
out of earlycon which is where the clang built kernel dies.

I had forgotten something obvious: keep_bootcon. With that, on the sbi
console I get a nice:

[    0.581754] Unable to handle kernel NULL pointer dereference at virtual address 00000000000000b1
[    0.591520] Oops [#1]
[    0.594045] Modules linked in:
[    0.597435] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.0.0-rc1-00011-g8e1459cf4eca #1
[    0.606188] Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
[    0.613012] epc : __clk_register+0x4a6/0x85c
[    0.617759]  ra : __clk_register+0x49e/0x85c
[    0.622489] epc : ffffffff803faf7c ra : ffffffff803faf74 sp : ffffffc80400b720
[    0.630466]  gp : ffffffff810e93f8 tp : ffffffe77fe60000 t0 : ffffffe77ffb3800
[    0.638443]  t1 : 000000000000000a t2 : ffffffffffffffff s0 : ffffffc80400b7c0
[    0.646420]  s1 : 0000000000000001 a0 : 0000000000000001 a1 : 0000000000000000
[    0.654396]  a2 : 0000000000000001 a3 : 0000000000000000 a4 : 0000000000000000
[    0.662373]  a5 : ffffffff803a5810 a6 : 0000000200000022 a7 : 0000000000000006
[    0.670350]  s2 : ffffffff81099d48 s3 : ffffffff80d6e28e s4 : 0000000000000028
[    0.678327]  s5 : ffffffff810ed3c8 s6 : ffffffff810ed3d0 s7 : ffffffe77ffbc100
[    0.686304]  s8 : ffffffe77ffb1540 s9 : ffffffe77ffb1540 s10: 0000000000000008
[    0.694281]  s11: 0000000000000000 t3 : 00000000000000c6 t4 : 0000000000000007
[    0.702258]  t5 : ffffffff810c78c0 t6 : ffffffe77ff88cd0
[    0.708125] status: 0000000200000120 badaddr: 00000000000000b1 cause: 000000000000000d
[    0.716869] [<ffffffff803fb892>] devm_clk_hw_register+0x62/0xaa
[    0.723420] [<ffffffff80403412>] mpfs_clk_probe+0x1e0/0x244
[    0.729592] [<ffffffff80457dea>] platform_probe+0x82/0xa6
[    0.735581] [<ffffffff8045593c>] call_driver_probe+0x22/0xa4
[    0.741848] [<ffffffff804557da>] really_probe+0x13a/0x27a
[    0.747819] [<ffffffff804549f8>] __driver_probe_device+0xc4/0xee
[    0.754460] [<ffffffff804554d0>] driver_probe_device+0x3c/0x196
[    0.761013] [<ffffffff804552cc>] __device_attach_driver+0xa2/0x18c
[    0.767853] [<ffffffff8045284a>] bus_for_each_drv+0x76/0xba
[    0.774016] [<ffffffff804547ca>] __device_attach+0xa0/0x15a
[    0.780179] [<ffffffff80454896>] device_initial_probe+0x12/0x1a
[    0.786732] [<ffffffff804529c6>] bus_probe_device+0x2e/0x7c
[    0.792895] [<ffffffff8044f290>] device_add+0x21a/0x3aa
[    0.798693] [<ffffffff805c323a>] of_device_add+0x28/0x32
[    0.804578] [<ffffffff805c3c54>] of_platform_device_create_pdata+0x88/0xb2
[    0.812181] [<ffffffff805c3e70>] of_platform_bus_create+0x14a/0x1a8
[    0.819117] [<ffffffff805c3ea6>] of_platform_bus_create+0x180/0x1a8
[    0.826053] [<ffffffff805c3f2a>] of_platform_populate+0x5c/0x96
[    0.832606] [<ffffffff8082bbaa>] of_platform_default_populate_init+0xc2/0xd4
[    0.840400] [<ffffffff800020e8>] do_one_initcall+0xbc/0x216
[    0.846563] [<ffffffff8080126c>] do_initcall_level+0x82/0x94
[    0.852830] [<ffffffff808011ae>] do_initcalls+0x5c/0x98
[    0.858611] [<ffffffff8080114a>] do_basic_setup+0x20/0x28
[    0.864583] [<ffffffff808010e0>] kernel_init_freeable+0xba/0x104
[    0.871223] [<ffffffff80746856>] kernel_init+0x22/0x1ba
[    0.877013] [<ffffffff80003700>] ret_from_exception+0x0/0xc
[    0.883218] ---[ end trace 0000000000000000 ]---

At least I had somewhere to start. Since QEMU repros, I continued there with:
	$(QEMU)/qemu-system-riscv64 -s -S -M microchip-icicle-kit \
		-m 2G -smp 5 \
		-kernel $(vmlinux_bin) \
		-dtb $(dtb) \
		-initrd $(initramfs) \
		-display none -serial null \
		-serial stdio

Dumping the backtrace at the devm_clk_hw_register() callsites shows that
it passes for:
- the main pll in mpfs_clk_register_mssplls()
- the "cfg" clocks in mpfs_clk_register_cfgs()
- the first 4 periph clocks...

It fails on "clk_periph_timer" - which uses a different parent, that it
tries to find using the macro:
#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT].cfg.hw)

If parent is RTCREF, that becomes: &mpfs_cfg_clks[33].cfg.hw

And yeah, thats well beyond the end of the array!

Booting with gcc, and looking at the clock summary:
  mssrefclk                            1        1        0   125000000          0     0  50000         Y
     clk_rtcref                        0        0        0     1000000          0     0  50000         Y
        clk_periph_timer               0        0        0     1000000          0     0  50000         N

This is reported correctly, so how this works for either gcc or clang now, or
gcc with the patches applied, I am not really too sure..

On v6.0-rc1, gcc and clang show:
  mssrefclk                            1        1        0   125000000          0     0  50000         Y
     clk_rtcref                        0        0        0     1000000          0     0  50000         Y

I guess this may fail gracefully as the pointer is actually null, but
the extra level of indirection added by my patch started tripping up
the error checking. I must just not have noticed because I added RTCREF
in a bit of a hurry trying to fix a bug in time for 5.18:
https://lore.kernel.org/linux-clk/986c73df-9634-d18b-eed3-37584fa2ea89@conchuod.ie/#t (local)

Obviously I've got to fix the bug itself now, but now I am left wondering
about GCC's behaviour rather than clang/llvm's!

Thanks Nathan - if even just for being a rubber duck :)

Conor.

quoted

Didn't get a chance to look at disassembly etc today, but as I said
last night it reproduces with GNU binutils.

Thanks,
Conor.

On another note, brought up our QEMU port's state today so fixing
it is now on the good ole, ever expanding todo list :)