Re: [PATCH E 11/14] OMAP clock: track child clocks

From: Russell King - ARM Linux
Date: Thu Jan 29 2009 - 17:06:37 EST

On Thu, Jan 29, 2009 at 03:14:01PM +0000, Russell King - ARM Linux wrote:
> On Wed, Jan 28, 2009 at 12:27:59PM -0700, Paul Walmsley wrote:
> > The price paid is additional runtime memory consumption - 8 bytes per
> > clock and 16 bytes per child clock - roughly 4.5KiB on OMAP3.
>
> For OMAP3, that's 222 struct clks of which 182 are children, and indeed
> 222 * 8 + 182 * 16 gives about 4.5K. On OMAP2, it's 140 and 136,
> giving 140 * 8 + 136 * 16 = 3.3K.
>
> Moving struct clk_child into struct clk means that its clk and flags
> members can be deleted, making it 8 bytes in size - effectively just
> the list_head. We need a list_head for the 'children' as you have it.
> So, that works out as 16 bytes per clock. That gives 3.5K on OMAP3
> and 2.2K on OMAP2.
>
> So, by taking that alternative approach, not only do you end up using
> less memory, but you also don't have to have the overhead of your
> custom memory bookkeeping.
>
> The other change I'd suggest is that you have one function which deals
> with setting the parent of a clock:
>
> void clk_reparent(struct clk *child, struct clk *parent)
> {
> list_del_init(&child->sibling);
> if (parent)
> list_add(&child->sibling, &parent->children);
> child->parent = parent;
>
> /* now do the debugfs renaming to reattach the child
> to the proper parent */
> }
>
> which is a lot simpler than your omap_clk_add_child() and omap_clk_del_child().
>
> These should be in the _core_ OMAP clock code, not just in the OMAP2
> clock code. OMAP1 has child clocks as well as OMAP2.

And here is my version of this patch:

arch/arm/mach-omap2/clock.c | 4 +-
arch/arm/plat-omap/clock.c | 49 +++++++++++++++++++------------
arch/arm/plat-omap/include/mach/clock.h | 3 ++
3 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index 56d9ea4..14bf566 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -175,7 +175,7 @@ void omap2_init_clksel_parent(struct clk *clk)
clk->name, clks->parent->name,
((clk->parent) ?
clk->parent->name : "NULL"));
- clk->parent = clks->parent;
+ clk_reparent(clk, clks->parent);
};
found = 1;
}
@@ -780,7 +780,7 @@ int omap2_clk_set_parent(struct clk *clk, struct clk *new_parent)
if (clk->usecount > 0)
_omap2_clk_enable(clk);

- clk->parent = new_parent;
+ clk_reparent(clk, new_parent);

/* CLKSEL clocks follow their parents' rates, divided by a divisor */
clk->rate = new_parent->rate;
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index 3688400..58e42b1 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -127,8 +127,7 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
if (ret == 0) {
if (clk->recalc)
clk->recalc(clk);
- if (clk->flags & RATE_PROPAGATES)
- propagate_rate(clk);
+ propagate_rate(clk);
}
spin_unlock_irqrestore(&clockfw_lock, flags);

@@ -150,8 +149,7 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
if (ret == 0) {
if (clk->recalc)
clk->recalc(clk);
- if (clk->flags & RATE_PROPAGATES)
- propagate_rate(clk);
+ propagate_rate(clk);
}
spin_unlock_irqrestore(&clockfw_lock, flags);

@@ -192,27 +190,34 @@ void followparent_recalc(struct clk *clk)
clk->rate = clk->parent->rate;
}

+void clk_reparent(struct clk *child, struct clk *parent)
+{
+ list_del_init(&child->sibling);
+ if (parent)
+ list_add(&child->sibling, &parent->children);
+ child->parent = parent;
+
+ /* now do the debugfs renaming to reattach the child
+ to the proper parent */
+}
+
/* Propagate rate to children */
void propagate_rate(struct clk * tclk)
{
struct clk *clkp;

- if (tclk == NULL || IS_ERR(tclk))
- return;
-
- list_for_each_entry(clkp, &clocks, node) {
- if (likely(clkp->parent != tclk))
- continue;
+ list_for_each_entry(clkp, &tclk->children, sibling) {
if (clkp->recalc) {
unsigned long old_rate = clkp->rate;
clkp->recalc(clkp);
- if (clkp->rate != old_rate &&
- (clkp->flags & RATE_PROPAGATES))
+ if (clkp->rate != old_rate)
propagate_rate(clkp);
}
}
}

+static LIST_HEAD(root_clks);
+
/**
* recalculate_root_clocks - recalculate and propagate all root clocks
*
@@ -224,13 +229,10 @@ void recalculate_root_clocks(void)
{
struct clk *clkp;

- list_for_each_entry(clkp, &clocks, node) {
- if (!clkp->parent) {
- if (clkp->recalc)
- clkp->recalc(clkp);
- if (clkp->flags & RATE_PROPAGATES)
- propagate_rate(clkp);
- }
+ list_for_each_entry(clkp, &root_clks, sibling) {
+ if (clkp->recalc)
+ clkp->recalc(clkp);
+ propagate_rate(clkp);
}
}

@@ -246,6 +248,15 @@ int clk_register(struct clk *clk)
return 0;

mutex_lock(&clocks_mutex);
+ if (!clk->children.next)
+ INIT_LIST_HEAD(&clk->children);
+ if (clk->parent && !clk->parent->children.next)
+ INIT_LIST_HEAD(&clk->parent->children);
+ if (clk->parent)
+ list_add(&clk->sibling, &clk->parent->children);
+ else
+ list_add(&clk->sibling, &root_clks);
+
list_add(&clk->node, &clocks);
if (clk->init)
clk->init(clk);
diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
index ad0345f..3504c2b 100644
--- a/arch/arm/plat-omap/include/mach/clock.h
+++ b/arch/arm/plat-omap/include/mach/clock.h
@@ -70,6 +70,8 @@ struct clk {
const char *name;
int id;
struct clk *parent;
+ struct list_head children;
+ struct list_head sibling; /* node for children */
unsigned long rate;
__u32 flags;
void __iomem *enable_reg;
@@ -116,6 +118,7 @@ extern unsigned int mpurate;

extern int clk_init(struct clk_functions *custom_clocks);
extern int clk_register(struct clk *clk);
+extern void clk_reparent(struct clk *child, struct clk *parent);
extern void clk_unregister(struct clk *clk);
extern void propagate_rate(struct clk *clk);
extern void recalculate_root_clocks(void);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/