On 2020/11/13 17:11, Stephen Boyd wrote: > Quoting Damien Le Moal (2020-11-12 23:57:19) >> On 2020/11/13 16:53, Stephen Boyd wrote: >>> >>> Yes it's fine. Just the commit text reads as "If of_clk_init() is not >>> called in time_init() then nothing works" which is true but made me >>> wonder if it was because it needed to be super early or not. The commit >>> text could be a little clearer here. >> >> OK. I will clarify the commit message in V2. Working on it now. > > Thanks! > >> >>> We don't have any good solution for a fallback to call of_clk_init() >>> somewhere later. I do wonder if we should generalize this though and >>> call of_clk_init() from start_kernel() directly via some Kconfig that >>> architectures select if they need it for their timer and then move it to >>> an initcall if architectures don't select the config. Or throw it into >>> the of_platform_default_populate_init() hook if the architecture doesn't >>> need to call it early. >> >> This last idea seems reasonable and probably the easiest. And I think it could >> be done unconditionally even if the arch calls of_clk_init() early as the >> already populated clock provider nodes would not be initialized again. >> > > Also of_clk_init() is for the CLK_OF_DECLARE() users and if they can > wait to platform bus population time then they could be converted to > regular old platform device drivers. Maybe the problem is the clk driver > you're using is only using CLK_OF_DECLARE() and not registering a > platform driver? Yep, correct, that is what I did. SO yes, indeed, if I where to use a regular platform_driver, then the of_clk_init() change would not be needed. For the clock driver, I followed the pattern used by most other drivers with the majority I think using CLK_OF_DECLARE(). I did see some using the platform driver declaration though. I could spend time trying to figure out if I can get away without using CLK_OF_DECLARE(), but since I think other riscv board clock driver are arriving, we may as well keep that of_clk_init() change, no ? -- Damien Le Moal Western Digital Research
