[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200930115915.GB1611809@kroah.com>
Date: Wed, 30 Sep 2020 13:59:15 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc: broonie@...nel.org, rafael@...nel.org, lgirdwood@...il.com,
perex@...ex.cz, tiwai@...e.com, linux-kernel@...r.kernel.org,
alsa-devel@...a-project.org, srivasam@...eaurora.org,
rohitkr@...eaurora.org
Subject: Re: [PATCH v2 1/2] regmap: add support to
regmap_field_bulk_alloc/free apis
On Fri, Sep 25, 2020 at 05:48:55PM +0100, 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_fields.
>
> Using new bulk api to allocate fields makes it much more cleaner code to read!
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> Tested-by: Srinivasa Rao Mandadapu <srivasam@...eaurora.org>
> ---
> 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..8d6aedce666d 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 fields within the bank.
> + * @reg_field: Register fields within the bank.
> + * @num_fields: Number of register fields.
> + *
> + * 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 fields within the bank.
> + * @reg_field: Register fields within the bank.
> + * @num_fields: Number of register fields.
> + *
> + * 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-cycle.
> + */
> +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);
> +
You only have a patch that uses these last two functions, so why add all
4? We don't add infrastructure to the kernel without users.
thanks,
greg k-h
Powered by blists - more mailing lists