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

[Nishanth Menon]

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