| Message ID | c0838592-5613-660d-56fc-e7022a38a86d@free.fr (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 11/07/2017 13:09, Mason wrote: > On 11/07/2017 12:25, Viresh Kumar wrote: >> On 11-07-17, 11:27, Mason wrote: >>> On 29/06/2017 16:34, Viresh Kumar wrote: >>> >>>> On 29-06-17, 13:41, Mason wrote: >>>> >>>>> I'm on SoC B, where nominal/max freq is expected to be 1206 MHz. >>>>> So the OPPs in the DT are: >>>>> operating-points = <1206000 0 603000 0 402000 0 241200 0 134000 0>; >>>>> *But* FW changed the max freq behind my back, to 1215 MHz. >> >> What does this line mean really? Where is this frequency changed? >> In the OPP table in DT? > > I apologize for being unclear. > > What I meant is that the bootloader originally set the max frequency > to 1206 MHz. The OPP table in DTS was written based on that value. > > Later, someone changed the bootloader code to set a slightly higher > max frequency. When I flashed the new bootloader on my board, the > OPP table no longer matches the actual frequency. > > But I am not notified when bootloader authors change max frequencies, > which is why I wrote "changed the max freq behind my back". > > Again, sorry for the confusing statements. > > (The bootloader is not DT-aware, so it leaves the DT untouched.) I just realized that there's another unstated assumption. Since the bootloader is not DT-aware, the DTB is, in fact, appended to the kernel image. This is why it's possible to have "mismatching" kernel and bootloader. Regards.
On 11-07-17, 13:09, Mason wrote: > I apologize for being unclear. > > What I meant is that the bootloader originally set the max frequency > to 1206 MHz. The OPP table in DTS was written based on that value. > > Later, someone changed the bootloader code to set a slightly higher > max frequency. When I flashed the new bootloader on my board, the > OPP table no longer matches the actual frequency. > > But I am not notified when bootloader authors change max frequencies, > which is why I wrote "changed the max freq behind my back". > > Again, sorry for the confusing statements. > > (The bootloader is not DT-aware, so it leaves the DT untouched.) Here we go. Finally I have understood what the problem you are facing is :) And yes, it was really not clear to me until now. I though that someone just changed the max in DT and that's making things go bad :) Anyway, how does the bootloader control the max frequency? For the boards I worked on, its just a PLL that the kernel needs to set and kernel can choose to program it the way it wants to irrespective of the way bootloader has worked on it. > Maybe this is the real issue that needs to be addressed, > rather than the symptoms that turn up later because of > the root issue. Sure, I just need a bit more of input from you :)
On 12/07/2017 05:41, Viresh Kumar wrote: > On 11-07-17, 13:09, Mason wrote: > >> What I meant is that the bootloader originally set the max frequency >> to 1206 MHz. The OPP table in DTS was written based on that value. >> >> Later, someone changed the bootloader code to set a slightly higher >> max frequency. When I flashed the new bootloader on my board, the >> OPP table no longer matches the actual frequency. >> >> But I am not notified when bootloader authors change max frequencies, >> which is why I wrote "changed the max freq behind my back". >> >> (The bootloader is not DT-aware, so it leaves the DT untouched.) > > Here we go. Finally I have understood what the problem you are facing is :) > And yes, it was really not clear to me until now. I though that someone just > changed the max in DT and that's making things go bad :) > > Anyway, how does the bootloader control the max frequency? For the boards I > worked on, it's just a PLL that the kernel needs to set and kernel can choose to > program it the way it wants to irrespective of the way bootloader has worked on it. I would object to the characterization of "just a PLL" :-) The PLL outputs "garbage" before actually "locking" a target frequency. It is not possible for the CPU to blindly change the PLL settings, because that crashes the system. The bootloader implements the steps required to change said settings, so the strategy has been: have Linux use whatever PLL frequency the bootloader programs. Behind the PLL, there is a glitch-free divider, which is able to divide the PLL output without crashing the system. I've been using that divider for DFS. drivers/clk/clk-tango4.c Regards.
On 12-07-17, 11:58, Mason wrote: > I would object to the characterization of "just a PLL" :-) > > The PLL outputs "garbage" before actually "locking" a target > frequency. It is not possible for the CPU to blindly change > the PLL settings, because that crashes the system. > > The bootloader implements the steps required to change said > settings, so the strategy has been: have Linux use whatever > PLL frequency the bootloader programs. > > Behind the PLL, there is a glitch-free divider, which is able > to divide the PLL output without crashing the system. I've > been using that divider for DFS. > > drivers/clk/clk-tango4.c Okay, got it now. Yes, you *really* need to create these OPPs dynamically. I am convinced now :)
On 12/07/2017 12:09, Viresh Kumar wrote: > On 12-07-17, 11:58, Mason wrote: >> I would object to the characterization of "just a PLL" :-) >> >> The PLL outputs "garbage" before actually "locking" a target >> frequency. It is not possible for the CPU to blindly change >> the PLL settings, because that crashes the system. >> >> The bootloader implements the steps required to change said >> settings, so the strategy has been: have Linux use whatever >> PLL frequency the bootloader programs. >> >> Behind the PLL, there is a glitch-free divider, which is able >> to divide the PLL output without crashing the system. I've >> been using that divider for DFS. >> >> drivers/clk/clk-tango4.c > > Okay, got it now. > > Yes, you *really* need to create these OPPs dynamically. > I am convinced now :) I will test your patch on my 4.9 branch. Does tango-cpufreq.c look acceptable to you? (I am aware the patch needs work in the the Kconfig/Makefile part. That was quick and dirty.) Regards.
On 12-07-17, 13:25, Mason wrote: > I will test your patch on my 4.9 branch. > > Does tango-cpufreq.c look acceptable to you? > (I am aware the patch needs work in the the Kconfig/Makefile part. > That was quick and dirty.) Mostly yes. Just make sure everything is well handled when u send it.
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 6441dfda489f..53ec09eaa01c 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -603,6 +603,9 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) old_freq = clk_get_rate(clk); + printk("%s: target_freq=%lu freq=%lu old_freq=%lu\n", + __func__, target_freq, freq, old_freq); + /* Return early if nothing to do */ if (old_freq == freq) { dev_dbg(dev, "%s: old/new frequencies (%lu Hz) are same, nothing to do\n",