[PATCHv9 01/43] clk: Add support for regmap register read/write

[Gerhard Sittig]

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 at denx.de