[PATCHv9 01/43] clk: Add support for regmap register read/write
Tero Kristo
t-kristo at ti.com
Fri Nov 1 04:57:28 EDT 2013
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
More information about the linux-arm-kernel
mailing list