All of lore.kernel.org
 help / color / mirror / Atom feed
From: mturquette@linaro.org (Mike Turquette)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clk: Properly handle notifier return values
Date: Wed, 10 Apr 2013 16:51:15 -0700	[thread overview]
Message-ID: <20130410235115.14528.14648@quantum> (raw)
In-Reply-To: <a2f153b2-9266-4ddc-94a9-3efbcbe9768c@CH1EHSMHS018.ehs.local>

Quoting Soren Brinkmann (2013-04-03 12:17:12)
> Notifiers may return NOTIFY_(OK|DONE|STOP|BAD). The CCF uses an
> inconsistent mix of checking against NOTIFY_STOP or NOTIFY_BAD.
> This inconsistency leaves errors undetected in some cases:
> clk_set_parent() calls __clk_speculate_rates(), which stops when it
> hits a NOTIFIER_BAD (STOP is ignored), and passes this value back to the
> caller.
> clk_set_parent() compares this return value against NOTIFY_STOP only,
> ignoring NOTIFY_BAD returns.
> 
> Use NOTIFY_STOP_MASK to detect a negative notifier return value and
> document all four return value options.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>

Taken into clk-next.

Thanks for the fix,
Mike

> ---
>  drivers/clk/clk.c   | 10 +++++-----
>  include/linux/clk.h |  8 ++++----
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ed87b24..68bd38c 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -876,16 +876,16 @@ static int __clk_speculate_rates(struct clk *clk, unsigned long parent_rate)
>         else
>                 new_rate = parent_rate;
>  
> -       /* abort the rate change if a driver returns NOTIFY_BAD */
> +       /* abort rate change if a driver returns NOTIFY_BAD or NOTIFY_STOP */
>         if (clk->notifier_count)
>                 ret = __clk_notify(clk, PRE_RATE_CHANGE, clk->rate, new_rate);
>  
> -       if (ret == NOTIFY_BAD)
> +       if (ret & NOTIFY_STOP_MASK)
>                 goto out;
>  
>         hlist_for_each_entry(child, &clk->children, child_node) {
>                 ret = __clk_speculate_rates(child, new_rate);
> -               if (ret == NOTIFY_BAD)
> +               if (ret & NOTIFY_STOP_MASK)
>                         break;
>         }
>  
> @@ -978,7 +978,7 @@ static struct clk *clk_propagate_rate_change(struct clk *clk, unsigned long even
>  
>         if (clk->notifier_count) {
>                 ret = __clk_notify(clk, event, clk->rate, clk->new_rate);
> -               if (ret == NOTIFY_BAD)
> +               if (ret & NOTIFY_STOP_MASK)
>                         fail_clk = clk;
>         }
>  
> @@ -1296,7 +1296,7 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
>                 ret = __clk_speculate_rates(clk, parent->rate);
>  
>         /* abort if a driver objects */
> -       if (ret == NOTIFY_STOP)
> +       if (ret & NOTIFY_STOP_MASK)
>                 goto out;
>  
>         /* only re-parent if the clock is not in use */
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index b3ac22d..9a6d045 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -28,16 +28,16 @@ struct clk;
>   * PRE_RATE_CHANGE - called immediately before the clk rate is changed,
>   *     to indicate that the rate change will proceed.  Drivers must
>   *     immediately terminate any operations that will be affected by the
> - *     rate change.  Callbacks may either return NOTIFY_DONE or
> - *     NOTIFY_STOP.
> + *     rate change.  Callbacks may either return NOTIFY_DONE, NOTIFY_OK,
> + *     NOTIFY_STOP or NOTIFY_BAD.
>   *
>   * ABORT_RATE_CHANGE: called if the rate change failed for some reason
>   *     after PRE_RATE_CHANGE.  In this case, all registered notifiers on
>   *     the clk will be called with ABORT_RATE_CHANGE. Callbacks must
> - *     always return NOTIFY_DONE.
> + *     always return NOTIFY_DONE or NOTIFY_OK.
>   *
>   * POST_RATE_CHANGE - called after the clk rate change has successfully
> - *     completed.  Callbacks must always return NOTIFY_DONE.
> + *     completed.  Callbacks must always return NOTIFY_DONE or NOTIFY_OK.
>   *
>   */
>  #define PRE_RATE_CHANGE                        BIT(0)
> -- 
> 1.8.2

WARNING: multiple messages have this Message-ID (diff)
From: Mike Turquette <mturquette@linaro.org>
To: Soren Brinkmann <soren.brinkmann@xilinx.com>,
	Russell King <linux@arm.linux.org.uk>
Cc: <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Soren Brinkmann <soren.brinkmann@xilinx.com>
Subject: Re: [PATCH] clk: Properly handle notifier return values
Date: Wed, 10 Apr 2013 16:51:15 -0700	[thread overview]
Message-ID: <20130410235115.14528.14648@quantum> (raw)
In-Reply-To: <a2f153b2-9266-4ddc-94a9-3efbcbe9768c@CH1EHSMHS018.ehs.local>

Quoting Soren Brinkmann (2013-04-03 12:17:12)
> Notifiers may return NOTIFY_(OK|DONE|STOP|BAD). The CCF uses an
> inconsistent mix of checking against NOTIFY_STOP or NOTIFY_BAD.
> This inconsistency leaves errors undetected in some cases:
> clk_set_parent() calls __clk_speculate_rates(), which stops when it
> hits a NOTIFIER_BAD (STOP is ignored), and passes this value back to the
> caller.
> clk_set_parent() compares this return value against NOTIFY_STOP only,
> ignoring NOTIFY_BAD returns.
> 
> Use NOTIFY_STOP_MASK to detect a negative notifier return value and
> document all four return value options.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>

Taken into clk-next.

Thanks for the fix,
Mike

> ---
>  drivers/clk/clk.c   | 10 +++++-----
>  include/linux/clk.h |  8 ++++----
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ed87b24..68bd38c 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -876,16 +876,16 @@ static int __clk_speculate_rates(struct clk *clk, unsigned long parent_rate)
>         else
>                 new_rate = parent_rate;
>  
> -       /* abort the rate change if a driver returns NOTIFY_BAD */
> +       /* abort rate change if a driver returns NOTIFY_BAD or NOTIFY_STOP */
>         if (clk->notifier_count)
>                 ret = __clk_notify(clk, PRE_RATE_CHANGE, clk->rate, new_rate);
>  
> -       if (ret == NOTIFY_BAD)
> +       if (ret & NOTIFY_STOP_MASK)
>                 goto out;
>  
>         hlist_for_each_entry(child, &clk->children, child_node) {
>                 ret = __clk_speculate_rates(child, new_rate);
> -               if (ret == NOTIFY_BAD)
> +               if (ret & NOTIFY_STOP_MASK)
>                         break;
>         }
>  
> @@ -978,7 +978,7 @@ static struct clk *clk_propagate_rate_change(struct clk *clk, unsigned long even
>  
>         if (clk->notifier_count) {
>                 ret = __clk_notify(clk, event, clk->rate, clk->new_rate);
> -               if (ret == NOTIFY_BAD)
> +               if (ret & NOTIFY_STOP_MASK)
>                         fail_clk = clk;
>         }
>  
> @@ -1296,7 +1296,7 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
>                 ret = __clk_speculate_rates(clk, parent->rate);
>  
>         /* abort if a driver objects */
> -       if (ret == NOTIFY_STOP)
> +       if (ret & NOTIFY_STOP_MASK)
>                 goto out;
>  
>         /* only re-parent if the clock is not in use */
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index b3ac22d..9a6d045 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -28,16 +28,16 @@ struct clk;
>   * PRE_RATE_CHANGE - called immediately before the clk rate is changed,
>   *     to indicate that the rate change will proceed.  Drivers must
>   *     immediately terminate any operations that will be affected by the
> - *     rate change.  Callbacks may either return NOTIFY_DONE or
> - *     NOTIFY_STOP.
> + *     rate change.  Callbacks may either return NOTIFY_DONE, NOTIFY_OK,
> + *     NOTIFY_STOP or NOTIFY_BAD.
>   *
>   * ABORT_RATE_CHANGE: called if the rate change failed for some reason
>   *     after PRE_RATE_CHANGE.  In this case, all registered notifiers on
>   *     the clk will be called with ABORT_RATE_CHANGE. Callbacks must
> - *     always return NOTIFY_DONE.
> + *     always return NOTIFY_DONE or NOTIFY_OK.
>   *
>   * POST_RATE_CHANGE - called after the clk rate change has successfully
> - *     completed.  Callbacks must always return NOTIFY_DONE.
> + *     completed.  Callbacks must always return NOTIFY_DONE or NOTIFY_OK.
>   *
>   */
>  #define PRE_RATE_CHANGE                        BIT(0)
> -- 
> 1.8.2

  reply	other threads:[~2013-04-10 23:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-03 19:17 [PATCH] clk: Properly handle notifier return values Soren Brinkmann
2013-04-03 19:17 ` Soren Brinkmann
2013-04-10 23:51 ` Mike Turquette [this message]
2013-04-10 23:51   ` Mike Turquette

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=20130410235115.14528.14648@quantum \
    --to=mturquette@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.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.