From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754780AbbCEJUL (ORCPT ); Thu, 5 Mar 2015 04:20:11 -0500 Received: from metis.ext.pengutronix.de ([92.198.50.35]:38711 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754249AbbCEJUH (ORCPT ); Thu, 5 Mar 2015 04:20:07 -0500 Date: Thu, 5 Mar 2015 10:19:48 +0100 From: Sascha Hauer To: Viresh Kumar Cc: Pi-Cheng Chen , Mike Turquette , Stephen Boyd , Matthias Brugger , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Henry Chen , James Liao , Chen Fan , Eddie Huang , "Joe.C" , Linux Kernel Mailing List , "linux-arm-kernel@lists.infradead.org" , "devicetree@vger.kernel.org" , Linaro Kernel Mailman List , linux-mediatek@lists.infradead.org Subject: Re: [PATCH] clk: mediatek: Export CPU mux clocks for CPU frequency control Message-ID: <20150305091948.GH11010@pengutronix.de> References: <1425466152-7867-1-git-send-email-pi-cheng.chen@linaro.org> <20150304112109.GB11010@pengutronix.de> <20150305074207.GC11010@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 10:06:41 up 4 days, 21:20, 54 users, load average: 0.01, 0.13, 0.14 User-Agent: Mutt/1.5.21 (2010-09-15) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: sha@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 05, 2015 at 02:29:50PM +0530, Viresh Kumar wrote: > On 5 March 2015 at 13:12, Sascha Hauer 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. 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. > > > My suggestion is to take another approach. Implement clk_set_rate for > > these muxes and in the set_rate hook: > > > > - switch mux to intermediate PLL parent > > - call clk_set_rate() for the real parent PLL > > - switch mux back to real parent PLL > > > > This way the things happening behind the scenes are completely transparent > > to the cpufreq driver and you can use cpufreq-dt as is without changes. > > CPUFreq wants to change to intermediate frequency by itself against > some magic change behind the scene. The major requirement for that > comes from the fact that we want to send PRE/POST freq notifiers on > which loops-per-jiffie depends. Maybe, I don't know the internals of CPUFreq. But here the important thing is the rate change, not the parent change. 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 |