linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Russell King <rmk@arm.linux.org.uk>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: John Jacques <john.jacques@lsi.com>,
	linuxppc-dev list <linuxppc-dev@ozlabs.org>,
	devicetree-discuss@lists.ozlabs.org,
	Torez Smith <torez@us.ibm.com>,
	Mark Brown <broonie@sirena.org.uk>
Subject: Re: ARM clock API to PowerPC
Date: Wed, 12 Aug 2009 23:28:43 +0100	[thread overview]
Message-ID: <20090812222843.GA7118@flint.arm.linux.org.uk> (raw)
In-Reply-To: <1250114192.3587.41.camel@pasglop>

On Thu, Aug 13, 2009 at 07:56:32AM +1000, Benjamin Herrenschmidt wrote:
> Maybe we can make clock-names non-optional though in the DT as an
> incentive not to use the simple 1-clock "NULL" path.

We used to pass names.  Everyone got the idea that they could ignore
the struct device argument, and chaos ensued in drivers - people wanted
to name each of their individual clk structures uniquely, and pass
clock names, or even struct clk pointers into drivers via platform data.
Some drivers conditionalized the clock name depending on the SoC they
were built for in the driver code.

Providing the clkdev infrastructure (which I'll talk about in another
email, probably tomorrow) and ensuring that single-clock drivers pass
a NULL name has ensured that people back away from that broken kind
of thinking.  It has certainly cut down on the code size and the
complexity in drivers.

IIRC, there were some drivers shrunk by about 100 LOC by using the
clk API as I originally intended it to be used - which clkdev
facilitates.

> > It's not just the device tree, it's also the drivers which have to be
> > able to cope with whatever random device tree that's thrown at them.
> 
> Well, the clocks are named. At some stage, the binding for a given
> device will define what clock names it expects. I don't see that
> differing from what the ARM folks do.

The difference is that I'm trying to avoid the "name each clock source
and have each driver ask for the clock by name".  Such an approach
at first seems simple and logical, but experience has shown that it
eventually creates more problems as things progress.

Take a look at these two commits:

39a80c7f379e1c1d3e63b204b8353b7381d0a3d5
4c5e1946b5f89c33e3bc8ed73fa7ba8f31e37cc5

to see how moving from a per-clk naming system to a dev+consumer naming
allowed omap_wdt to be cleaned up.  (OMAP3 added more clk naming
conditions in the driver, so had this cleanup not happened the driver
would have more stuff in it.)

What I'm saying is that always passing a bunch of names has been well
proven to lead people down the wrong path of matching only by names
and then running into problems later.  We need drivers passing a NULL
name to ensure that people get the right idea.  Comments in code/headers
don't seem to work. ;(

-- 
Russell King

  parent reply	other threads:[~2009-08-12 22:29 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-12  7:57 ARM clock API to PowerPC Benjamin Herrenschmidt
2009-08-12  8:29 ` Benjamin Herrenschmidt
2009-08-12 17:31   ` Mitch Bradley
2009-08-12 21:30     ` Benjamin Herrenschmidt
2009-08-12 11:19 ` Josh Boyer
2009-08-12 13:40   ` Kumar Gala
2009-08-12 21:29     ` Benjamin Herrenschmidt
2009-08-13  8:59       ` Li Yang-R58472
2009-08-14  9:29         ` Benjamin Herrenschmidt
2009-08-14 11:29           ` Guennadi Liakhovetski
2009-08-14 12:07             ` Benjamin Herrenschmidt
2009-08-15 12:43               ` Russell King
2009-08-15 22:18                 ` Benjamin Herrenschmidt
2009-08-16  5:09                   ` Grant Likely
2009-08-12 12:35 ` Mark Brown
2009-08-12 21:34   ` Benjamin Herrenschmidt
2009-08-12 21:44     ` Mark Brown
2009-08-12 21:56       ` Benjamin Herrenschmidt
2009-08-12 22:20         ` Mark Brown
2009-08-12 22:32           ` Benjamin Herrenschmidt
2009-08-12 23:00             ` Mark Brown
2009-08-12 23:15               ` Benjamin Herrenschmidt
2009-08-12 22:28         ` Russell King [this message]
2009-08-12 22:45           ` Mark Brown
2009-08-12 22:52           ` Benjamin Herrenschmidt
2009-08-12 23:40             ` Russell King
2009-08-12 23:47               ` Benjamin Herrenschmidt
2009-08-13  3:45               ` Benjamin Herrenschmidt

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=20090812222843.GA7118@flint.arm.linux.org.uk \
    --to=rmk@arm.linux.org.uk \
    --cc=benh@kernel.crashing.org \
    --cc=broonie@sirena.org.uk \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=john.jacques@lsi.com \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=torez@us.ibm.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).