All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@codeaurora.org>
To: Heiko Stuebner <heiko@sntech.de>, mturquette@linaro.org
Cc: linux@arm.linux.org.uk, linux-kernel@vger.kernel.org,
	dianders@chromium.org, linux-rockchip@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 3/4] clk: add CLK_DEFER_ORPHAN flag to prevent orphans from being used
Date: Fri, 10 Apr 2015 17:52:43 -0700	[thread overview]
Message-ID: <5528705B.3040102@codeaurora.org> (raw)
In-Reply-To: <1427988853-9549-4-git-send-email-heiko@sntech.de>

On 04/02/15 08:34, Heiko Stuebner wrote:
> The usage of clocks derived from an orphan can produce issues when trying
> to set rates etc. So ideally a clk_get to such a clock should defer till
> the clock hierarchy is complete.
> But as some arches probably rely on such clocks we can't disable them all.
> Therefore add a new clk flag where arches can enable this behaviour for
> their clocks.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/clk/clk.c            | 6 ++++++
>  include/linux/clk-provider.h | 1 +
>  2 files changed, 7 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 476f491..8511c62 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3041,6 +3041,12 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
>  		if (provider->node == clkspec->np)
>  			clk = provider->get(clkspec, provider->data);
>  		if (!IS_ERR(clk)) {
> +			if ((__clk_get_flags(clk) & CLK_DEFER_ORPHAN)
> +						&& clk_is_orphan(clk)) {
> +				clk = ERR_PTR(-EPROBE_DEFER);
> +				break;
> +			}
> +
>  			clk = __clk_create_clk(__clk_get_hw(clk), dev_id,
>  					       con_id);
>  
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 28abf1b..ef8d669 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -31,6 +31,7 @@
>  #define CLK_GET_RATE_NOCACHE	BIT(6) /* do not use the cached clk rate */
>  #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
>  #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
> +#define CLK_DEFER_ORPHAN	BIT(9) /* defer clk_get calls for orphans */
>  

I don't see why this is an opt-in feature. If we think there are arches
that are setting rates of clocks before they're ready then we have a
problem and we should fix it. These last two patches don't look necessary.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

WARNING: multiple messages have this Message-ID (diff)
From: sboyd@codeaurora.org (Stephen Boyd)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/4] clk: add CLK_DEFER_ORPHAN flag to prevent orphans from being used
Date: Fri, 10 Apr 2015 17:52:43 -0700	[thread overview]
Message-ID: <5528705B.3040102@codeaurora.org> (raw)
In-Reply-To: <1427988853-9549-4-git-send-email-heiko@sntech.de>

On 04/02/15 08:34, Heiko Stuebner wrote:
> The usage of clocks derived from an orphan can produce issues when trying
> to set rates etc. So ideally a clk_get to such a clock should defer till
> the clock hierarchy is complete.
> But as some arches probably rely on such clocks we can't disable them all.
> Therefore add a new clk flag where arches can enable this behaviour for
> their clocks.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/clk/clk.c            | 6 ++++++
>  include/linux/clk-provider.h | 1 +
>  2 files changed, 7 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 476f491..8511c62 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3041,6 +3041,12 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
>  		if (provider->node == clkspec->np)
>  			clk = provider->get(clkspec, provider->data);
>  		if (!IS_ERR(clk)) {
> +			if ((__clk_get_flags(clk) & CLK_DEFER_ORPHAN)
> +						&& clk_is_orphan(clk)) {
> +				clk = ERR_PTR(-EPROBE_DEFER);
> +				break;
> +			}
> +
>  			clk = __clk_create_clk(__clk_get_hw(clk), dev_id,
>  					       con_id);
>  
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 28abf1b..ef8d669 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -31,6 +31,7 @@
>  #define CLK_GET_RATE_NOCACHE	BIT(6) /* do not use the cached clk rate */
>  #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
>  #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
> +#define CLK_DEFER_ORPHAN	BIT(9) /* defer clk_get calls for orphans */
>  

I don't see why this is an opt-in feature. If we think there are arches
that are setting rates of clocks before they're ready then we have a
problem and we should fix it. These last two patches don't look necessary.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

  reply	other threads:[~2015-04-11  0:52 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-02 15:34 [PATCH 0/4] clk: improve handling of orphan clocks Heiko Stuebner
2015-04-02 15:34 ` Heiko Stuebner
2015-04-02 15:34 ` [PATCH 1/4] clk: Propagate prepare and enable when reparenting orphans Heiko Stuebner
2015-04-02 15:34   ` Heiko Stuebner
2015-04-11  0:59   ` Stephen Boyd
2015-04-11  0:59     ` Stephen Boyd
2015-04-02 15:34 ` [PATCH 2/4] clk: add clk_is_orphan() to check if a clocks inherits from an orphan clock Heiko Stuebner
2015-04-02 15:34   ` Heiko Stuebner
2015-04-11  0:49   ` Stephen Boyd
2015-04-11  0:49     ` Stephen Boyd
2015-04-02 15:34 ` [PATCH 3/4] clk: add CLK_DEFER_ORPHAN flag to prevent orphans from being used Heiko Stuebner
2015-04-02 15:34   ` Heiko Stuebner
2015-04-11  0:52   ` Stephen Boyd [this message]
2015-04-11  0:52     ` Stephen Boyd
2015-04-11  1:32     ` Heiko Stübner
2015-04-11  1:32       ` Heiko Stübner
2015-04-02 15:34 ` [PATCH 4/4] clk: rockchip: enable CLK_DEFER_ORPHAN for all branches Heiko Stuebner
2015-04-02 15:34   ` Heiko Stuebner

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=5528705B.3040102@codeaurora.org \
    --to=sboyd@codeaurora.org \
    --cc=dianders@chromium.org \
    --cc=heiko@sntech.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mturquette@linaro.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.