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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/31/2013 05:46 PM, Nishanth Menon wrote:
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..

Initial work I did for v9 had clk_reg_ops struct in place which pretty much did this, however Mike recommended looking at the regmap so I did. I could put the reg_ops struct back and just have it use internally regmap if that would be better, but I guess we need comment from Mike how he wants this to be done.

We could have:

struct clk_reg_ops {
	int (*clk_readl)(u32 addr, u32 *val);
	int (*clk_writel)(u32 addr, u32 val);
};

struct clk_foo {
	...
	void __iomem *reg;
	struct clk_reg_ops *regops;
};


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.

Currently, none of the clock drivers handle failures for regmap operations, I can of course add some sort of support for this given what we do with above point and what the API for the register accesses ends up like.

-Tero
--
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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux