Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1321014 > unrolled thread

[PATCH 1/2] regulator: gpio: Avoid unnecessarily copying data structures in probe()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/2] regulator: gpio: Avoid unnecessarily copying data structures in probe() Harald Geyer <harald@ccbib.org> - 2016-01-28 21:10 +0100
    Re: [PATCH 1/2] regulator: gpio: Avoid unnecessarily copying data  structures in probe() Mark Brown <broonie@kernel.org> - 2016-01-29 00:10 +0100
      Re: [PATCH 1/2] regulator: gpio: Avoid unnecessarily copying data structures in probe() Harald Geyer <harald@ccbib.org> - 2016-01-29 17:30 +0100
        Re: [PATCH 1/2] regulator: gpio: Avoid unnecessarily copying data  structures in probe() Mark Brown <broonie@kernel.org> - 2016-02-03 18:30 +0100

#1321014 — [PATCH 1/2] regulator: gpio: Avoid unnecessarily copying data structures in probe()

The data structures either have been copied in
of_get_gpio_regulator_config() already or are part of platform data,
which we keep a pointer to for the life time of the device anyway.

As a side effect this fixes the cleanup pathes if probe() fails.

Signed-off-by: Harald Geyer <harald@ccbib.org>
---
 drivers/regulator/gpio-regulator.c | 34 ++++++----------------------------
 1 file changed, 6 insertions(+), 28 deletions(-)

diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index 7bba8b7..5e2e14d 100644
--- a/drivers/regulator/gpio-regulator.c
+++ b/drivers/regulator/gpio-regulator.c
@@ -271,33 +271,18 @@ static int gpio_regulator_probe(struct platform_device *pdev)
 	}
 
 	if (config->nr_gpios != 0) {
-		drvdata->gpios = kmemdup(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;
-		}
+		drvdata->gpios = config->gpios;
 
 		drvdata->nr_gpios = config->nr_gpios;
 		ret = gpio_request_array(drvdata->gpios, drvdata->nr_gpios);
 		if (ret) {
 			dev_err(&pdev->dev,
 			"Could not obtain regulator setting GPIOs: %d\n", ret);
-			goto err_memstate;
+			goto err_name;
 		}
 	}
 
-	drvdata->states = kmemdup(config->states,
-				  config->nr_states *
-					 sizeof(struct gpio_regulator_state),
-				  GFP_KERNEL);
-	if (drvdata->states == NULL) {
-		dev_err(&pdev->dev, "Failed to allocate state data\n");
-		ret = -ENOMEM;
-		goto err_memgpio;
-	}
+	drvdata->states = config->states;
 	drvdata->nr_states = config->nr_states;
 
 	drvdata->desc.owner = THIS_MODULE;
@@ -317,7 +302,7 @@ static int gpio_regulator_probe(struct platform_device *pdev)
 	default:
 		dev_err(&pdev->dev, "No regulator type set\n");
 		ret = -EINVAL;
-		goto err_memgpio;
+		goto err_gpio;
 	}
 
 	/* build initial state from gpio init data. */
@@ -354,19 +339,15 @@ static int gpio_regulator_probe(struct platform_device *pdev)
 	if (IS_ERR(drvdata->dev)) {
 		ret = PTR_ERR(drvdata->dev);
 		dev_err(&pdev->dev, "Failed to register regulator: %d\n", ret);
-		goto err_stategpio;
+		goto err_gpio;
 	}
 
 	platform_set_drvdata(pdev, drvdata);
 
 	return 0;
 
-err_stategpio:
+err_gpio:
 	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:
@@ -381,9 +362,6 @@ static int gpio_regulator_remove(struct platform_device *pdev)
 
 	gpio_free_array(drvdata->gpios, drvdata->nr_gpios);
 
-	kfree(drvdata->states);
-	kfree(drvdata->gpios);
-
 	kfree(drvdata->desc.name);
 
 	return 0;
-- 
2.1.4

[toc] | [next] | [standalone]

#1321130 — Re: [PATCH 1/2] regulator: gpio: Avoid unnecessarily copying data structures in probe()

[Multipart message — attachments visible in raw view] — view raw

On Thu, Jan 28, 2016 at 07:55:17PM +0000, Harald Geyer wrote:
> The data structures either have been copied in
> of_get_gpio_regulator_config() already or are part of platform data,
> which we keep a pointer to for the life time of the device anyway.

The point of this code is to avoid referring to platform data after
probe for robustness.

[toc] | [prev] | [next] | [standalone]

#1321863

Mark Brown writes:
> On Thu, Jan 28, 2016 at 07:55:17PM +0000, Harald Geyer wrote:
> > The data structures either have been copied in
> > of_get_gpio_regulator_config() already or are part of platform data,
> > which we keep a pointer to for the life time of the device anyway.
> 
> The point of this code is to avoid referring to platform data after
> probe for robustness.

Well, if we can't rely on platform data staying available after probe()
then probably we shouldn't keep a pointer to it - but thats a different
issue, so lets focus on the original problem:

What's your preferred way to fix the error path of probe() then?

Harald

[toc] | [prev] | [next] | [standalone]

#1325728 — Re: [PATCH 1/2] regulator: gpio: Avoid unnecessarily copying data structures in probe()

[Multipart message — attachments visible in raw view] — view raw

On Fri, Jan 29, 2016 at 05:25:28PM +0100, Harald Geyer wrote:
> Mark Brown writes:
> > On Thu, Jan 28, 2016 at 07:55:17PM +0000, Harald Geyer wrote:
> > > The data structures either have been copied in
> > > of_get_gpio_regulator_config() already or are part of platform data,
> > > which we keep a pointer to for the life time of the device anyway.

> > The point of this code is to avoid referring to platform data after
> > probe for robustness.

> Well, if we can't rely on platform data staying available after probe()
> then probably we shouldn't keep a pointer to it - but thats a different
> issue, so lets focus on the original problem:

> What's your preferred way to fix the error path of probe() then?

Add the matching frees?  I'm not sure I know what the original problem
is mind you.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web