Re: [PATCH v5 3/4] clk: introduce the common clock framework

[Turquette, Mike]

On Thu, Mar 8, 2012 at 11:57 PM, Andrew Lunn <andrew@xxxxxxx> wrote:
>> I'd say use the nonstatic ones. I think using the static initializers
>> will cause us much pain in the future. I've been through several rebases
>> on the i.MX clock rework and everytime I wish my sed foo would be
>> better. Now imagine what happens when it turns out that the internal
>> struct clk layout or the structs for the muxes/dividers/gates have to
>> be changed.
>
> /*****************************************************************************
>  * CLK tree
>  ****************************************************************************/
> static DEFINE_SPINLOCK(gating_lock);
>
> #define DEFINE_KIRKWOOD_CLK_GATE(_name, _bit)                           \
>        DEFINE_CLK_GATE(_name, "tclk", NULL, 0,                         \
>                        (void __iomem *)CLOCK_GATING_CTRL,              \
>                        _bit, 0, &gating_lock)
>
> DEFINE_KIRKWOOD_CLK_GATE(clk_ge0,    CGC_BIT_GE0);
> DEFINE_KIRKWOOD_CLK_GATE(clk_pex0,   CGC_BIT_PEX0);
> DEFINE_KIRKWOOD_CLK_GATE(clk_usb0,   CGC_BIT_USB0);
> DEFINE_KIRKWOOD_CLK_GATE(clk_sdio,   CGC_BIT_SDIO);
> DEFINE_KIRKWOOD_CLK_GATE(clk_tsu,    CGC_BIT_TSU);
> DEFINE_KIRKWOOD_CLK_GATE(clk_dunit,  CGC_BIT_DUNIT);
> DEFINE_KIRKWOOD_CLK_GATE(clk_runit,  CGC_BIT_RUNIT);
>
> I've so far not had any problems, and not needed an sed foo.  I do
> only have a dozen or so clocks, which helps. But even so, all the real
> pain is hidden inside DEFINE_CLK_GATE() which Mike maintains.

It's true that if the argument list for the macros doesn't change then
the pain of static initialization is hidden from the platform data.

However if you have the ability to use the clk_foo_register functions
please do use them in place of static initialization.  The static init
stuff is only for folks backed into a corner and forced to use it...
for now.  I'm looking at ways to allow for kmalloc'ing in early boot,
as well as reducing the number of clocks that my platform registers
during early boot drastically.

>
> I guess the problem comes when you are not using the basic clk
> providers, but your own provider. What might help is if
> linux/clk-provider.h could provide some macros to do most of the
> generic definitions. Something like:

I'd rather people just used the registration functions instead.

Thanks,
Mike

>
> #define DEFINE_CLK_GENERIC(_name, _flags, _ops)                 \
>        static struct clk _name;                                \
>        static char *_name##_parent_names[] = {};               \
>        static struct clk _name = {                             \
>                .name = #_name,                                 \
>                .ops = &_ops,                                   \
>                .hw = &_name##_hw.hw,                           \
>                .parent_names = _name##_parent_names,           \
>                .num_parents =                                  \
>                        ARRAY_SIZE(_name##_parent_names),       \
>                .flags = _flags,                                \
>        };
>
> and then you have something like
>
> #define DEFINE_CLK_IMX(_name, _flags, _foo, _bar)               \
>        static struct clk_imx _name##_hw = {                    \
>                .hw = {                                         \
>                        .clk = &_name,                          \
>                },                                              \
>                .foo = _foo,                                    \
>                .bar = _bar,                                    \
>        };                                                      \
>        DEFINE_CLK_GENERIC(_name, _flags, clk_imx_ops)
>
>        Andrew
--
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/