On Friday 20 November 2015 20:07:46 Vladimir Zapolskiy wrote: > On 20.11.2015 16:04, Arnd Bergmann wrote: > > On Friday 20 November 2015 03:05:09 Vladimir Zapolskiy wrote: > >> + > >> +struct clk_proto_t { > >> + const char *name; > >> + const u8 parents[LPC32XX_CLK_PARENTS_MAX]; > >> + u8 num_parents; > >> + unsigned long flags; > >> +}; > >> + > >> +#define CLK_PREFIX(LITERAL) LPC32XX_CLK_ ## LITERAL > >> +#define NUMARGS(...) (sizeof((int[]){__VA_ARGS__})/sizeof(int)) > >> + > >> +#define LPC32XX_CLK_DEFINE(_idx, _name, _flags, ...) \ > >> + [CLK_PREFIX(_idx)] = { \ > >> + .name = #_name, \ > >> + .flags = _flags, \ > >> + .parents = { __VA_ARGS__ }, \ > >> + .num_parents = NUMARGS(__VA_ARGS__), \ > >> + } > >> + > > > > Try to not outsmart yourself with the macros. It's better to avoid > > string concatenation so it's possible to grep for uses of some > > constant. > > > > I would probably not use a macro at all here and just open-code the > > entire table. If you ensure that '0' is not a valid parent, then > > you can leave out the .num_parents field and just look for the > > zero-termination. > > Macros are here for simplicity, code size reduction and to avoid some > stupid mistakes like different number of .parents and .num_parents. > > I believe macro unwrapping in this code will add another 1000 LoC and > will result in quite unreadable and less maintainable code. I mean specifically the macro above: static const struct clk_proto_t clk_proto[LPC32XX_CLK_CCF_MAX] __initconst = { + LPC32XX_CLK_DEFINE(XTAL, xtal, 0x0), + LPC32XX_CLK_DEFINE(XTAL_32K, xtal_32k, 0x0), + + LPC32XX_CLK_DEFINE(RTC, rtc, 0x0, LPC32XX_CLK_XTAL_32K), + LPC32XX_CLK_DEFINE(OSC, osc, CLK_IGNORE_UNUSED, LPC32XX_CLK_XTAL), + LPC32XX_CLK_DEFINE(SYS, sys, CLK_IGNORE_UNUSED, + LPC32XX_CLK_OSC, LPC32XX_CLK_PLL397X), + LPC32XX_CLK_DEFINE(PLL397X, pll_397x, CLK_IGNORE_UNUSED, + LPC32XX_CLK_RTC), can become static const struct clk_proto_t clk_proto[] __initconst = { [LPC32XX_CLK_XTAL] = { "xtal" }, [LPC32XX_CLK_XTAL_32K] = { "xtal_32k" }, [LPC32XX_CLK_RTC] = { "rtc", .parents = { LPC32XX_CLK_XTAL_32K, 0 } }, [LPC32XX_CLK_OSC] = { "osc", CLK_IGNORE_UNUSED, .parents = { LPC32XX_CLK_XTAL, 0 } }, [LPC32XX_CLK_SYS] = { "sys", CLK_IGNORE_UNUSED, .parents = { LPC32XX_CLK_OSC, LPC32XX_CLK_PLL397X, 0) }, [LPC32XX_CLK_PLL397X] = { "pll_397x", CLK_IGNORE_UNUSED, .parents = { LPC32XX_CLK_RTC, 0 }, Not harder to read at all, not really longer, but easier to grep for. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html