All of lore.kernel.org
 help / color / mirror / Atom feed
From: sylvester.nawrocki@gmail.com (Sylwester Nawrocki)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clkdev: only fall through to clk_get_sys() if DT indicates no entry
Date: Sun, 27 Apr 2014 21:08:54 +0200	[thread overview]
Message-ID: <535D55C6.9090609@gmail.com> (raw)
In-Reply-To: <E1We4rQ-0001mv-Ps@rmk-PC.arm.linux.org.uk>

On 04/26/2014 05:49 PM, Russell King wrote:
> There are two failures which DT can return when looking up a phandle:
> -ENOENT - when DT finds that the desired property/index is not present
> -EINVAL - when DT finds that the desired property/index is present, but
>   there is a problem parsing it.
>
> We should only fall through to clk_get_sys() (the table driven clock
> lookup) when DT indicates that there was no entry in the OF tables
> for the clock.  Doing otherwise causes the clk API to always indicate
> that there is no entry for this clock, which is not correct behaviour.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> Grant, Rob,
>
> Please let me know ASAP if this gives you any cause for concern.  This
> looks to me like a long standing bug which really needs fixing.  Part
> of the motivation here is similar to Jean-Francois' patch to clkdev
> which allows a DT specified clock which isn't get present to have
> clk_get() and friends return -EPROBE_DEFER - again, something that I
> think really should happen.  Jean-Francois hasn't been able to get
> much traction for his patch, so let's start with fixing this bug first.

How about upstream commit a34cd4666f3da84228a82f70c94b8d9b692034ea,
I thought it already solves the issue you're trying to address in
this patch ?

>   drivers/clk/clkdev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index 48f67218247c..4e65d0e10b05 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -165,7 +165,7 @@ struct clk *clk_get(struct device *dev, const char *con_id)
>
>   	if (dev) {
>   		clk = of_clk_get_by_name(dev->of_node, con_id);
> -		if (!IS_ERR(clk))
> +		if (!IS_ERR(clk) || PTR_ERR(clk) != -ENOENT)
>   			return clk;
>   	}

--
Regards,
Sylwester

WARNING: multiple messages have this Message-ID (diff)
From: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
To: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Grant Likely <grant.likely@linaro.org>,
	Rob Herring <robh+dt@kernel.org>
Subject: Re: [PATCH] clkdev: only fall through to clk_get_sys() if DT indicates no entry
Date: Sun, 27 Apr 2014 21:08:54 +0200	[thread overview]
Message-ID: <535D55C6.9090609@gmail.com> (raw)
In-Reply-To: <E1We4rQ-0001mv-Ps@rmk-PC.arm.linux.org.uk>

On 04/26/2014 05:49 PM, Russell King wrote:
> There are two failures which DT can return when looking up a phandle:
> -ENOENT - when DT finds that the desired property/index is not present
> -EINVAL - when DT finds that the desired property/index is present, but
>   there is a problem parsing it.
>
> We should only fall through to clk_get_sys() (the table driven clock
> lookup) when DT indicates that there was no entry in the OF tables
> for the clock.  Doing otherwise causes the clk API to always indicate
> that there is no entry for this clock, which is not correct behaviour.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> Grant, Rob,
>
> Please let me know ASAP if this gives you any cause for concern.  This
> looks to me like a long standing bug which really needs fixing.  Part
> of the motivation here is similar to Jean-Francois' patch to clkdev
> which allows a DT specified clock which isn't get present to have
> clk_get() and friends return -EPROBE_DEFER - again, something that I
> think really should happen.  Jean-Francois hasn't been able to get
> much traction for his patch, so let's start with fixing this bug first.

How about upstream commit a34cd4666f3da84228a82f70c94b8d9b692034ea,
I thought it already solves the issue you're trying to address in
this patch ?

>   drivers/clk/clkdev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index 48f67218247c..4e65d0e10b05 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -165,7 +165,7 @@ struct clk *clk_get(struct device *dev, const char *con_id)
>
>   	if (dev) {
>   		clk = of_clk_get_by_name(dev->of_node, con_id);
> -		if (!IS_ERR(clk))
> +		if (!IS_ERR(clk) || PTR_ERR(clk) != -ENOENT)
>   			return clk;
>   	}

--
Regards,
Sylwester

  reply	other threads:[~2014-04-27 19:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-26 15:49 [PATCH] clkdev: only fall through to clk_get_sys() if DT indicates no entry Russell King
2014-04-26 15:49 ` Russell King
2014-04-27 19:08 ` Sylwester Nawrocki [this message]
2014-04-27 19:08   ` Sylwester Nawrocki
2014-04-27 19:12   ` Russell King - ARM Linux
2014-04-27 19:12     ` Russell King - ARM Linux
2014-04-27 21:37 ` Rob Herring
2014-04-27 21:37   ` Rob Herring

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=535D55C6.9090609@gmail.com \
    --to=sylvester.nawrocki@gmail.com \
    --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.