lkml.org 
[lkml]   [2013]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC] regmap: Add regmap_field APIs
On 04/06/13 22:01, Mark Brown wrote:
> On Tue, May 28, 2013 at 03:58:00PM +0100, Srinivas KANDAGATLA wrote:
>
>> +#define REGMAP_FIELD_INIT(regmap, reg, lsb, msb) { \
>> + .regmap = regmap, \
>> + .reg = reg, \
>> + .lsb = lsb, \
>> + .msb = msb, \
>> + }
>
> Having a macro for this is really odd since macros are generally only
> used at compile time but the regmap is only available at runtime and
> this...
>
Yes, I think the macro is bit over do.. I will remove it.

>> +static inline void regmap_field_init(struct regmap_field *field,
>> + struct regmap *regmap, unsigned int reg,
>> + unsigned int lsb, unsigned int msb)
>> +{
>> + field->regmap = regmap;
>> + field->reg = reg;
>> + field->lsb = lsb;
>> + field->msb = msb;
>> +}
>
> ...is a bit awkward since you can't use it with static data. I think
> either the read/write/modify APIs should be changed to take both the map
> and the field as arguments (with the field only containing the bitfield
> definitions) or the init function should be something that allocates a
> new, runtime only structure from static data.
I agree with you and I think init function should allocate and
initialize the regmap_field and return it, and the read/write apis will
take the field and value. This approach looks neat.

Is it ok if we rename the regmap_field_init function to
regmap_field_alloc, as it will make it obvious that its allocating
memory which should be freed?
I also thought we could add devm version of it as well.

With this change here is what the init/alloc function would look like:

static void _regmap_field_init(struct regmap_field *field,
struct regmap *regmap, unsigned int reg,
unsigned int lsb, unsigned int msb)
{
field->regmap = regmap;
field->reg = reg;
field->lsb = lsb;
field->msb = msb;
}

struct regmap_field *devm_regmap_field_alloc(struct device *dev,
struct regmap *regmap, unsigned int reg,
unsigned int lsb, unsigned int msb)
{
struct regmap_field *field = devm_kzalloc(dev,
sizeof(*field), GFP_KERNEL);
if (!field)
return ERR_PTR(-ENOMEM);

_regmap_field_init(field, regmap, reg, lsb, msb);
return field;

}
EXPORT_SYMBOL_GPL(devm_regmap_field_alloc);

struct regmap_field *regmap_field_alloc(struct regmap *regmap,
unsigned int reg, unsigned int lsb, unsigned int msb)
{
struct regmap_field *field = kzalloc(sizeof(*field), GFP_KERNEL);
if (!field)
return ERR_PTR(-ENOMEM);

_regmap_field_init(field, regmap, reg, lsb, msb);
return field;
}
EXPORT_SYMBOL_GPL(regmap_field_alloc);

In header file..
static void inline regmap_field_free(struct regmap_field *field)
{
kfree(field);
}

static void inline devm_regmap_field_free(struct device *dev, struct
regmap_field *field)
{
devm_kfree(dev, field);
}

Thanks,
srini
>



\
 
 \ /
  Last update: 2013-06-05 14:41    [W:1.413 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site