From: tglx@linutronix.de (Thomas Gleixner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 01/10] Add a common struct clk
Date: Fri, 29 Apr 2011 12:45:00 +0200 (CEST) [thread overview]
Message-ID: <alpine.LFD.2.02.1104291223250.3005@ionos> (raw)
In-Reply-To: <20110429100927.GC5126@n2100.arm.linux.org.uk>
On Fri, 29 Apr 2011, Russell King - ARM Linux wrote:
> On Thu, Apr 21, 2011 at 09:48:28PM +0200, Thomas Gleixner wrote:
> > On Fri, 15 Apr 2011, Sascha Hauer wrote:
> > > From: Jeremy Kerr <jeremy.kerr@canonical.com>
> > > + * @get: Called by the core clock code when a device driver acquires a
> > > + * clock via clk_get(). Optional.
> > > + *
> > > + * @put: Called by the core clock code when a devices driver releases a
> > > + * clock via clk_put(). Optional.
> >
> > These callbacks are completely pointless. There are only two non empty
> > implementations in tree:
> >
> > One does a try_module_get(clk->owner), which should be done in generic
> > code. The other does special clock enabling magic which wants to go to
> > clk->prepare().
>
> I disagree. Most clocks don't live in a module - there's only one
> platform which does at present. To force every clock to have an owner
> field is rediculous. We already know that the OMAP tree represents a
So we trade an owner field (which can be NULL) versus two function
pointers in the clk_ops struct, which are of course subject to be
abused for all kind of crap which does not belong there at all.
> significant amount of data, and for every additional unnecessary word,
> it gobbles up another 4K of data space.
What are we talking about?
arch/arm/mach-omap2/clock2420_data.c:117
arch/arm/mach-omap2/clock2430_data.c:126
arch/arm/mach-omap2/clock3xxx_data.c:235
arch/arm/mach-omap2/clock44xx_data.c:228
So even if we could build a common kernel for all these omaps, then
adding owner to struct clk will consume ~2800 bytes.
> If we go down the route of separating clock stuff into bite-size pieces,
> OMAP is going to end up with yet more struct clks, which is going to add
> to the bloat even more.
Make it 8k then and it's still not any significant number.
Abstractions and I mean abstractions which encapsulate common
functionality have always a tradeoff in larger data size. And one
lesson I learned the hard way is that if you make abstractions and
frameworks by design permissive, then you end up with tons of extra
private code because people prefer to hack their "special needs" into
their private code instead of talking to the people who maintain the
core code where it could be solved in a generalized way.
Yes, we want to avoid unneccesary data size growth, but at the same
time we have to make it as hard as it gets for users of an
abstraction/framework to tinker around the core code as they see
fit. Otherwise you wont get any consolidation and reduction of over
and over copied and modified code doing basically the same thing.
If I can trade 2800 bytes of data vs. 280 lines of duplicated code and
a can of worms opened by interfaces which are an invitation for abuse,
then I chose the 2800 bytes any time.
Thanks,
tglx
next prev parent reply other threads:[~2011-04-29 10:45 UTC|newest]
Thread overview: 157+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-15 19:08 [RFC] sanitizing crazy clock data files Sascha Hauer
2011-04-15 19:08 ` [PATCH 01/10] Add a common struct clk Sascha Hauer
2011-04-21 19:48 ` Thomas Gleixner
2011-04-22 0:28 ` Richard Zhao
2011-04-22 9:23 ` Thomas Gleixner
2011-04-23 14:08 ` Richard Zhao
2011-04-23 15:30 ` Thomas Gleixner
2011-04-24 2:54 ` Richard Zhao
2011-04-24 7:20 ` Linus Walleij
2011-04-24 9:55 ` Richard Zhao
2011-04-22 4:57 ` Saravana Kannan
2011-04-22 9:13 ` Thomas Gleixner
2011-04-29 10:09 ` Russell King - ARM Linux
2011-04-29 10:45 ` Thomas Gleixner [this message]
2011-04-29 10:58 ` Tony Lindgren
2011-04-29 11:01 ` Russell King - ARM Linux
2011-04-30 18:30 ` Pavel Machek
2011-04-30 8:06 ` Linus Walleij
2011-04-30 9:01 ` Russell King - ARM Linux
2011-04-30 16:30 ` Linus Walleij
2011-05-01 20:33 ` Rob Herring
2011-05-02 1:09 ` Jeremy Kerr
2011-05-02 3:09 ` Rob Herring
2011-05-02 3:40 ` Jeremy Kerr
2011-05-02 16:30 ` Rob Herring
2011-05-02 22:36 ` Russell King - ARM Linux
2011-05-03 0:22 ` Saravana Kannan
2011-05-04 6:40 ` Sascha Hauer
2011-05-04 18:33 ` Saravana Kannan
2011-05-04 23:35 ` Paul Walmsley
2011-05-10 20:06 ` Saravana Kannan
2011-05-02 16:55 ` David Brown
2011-05-02 17:31 ` Stephen Boyd
2011-04-15 19:08 ` [PATCH 02/10] clk: Generic support for fixed-rate clocks Sascha Hauer
2011-04-15 19:08 ` [PATCH 03/10] clk: Make NULL a valid clock again Sascha Hauer
2011-04-19 0:53 ` Jeremy Kerr
2011-04-19 6:25 ` Sascha Hauer
2011-04-20 12:53 ` Uwe Kleine-König
2011-04-15 19:08 ` [PATCH 04/10] clk: implement parent pass through functions Sascha Hauer
2011-04-18 9:25 ` Uwe Kleine-König
2011-04-18 9:48 ` Sascha Hauer
2011-04-19 17:20 ` Stephen Boyd
2011-04-19 17:53 ` Sascha Hauer
2011-04-19 19:09 ` Uwe Kleine-König
2011-04-19 20:58 ` Sascha Hauer
2011-04-19 21:54 ` Thomas Gleixner
2011-04-20 7:16 ` Uwe Kleine-König
2011-04-20 8:34 ` Thomas Gleixner
2011-04-20 14:07 ` [PATCH RFC] clk: add support for automatic parent handling Uwe Kleine-König
2011-04-20 16:16 ` Thomas Gleixner
2011-04-20 18:59 ` Uwe Kleine-König
2011-04-20 19:52 ` Thomas Gleixner
2011-04-21 6:58 ` Saravana Kannan
2011-04-21 10:33 ` Thomas Gleixner
2011-04-21 19:22 ` torbenh
2011-04-23 23:26 ` Benjamin Herrenschmidt
2011-04-21 7:22 ` Uwe Kleine-König
2011-04-21 9:12 ` Thomas Gleixner
2011-04-21 10:31 ` Mark Brown
2011-04-21 11:42 ` Tony Lindgren
2011-04-21 14:52 ` Thomas Gleixner
2011-04-22 7:09 ` Tony Lindgren
2011-04-22 8:22 ` Thomas Gleixner
2011-04-21 14:29 ` Thomas Gleixner
2011-04-29 10:37 ` Russell King - ARM Linux
2011-04-29 11:01 ` Thomas Gleixner
2011-04-29 11:06 ` Russell King - ARM Linux
2011-04-29 12:13 ` Thomas Gleixner
2011-04-29 13:26 ` Russell King - ARM Linux
2011-04-29 15:31 ` Thomas Gleixner
2011-04-29 22:07 ` Russell King - ARM Linux
2011-04-29 22:16 ` Thomas Gleixner
2011-04-29 22:19 ` Russell King - ARM Linux
2011-04-29 22:47 ` Thomas Gleixner
2011-04-30 14:27 ` torbenh
2011-05-03 6:35 ` Colin Cross
2011-05-05 8:35 ` torbenh
2011-05-03 2:44 ` Saravana Kannan
2011-05-03 2:46 ` Saravana Kannan
2011-04-21 10:13 ` Mark Brown
2011-04-21 11:39 ` Tony Lindgren
2011-04-21 7:42 ` Sascha Hauer
2011-04-21 9:21 ` Thomas Gleixner
2011-04-21 11:50 ` Mark Brown
2011-04-21 12:20 ` Thomas Gleixner
2011-04-21 12:35 ` Mark Brown
2011-04-25 2:03 ` Richard Zhao
2011-04-25 10:57 ` Mark Brown
2011-04-25 14:41 ` Richard Zhao
2011-04-25 14:44 ` Mark Brown
2011-04-29 10:49 ` Russell King - ARM Linux
2011-04-29 11:11 ` Thomas Gleixner
2011-04-29 11:38 ` Russell King - ARM Linux
2011-04-29 12:19 ` Thomas Gleixner
2011-04-29 13:27 ` Russell King - ARM Linux
2011-04-29 15:47 ` Thomas Gleixner
2011-04-21 12:06 ` Sascha Hauer
2011-04-21 15:38 ` Thomas Gleixner
2011-04-22 0:23 ` Colin Cross
2011-04-22 9:51 ` Thomas Gleixner
2011-04-22 16:14 ` Thomas Gleixner
2011-04-22 16:39 ` Colin Cross
2011-04-22 16:57 ` Thomas Gleixner
2011-04-22 22:26 ` Saravana Kannan
2011-04-22 22:55 ` Thomas Gleixner
2011-04-23 0:48 ` Saravana Kannan
2011-04-23 23:34 ` Benjamin Herrenschmidt
2011-04-22 4:54 ` Saravana Kannan
2011-04-22 9:06 ` Thomas Gleixner
2011-04-29 10:30 ` Russell King - ARM Linux
2011-04-29 10:51 ` Thomas Gleixner
2011-04-29 10:56 ` Russell King - ARM Linux
2011-04-24 9:45 ` Richard Zhao
2011-04-24 20:14 ` Uwe Kleine-König
2011-04-29 10:20 ` [PATCH 04/10] clk: implement parent pass through functions Russell King - ARM Linux
2011-04-15 19:08 ` [PATCH 05/10] clk: Add support for simple dividers Sascha Hauer
2011-04-18 9:49 ` Uwe Kleine-König
2011-04-18 10:07 ` Sascha Hauer
2011-04-19 2:45 ` Saravana Kannan
2011-04-19 7:32 ` Uwe Kleine-König
2011-04-19 8:55 ` Saravana Kannan
2011-04-19 9:31 ` Sascha Hauer
2011-04-19 22:28 ` Saravana Kannan
2011-04-20 6:36 ` Sascha Hauer
2011-04-20 21:45 ` Saravana Kannan
2011-04-21 7:39 ` Uwe Kleine-König
2011-04-28 15:14 ` Russell King - ARM Linux
2011-05-03 3:37 ` Saravana Kannan
2011-05-03 7:12 ` Uwe Kleine-König
2011-04-28 15:22 ` Russell King - ARM Linux
2011-05-02 7:58 ` Sascha Hauer
2011-04-18 22:40 ` Stephen Boyd
2011-04-19 0:32 ` Jeremy Kerr
2011-04-19 5:41 ` Stephen Boyd
2011-04-24 13:48 ` Richard Zhao
2011-04-25 18:51 ` Sascha Hauer
2011-04-26 1:54 ` Richard Zhao
2011-04-15 19:08 ` [PATCH 06/10] clk: Add support for a generic clock multiplexer Sascha Hauer
2011-04-18 13:15 ` Uwe Kleine-König
2011-04-18 13:33 ` Sascha Hauer
2011-04-18 13:54 ` Uwe Kleine-König
2011-04-18 17:54 ` Stephen Boyd
2011-04-18 18:34 ` Russell King - ARM Linux
2011-04-18 18:41 ` Russell King - ARM Linux
2011-04-18 18:46 ` Stephen Boyd
2011-04-15 19:08 ` [PATCH 07/10] ARM i.MX: Support for clock building blocks Sascha Hauer
2011-04-15 19:08 ` [PATCH 08/10] ARM i.MX: Add generic support for pllv2 Sascha Hauer
2011-04-15 19:08 ` [PATCH 09/10] ARM i.MX51/53: reimplement clock support Sascha Hauer
2011-04-15 19:08 ` [PATCH 10/10] ARM i.MX51/53: remove old " Sascha Hauer
2011-04-15 19:36 ` [RFC] sanitizing crazy clock data files Russell King - ARM Linux
2011-04-15 20:12 ` Sascha Hauer
2011-04-15 20:25 ` Russell King - ARM Linux
2011-04-15 20:28 ` Russell King - ARM Linux
2011-04-15 20:49 ` Uwe Kleine-König
2011-04-18 4:07 ` Shawn Guo
2011-04-15 20:45 ` Uwe Kleine-König
2011-04-18 7:42 ` Sascha Hauer
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=alpine.LFD.2.02.1104291223250.3005@ionos \
--to=tglx@linutronix.de \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox