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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJOA=zPztBfTY_WDwZ5iFEa94eCtHyrzN422RTACMQXs625gRA@mail.gmail.com>
Date:	Fri, 9 Mar 2012 10:25:00 -0800
From:	"Turquette, Mike" <mturquette@...com>
To:	Sascha Hauer <s.hauer@...gutronix.de>,
	"Turquette, Mike" <mturquette@...com>,
	Andrew Lunn <andrew@...n.ch>, Paul Walmsley <paul@...an.com>,
	linaro-dev@...ts.linaro.org,
	Linus Walleij <linus.walleij@...ricsson.com>,
	patches@...aro.org, Stephen Boyd <sboyd@...eaurora.org>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Magnus Damm <magnus.damm@...il.com>,
	linux-kernel@...r.kernel.org,
	Rob Herring <rob.herring@...xeda.com>,
	Richard Zhao <richard.zhao@...aro.org>,
	Grant Likely <grant.likely@...retlab.ca>,
	Deepak Saxena <dsaxena@...aro.org>,
	Saravana Kannan <skannan@...eaurora.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Shawn Guo <shawn.guo@...escale.com>,
	Amit Kucheria <amit.kucheria@...aro.org>,
	Russell King <linux@....linux.org.uk>,
	Jeremy Kerr <jeremy.kerr@...onical.com>,
	Arnd Bergman <arnd.bergmann@...aro.org>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v5 3/4] clk: introduce the common clock framework

On Thu, Mar 8, 2012 at 11:57 PM, Andrew Lunn <andrew@...n.ch> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/