On Wed, Aug 14, 2019 at 11:10 PM Aaro Koskinen <aaro.koski...@iki.fi> wrote: > > Hi, > > On Thu, Aug 08, 2019 at 11:43:39PM +0200, Arnd Bergmann wrote: > > The omap1 clock driver now uses types and calling conventions > > that are compatible with the common clk core. > > > > Turn on CONFIG_COMMON_CLK and remove all the code that is > > now duplicated. > > > > Note: if this previous steps didn't already break it, this one > > most likely will, because the interfaces are very likely to > > have different semantics. > > QEMU SX1 board works up to this patch (the I/O virtual address change > included). With this patch, it seems to fail to allocate memory during > omap1_init_early() (the log is a bit messy as I extracted it using QEMU > memory dumping):
That sounds pretty good, I definitely did not expect this patch to work without first dealing with a few bugs, and it it did not break earlier, I'm willing to call that success ;-) Unfortunately, doing it in qemu does not guarantee that the clocks are set up right at this point: if any of the clocks are disabled when they should not be, qemu won't care as much as real hardware would. > swapper: page allocation failure: order:0, mode:0x0(), nodemask=(null) > CPU: 0 PID: 0 Comm: swapper Not tainted 5.3.0-rc4-sx1-los_80efa++ #1 > Hardware name: OMAP310 based Siemens SX1 > [<c000dc44>] (unwind_backtrace) from [<c000cb00>] (show_stack+0x10/0x18) > [<c000cb00>] (show_stack) from [<c0172ba8>] (dump_stack+0x18/0x24) > [<c0172ba8>] (dump_stack) from [<c00844e8>] (warn_alloc+0x90/0x140) > [<c00844e8>] (warn_alloc) from [<c0084dcc>] > (__alloc_pages_nodemask+0x7a4/0x9cc) > [<c0084dcc>] (__alloc_pages_nodemask) from [<c008df24>] > (slob_new_pages.constpro > p.2+0x10/0x3c) > [<c008df24>] (slob_new_pages.constprop.2) from [<c008e208>] > (slob_alloc.constprop.1+0xe4/0x1e8) > [<c008e208>] (slob_alloc.constprop.1) from [<c008e344>] (__kmalloc+0x38/0xb0) > [<c008e344>] (__kmalloc) from [<c0126514>] (__clk_register+0x20/0x62c) > [<c0126514>] (__clk_register) from [<c01f6614>] (omap1_clk_init+0x88/0x220) > [<c01f6614>] (omap1_clk_init) from [<c01f5820>] (omap1_init_early+0x20/0x30) > [<c01f5820>] (omap1_init_early) from [<c01f09e8>] (start_kernel+0x48/0x408) > [<c01f09e8>] (start_kernel) from [<00000000>] (0x0) > Clocks: ARM_SYSST: 0x003a DPLL_CTL: 0x2002 ARM_CKCTL: 0x3000 > Clocking rate (xtal/DPLL1/MPU): 12.0/12.0/0.0 MHz Ok, so here the problem is that we call the omap1_clk_init() function from setup_arch(), which is before we can even allocate memory with kmalloc. Most other machines do it from init_time(), which comes after the initialization of the memory allocator. Something like this would be needed: diff --git a/arch/arm/mach-omap1/io.c b/arch/arm/mach-omap1/io.c index b0465a956ea8..17ba8dfd8e19 100644 --- a/arch/arm/mach-omap1/io.c +++ b/arch/arm/mach-omap1/io.c @@ -125,9 +125,6 @@ void __init omap1_init_early(void) omap_writew(0x0, MPU_PUBLIC_TIPB_CNTL); omap_writew(0x0, MPU_PRIVATE_TIPB_CNTL); - /* Must init clocks early to assure that timer interrupt works - */ - omap1_clk_init(); omap1_mux_init(); } diff --git a/arch/arm/mach-omap1/time.c b/arch/arm/mach-omap1/time.c index 7cc1a968230e..4e5ddd1db429 100644 --- a/arch/arm/mach-omap1/time.c +++ b/arch/arm/mach-omap1/time.c @@ -228,6 +228,8 @@ static inline void omap_mpu_timer_init(void) */ void __init omap1_timer_init(void) { + omap1_clk_init(); + if (omap_32k_timer_init() != 0) omap_mpu_timer_init(); } but the removed comment up there makes me suspect that it introduces a different issue. > "8<--- cut here --- > "Unable to handle kernel NULL pointer dereference at virtual address 00000018 > "pgd = (ptrval) > "[00000018] *pgd=00000000 > Internal error: Oops: 5 [#1] ARM > CPU: 0 PID: 0 Comm: swapper Not tainted 5.3.0-rc4-sx1-los_80efa++ #1 > Hardware name: OMAP310 based Siemens SX1 > PC is at clk_hw_get_parent+0x4/0x14 > LR is at omap1_clk_enable+0xc/0xcc > OMAP310 based Siemens SX1 > [ 0.000000] free:0 free_pcp:0 free_cma:0 > pc : [<c0126cd0>] lr : [<c00128d4>] psr: 600001d3 > sp : c03aff88 ip : 00000000 fp : 00000000 > r10: 00000001 r9 : 54029252 r8 : 10000100 > r7 : c03b1000 r6 : 00002002 r5 : 0000003a r4 : c03b5444 > r3 : 00000000 r2 : c03b9818 r1 : ff03ce08 r0 : c03b5444 > Flags: nZCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment user > Control: 0000317f Table: 10004000 DAC: 00000055 > Process swapper (pid: 0, stack limit = 0x(ptrval)) > Stack: (0xc03aff88 to 0xc03b0000) > ff80: c03b5438 0000003a 00002002 c01f6734 00000000 00000057 > ffa0: 0000313d c01f5820 00000000 c01f09e8 00000000 00000000 00000000 00000000 > ffc0: 00000000 00000000 00000000 c0201a38 00000000 c01f0330 00000057 0000313d > ffe0: 00000265 10000100 54029252 0000317f 00000000 00000000 00000000 00000000 > [<c0126cd0>] (clk_hw_get_parent) from [<c00128d4>] (omap1_clk_enable+0xc/0xcc) > [<c00128d4>] (omap1_clk_enable) from [<c01f6734>] (omap1_clk_init+0x1a8/0x220) > [<c01f6734>] (omap1_clk_init) from [<c01f5820>] (omap1_init_early+0x20/0x30) > [<c01f5820>] (omap1_init_early) from [<c01f09e8>] (start_kernel+0x48/0x408) > [<c01f09e8>] (start_kernel) from [<00000000>] (0x0) clk_hw->core is NULL here, presumably as a result of the first issue. Arnd