All of lore.kernel.org
 help / color / mirror / Atom feed
From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 3/5] of: add clock providers
Date: Tue, 01 Feb 2011 16:42:34 -0600	[thread overview]
Message-ID: <4D488C5A.7020107@gmail.com> (raw)
In-Reply-To: <20110126044416.16410.68942.stgit@localhost6.localdomain6>

Grant,

On 01/25/2011 10:44 PM, Grant Likely wrote:
> +struct clk *of_clk_get(struct device *dev, const char *id)

It would help if this took a struct device_node rather than struct 
device. This would allow using of_clk_get directly for cases where you 
don't have a struct device. This is fairly common in core platform core 
which are not full drivers. The case I have run into is the timer init 
code. To make clk_get work, I would have to create a dummy struct device.

Alternatively, an of_clk_get_sys function is needed.

 > +{
> +	struct device_node *provnode;
> +	u32 provhandle;
> +	int sz;
> +	struct clk *clk;
> +	char prop_name[32]; /* 32 is max size of property name */
> +	const void *prop;
> +
> +	dev_dbg(dev, "Looking up %s-clock from device tree\n", id);
> +
> +	snprintf(prop_name, 32, "%s-clock", id ? id : "bus");

Many times a module just has 1 clock and id will be NULL. Is "bus-clock" 
really the best choice of name? A bus clock may exist, but be 
transparent to s/w. How about allowing "clock" or "%s-clock"?

If bus-clock is the default, then you should add this to the clock 
binding wiki page.

Rob

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robherring2@gmail.com>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: devicetree-discuss@lists.ozlabs.org, jeremy.kerr@canonical.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, nicolas.pitre@linaro.org
Subject: Re: [RFC PATCH 3/5] of: add clock providers
Date: Tue, 01 Feb 2011 16:42:34 -0600	[thread overview]
Message-ID: <4D488C5A.7020107@gmail.com> (raw)
In-Reply-To: <20110126044416.16410.68942.stgit@localhost6.localdomain6>

Grant,

On 01/25/2011 10:44 PM, Grant Likely wrote:
> +struct clk *of_clk_get(struct device *dev, const char *id)

It would help if this took a struct device_node rather than struct 
device. This would allow using of_clk_get directly for cases where you 
don't have a struct device. This is fairly common in core platform core 
which are not full drivers. The case I have run into is the timer init 
code. To make clk_get work, I would have to create a dummy struct device.

Alternatively, an of_clk_get_sys function is needed.

 > +{
> +	struct device_node *provnode;
> +	u32 provhandle;
> +	int sz;
> +	struct clk *clk;
> +	char prop_name[32]; /* 32 is max size of property name */
> +	const void *prop;
> +
> +	dev_dbg(dev, "Looking up %s-clock from device tree\n", id);
> +
> +	snprintf(prop_name, 32, "%s-clock", id ? id : "bus");

Many times a module just has 1 clock and id will be NULL. Is "bus-clock" 
really the best choice of name? A bus clock may exist, but be 
transparent to s/w. How about allowing "clock" or "%s-clock"?

If bus-clock is the default, then you should add this to the clock 
binding wiki page.

Rob

  parent reply	other threads:[~2011-02-01 22:42 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-26  4:44 [RFC PATCH 0/5] arm/dt: register devices from device tree Grant Likely
2011-01-26  4:44 ` Grant Likely
2011-01-26  4:44 ` Grant Likely
2011-01-26  4:44 ` [RFC PATCH 1/5] arm/dt: Add dt machine definition Grant Likely
2011-01-26  4:44   ` Grant Likely
2011-01-26  4:44   ` Grant Likely
2011-01-26 18:27   ` [RFC,1/5] " Milton Miller
2011-01-26 18:27     ` Milton Miller
2011-01-27  3:31     ` Grant Likely
2011-01-26  4:44 ` [RFC PATCH 2/5] drivers/amba: probe via device tree Grant Likely
2011-01-26  4:44   ` Grant Likely
2011-01-26  4:44   ` Grant Likely
2011-01-26  4:44 ` [RFC PATCH 3/5] of: add clock providers Grant Likely
2011-01-26  4:44   ` Grant Likely
2011-01-26  4:44   ` Grant Likely
2011-01-26  5:47   ` Paul Mundt
2011-01-26  5:47     ` Paul Mundt
2011-02-01 22:42   ` Rob Herring [this message]
2011-02-01 22:42     ` Rob Herring
2011-01-26  4:44 ` [RFC PATCH 4/5] arm/clkdev: lookup clocks from OF " Grant Likely
2011-01-26  4:44   ` Grant Likely
2011-01-26  4:44   ` Grant Likely
2011-01-26  4:44 ` [RFC PATCH 5/5] arm/versatile: add a device tree versatile platform Grant Likely
2011-01-26  4:44   ` Grant Likely
2011-01-26  4:44   ` Grant Likely

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=4D488C5A.7020107@gmail.com \
    --to=robherring2@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.