All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks
Date: Mon, 11 Jul 2011 10:28:56 +0000	[thread overview]
Message-ID: <20110711102856.GG3239@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20110711100759.GG26347@pengutronix.de>

On Mon, Jul 11, 2011 at 12:07:59PM +0200, Sascha Hauer wrote:
> On Mon, Jul 11, 2011 at 10:31:24AM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote:
> > > The biggest issue I ran into was that as the clocks are all registered
> > > by name with the API if you've got two instances of the same off-SoC
> > > device in the system you'll not be able to disambiguate between the
> > > clocks it provides.
> > 
> > Sigh.  That sounds like yet more trash.  Obviously whoever thought
> > up that doesn't actually understand clks.
> 
> Nope. In the patches Jeremy posted clocks have a name, but this name
> is not meant to be used with clk_get. clk_get is still implemented
> in clkdev, so the matching between clocks and devices is independent
> of the clock name.
> 
> In earlier versions of Jeremys patches the clock name was only present
> when debugfs was compiled in and I think it can be changed back to this.
> 
> That said the debugfs support (which is not present in Jeremys latest
> series) would break if two clocks with the same name have the same
> parent, because the clock core would try to create to debugfs entries
> with the same name.

If that's all its for, then can't some other solution for debugfs names
(lets stop calling them clock names to avoid confusion) be found -
such as

	sprintf(debugfsname, "%s@%p", clk->debugfsprefix, clk);

Or how about using something like this:

$debugfs/clkdev/device/connection -> ../../clk/<name-of-clk>
$debugfs/clk/<name-of-clk>

where <name-of-clk> could just be the address of the struct clk (which
eliminates the need to pass any names in) or some prefix plus an
incrementing identifier, or just an incrementing number.

I did think about introducing such a scheme along with clkdev (and did
have some code but that's long gone), but without the $debugfs/clk/ bit
being standardized, it wouldn't have worked.

WARNING: multiple messages have this Message-ID (diff)
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>,
	Grant Likely <grant@secretlab.ca>,
	linux-sh@vger.kernel.org, patches@opensource.wolfsonmicro.com,
	linux-kernel@vger.kernel.org,
	Jeremy Kerr <jeremy.kerr@canonical.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks
Date: Mon, 11 Jul 2011 11:28:56 +0100	[thread overview]
Message-ID: <20110711102856.GG3239@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20110711100759.GG26347@pengutronix.de>

On Mon, Jul 11, 2011 at 12:07:59PM +0200, Sascha Hauer wrote:
> On Mon, Jul 11, 2011 at 10:31:24AM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote:
> > > The biggest issue I ran into was that as the clocks are all registered
> > > by name with the API if you've got two instances of the same off-SoC
> > > device in the system you'll not be able to disambiguate between the
> > > clocks it provides.
> > 
> > Sigh.  That sounds like yet more trash.  Obviously whoever thought
> > up that doesn't actually understand clks.
> 
> Nope. In the patches Jeremy posted clocks have a name, but this name
> is not meant to be used with clk_get. clk_get is still implemented
> in clkdev, so the matching between clocks and devices is independent
> of the clock name.
> 
> In earlier versions of Jeremys patches the clock name was only present
> when debugfs was compiled in and I think it can be changed back to this.
> 
> That said the debugfs support (which is not present in Jeremys latest
> series) would break if two clocks with the same name have the same
> parent, because the clock core would try to create to debugfs entries
> with the same name.

If that's all its for, then can't some other solution for debugfs names
(lets stop calling them clock names to avoid confusion) be found -
such as

	sprintf(debugfsname, "%s@%p", clk->debugfsprefix, clk);

Or how about using something like this:

$debugfs/clkdev/device/connection -> ../../clk/<name-of-clk>
$debugfs/clk/<name-of-clk>

where <name-of-clk> could just be the address of the struct clk (which
eliminates the need to pass any names in) or some prefix plus an
incrementing identifier, or just an incrementing number.

I did think about introducing such a scheme along with clkdev (and did
have some code but that's long gone), but without the $debugfs/clk/ bit
being standardized, it wouldn't have worked.

WARNING: multiple messages have this Message-ID (diff)
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks
Date: Mon, 11 Jul 2011 11:28:56 +0100	[thread overview]
Message-ID: <20110711102856.GG3239@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20110711100759.GG26347@pengutronix.de>

On Mon, Jul 11, 2011 at 12:07:59PM +0200, Sascha Hauer wrote:
> On Mon, Jul 11, 2011 at 10:31:24AM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jul 11, 2011 at 11:53:44AM +0900, Mark Brown wrote:
> > > The biggest issue I ran into was that as the clocks are all registered
> > > by name with the API if you've got two instances of the same off-SoC
> > > device in the system you'll not be able to disambiguate between the
> > > clocks it provides.
> > 
> > Sigh.  That sounds like yet more trash.  Obviously whoever thought
> > up that doesn't actually understand clks.
> 
> Nope. In the patches Jeremy posted clocks have a name, but this name
> is not meant to be used with clk_get. clk_get is still implemented
> in clkdev, so the matching between clocks and devices is independent
> of the clock name.
> 
> In earlier versions of Jeremys patches the clock name was only present
> when debugfs was compiled in and I think it can be changed back to this.
> 
> That said the debugfs support (which is not present in Jeremys latest
> series) would break if two clocks with the same name have the same
> parent, because the clock core would try to create to debugfs entries
> with the same name.

If that's all its for, then can't some other solution for debugfs names
(lets stop calling them clock names to avoid confusion) be found -
such as

	sprintf(debugfsname, "%s@%p", clk->debugfsprefix, clk);

Or how about using something like this:

$debugfs/clkdev/device/connection -> ../../clk/<name-of-clk>
$debugfs/clk/<name-of-clk>

where <name-of-clk> could just be the address of the struct clk (which
eliminates the need to pass any names in) or some prefix plus an
incrementing identifier, or just an incrementing number.

I did think about introducing such a scheme along with clkdev (and did
have some code but that's long gone), but without the $debugfs/clk/ bit
being standardized, it wouldn't have worked.

  reply	other threads:[~2011-07-11 10:28 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-11  2:53 [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks Mark Brown
2011-07-11  2:53 ` Mark Brown
2011-07-11  2:53 ` Mark Brown
2011-07-11  2:53 ` [PATCH 1/6] clk: Prototype and document clk_register() Mark Brown
2011-07-11  2:53   ` Mark Brown
2011-07-11  2:53   ` Mark Brown
2011-07-11  2:53   ` [PATCH 2/6] clk: Provide a dummy clk_unregister() Mark Brown
2011-07-11  2:53     ` Mark Brown
2011-07-11  2:53     ` Mark Brown
2011-07-11  2:53   ` [PATCH 3/6] clk: Constify struct clk_hw_ops Mark Brown
2011-07-11  2:53     ` Mark Brown
2011-07-11  2:53     ` Mark Brown
2011-07-11  2:53   ` [PATCH 4/6] clk: Add Kconfig option to build all generic clk drivers Mark Brown
2011-07-11  2:53     ` Mark Brown
2011-07-11  2:53     ` Mark Brown
2011-07-11  2:53   ` [PATCH 5/6] clk: Support multiple instances of the same clock provider Mark Brown
2011-07-11  2:53     ` Mark Brown
2011-07-11  2:53     ` Mark Brown
2011-07-11  9:34     ` [PATCH 5/6] clk: Support multiple instances of the same clock Russell King - ARM Linux
2011-07-11  9:34       ` [PATCH 5/6] clk: Support multiple instances of the same clock provider Russell King - ARM Linux
2011-07-11  9:34       ` Russell King - ARM Linux
2011-07-11 10:53       ` [PATCH 5/6] clk: Support multiple instances of the same clock Mark Brown
2011-07-11 10:53         ` [PATCH 5/6] clk: Support multiple instances of the same clock provider Mark Brown
2011-07-11 10:53         ` Mark Brown
2011-07-11 11:11         ` [PATCH 5/6] clk: Support multiple instances of the same clock Russell King - ARM Linux
2011-07-11 11:11           ` [PATCH 5/6] clk: Support multiple instances of the same clock provider Russell King - ARM Linux
2011-07-11 11:11           ` Russell King - ARM Linux
2011-07-11 11:41           ` [PATCH 5/6] clk: Support multiple instances of the same clock Mark Brown
2011-07-11 11:41             ` [PATCH 5/6] clk: Support multiple instances of the same clock provider Mark Brown
2011-07-11 11:41             ` Mark Brown
2011-07-11  2:53   ` [PATCH 6/6] clk: Add initial WM831x clock driver Mark Brown
2011-07-11  2:53     ` Mark Brown
2011-07-11  2:53     ` Mark Brown
2011-07-15  2:53     ` Grant Likely
2011-07-15  2:53       ` Grant Likely
2011-07-15  2:53       ` Grant Likely
2011-07-15  5:05       ` Mark Brown
2011-07-15  5:05         ` Mark Brown
2011-07-15  5:05         ` Mark Brown
2011-07-15  5:14         ` Ryan Mallon
2011-07-15  5:14           ` Ryan Mallon
2011-07-15  5:14           ` Ryan Mallon
2011-07-15  2:53   ` [PATCH 1/6] clk: Prototype and document clk_register() Grant Likely
2011-07-15  2:53     ` Grant Likely
2011-07-15  2:53     ` Grant Likely
2011-07-11  3:57 ` [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks Mark Brown
2011-07-11  3:57   ` Mark Brown
2011-07-11  3:57   ` Mark Brown
2011-07-11  4:30   ` Mike Frysinger
2011-07-11  4:30     ` Mike Frysinger
2011-07-11  4:30     ` Mike Frysinger
2011-07-11  4:56     ` Barry Song
2011-07-11  4:56       ` Barry Song
2011-07-11  4:56       ` Barry Song
2011-07-11  5:01       ` [uclinux-dist-devel] [PATCH 0/6] clk: Initial feedback for Mike Frysinger
2011-07-11  5:01         ` [uclinux-dist-devel] [PATCH 0/6] clk: Initial feedback for off-SoC slow bus clocks Mike Frysinger
2011-07-11  5:01         ` Mike Frysinger
2011-07-11  9:31 ` Russell King - ARM Linux
2011-07-11  9:31   ` Russell King - ARM Linux
2011-07-11  9:31   ` Russell King - ARM Linux
2011-07-11 10:07   ` Sascha Hauer
2011-07-11 10:07     ` Sascha Hauer
2011-07-11 10:07     ` Sascha Hauer
2011-07-11 10:28     ` Russell King - ARM Linux [this message]
2011-07-11 10:28       ` Russell King - ARM Linux
2011-07-11 10:28       ` Russell King - ARM Linux
2011-07-11 10:46       ` Sascha Hauer
2011-07-11 10:46         ` Sascha Hauer
2011-07-11 10:46         ` Sascha Hauer
2011-07-11 11:43         ` Mark Brown
2011-07-11 11:43           ` Mark Brown
2011-07-11 11:43           ` Mark Brown

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=20110711102856.GG3239@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --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.