| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 5 Mar 2015 19:33:58 +0800 From: Pi-Cheng Chen <pi-cheng.chen@...aro.org> To: Sascha Hauer <s.hauer@...gutronix.de> Cc: Mark Rutland <mark.rutland@....com>, James Liao <jamesjj.liao@...iatek.com>, Linaro Kernel Mailman List <linaro-kernel@...ts.linaro.org>, Mike Turquette <mturquette@...aro.org>, Pawel Moll <pawel.moll@....com>, Ian Campbell <ijc+devicetree@...lion.org.uk>, Viresh Kumar <viresh.kumar@...aro.org>, Stephen Boyd <sboyd@...eaurora.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Henry Chen <henryc.chen@...iatek.com>, Chen Fan <fan.chen@...iatek.com>, "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, Rob Herring <robh+dt@...nel.org>, linux-mediatek@...ts.infradead.org, Kumar Gala <galak@...eaurora.org>, Matthias Brugger <matthias.bgg@...il.com>, "Joe.C" <yingjoe.chen@...iatek.com>, Eddie Huang <eddie.huang@...iatek.com>, "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org> Subject: Re: [PATCH] clk: mediatek: Export CPU mux clocks for CPU frequency control On 5 March 2015 at 18:51, Sascha Hauer <s.hauer@...gutronix.de> wrote: > On Thu, Mar 05, 2015 at 05:39:12PM +0800, Pi-Cheng Chen wrote: >> On 5 March 2015 at 17:19, Sascha Hauer <s.hauer@...gutronix.de> wrote: >> > On Thu, Mar 05, 2015 at 02:29:50PM +0530, Viresh Kumar wrote: >> >> On 5 March 2015 at 13:12, Sascha Hauer <s.hauer@...gutronix.de> wrote: >> >> > We have clk_set_parent for changing the parent and clk_set_rate to >> >> > change the rate. Use the former for changing the parent and the latter >> >> > for changing the rate. What you are interested in is changing the >> >> > parent, so use clk_set_parent for this and not abuse a side effect >> >> > of clk_set_rate. >> >> >> >> clk_set_rate() for CPUs clock is responsible to change clock rate >> >> of the CPU. Whether it plays with PLLs or muxes, its not that relevant. >> > >> > The sequence to change the CPU frequency on the Mediatek SoCs is like this: >> > >> > - Change CPU from CPU PLL to another clock source (intermediate source) >> > - Change CPU PLL frequency >> > - wait until PLL has settled >> > - switch back to CPU PLL >> > >> > The frequency of the intermediate source is irrelevant, the important >> > thing is that the CPU is switched to this source while the CPU PLL is >> > reconfigured. >> > >> > In Pi-Chengs patches the switch to th eintermediate clock is done like: >> > >> > rate = clk_get_rate(intermediate_clk); >> > clk_set_rate(cpu_clk, rate); >> > >> > Now the clk framework does the switch not because it's told to switch >> > to another parent, but only because the other parent happens to be the >> > only provider for that rate. That's rubbish, when the parent must be >> > changed, then it should be done explicitly. >> >> Hi, >> >> Please correct me if I was wrong. But I think that's the reason why we have >> a determine_rate hook here, isn't it? >> >> > What if the CPU PLL and the intermediate clk happen to have the same >> > rate? Then the clk_set_rate above simply does nothing, no parent is >> > changed and the following rate change of the CPU PLL just crashes the >> > system. >> >> I think what I am trying to do in the determine_rate hook of cpumux is: >> >> * if the target rate of clk_set_rate is equal to the rate of MAINPLL, >> then switch >> the mux to MAINPLL >> * otherwise, set the rate directly on ARMPLL and switch the mux back to >> ARMPLL if the current parent of mux is not ARMPLL >> >> And if the CPU PLL and the intermediate clk happen to have the same rate, >> I think the cpufreq-dt driver will change nothing on the clk framework. Or do >> I misunderstand your point? > > Imagine the board starts with both the CPU PLL and Intermediate PLL > running with the same frequency with the CPU driven from the CPU PLL. > Now the cpufreq driver does it's very first frequency transition. Now > when clk_set_rate is used with the intention to switch to the > intermediate PLL nothing will happen because the CPU PLL already runs > at the desired frequency. Reconfiguring the CPU PLL then crashes your > system. > > Another reason why abusing clk_set_rate to change the parent is this: > You have this in your SoC: > > --- > CPU_PLL ---| | > | | > | |----- CPU > | | > IM_PLL ----| | > --- > > Now you do a clk_set_rate(CPU, clk_get_rate(IM_PLL)) which (often) works > in your case. Many SoCs have an additional divider though, like this: > > --- > CPU_PLL ---| | > | | ---- > | |--| :x | --- CPU > | | ---- > IM_PLL ----| | > --- > > Now clk_set_rate(CPU, clk_get_rate(IM_PLL)) doesn't give anything > sensible anymore. > > Given the variance of different SoCs I don't think it makes sense > to try to handle all these cases. Instead the cpufreq-dt driver > should just call clk_set_rate() on the CPU clock with the desired > target frequency. Everything else should be handled in the clock > driver which has intimate knowledge about the SoC anyway. Now I got it. Thanks for your elaboration. I will investigate the way you suggested to implement intermediate clock switching. Best Regards, Pi-Cheng > > Sascha > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists