[PATCH resend 0/2] Input: twl*-vibra probe cleanups (devm_* conversion)

[PATCH resend 0/2] Input: twl*-vibra probe cleanups (devm_* conversion)

[Peter Ujfalusi]

Hi Dmitry,

This series seams to got lost:
https://lkml.org/lkml/2012/6/15/128

Can you take it for 3.9?

Convert the twl4030, twl6040 vibra drivers to use devm_ at probe time to clean
up the code a bit.
The twl6040-vibra driver needed bigger change since the ordering of the
initialization was not correct.

Regards,
Peter
---
Peter Ujfalusi (2):
  Input: twl4030-vibra: Convert to use devm_* in probe
  Input: twl6040-vibra: Code cleanup in probe with devm_* conversion

 drivers/input/misc/twl4030-vibra.c |  8 +--
 drivers/input/misc/twl6040-vibra.c | 99 +++++++++++++++++---------------------
 2 files changed, 47 insertions(+), 60 deletions(-)

-- 
1.8.1

[PATCH resend 0/2] Input: twl*-vibra probe cleanups (devm_* conversion)

[Peter Ujfalusi]

Hi Dmitry,

This series seams to got lost:
https://lkml.org/lkml/2012/6/15/128

Can you take it for 3.9?

Convert the twl4030, twl6040 vibra drivers to use devm_ at probe time to clean
up the code a bit.
The twl6040-vibra driver needed bigger change since the ordering of the
initialization was not correct.

Regards,
Peter
---
Peter Ujfalusi (2):
  Input: twl4030-vibra: Convert to use devm_* in probe
  Input: twl6040-vibra: Code cleanup in probe with devm_* conversion

 drivers/input/misc/twl4030-vibra.c |  8 +--
 drivers/input/misc/twl6040-vibra.c | 99 +++++++++++++++++---------------------
 2 files changed, 47 insertions(+), 60 deletions(-)

-- 
1.8.1

[PATCH resend 1/2] Input: twl4030-vibra: Convert to use devm_* in probe

[Peter Ujfalusi]

Cleanup to use devm_* when it is possible at probe time.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/input/misc/twl4030-vibra.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/twl4030-vibra.c b/drivers/input/misc/twl4030-vibra.c
index 78eb6b3..1fe7644 100644
--- a/drivers/input/misc/twl4030-vibra.c
+++ b/drivers/input/misc/twl4030-vibra.c
@@ -219,7 +219,7 @@ static int twl4030_vibra_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
 
@@ -230,8 +230,7 @@ static int twl4030_vibra_probe(struct platform_device *pdev)
 	info->input_dev = input_allocate_device();
 	if (info->input_dev == NULL) {
 		dev_err(&pdev->dev, "couldn't allocate input device\n");
-		ret = -ENOMEM;
-		goto err_kzalloc;
+		return -ENOMEM;
 	}
 
 	input_set_drvdata(info->input_dev, info);
@@ -264,8 +263,6 @@ err_iff:
 	input_ff_destroy(info->input_dev);
 err_ialloc:
 	input_free_device(info->input_dev);
-err_kzalloc:
-	kfree(info);
 	return ret;
 }
 
@@ -275,7 +272,6 @@ static int twl4030_vibra_remove(struct platform_device *pdev)
 
 	/* this also free ff-memless and calls close if needed */
 	input_unregister_device(info->input_dev);
-	kfree(info);
 	platform_set_drvdata(pdev, NULL);
 
 	return 0;
-- 
1.8.1

[PATCH resend 1/2] Input: twl4030-vibra: Convert to use devm_* in probe

[Peter Ujfalusi]

Cleanup to use devm_* when it is possible at probe time.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/input/misc/twl4030-vibra.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/twl4030-vibra.c b/drivers/input/misc/twl4030-vibra.c
index 78eb6b3..1fe7644 100644
--- a/drivers/input/misc/twl4030-vibra.c
+++ b/drivers/input/misc/twl4030-vibra.c
@@ -219,7 +219,7 @@ static int twl4030_vibra_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
 
@@ -230,8 +230,7 @@ static int twl4030_vibra_probe(struct platform_device *pdev)
 	info->input_dev = input_allocate_device();
 	if (info->input_dev == NULL) {
 		dev_err(&pdev->dev, "couldn't allocate input device\n");
-		ret = -ENOMEM;
-		goto err_kzalloc;
+		return -ENOMEM;
 	}
 
 	input_set_drvdata(info->input_dev, info);
@@ -264,8 +263,6 @@ err_iff:
 	input_ff_destroy(info->input_dev);
 err_ialloc:
 	input_free_device(info->input_dev);
-err_kzalloc:
-	kfree(info);
 	return ret;
 }
 
@@ -275,7 +272,6 @@ static int twl4030_vibra_remove(struct platform_device *pdev)
 
 	/* this also free ff-memless and calls close if needed */
 	input_unregister_device(info->input_dev);
-	kfree(info);
 	platform_set_drvdata(pdev, NULL);
 
 	return 0;
-- 
1.8.1

[PATCH resend 2/2] Input: twl6040-vibra: Code cleanup in probe with devm_* conversion

[Peter Ujfalusi]

Convert the probe to use devm_*. At the same time reorder the calls
so we will register the input device as the last step when the driver
is loaded.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/input/misc/twl6040-vibra.c | 99 +++++++++++++++++---------------------
 1 file changed, 45 insertions(+), 54 deletions(-)

diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c
index 71a28ee..d1e31b8 100644
--- a/drivers/input/misc/twl6040-vibra.c
+++ b/drivers/input/misc/twl6040-vibra.c
@@ -275,7 +275,7 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info) {
 		dev_err(&pdev->dev, "couldn't allocate memory\n");
 		return -ENOMEM;
@@ -309,53 +309,23 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 	if ((!info->vibldrv_res && !info->viblmotor_res) ||
 	    (!info->vibrdrv_res && !info->vibrmotor_res)) {
 		dev_err(info->dev, "invalid vibra driver/motor resistance\n");
-		ret = -EINVAL;
-		goto err_kzalloc;
+		return -EINVAL;
 	}
 
 	info->irq = platform_get_irq(pdev, 0);
 	if (info->irq < 0) {
 		dev_err(info->dev, "invalid irq\n");
-		ret = -EINVAL;
-		goto err_kzalloc;
+		return -EINVAL;
 	}
 
 	mutex_init(&info->mutex);
 
-	info->input_dev = input_allocate_device();
-	if (info->input_dev == NULL) {
-		dev_err(info->dev, "couldn't allocate input device\n");
-		ret = -ENOMEM;
-		goto err_kzalloc;
-	}
-
-	input_set_drvdata(info->input_dev, info);
-
-	info->input_dev->name = "twl6040:vibrator";
-	info->input_dev->id.version = 1;
-	info->input_dev->dev.parent = pdev->dev.parent;
-	info->input_dev->close = twl6040_vibra_close;
-	__set_bit(FF_RUMBLE, info->input_dev->ffbit);
-
-	ret = input_ff_create_memless(info->input_dev, NULL, vibra_play);
-	if (ret < 0) {
-		dev_err(info->dev, "couldn't register vibrator to FF\n");
-		goto err_ialloc;
-	}
-
-	ret = input_register_device(info->input_dev);
-	if (ret < 0) {
-		dev_err(info->dev, "couldn't register input device\n");
-		goto err_iff;
-	}
-
-	platform_set_drvdata(pdev, info);
-
-	ret = request_threaded_irq(info->irq, NULL, twl6040_vib_irq_handler, 0,
-				   "twl6040_irq_vib", info);
+	ret = devm_request_threaded_irq(&pdev->dev, info->irq, NULL,
+					twl6040_vib_irq_handler, 0,
+					"twl6040_irq_vib", info);
 	if (ret) {
 		dev_err(info->dev, "VIB IRQ request failed: %d\n", ret);
-		goto err_irq;
+		return ret;
 	}
 
 	info->supplies[0].supply = "vddvibl";
@@ -368,7 +338,7 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 				 ARRAY_SIZE(info->supplies), info->supplies);
 	if (ret) {
 		dev_err(info->dev, "couldn't get regulators %d\n", ret);
-		goto err_regulator;
+		return ret;
 	}
 
 	if (vddvibl_uV) {
@@ -377,7 +347,7 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 		if (ret) {
 			dev_err(info->dev, "failed to set VDDVIBL volt %d\n",
 				ret);
-			goto err_voltage;
+			goto err_regulator;
 		}
 	}
 
@@ -387,7 +357,7 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 		if (ret) {
 			dev_err(info->dev, "failed to set VDDVIBR volt %d\n",
 				ret);
-			goto err_voltage;
+			goto err_regulator;
 		}
 	}
 
@@ -395,26 +365,49 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 	if (info->workqueue == NULL) {
 		dev_err(info->dev, "couldn't create workqueue\n");
 		ret = -ENOMEM;
-		goto err_voltage;
+		goto err_regulator;
 	}
 	INIT_WORK(&info->play_work, vibra_play_work);
 
+	info->input_dev = input_allocate_device();
+	if (info->input_dev == NULL) {
+		dev_err(info->dev, "couldn't allocate input device\n");
+		ret = -ENOMEM;
+		goto err_wq;
+	}
+
+	input_set_drvdata(info->input_dev, info);
+
+	info->input_dev->name = "twl6040:vibrator";
+	info->input_dev->id.version = 1;
+	info->input_dev->dev.parent = pdev->dev.parent;
+	info->input_dev->close = twl6040_vibra_close;
+	__set_bit(FF_RUMBLE, info->input_dev->ffbit);
+
+	ret = input_ff_create_memless(info->input_dev, NULL, vibra_play);
+	if (ret < 0) {
+		dev_err(info->dev, "couldn't register vibrator to FF\n");
+		goto err_ialloc;
+	}
+
+	ret = input_register_device(info->input_dev);
+	if (ret < 0) {
+		dev_err(info->dev, "couldn't register input device\n");
+		goto err_iff;
+	}
+
+	platform_set_drvdata(pdev, info);
+
 	return 0;
 
-err_voltage:
-	regulator_bulk_free(ARRAY_SIZE(info->supplies), info->supplies);
-err_regulator:
-	free_irq(info->irq, info);
-err_irq:
-	input_unregister_device(info->input_dev);
-	info->input_dev = NULL;
 err_iff:
-	if (info->input_dev)
-		input_ff_destroy(info->input_dev);
+	input_ff_destroy(info->input_dev);
 err_ialloc:
 	input_free_device(info->input_dev);
-err_kzalloc:
-	kfree(info);
+err_wq:
+	destroy_workqueue(info->workqueue);
+err_regulator:
+	regulator_bulk_free(ARRAY_SIZE(info->supplies), info->supplies);
 	return ret;
 }
 
@@ -423,10 +416,8 @@ static int twl6040_vibra_remove(struct platform_device *pdev)
 	struct vibra_info *info = platform_get_drvdata(pdev);
 
 	input_unregister_device(info->input_dev);
-	free_irq(info->irq, info);
 	regulator_bulk_free(ARRAY_SIZE(info->supplies), info->supplies);
 	destroy_workqueue(info->workqueue);
-	kfree(info);
 
 	return 0;
 }
-- 
1.8.1

[PATCH resend 2/2] Input: twl6040-vibra: Code cleanup in probe with devm_* conversion

[Peter Ujfalusi]

Convert the probe to use devm_*. At the same time reorder the calls
so we will register the input device as the last step when the driver
is loaded.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/input/misc/twl6040-vibra.c | 99 +++++++++++++++++---------------------
 1 file changed, 45 insertions(+), 54 deletions(-)

diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c
index 71a28ee..d1e31b8 100644
--- a/drivers/input/misc/twl6040-vibra.c
+++ b/drivers/input/misc/twl6040-vibra.c
@@ -275,7 +275,7 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info) {
 		dev_err(&pdev->dev, "couldn't allocate memory\n");
 		return -ENOMEM;
@@ -309,53 +309,23 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 	if ((!info->vibldrv_res && !info->viblmotor_res) ||
 	    (!info->vibrdrv_res && !info->vibrmotor_res)) {
 		dev_err(info->dev, "invalid vibra driver/motor resistance\n");
-		ret = -EINVAL;
-		goto err_kzalloc;
+		return -EINVAL;
 	}
 
 	info->irq = platform_get_irq(pdev, 0);
 	if (info->irq < 0) {
 		dev_err(info->dev, "invalid irq\n");
-		ret = -EINVAL;
-		goto err_kzalloc;
+		return -EINVAL;
 	}
 
 	mutex_init(&info->mutex);
 
-	info->input_dev = input_allocate_device();
-	if (info->input_dev == NULL) {
-		dev_err(info->dev, "couldn't allocate input device\n");
-		ret = -ENOMEM;
-		goto err_kzalloc;
-	}
-
-	input_set_drvdata(info->input_dev, info);
-
-	info->input_dev->name = "twl6040:vibrator";
-	info->input_dev->id.version = 1;
-	info->input_dev->dev.parent = pdev->dev.parent;
-	info->input_dev->close = twl6040_vibra_close;
-	__set_bit(FF_RUMBLE, info->input_dev->ffbit);
-
-	ret = input_ff_create_memless(info->input_dev, NULL, vibra_play);
-	if (ret < 0) {
-		dev_err(info->dev, "couldn't register vibrator to FF\n");
-		goto err_ialloc;
-	}
-
-	ret = input_register_device(info->input_dev);
-	if (ret < 0) {
-		dev_err(info->dev, "couldn't register input device\n");
-		goto err_iff;
-	}
-
-	platform_set_drvdata(pdev, info);
-
-	ret = request_threaded_irq(info->irq, NULL, twl6040_vib_irq_handler, 0,
-				   "twl6040_irq_vib", info);
+	ret = devm_request_threaded_irq(&pdev->dev, info->irq, NULL,
+					twl6040_vib_irq_handler, 0,
+					"twl6040_irq_vib", info);
 	if (ret) {
 		dev_err(info->dev, "VIB IRQ request failed: %d\n", ret);
-		goto err_irq;
+		return ret;
 	}
 
 	info->supplies[0].supply = "vddvibl";
@@ -368,7 +338,7 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 				 ARRAY_SIZE(info->supplies), info->supplies);
 	if (ret) {
 		dev_err(info->dev, "couldn't get regulators %d\n", ret);
-		goto err_regulator;
+		return ret;
 	}
 
 	if (vddvibl_uV) {
@@ -377,7 +347,7 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 		if (ret) {
 			dev_err(info->dev, "failed to set VDDVIBL volt %d\n",
 				ret);
-			goto err_voltage;
+			goto err_regulator;
 		}
 	}
 
@@ -387,7 +357,7 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 		if (ret) {
 			dev_err(info->dev, "failed to set VDDVIBR volt %d\n",
 				ret);
-			goto err_voltage;
+			goto err_regulator;
 		}
 	}
 
@@ -395,26 +365,49 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 	if (info->workqueue == NULL) {
 		dev_err(info->dev, "couldn't create workqueue\n");
 		ret = -ENOMEM;
-		goto err_voltage;
+		goto err_regulator;
 	}
 	INIT_WORK(&info->play_work, vibra_play_work);
 
+	info->input_dev = input_allocate_device();
+	if (info->input_dev == NULL) {
+		dev_err(info->dev, "couldn't allocate input device\n");
+		ret = -ENOMEM;
+		goto err_wq;
+	}
+
+	input_set_drvdata(info->input_dev, info);
+
+	info->input_dev->name = "twl6040:vibrator";
+	info->input_dev->id.version = 1;
+	info->input_dev->dev.parent = pdev->dev.parent;
+	info->input_dev->close = twl6040_vibra_close;
+	__set_bit(FF_RUMBLE, info->input_dev->ffbit);
+
+	ret = input_ff_create_memless(info->input_dev, NULL, vibra_play);
+	if (ret < 0) {
+		dev_err(info->dev, "couldn't register vibrator to FF\n");
+		goto err_ialloc;
+	}
+
+	ret = input_register_device(info->input_dev);
+	if (ret < 0) {
+		dev_err(info->dev, "couldn't register input device\n");
+		goto err_iff;
+	}
+
+	platform_set_drvdata(pdev, info);
+
 	return 0;
 
-err_voltage:
-	regulator_bulk_free(ARRAY_SIZE(info->supplies), info->supplies);
-err_regulator:
-	free_irq(info->irq, info);
-err_irq:
-	input_unregister_device(info->input_dev);
-	info->input_dev = NULL;
 err_iff:
-	if (info->input_dev)
-		input_ff_destroy(info->input_dev);
+	input_ff_destroy(info->input_dev);
 err_ialloc:
 	input_free_device(info->input_dev);
-err_kzalloc:
-	kfree(info);
+err_wq:
+	destroy_workqueue(info->workqueue);
+err_regulator:
+	regulator_bulk_free(ARRAY_SIZE(info->supplies), info->supplies);
 	return ret;
 }
 
@@ -423,10 +416,8 @@ static int twl6040_vibra_remove(struct platform_device *pdev)
 	struct vibra_info *info = platform_get_drvdata(pdev);
 
 	input_unregister_device(info->input_dev);
-	free_irq(info->irq, info);
 	regulator_bulk_free(ARRAY_SIZE(info->supplies), info->supplies);
 	destroy_workqueue(info->workqueue);
-	kfree(info);
 
 	return 0;
 }
-- 
1.8.1

Re: [PATCH resend 2/2] Input: twl6040-vibra: Code cleanup in probe with devm_* conversion

[Dmitry Torokhov]

Hi Peter,

On Thu, Jan 10, 2013 at 02:36:05PM +0100, Peter Ujfalusi wrote:
> Convert the probe to use devm_*. At the same time reorder the calls
> so we will register the input device as the last step when the driver
> is loaded.

If you are doing this please also use devm_input_allocate_device() and
devm_regulator_bulk_get(). I alos think it is high time to get rid of
dedicated work queue - cwq hasn't actually given you a separate therad
for a while now and if you get rid of workqueue you can get ridof entire
twl6040_vibra_remove().

Thanks.

-- 
Dmitry

Re: [PATCH resend 2/2] Input: twl6040-vibra: Code cleanup in probe with devm_* conversion

[Peter Ujfalusi]

Hi Dmitry,

On 01/10/2013 06:54 PM, Dmitry Torokhov wrote:
> Hi Peter,
> 
> On Thu, Jan 10, 2013 at 02:36:05PM +0100, Peter Ujfalusi wrote:
>> Convert the probe to use devm_*. At the same time reorder the calls
>> so we will register the input device as the last step when the driver
>> is loaded.
> 
> If you are doing this please also use devm_input_allocate_device() and
> devm_regulator_bulk_get().

Now I remember: I can not switch to devm_regulator_bulk_get() unfortunately
since when we boot with DT the code needs to reach up for the parent's dev to
get the needed regulator. If I use devm_regulator_bulk_get() the resource will
be bind to the parent dev and not to the vibra driver.

> I also think it is high time to get rid of
> dedicated work queue - cwq hasn't actually given you a separate therad
> for a while now and if you get rid of workqueue you can get ridof entire
> twl6040_vibra_remove().

I agree with the rest. I'll move both twl4030/6040 vibra to use
devm_input_allocate_device() and also I will move them to use system wide
workqueue.

I'm going to resend the series soon with the suggested changes.

Thanks,
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH resend 2/2] Input: twl6040-vibra: Code cleanup in probe with devm_* conversion

[Peter Ujfalusi]

Hi Dmitry,

On 01/10/2013 06:54 PM, Dmitry Torokhov wrote:
> Hi Peter,
> 
> On Thu, Jan 10, 2013 at 02:36:05PM +0100, Peter Ujfalusi wrote:
>> Convert the probe to use devm_*. At the same time reorder the calls
>> so we will register the input device as the last step when the driver
>> is loaded.
> 
> If you are doing this please also use devm_input_allocate_device() and
> devm_regulator_bulk_get().

Now I remember: I can not switch to devm_regulator_bulk_get() unfortunately
since when we boot with DT the code needs to reach up for the parent's dev to
get the needed regulator. If I use devm_regulator_bulk_get() the resource will
be bind to the parent dev and not to the vibra driver.

> I also think it is high time to get rid of
> dedicated work queue - cwq hasn't actually given you a separate therad
> for a while now and if you get rid of workqueue you can get ridof entire
> twl6040_vibra_remove().

I agree with the rest. I'll move both twl4030/6040 vibra to use
devm_input_allocate_device() and also I will move them to use system wide
workqueue.

I'm going to resend the series soon with the suggested changes.

Thanks,
Péter