Add new managed resource functions as needed to achieve this.
Amend the following functions:
devm_kstrdup()
gpio_regulator_probe()
Add the following functions:
devm_kmemdup()
devm_gpio_request_array()
devm_gpio_free_array()
Remove the following functions:
gpio_regulator_remove()
>From Rob Jones <[email protected]> # This line is ignored.
From: Rob Jones <[email protected]>
Subject:
In-Reply-To:
Add functions devm_gpio_request_array() and devm_gpio_free_array()
which are exactly analogous to the equivalent non-managed device
functions gpio_request_array() and gpio_free_array(), which can be
found in the module gpiolib.c.
Note that if devm_gpio_request_array() fails, no gpios are obtained.
No indication is provided as to which particular request failed.
Reviewed-by: Ian Molton <[email protected]>
Signed-off-by: Rob Jones <[email protected]>
---
drivers/gpio/devres.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/gpio.h | 4 ++++
2 files changed, 61 insertions(+)
diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c
index 307464f..d4c2161 100644
--- a/drivers/gpio/devres.c
+++ b/drivers/gpio/devres.c
@@ -186,6 +186,63 @@ int devm_gpio_request_one(struct device *dev, unsigned gpio,
EXPORT_SYMBOL(devm_gpio_request_one);
/**
+ * devm_gpio_request_array - request multiple GPIOs in a single call
+ * for a managed device
+ * @dev: device requesting the gpio
+ * @array: array of the 'struct gpio'
+ * @num: how many GPIOs in the array
+ *
+ * Except for the extra @dev argument, this function takes the
+ * same arguments and performs the same function as
+ * gpio_request(). GPIOs requested with this function will be
+ * automatically freed on driver detach.
+ *
+ * If GPIOs allocated with this function need to be freed separately,
+ * devm_gpio_free_array() or devm_gpio_free() must be used.
+ */
+int devm_gpio_request_array(struct device *dev,
+ const struct gpio *array,
+ size_t num)
+{
+ int i, err = 0;
+
+ for (i = 0; i < num; i++, array++) {
+ err = devm_gpio_request_one(dev,
+ array->gpio,
+ array->flags,
+ array->label);
+ if (err) {
+ while (i--)
+ devm_gpio_free(dev, (--array)->gpio);
+ }
+ }
+
+ return err;
+}
+EXPORT_SYMBOL(devm_gpio_request_array);
+
+/**
+ * devm_gpio_free_array - release multiple GPIOs in a single call
+ * for a managed device
+ * @dev: device requesting the gpio
+ * @array: array of the 'struct gpio'
+ * @num: how many GPIOs in the array
+ *
+ * Except for the extra @dev argument, this function takes the
+ * same arguments and performs the same function as gpio_free_array().
+ * This function instead of gpio_free_array() should be used to
+ * manually free GPIOs allocated with devm_gpio_request().
+ */
+void devm_gpio_free_array(struct device *dev,
+ const struct gpio *array,
+ size_t num)
+{
+ while (num--)
+ devm_gpio_free(dev, (array++)->gpio);
+}
+EXPORT_SYMBOL(devm_gpio_free_array);
+
+/**
* devm_gpio_free - free a GPIO
* @dev: device to free GPIO for
* @gpio: GPIO to free
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index 85aa5d0..12d5648 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -84,6 +84,10 @@ struct device;
int devm_gpio_request(struct device *dev, unsigned gpio, const char *label);
int devm_gpio_request_one(struct device *dev, unsigned gpio,
unsigned long flags, const char *label);
+int devm_gpio_request_array(struct device *dev, const struct gpio *array,
+ size_t num);
+void devm_gpio_free_array(struct device *dev, const struct gpio *array,
+ size_t num);
void devm_gpio_free(struct device *dev, unsigned int gpio);
#else /* ! CONFIG_GPIOLIB */
--
1.7.10.4
Use managed resource functions for all resource allocations in
gpio_regulator_probe().
Remove gpio_regulator_remove() as it is now redundant.
Reviewed-by: Ian Molton <[email protected]>
Signed-off-by: Rob Jones <[email protected]>
---
drivers/regulator/gpio-regulator.c | 70 ++++++++++++------------------------
1 file changed, 23 insertions(+), 47 deletions(-)
diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index 989b23b..6edde12 100644
--- a/drivers/regulator/gpio-regulator.c
+++ b/drivers/regulator/gpio-regulator.c
@@ -254,30 +254,31 @@ static int gpio_regulator_probe(struct platform_device *pdev)
if (drvdata == NULL)
return -ENOMEM;
- drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
+ drvdata->desc.name = devm_kstrdup(&pdev->dev,
+ config->supply_name,
+ GFP_KERNEL);
if (drvdata->desc.name == NULL) {
dev_err(&pdev->dev, "Failed to allocate supply name\n");
- ret = -ENOMEM;
- goto err;
+ return -ENOMEM;
}
- drvdata->gpios = kmemdup(config->gpios,
- config->nr_gpios * sizeof(struct gpio),
- GFP_KERNEL);
+ drvdata->gpios = devm_kmemdup(&pdev->dev,
+ config->gpios,
+ config->nr_gpios * sizeof(struct gpio),
+ GFP_KERNEL);
if (drvdata->gpios == NULL) {
dev_err(&pdev->dev, "Failed to allocate gpio data\n");
- ret = -ENOMEM;
- goto err_name;
+ return -ENOMEM;
}
- drvdata->states = kmemdup(config->states,
- config->nr_states *
+ drvdata->states = devm_kmemdup(&pdev->dev,
+ config->states,
+ config->nr_states *
sizeof(struct gpio_regulator_state),
- GFP_KERNEL);
+ GFP_KERNEL);
if (drvdata->states == NULL) {
dev_err(&pdev->dev, "Failed to allocate state data\n");
- ret = -ENOMEM;
- goto err_memgpio;
+ return -ENOMEM;
}
drvdata->nr_states = config->nr_states;
@@ -297,16 +298,17 @@ static int gpio_regulator_probe(struct platform_device *pdev)
break;
default:
dev_err(&pdev->dev, "No regulator type set\n");
- ret = -EINVAL;
- goto err_memgpio;
+ return -EINVAL;
}
drvdata->nr_gpios = config->nr_gpios;
- ret = gpio_request_array(drvdata->gpios, drvdata->nr_gpios);
+ ret = devm_gpio_request_array(&pdev->dev,
+ drvdata->gpios,
+ drvdata->nr_gpios);
if (ret) {
dev_err(&pdev->dev,
"Could not obtain regulator setting GPIOs: %d\n", ret);
- goto err_memstate;
+ return ret;
}
/* build initial state from gpio init data. */
@@ -337,43 +339,18 @@ static int gpio_regulator_probe(struct platform_device *pdev)
cfg.ena_gpio_flags |= GPIOF_OUT_INIT_HIGH;
}
- drvdata->dev = regulator_register(&drvdata->desc, &cfg);
+ drvdata->dev = devm_regulator_register(&pdev->dev,
+ &drvdata->desc,
+ &cfg);
if (IS_ERR(drvdata->dev)) {
ret = PTR_ERR(drvdata->dev);
dev_err(&pdev->dev, "Failed to register regulator: %d\n", ret);
- goto err_stategpio;
+ return ret;
}
platform_set_drvdata(pdev, drvdata);
return 0;
-
-err_stategpio:
- gpio_free_array(drvdata->gpios, drvdata->nr_gpios);
-err_memstate:
- kfree(drvdata->states);
-err_memgpio:
- kfree(drvdata->gpios);
-err_name:
- kfree(drvdata->desc.name);
-err:
- return ret;
-}
-
-static int gpio_regulator_remove(struct platform_device *pdev)
-{
- struct gpio_regulator_data *drvdata = platform_get_drvdata(pdev);
-
- regulator_unregister(drvdata->dev);
-
- gpio_free_array(drvdata->gpios, drvdata->nr_gpios);
-
- kfree(drvdata->states);
- kfree(drvdata->gpios);
-
- kfree(drvdata->desc.name);
-
- return 0;
}
#if defined(CONFIG_OF)
@@ -385,7 +362,6 @@ static const struct of_device_id regulator_gpio_of_match[] = {
static struct platform_driver gpio_regulator_driver = {
.probe = gpio_regulator_probe,
- .remove = gpio_regulator_remove,
.driver = {
.name = "gpio-regulator",
.owner = THIS_MODULE,
--
1.7.10.4
Avoid code duplication by using devm_kmemdup() to copy data instead
of having a separate loop within devm_kstrdup().
Reviewed-by: Ian Molton <[email protected]>
Signed-off-by: Rob Jones <[email protected]>
---
drivers/base/devres.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 9d0f64c..cf19664 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -802,17 +802,10 @@ EXPORT_SYMBOL_GPL(devm_kmalloc);
*/
char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp)
{
- size_t size;
- char *buf;
-
if (!s)
return NULL;
- size = strlen(s) + 1;
- buf = devm_kmalloc(dev, size, gfp);
- if (buf)
- memcpy(buf, s, size);
- return buf;
+ return devm_kmemdup(dev, s, (strlen(s) + 1), gfp);
}
EXPORT_SYMBOL_GPL(devm_kstrdup);
--
1.7.10.4
Add function devm_kmemdup().
Reviewed-by: Ian Molton <[email protected]>
Signed-off-by: Rob Jones <[email protected]>
---
drivers/base/devres.c | 27 ++++++++++++++++++++++++++-
include/linux/device.h | 2 ++
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index db4e264..9d0f64c 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -793,7 +793,7 @@ EXPORT_SYMBOL_GPL(devm_kmalloc);
/**
* devm_kstrdup - Allocate resource managed space and
* copy an existing string into that.
- * @dev: Device to allocate memory for
+ * @dev:Device to allocate memory for
* @s: the string to duplicate
* @gfp: the GFP mask used in the devm_kmalloc() call when
* allocating memory
@@ -817,6 +817,31 @@ char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp)
EXPORT_SYMBOL_GPL(devm_kstrdup);
/**
+ * devm_kmemdup - Allocate resource managed space and
+ * copy existing data into that.
+ * @dev: Device to allocate memory for
+ * @s: The memory block to duplicate
+ * @gfp: The GFP mask used in the devm_kmalloc() call when
+ * allocating memory
+ * RETURNS:
+ * Pointer to allocated string on success, NULL on failure.
+ */
+void *devm_kmemdup(struct device *dev, const void *s, size_t size, gfp_t gfp)
+{
+ void *buf;
+
+ if (!s || (size == 0))
+ return NULL;
+
+ buf = devm_kmalloc(dev, size, gfp);
+ if (buf)
+ memcpy(buf, s, size);
+
+ return buf;
+}
+EXPORT_SYMBOL_GPL(devm_kmemdup);
+
+/**
* devm_kfree - Resource-managed kfree
* @dev: Device this memory belongs to
* @p: Memory to free
diff --git a/include/linux/device.h b/include/linux/device.h
index d1d1c05..7ace116 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -623,6 +623,8 @@ static inline void *devm_kcalloc(struct device *dev,
}
extern void devm_kfree(struct device *dev, void *p);
extern char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp);
+extern void *devm_kmemdup(struct device *dev,
+ const void *s, size_t size, gfp_t gfp);
void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res);
void __iomem *devm_request_and_ioremap(struct device *dev,
--
1.7.10.4
On Thu, 2014-06-19 at 16:46 +0100, Rob Jones wrote:
> Avoid code duplication by using devm_kmemdup() to copy data instead
> of having a separate loop within devm_kstrdup().
>
> Reviewed-by: Ian Molton <[email protected]>
> Signed-off-by: Rob Jones <[email protected]>
[]
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
[]
> @@ -802,17 +802,10 @@ EXPORT_SYMBOL_GPL(devm_kmalloc);
> */
> char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp)
> {
[]
> + return devm_kmemdup(dev, s, (strlen(s) + 1), gfp);
> }
> EXPORT_SYMBOL_GPL(devm_kstrdup);
Making this static inline in the header and dropping
EXPORT_SYMBOL_GPL might be smaller code.
static inline
char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp)
{
return devm_kmemdup(dev, s, strlen(s) + 1, gfp);
}
On Thu, Jun 19, 2014 at 04:46:01PM +0100, Rob Jones wrote:
> Add new managed resource functions as needed to achieve this.
>
> Amend the following functions:
You appear to have omitted the actual patch, I'm quoting the entire
mail here. Please see Documentation/SubmittingPatches for some tips
on how to do things.
>
> devm_kstrdup()
> gpio_regulator_probe()
>
> Add the following functions:
>
> devm_kmemdup()
> devm_gpio_request_array()
> devm_gpio_free_array()
>
> Remove the following functions:
>
> gpio_regulator_remove()
>
>
> From Rob Jones <[email protected]> # This line is ignored.
> From: Rob Jones <[email protected]>
> Subject:
> In-Reply-To:
>
>
On 19/06/14 18:57, Mark Brown wrote:
> On Thu, Jun 19, 2014 at 04:46:01PM +0100, Rob Jones wrote:
>> Add new managed resource functions as needed to achieve this.
>>
>> Amend the following functions:
> You appear to have omitted the actual patch, I'm quoting the entire
> mail here. Please see Documentation/SubmittingPatches for some tips
> on how to do things.
I omitted to put "[Patch 0/4][v2]" on the front of the subject line.The
actual patches followed.
It was a consequence of lack of familiarity with the tools; I'm
learning, so it shouldn't happen again. Would you like me to resubmit?
>
>> devm_kstrdup()
>> gpio_regulator_probe()
>>
>> Add the following functions:
>>
>> devm_kmemdup()
>> devm_gpio_request_array()
>> devm_gpio_free_array()
>>
>> Remove the following functions:
>>
>> gpio_regulator_remove()
>>
>>
>> From Rob Jones <[email protected]> # This line is ignored.
>> From: Rob Jones <[email protected]>
>> Subject:
>> In-Reply-To:
>>
>>
--
Rob Jones
Project Manager
Codethink Ltd
mailto:[email protected]
tel:+44 161 236 5575
On 19/06/14 16:59, Joe Perches wrote:
> On Thu, 2014-06-19 at 16:46 +0100, Rob Jones wrote:
>> Avoid code duplication by using devm_kmemdup() to copy data instead
>> of having a separate loop within devm_kstrdup().
>>
>> Reviewed-by: Ian Molton <[email protected]>
>> Signed-off-by: Rob Jones <[email protected]>
> []
>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> []
>> @@ -802,17 +802,10 @@ EXPORT_SYMBOL_GPL(devm_kmalloc);
>> */
>> char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp)
>> {
> []
>> + return devm_kmemdup(dev, s, (strlen(s) + 1), gfp);
>> }
>> EXPORT_SYMBOL_GPL(devm_kstrdup);
> Making this static inline in the header and dropping
> EXPORT_SYMBOL_GPL might be smaller code.
>
> static inline
> char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp)
> {
> return devm_kmemdup(dev, s, strlen(s) + 1, gfp);
> }
>
Is it wise not to check for (s == NULL)? Surely the behaviour of
strlen(NULL) is undefined.
Consequently, I am, on balance, against making the function static
inline. Given that inline is only a recommendation to the compiler, it
may well not inline it anyway if the NULL test is present, in which case
we would end up with a copy of the code in each module that uses it
rather than a single, global, copy. If it didn't need the NULL test, I
would agree that it would be best as a static inline but I think it does
need it.
--
Rob Jones
Project Manager
Codethink Ltd
mailto:[email protected]
tel:+44 161 236 5575
On Fri, Jun 20, 2014 at 09:38:29AM +0100, Rob Jones wrote:
> I omitted to put "[Patch 0/4][v2]" on the front of the subject line.The
> actual patches followed.
Oh, so this is associated with a patch series?
> It was a consequence of lack of familiarity with the tools; I'm learning, so
> it shouldn't happen again. Would you like me to resubmit?
I do see another patch from you for the regulator subsystem which is
part of a series so I guess no need.
Please, if you are resubmitting try to use a changelog style consistent
with the code you're submitting against - you've got subject lines with
"drivers/" which the kernel doesn't do.
On Thu, 2014-06-19 at 16:46 +0100, Rob Jones wrote:
[...]
> +int devm_gpio_request_array(struct device *dev,
> + const struct gpio *array,
> + size_t num)
> +{
> + int i, err = 0;
> +
> + for (i = 0; i < num; i++, array++) {
> + err = devm_gpio_request_one(dev,
> + array->gpio,
> + array->flags,
> + array->label);
> + if (err) {
> + while (i--)
> + devm_gpio_free(dev, (--array)->gpio);
Missing break here.
> + }
> + }
> +
> + return err;
> +}
> +EXPORT_SYMBOL(devm_gpio_request_array);
[...]
On Thu, 2014-06-19 at 16:46 +0100, Rob Jones wrote:
[...]
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -793,7 +793,7 @@ EXPORT_SYMBOL_GPL(devm_kmalloc);
> /**
> * devm_kstrdup - Allocate resource managed space and
> * copy an existing string into that.
> - * @dev: Device to allocate memory for
> + * @dev:Device to allocate memory for
You shouldn't be changing this comment...
Ben.
> * @s: the string to duplicate
> * @gfp: the GFP mask used in the devm_kmalloc() call when
> * allocating memory
[...]