devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Tero Kristo <t-kristo@ti.com>
Cc: linux-omap@vger.kernel.org, paul@pwsan.com, tony@atomide.com,
	nm@ti.com, rnayak@ti.com, mturquette@linaro.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
Subject: Re: [PATCHv5 01/31] CLK: clkdev: add support for looking up clocks from DT
Date: Mon, 26 Aug 2013 18:03:27 +0100	[thread overview]
Message-ID: <20130826170327.GG25647@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <521B67DF.4000005@ti.com>

On Mon, Aug 26, 2013 at 05:36:15PM +0300, Tero Kristo wrote:
> On 08/03/2013 09:31 PM, Russell King - ARM Linux wrote:
>> On Fri, Aug 02, 2013 at 07:25:20PM +0300, Tero Kristo wrote:
>>> +
>>> +	if (cl)
>>> +		return cl;
>>> +
>>> +	/* If clock was not found, attempt to look-up from DT */
>>> +	node = of_find_node_by_name(NULL, con_id);
>>
>> This is utterly broken if you're looking up purely by a connection ID.
>> Please, go back and read clk_get()'s documentation in linux/clk.h.
>> It's spelt out very plainly there.
>>
>> NAK.
>>
>
> Hi Russell,
>
> After looking at this a bit more, it seems difficult to get rid of the  
> clk_get -> of_clk_get conversion, however based on this comment I am  
> somewhat stuck. Do you think it would be acceptable if I change the  
> implementation of this patch to work like this:

Firstly, for clk_get(), you don't need to do anything - clk_get() already
deals with DT, and if your DT is correct, it should already be returning
the appropriate clock(s).

I guess the problem here is clk_get_sys(), because that doesn't take a
struct device (and it doesn't take a struct device because it's used
from code where there is no struct device to use with it.)

If you have an OF node, what's wrong with using of_clk_get_by_name()?
If you don't have an OF node, how do you expect to find the clock in
the device tree, remembering that clocks are specific to devices, and
the 2nd argument to clk_get()/clk_get_sys() is not a unique identifier.

  reply	other threads:[~2013-08-26 17:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1375460751-23676-1-git-send-email-t-kristo@ti.com>
     [not found] ` <1375460751-23676-2-git-send-email-t-kristo@ti.com>
     [not found]   ` <20130803183143.GE23053@n2100.arm.linux.org.uk>
2013-08-26 14:36     ` [PATCHv5 01/31] CLK: clkdev: add support for looking up clocks from DT Tero Kristo
2013-08-26 17:03       ` Russell King - ARM Linux [this message]
2013-08-26 18:12         ` Tero Kristo
2013-08-27  6:55           ` Tony Lindgren

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=20130826170327.GG25647@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=mturquette@linaro.org \
    --cc=nm@ti.com \
    --cc=paul@pwsan.com \
    --cc=rnayak@ti.com \
    --cc=t-kristo@ti.com \
    --cc=tony@atomide.com \
    /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 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).