LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jerome Brunet <jbrunet@baylibre.com>
To: Stephen Boyd <sboyd@codeaurora.org>,
	Michael Turquette <mturquette@baylibre.com>
Cc: linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
	Russell King <linux@armlinux.org.uk>,
	Linus Walleij <linus.walleij@linaro.org>,
	Quentin Schulz <quentin.schulz@free-electrons.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Maxime Ripard <maxime.ripard@free-electrons.com>
Subject: Re: [PATCH v5 08/10] clk: fix CLK_SET_RATE_GATE with clock rate protection
Date: Fri, 30 Mar 2018 10:20:50 +0200	[thread overview]
Message-ID: <1522398050.3871.12.camel@baylibre.com> (raw)
In-Reply-To: <20171201215200.23523-9-jbrunet@baylibre.com>

On Fri, 2017-12-01 at 22:51 +0100, Jerome Brunet wrote:
> Using clock rate protection, we can now enforce CLK_SET_RATE_GATE along the
> clock tree

Hi Mike, Stephen,

Gentle ping.
This patch has been waiting for while now.
As far as I know, the only blocking point to merging this patch is the qcom mmc
driver. The clocks used in this driver rely on the broken implementation of
CLK_SET_RATE_GATE, effectively ignoring the flag.

Since the flag is ignored, removing it from this particular clock won't make any
difference. Can we just do that until a better fix is available for this qcom
mmc clock ?

--- A bit of history ----

I managed to have a run on kci - based on v4.12-rc6:
https://kernelci.org/boot/all/job/khilman/branch/to-build/kernel/v4.12-rc6-10-ge
a373ddef830/

There was no build regression but kci did find one boot regression on qcom
platforms:
* qcom-apq8064-cm-qs600
* qcom-apq8064-ifc6410

it seems the problem is coming from the clock used by the mmc driver
(drivers/mmc/host/mmci.c)

the driver does following sequence:
* get_clk
* prepare_enable
* get_rate
* set_rate
* ...

with clock SDCx_clk (qcom_apq8064.dtsi:1037). This clock has CLK_SET_RATE_PARENT
flag so it will transmit the request to its parent.
The parent of this clock is SDCx_src which has the CLK_SET_RATE_GATE flag.

So obviously, is CLK_SET_RATE_GATE was enforced, the sequence used by mmci
driver would never had worked.

This particular driver rely on the fact that the clock can while the clock is
on. Either it actually works and we can remove CLK_SET_RATE_GATE until a better
solution comes.
OR, it does not work which means nobody has been using this driver for a long
time.

> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Tested-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> Tested-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Acked-by: Michael Turquette <mturquette@baylibre.com>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  drivers/clk/clk.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index f6fe5e5595ca..1af843ae20ff 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -605,6 +605,9 @@ static void clk_core_unprepare(struct clk_core *core)
>  	if (WARN_ON(core->prepare_count == 1 && core->flags & CLK_IS_CRITICAL))
>  		return;
>  
> +	if (core->flags & CLK_SET_RATE_GATE)
> +		clk_core_rate_unprotect(core);
> +
>  	if (--core->prepare_count > 0)
>  		return;
>  
> @@ -679,6 +682,16 @@ static int clk_core_prepare(struct clk_core *core)
>  
>  	core->prepare_count++;
>  
> +	/*
> +	 * CLK_SET_RATE_GATE is a special case of clock protection
> +	 * Instead of a consumer claiming exclusive rate control, it is
> +	 * actually the provider which prevents any consumer from making any
> +	 * operation which could result in a rate change or rate glitch while
> +	 * the clock is prepared.
> +	 */
> +	if (core->flags & CLK_SET_RATE_GATE)
> +		clk_core_rate_protect(core);
> +
>  	return 0;
>  unprepare:
>  	clk_core_unprepare(core->parent);
> @@ -1780,9 +1793,6 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
>  	if (clk_core_rate_is_protected(core))
>  		return -EBUSY;
>  
> -	if ((core->flags & CLK_SET_RATE_GATE) && core->prepare_count)
> -		return -EBUSY;
> -
>  	/* calculate new rates and get the topmost changed clock */
>  	top = clk_calc_new_rates(core, req_rate);
>  	if (!top)

  reply	other threads:[~2018-03-30  8:20 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-01 21:51 [PATCH v5 00/10] clk: implement clock rate protection mechanism Jerome Brunet
2017-12-01 21:51 ` [PATCH v5 01/10] clk: fix incorrect usage of ENOSYS Jerome Brunet
2017-12-01 21:51 ` [PATCH v5 02/10] clk: take the prepare lock out of clk_core_set_parent Jerome Brunet
2017-12-01 21:51 ` [PATCH v5 03/10] clk: add clk_core_set_phase_nolock function Jerome Brunet
2017-12-01 21:51 ` [PATCH v5 04/10] clk: rework calls to round and determine rate callbacks Jerome Brunet
2017-12-01 21:51 ` [PATCH v5 05/10] clk: use round rate to bail out early in set_rate Jerome Brunet
2017-12-01 21:51 ` [PATCH v5 06/10] clk: add clock protection mechanism to clk core Jerome Brunet
2017-12-01 21:51 ` [PATCH v5 07/10] clk: cosmetic changes to clk_summary debugfs entry Jerome Brunet
2017-12-01 21:51 ` [PATCH v5 08/10] clk: fix CLK_SET_RATE_GATE with clock rate protection Jerome Brunet
2018-03-30  8:20   ` Jerome Brunet [this message]
2017-12-01 21:51 ` [PATCH v5 09/10] clk: add clk_rate_exclusive api Jerome Brunet
2017-12-01 21:52 ` [PATCH v5 10/10] clk: fix set_rate_range when current rate is out of range Jerome Brunet
2017-12-20  0:38 ` [PATCH v5 00/10] clk: implement clock rate protection mechanism Michael Turquette
2017-12-20 17:45   ` Jerome Brunet
2017-12-22  2:15   ` Stephen Boyd
2018-01-29  9:22     ` Jerome Brunet
2018-02-01 17:43       ` Stephen Boyd
2018-02-02 12:50         ` Jerome Brunet
2018-04-23 18:21           ` Michael Turquette
2018-05-24 14:53             ` Jerome Brunet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1522398050.3871.12.camel@baylibre.com \
    --to=jbrunet@baylibre.com \
    --cc=khilman@baylibre.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=maxime.ripard@free-electrons.com \
    --cc=mturquette@baylibre.com \
    --cc=quentin.schulz@free-electrons.com \
    --cc=sboyd@codeaurora.org \
    --subject='Re: [PATCH v5 08/10] clk: fix CLK_SET_RATE_GATE with clock rate protection' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).