From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1754984AbbCEKXj (ORCPT <rfc822;w@1wt.eu>);
	Thu, 5 Mar 2015 05:23:39 -0500
Received: from mail-oi0-f48.google.com ([209.85.218.48]:38691 "EHLO
	mail-oi0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1754944AbbCEKXg (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 5 Mar 2015 05:23:36 -0500
MIME-Version: 1.0
In-Reply-To: <20150305091948.GH11010@pengutronix.de>
References: <1425466152-7867-1-git-send-email-pi-cheng.chen@linaro.org>
	<20150304112109.GB11010@pengutronix.de>
	<CALx668WKKmifhuODsV+Uaoq4ggcmB1=wbpLzXBnjEtxbuB4qig@mail.gmail.com>
	<20150305074207.GC11010@pengutronix.de>
	<CAKohponSqLPcULA9-T8y9b_b44Torti0p6W3sJfRZ1oKUWqtTw@mail.gmail.com>
	<20150305091948.GH11010@pengutronix.de>
Date: Thu, 5 Mar 2015 15:53:35 +0530
Message-ID: <CAKohpokVxxxTY9DpQVxNPnq92pyK=2J2dDeLmis_+WD16aatOg@mail.gmail.com>
Subject: Re: [PATCH] clk: mediatek: Export CPU mux clocks for CPU frequency control
From: Viresh Kumar <viresh.kumar@linaro.org>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pi-Cheng Chen <pi-cheng.chen@linaro.org>,
        Mike Turquette <mturquette@linaro.org>,
        Stephen Boyd <sboyd@codeaurora.org>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Ian Campbell <ijc+devicetree@hellion.org.uk>,
        Kumar Gala <galak@codeaurora.org>,
        Henry Chen <henryc.chen@mediatek.com>,
        James Liao <jamesjj.liao@mediatek.com>,
        Chen Fan <fan.chen@mediatek.com>,
        Eddie Huang <eddie.huang@mediatek.com>,
        "Joe.C" <yingjoe.chen@mediatek.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        Linaro Kernel Mailman List <linaro-kernel@lists.linaro.org>,
        linux-mediatek@lists.infradead.org
Content-Type: text/plain; charset=UTF-8
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 5 March 2015 at 14:49, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 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

This should be the case for most of the intermediate-freq users..

> 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.

Right.

> 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.

The problem is that the code is common across platforms that need to
reparent or just change rate for intermediate clocks. And the best we
can do is clk_set_rate() and so probably the clk driver need to take care
of this somehow and make sure we don't result in a crash like you just
demonstrated.