On 10/31/2013 09:40 AM, 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. > using function pointers might be an appropriate solution. in general a low level reg access api that uses two different approaches sounds a little.. umm.. fishy.. >> regmap can also return error value that could also be handled as a result. > > True, however if the regmap fails for the clock code, not sure what we > can do but panic. If this code is expanded, it is probably better to not > inline it anymore. > let the compiler deal with that decision. regmap operation fail should be percollated back to initiator of the request. in some cases that will be ir-recoverable, but on other cases panic might be the right answer - at the low level we are in no position to make that distinction. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html