Re: [RFC,PATCH 0/7 v2] Common struct clk implementation
From: Jeremy Kerr <hidden>
Date: 2010-01-13 01:17:51
Also in:
linux-arm-kernel, lkml
Hi Russell,
But the point I was trying to convey is that OMAP doesn't work with _either_ a pure operations struct _or_ a bunch of per-clock function pointers - it currently uses a mixture of the two.
With the common clk, you can do exactly that:
struct clk_foo {
/* ->ops provides functions common to clk_foo */
struct clk;
/* provides a function for only this clock */
int (*single_clock_func)(struct clk_foo *);
}
The only real difference is that the public API is provided through struct =
clk=20
rather than redefined clk_* functions; whatever is implementing the clock-
type-specific struct clk can add whatever fields necessary.
=46rom your earlier mail about sizes on omap:
There are two function pointers in the struct clk which would be identical to the versions proposed in this generic struct clk. There's a total of 219 clk structures in OMAP3. So, 219 * (4 + 8) =3D 2628. Switching OMAP means 219 * (4 + 32) =3D 7884, which is an increase in overhead of 3x.
But we also can reduce the size of the struct clk in most cases; I believe =
the=20
separate clk_operations in v2 of this series will help with this.
Taking OMAP3 for example (I'm not very familiar with that platform, so am=20
basing this on a brief look through the clock code), the first step to a=20
common clk port would be to wrap the existing struct clk:
struct clk_omap {
struct clk clk;
struct list_head node;
const struct clkops *ops;
[...]
};
and define the clk_operations to be the current omap clk_* functions.
This results in one extra pointer per clock, plus the clk_operations, so 90=
8=20
bytes (4 * 219 + 32) overhead.
Next, we can start removing fields that are not used by all clocks; the fix=
ed=20
top-level clocks would be a good start; it looks like we can represent thos=
e=20
with a:
struct clk_omap_fixed {
struct clk clk;
char *name;
unsigned long rate;
}
[For these fixed clocks, we don't need to propagate changes to children, he=
nce=20
I'm assuming no child/sibling members]
The original struct clk is 96 bytes; clk_omap_fixed is 12, but we still nee=
d=20
one clk_operations (32 bytes). Since there are 8 of these, we save 640 byte=
s=20
((96 - 12) * 8 - 32).
If we then take the 'follow parent' clocks, it looks like we can represent=
=20
those with something like:
struct clk_omap_followparent =3D {
struct clk clk;
char *name;
struct clk *parent;
struct list_head children, siblings;
unsigned long rate;
void __iomem *enable_reg;
__u8 enable_bit;
char *clkdm_name;
int flags;
};
This would be 48 bytes, there are 140 of these, saving 6688 bytes ( (96 - 4=
8)=20
* 140 - 32).
Now, we could stop here, or keep looking for common usage patterns of struc=
t=20
clk to find cases where creating another clock type makes sense.
I know I've only looked at the easy OMAP cases here, but the principle stil=
l=20
applies: keep the original struct clk around as a fallback, but use the=20
smaller struct clks where possible.
Cheers,
Jeremy