On Thu, Oct 31, 2013 at 16:40 +0200, Tero Kristo wrote: > > On 10/31/2013 04:03 PM, Nishanth Menon wrote: > >On 10/25/2013 10:56 AM, Tero Kristo wrote: > >[...] > >>diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > >>index 7e59253..63ff78c 100644 > >>--- a/include/linux/clk-provider.h > >>+++ b/include/linux/clk-provider.h > > > >[...] > >>-static inline u32 clk_readl(u32 __iomem *reg) > >>+static inline u32 clk_readl(u32 __iomem *reg, struct regmap *regmap) > >> { > >>- return readl(reg); > >>+ u32 val; > >>+ > >>+ if (regmap) > >>+ regmap_read(regmap, (u32)reg, &val); > >>+ else > >>+ val = readl(reg); > >>+ return val; > >> } > >> > >>-static inline void clk_writel(u32 val, u32 __iomem *reg) > >>+static inline void clk_writel(u32 val, u32 __iomem *reg, struct regmap *regmap) > >> { > >>- writel(val, reg); > >>+ if (regmap) > >>+ regmap_write(regmap, (u32)reg, val); > >>+ else > >>+ writel(val, reg); > >> } > >> > >> #endif /* CONFIG_COMMON_CLK */ > >> > > > >Might it not be better to introduce regmap variants? > >static inline void clk_regmap_writel(u32 val, u32 reg, struct regmap > >*regmap) > >and corresponding readl? that allows cleaner readability for clk > >drivers that use regmap and those that dont. > > Well, doing that will introduce a lot of redundant code, as the > checks for the presence of regmap must be copied all over the place. > With this patch, all the generic clock drivers support internally > both regmap or non-regmap register accesses. Please keep in mind that the "identity" of clk_readl() and readl() only applies in the current source code (ARM only use of common CCF primitives), while patches are pending (currently under review and receiving further improvement) which introduce several alternative implementations of clk_readl() depending on the platform. Making all of them duplicate the "regmap vs direct register access" branch would be as bad. Keeping one set of clk_readl() and clk_writel() routines and adding #ifdefs around the direct register access would be rather ugly, and I understand that preprocessor ifdef counts should get reduced instead of introduced. virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@xxxxxxx -- 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