Re: [PATCH 1/2] clocksource/drivers/ostm: Delay driver registration

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 11/09/2018 20:42, Chris Brandt wrote:
> On Tuesday, September 11, 2018 1, Rob Herring wrote:
>> Well before we get to initcalls, the kernel calls the arch specific
>> time_init() which (on ARM) calls of_clk_init (for all the reasons
>> above) and then timer_probe(). When timer_probe returns, it is
>> expected that you have setup a clocksource and clockevent. If you
>> haven't, then at some point before we get to initcalls we should hang
>> because we're not getting any timer interrupts and time is not
>> advancing.
> 
> What I get now is:
> 
> [    0.000000] timer_probe: no matching timers found
> ...
> ...
>  [    0.000000] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
> ...
> ...
> 
> 
> But then later on in boot, I'll get (with my subsys_initcall timer fix)
> 
> ...
> ...
> [    0.000000] SCSI subsystem initialized
> [    0.000000] usbcore: registered new interface driver usbfs
> [    0.000000] usbcore: registered new interface driver hub
> [    0.000000] usbcore: registered new device driver usb
> [    0.000000] clocksource: ostm: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 28958491609 ns
> [    0.000619] sched_clock: 32 bits at 66MHz, resolution 15ns, wraps every 32537631224ns
> [    0.008599] ostm: used for clocksource
> [    0.018926] ostm: used for clock events
> [    0.133339] clocksource: Switched to clocksource ostm
> [    0.821474] NET: Registered protocol family 2
> [    0.840624] tcp_listen_portaddr_hash hash table entries: 512 (order: 0, 4096 bytes)
> [    0.850549] TCP established hash table entries: 1024 (order: 0, 4096 bytes)
> ...
> ...
> 
> 
> 
>> Maybe you
>> just get lucky and it works as long as no thread blocks (e.g. on a
>> msleep).
> 
> You're right. If I put in a msleep() before my timer is up and running, it hangs.
> 
> As far as I can tell, nothing before device_initcall seems to call anything like msleep.
> 
>> If things changed and you can setup a timer in an initcall,
>> then why are folks still trying to do things like early platform
>> drivers. Regular drivers would work and we should be able to
>> completely remove CLK_OF_DECLARE and TIMER_OF_DECLARE.
> 
> I wonder if the reason is because you can't assign a priority to your 
> driver when you declare it with xxx_initcall( ). So, your driver ends up 
> in the same table as all the other drivers and you are not guaranteed the
> order in which they probe. So, the answer was to make a NEW table and 
> register it using TIMER_OF_DECLARE and CLOCK_OF_DECLARE.
> 
> I wonder they just didn't make a clock_initcall() and timer_initcall() 
> instead.

What happens if you place the clk_init() before board_time_init() ? in
arch/sh/kernel/time.c

void __init time_init(void)
{
        if (board_time_init)
                board_time_init();

        clk_init();

        late_time_init = sh_late_time_init;
}



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog