Messages in this thread | | | Date | Wed, 05 Jun 2013 10:21:23 +0100 | From | Srinivas KANDAGATLA <> | Subject | Re: [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 >
| |