On 11/05/2013 11:43 PM, Gerhard Sittig wrote:
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.
So, it might be better to provide platform / or clock specific
clk_reg_ops or something, so we can cover all the possible cases easily.
The requirement from TI SoC point of view is that:
1) need to be able to specify custom register read/write ops
2) need to be able to provide an index parameter to the read/write ops
3) need to be able to provide a register offset to the read/write ops
This could be done in following way for example:
clk_readl / clk_writel would be accessed through a single platform
specific struct which provides function pointers to both.
The content of 'reg' provided to the clk_readl / clk_writel would be
internally interpreted as a
struct omap_clk_reg_internal {
u16 offset;
u16 index;
};
... the index part would be used to select the appropriate regmap to
use, and TI SoC:s would be using 3...4 regmaps for actually accessing
the physical registers themselves.
-Tero
virtually yours
Gerhard Sittig
--
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
