2014-06-10 14:43:06

by Rob Jones

[permalink] [raw]
Subject: [PATCH 0/3] use managed resources for gpio-regulator probe

Change gpio-reulator-probe() to use managed resources.

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()


2014-06-10 14:43:00

by Rob Jones

[permalink] [raw]
Subject: [PATCH 1/3] drivers/gpio: devres.c: allow gpio array requests for managed devices

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 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..fd95240d 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

2014-06-10 14:43:11

by Rob Jones

[permalink] [raw]
Subject: [PATCH 3/3] drivers/regulator: gpio-regulator.c: use managed resources for probe.

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 | 71 ++++++++++++------------------------
1 file changed, 24 insertions(+), 47 deletions(-)

diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index 989b23b..56341c3 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,7 @@ 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,
+ .remove = NULL,
.driver = {
.name = "gpio-regulator",
.owner = THIS_MODULE,
--
1.7.10.4

2014-06-10 14:43:53

by Rob Jones

[permalink] [raw]
Subject: [PATCH 2/3] drivers/base: devres.c: Add block copy func. for managed devices

Add function devm_kmemdup().
Rationalise with devm_kstrdup() to avoid code duplication.

Reviewed-by: Ian Molton <[email protected]>
Signed-off-by: Rob Jones <[email protected]>
---
drivers/base/devres.c | 42 ++++++++++++++++++++++++++++++++++--------
include/linux/device.h | 2 ++
2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index db4e264..978bbe2 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -791,6 +791,20 @@ void * devm_kmalloc(struct device *dev, size_t size, gfp_t gfp)
EXPORT_SYMBOL_GPL(devm_kmalloc);

/**
+ * static function to avoid code duplication when duplicating buffers
+ */
+static void *devm_kmemdup_nochecks(struct device *dev, const void *s,
+ size_t size, gfp_t gfp)
+{
+ void *buf;
+
+ buf = devm_kmalloc(dev, size, gfp);
+ if (buf)
+ memcpy(buf, s, size);
+ return buf;
+}
+
+/**
* devm_kstrdup - Allocate resource managed space and
* copy an existing string into that.
* @dev: Device to allocate memory for
@@ -802,21 +816,33 @@ 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_nochecks(dev, s, (strlen(s)+1), 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)
+{
+ if (!s || (size == 0))
+ return NULL;
+
+ return devm_kmemdup_nochecks(dev, s, size, gfp);
+}
+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

2014-06-18 13:04:16

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH 3/3] drivers/regulator: gpio-regulator.c: use managed resources for probe.

Am Dienstag, 10. Juni 2014, 15:41:48 schrieb Rob Jones:
> 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]>

Reviewed-by: Heiko Stuebner <[email protected]>


> ---
> drivers/regulator/gpio-regulator.c | 71
> ++++++++++++------------------------ 1 file changed, 24 insertions(+), 47
> deletions(-)
>
> diff --git a/drivers/regulator/gpio-regulator.c
> b/drivers/regulator/gpio-regulator.c index 989b23b..56341c3 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,7 @@ 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,
> + .remove = NULL,
> .driver = {
> .name = "gpio-regulator",
> .owner = THIS_MODULE,

2014-06-18 13:04:42

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/base: devres.c: Add block copy func. for managed devices

Am Dienstag, 10. Juni 2014, 15:41:47 schrieb Rob Jones:
> Add function devm_kmemdup().
> Rationalise with devm_kstrdup() to avoid code duplication.
>
> Reviewed-by: Ian Molton <[email protected]>
> Signed-off-by: Rob Jones <[email protected]>
> ---
> drivers/base/devres.c | 42 ++++++++++++++++++++++++++++++++++--------
> include/linux/device.h | 2 ++
> 2 files changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index db4e264..978bbe2 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -791,6 +791,20 @@ void * devm_kmalloc(struct device *dev, size_t size,
> gfp_t gfp) EXPORT_SYMBOL_GPL(devm_kmalloc);
>
> /**
> + * static function to avoid code duplication when duplicating buffers
> + */
> +static void *devm_kmemdup_nochecks(struct device *dev, const void *s,
> + size_t size, gfp_t gfp)
> +{
> + void *buf;
> +
> + buf = devm_kmalloc(dev, size, gfp);
> + if (buf)
> + memcpy(buf, s, size);
> + return buf;
> +}
> +
> +/**
> * devm_kstrdup - Allocate resource managed space and
> * copy an existing string into that.
> * @dev: Device to allocate memory for
> @@ -802,21 +816,33 @@ 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_nochecks(dev, s, (strlen(s)+1), gfp);
> }
> EXPORT_SYMBOL_GPL(devm_kstrdup);

hmm, this looks like an unrelated change to me and may warrant a separate
patch for the devm_kstrdup change.


>
> /**
> + * 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) +{
> + if (!s || (size == 0))
> + return NULL;
> +
> + return devm_kmemdup_nochecks(dev, s, size, gfp);
> +}
> +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,

2014-06-18 13:23:09

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH 1/3] drivers/gpio: devres.c: allow gpio array requests for managed devices

Am Dienstag, 10. Juni 2014, 15:41:46 schrieb Rob Jones:
> 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 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]>

apart from the doc issue below this looks ok to me


> ---
> 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..fd95240d 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

@dev uses a tab as spacing, while @array and @num use spaces, might be nice to
have this consistent.


> + *
> + * 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

same here

> + *
> + * 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 */

2014-06-18 15:52:30

by Rob Jones

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/base: devres.c: Add block copy func. for managed devices


On 18/06/14 14:05, Heiko St?bner wrote:
> Am Dienstag, 10. Juni 2014, 15:41:47 schrieb Rob Jones:
>> Add function devm_kmemdup().
>> Rationalise with devm_kstrdup() to avoid code duplication.
>>
>> Reviewed-by: Ian Molton <[email protected]>
>> Signed-off-by: Rob Jones <[email protected]>
>> ---
>> drivers/base/devres.c | 42 ++++++++++++++++++++++++++++++++++--------
>> include/linux/device.h | 2 ++
>> 2 files changed, 36 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
>> index db4e264..978bbe2 100644
>> --- a/drivers/base/devres.c
>> +++ b/drivers/base/devres.c
>> @@ -791,6 +791,20 @@ void * devm_kmalloc(struct device *dev, size_t size,
>> gfp_t gfp) EXPORT_SYMBOL_GPL(devm_kmalloc);
>>
>> /**
>> + * static function to avoid code duplication when duplicating buffers
>> + */
>> +static void *devm_kmemdup_nochecks(struct device *dev, const void *s,
>> + size_t size, gfp_t gfp)
>> +{
>> + void *buf;
>> +
>> + buf = devm_kmalloc(dev, size, gfp);
>> + if (buf)
>> + memcpy(buf, s, size);
>> + return buf;
>> +}
>> +
>> +/**
>> * devm_kstrdup - Allocate resource managed space and
>> * copy an existing string into that.
>> * @dev: Device to allocate memory for
>> @@ -802,21 +816,33 @@ 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_nochecks(dev, s, (strlen(s)+1), gfp);
>> }
>> EXPORT_SYMBOL_GPL(devm_kstrdup);
> hmm, this looks like an unrelated change to me and may warrant a separate
> patch for the devm_kstrdup change.

On reflection, I agree, I'll separate out the change to devm_kstrdup()
and resubmit.

>
>
>> /**
>> + * 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) +{
>> + if (!s || (size == 0))
>> + return NULL;
>> +
>> + return devm_kmemdup_nochecks(dev, s, size, gfp);
>> +}
>> +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,
>
>

--
Rob Jones
Project Manager
Codethink Ltd
mailto:[email protected]
tel:+44 161 236 5575

2014-06-18 15:56:19

by Rob Jones

[permalink] [raw]
Subject: Re: [PATCH 1/3] drivers/gpio: devres.c: allow gpio array requests for managed devices


On 18/06/14 14:24, Heiko St?bner wrote:
> Am Dienstag, 10. Juni 2014, 15:41:46 schrieb Rob Jones:
>> 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 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]>
> apart from the doc issue below this looks ok to me
>
>
>> ---
>> 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..fd95240d 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
> @dev uses a tab as spacing, while @array and @num use spaces, might be nice to
> have this consistent.

I'm not sure how that happened. I'll rework it before re-submitting the
series.

>
>
>> + *
>> + * 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
> same here
>
>> + *
>> + * 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 */
>
>

--
Rob Jones
Project Manager
Codethink Ltd
mailto:[email protected]
tel:+44 161 236 5575