Re: [PATCH 1/2] regmap: add support to regmap_field_bulk_alloc/free apis

[Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>]

Thanks Pierre,

On 25/09/2020 14:12, Pierre-Louis Bossart wrote:
> On 9/25/20 4:28 AM, Srinivas Kandagatla wrote:
> > Usage of regmap_field_alloc becomes much overhead when number of fields
> > exceed more than 3.
> > QCOM LPASS driver has extensively converted to use regmap_fileds.
>
> Multiple typos scattered in this patch: fileds -> fields?
thanks for checking this, I will fix that in next spin!

--srini
> > Using new bluk api to allocate fields makes it much more cleaner code 
> > to read!
>
> bulk api?
>
> > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> > Tested-by: Srinivasa Rao Mandadapu <srivasam@xxxxxxxxxxxxxx>
> > ---
> >   drivers/base/regmap/regmap.c | 100 +++++++++++++++++++++++++++++++++++
> >   include/linux/regmap.h       |  11 ++++
> >   2 files changed, 111 insertions(+)
> >
> > diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> > index aec3f26bf711..271740a163ad 100644
> > --- a/drivers/base/regmap/regmap.c
> > +++ b/drivers/base/regmap/regmap.c
> > @@ -1270,6 +1270,106 @@ struct regmap_field 
> > *devm_regmap_field_alloc(struct device *dev,
> >   }
> >   EXPORT_SYMBOL_GPL(devm_regmap_field_alloc);
> > +
> > +/**
> > + * regmap_field_bulk_alloc() - Allocate and initialise a bulk 
> > register field.
> > + *
> > + * @regmap: regmap bank in which this register field is located.
> > + * @rm_field: regmap register fileds with in the bank.
> > + * @reg_field: Register fields with in the bank.
> > + * @num_fields: Number of register fileds.
> > + *
> > + * The return value will be an -ENOMEM on error or zero for success.
> > + * Newly allocated regmap_fields should be freed by calling
> > + * regmap_field_bulk_free()
> > + */
> > +int regmap_field_bulk_alloc(struct regmap *regmap,
> > +                struct regmap_field **rm_field,
> > +                struct reg_field *reg_field,
> > +                int num_fields)
> > +{
> > +    struct regmap_field *rf;
> > +    int i;
> > +
> > +    rf = kcalloc(num_fields, sizeof(*rf), GFP_KERNEL);
> > +    if (!rf)
> > +        return -ENOMEM;
> > +
> > +    for (i = 0; i < num_fields; i++) {
> > +        regmap_field_init(&rf[i], regmap, reg_field[i]);
> > +        rm_field[i] = &rf[i];
> > +    }
> > +
> > +    return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(regmap_field_bulk_alloc);
> > +
> > +/**
> > + * devm_regmap_field_bulk_alloc() - Allocate and initialise a bulk 
> > register
> > + * fields.
> > + *
> > + * @dev: Device that will be interacted with
> > + * @regmap: regmap bank in which this register field is located.
> > + * @rm_field: regmap register fileds with in the bank.
> > + * @reg_field: Register fields with in the bank.
>
> within?
>
> > + * @num_fields: Number of register fileds.
> > + *
> > + * The return value will be an -ENOMEM on error or zero for success.
> > + * Newly allocated regmap_fields will be automatically freed by the
> > + * device management code.
> > + */
> > +int devm_regmap_field_bulk_alloc(struct device *dev,
> > +                 struct regmap *regmap,
> > +                 struct regmap_field **rm_field,
> > +                 struct reg_field *reg_field,
> > +                 int num_fields)
> > +{
> > +    struct regmap_field *rf;
> > +    int i;
> > +
> > +    rf = devm_kcalloc(dev, num_fields, sizeof(*rf), GFP_KERNEL);
> > +    if (!rf)
> > +        return -ENOMEM;
> > +
> > +    for (i = 0; i < num_fields; i++) {
> > +        regmap_field_init(&rf[i], regmap, reg_field[i]);
> > +        rm_field[i] = &rf[i];
> > +    }
> > +
> > +    return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_regmap_field_bulk_alloc);
> > +
> > +/**
> > + * regmap_field_bulk_free() - Free register field allocated using
> > + *                       regmap_field_bulk_alloc.
> > + *
> > + * @field: regmap fields which should be freed.
> > + */
> > +void regmap_field_bulk_free(struct regmap_field *field)
> > +{
> > +    kfree(field);
> > +}
> > +EXPORT_SYMBOL_GPL(regmap_field_bulk_free);
> > +
> > +/**
> > + * devm_regmap_field_bulk_free() - Free a bulk register field 
> > allocated using
> > + *                            devm_regmap_field_bulk_alloc.
> > + *
> > + * @dev: Device that will be interacted with
> > + * @field: regmap field which should be freed.
> > + *
> > + * Free register field allocated using 
> > devm_regmap_field_bulk_alloc(). Usually
> > + * drivers need not call this function, as the memory allocated via devm
> > + * will be freed as per device-driver life-cyle.
> > + */
> > +void devm_regmap_field_bulk_free(struct device *dev,
> > +                 struct regmap_field *field)
> > +{
> > +    devm_kfree(dev, field);
> > +}
> > +EXPORT_SYMBOL_GPL(devm_regmap_field_bulk_free);
> > +
> >   /**
> >    * devm_regmap_field_free() - Free a register field allocated using
> >    *                            devm_regmap_field_alloc.
> > diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> > index 0c49d59168b5..a35ec0a0d6e0 100644
> > --- a/include/linux/regmap.h
> > +++ b/include/linux/regmap.h
> > @@ -1189,6 +1189,17 @@ struct regmap_field 
> > *devm_regmap_field_alloc(struct device *dev,
> >           struct regmap *regmap, struct reg_field reg_field);
> >   void devm_regmap_field_free(struct device *dev,    struct 
> > regmap_field *field);
> > +int regmap_field_bulk_alloc(struct regmap *regmap,
> > +                 struct regmap_field **rm_field,
> > +                 struct reg_field *reg_field,
> > +                 int num_fields);
> > +void regmap_field_bulk_free(struct regmap_field *field);
> > +int devm_regmap_field_bulk_alloc(struct device *dev, struct regmap 
> > *regmap,
> > +                 struct regmap_field **field,
> > +                 struct reg_field *reg_field, int num_fields);
> > +void devm_regmap_field_bulk_free(struct device *dev,
> > +                 struct regmap_field *field);
> > +
> >   int regmap_field_read(struct regmap_field *field, unsigned int *val);
> >   int regmap_field_update_bits_base(struct regmap_field *field,
> >                     unsigned int mask, unsigned int val,