Input: Introduce the use of managed version of kzalloc

PATCH In review Himangi Saraogi 2 participants 2 messages May 8, 2014 – May 9, 2014

Subsystems (3)

HID Input drivers Dialog semiconductor drivers

lore

Share & tools

lore

Himangi Saraogi

author

2014-05-08 04:00 UTC #0 lore raw

PATCH 1/1 Input: Introduce the use of managed version of kzalloc

This patch moves data allocated using kzalloc to managed data allocated using devm_kzalloc and cleans now unnecessary kfrees in probe and remove functions. This data is the third argument to da9052_request_irq in the two cases below.

The following Coccinelle semantic patch was used for making the change:

@platform@
identifier p, probefn, removefn;
@@
struct platform_driver p = {
.probe = probefn,
.remove = removefn,
};

@prb@
identifier platform.probefn, pdev;
expression e, e1, e2;
@@
probefn(struct platform_device *pdev, ...) {
<+...
- e = kzalloc(e1, e2)
+ e = devm_kzalloc(&pdev->dev, e1, e2)
...
?-kfree(e);
...+>
}

@rem depends on prb@
identifier platform.removefn;
expression e;
@@
removefn(...) {
<...
- kfree(e);
...>
}

Signed-off-by Himangi Saraogi <[email protected]>

Acked-by Julia Lawall <[email protected]>

---
As a follow up patch I would like to know if it would be desirable to modify request_threaded_irq to devm_request_threaded_irq in the helper function da9052_request_irq :
int da9052_request_irq(struct da9052 *da9052, int irq, char *name,
irq_handler_t handler, void *data)

{
	irq = da9052_map_irq(da9052, irq);
	if (irq < 0)
		return irq;

	return request_threaded_irq(irq, NULL, handler,
				     IRQF_TRIGGER_LOW | IRQF_ONESHOT,
				     name, data);
}

 drivers/input/misc/da9052_onkey.c      | 4 +---
 drivers/input/touchscreen/da9052_tsi.c | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

drivers/input/misc/da9052_onkey.c +1 -3

index 184c8f2..6fc8243 100644
@@ -84,7 +84,7 @@ static int da9052_onkey_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	onkey = kzalloc(sizeof(*onkey), GFP_KERNEL);
+	onkey = devm_kzalloc(&pdev->dev, sizeof(*onkey), GFP_KERNEL);
 	input_dev = input_allocate_device();
 	if (!onkey || !input_dev) {
 		dev_err(&pdev->dev, "Failed to allocate memory\n");

@@ -126,7 +126,6 @@ err_free_irq:
 	cancel_delayed_work_sync(&onkey->work);
 err_free_mem:
 	input_free_device(input_dev);
-	kfree(onkey);
 
 	return error;
 }

@@ -139,7 +138,6 @@ static int da9052_onkey_remove(struct platform_device *pdev)
 	cancel_delayed_work_sync(&onkey->work);
 
 	input_unregister_device(onkey->input);
-	kfree(onkey);
 
 	return 0;
 }

drivers/input/touchscreen/da9052_tsi.c +1 -3

index ab64d58..dff6a2e 100644
@@ -238,7 +238,7 @@ static int da9052_ts_probe(struct platform_device *pdev)
 	if (!da9052)
 		return -EINVAL;
 
-	tsi = kzalloc(sizeof(struct da9052_tsi), GFP_KERNEL);
+	tsi = devm_kzalloc(&pdev->dev, sizeof(struct da9052_tsi), GFP_KERNEL);
 	input_dev = input_allocate_device();
 	if (!tsi || !input_dev) {
 		error = -ENOMEM;

@@ -311,7 +311,6 @@ err_free_datardy_irq:
 err_free_pendwn_irq:
 	da9052_free_irq(tsi->da9052, DA9052_IRQ_PENDOWN, tsi);
 err_free_mem:
-	kfree(tsi);
 	input_free_device(input_dev);
 
 	return error;

@@ -327,7 +326,6 @@ static int  da9052_ts_remove(struct platform_device *pdev)
 	da9052_free_irq(tsi->da9052, DA9052_IRQ_PENDOWN, tsi);
 
 	input_unregister_device(tsi->dev);
-	kfree(tsi);
 
 	return 0;
 }

Signature · 1 line

1.9.1

Opensource [Anthony Olech]

Acked-by on v1 1/1

2014-05-09 09:47 UTC #1 lore raw

Quoted 11 lines

-----Original Message-----
From: Himangi Saraogi [mailto:[email protected]]
Sent: 08 May 2014 05:00
To: Support Opensource; [email protected]; linux- [email protected]; [email protected]
Cc: [email protected]
Subject: [PATCH] Input: Introduce the use of managed version of kzalloc This patch moves data allocated using kzalloc to managed data allocated using devm_kzalloc and cleans now unnecessary kfrees in probe and remove functions. This data is the third argument to da9052_request_irq in the two cases below.

The remainder of the description .....

Quoted 21 lines

The following Coccinelle semantic patch was used for making the change:
@platform@
identifier p, probefn, removefn;
@@
struct platform_driver p = {
.probe = probefn,
.remove = removefn,
};
@prb@
identifier platform.probefn, pdev;
expression e, e1, e2;
@@
probefn(struct platform_device *pdev, ...) {
<+...
- e = kzalloc(e1, e2)
+ e = devm_kzalloc(&pdev->dev, e1, e2)
...
?-kfree(e);
...+>
}
@rem depends on prb@

Quoted 8 lines

identifier platform.removefn;
expression e;
@@
removefn(...) {
<...
- kfree(e);
...>
}

.... does not seems appropriate for the commit message, it should IMHO go in the following section

Quoted 7 lines

Signed-off-by Himangi Saraogi <[email protected]>
Acked-by Julia Lawall <[email protected]>
---
As a follow up patch I would like to know if it would be desirable to modify request_threaded_irq to devm_request_threaded_irq in the helper function da9052_request_irq :
int da9052_request_irq(struct da9052 *da9052, int irq, char *name,

Quoted 8 lines

			   irq_handler_t handler, void *data) {
	irq = da9052_map_irq(da9052, irq);
	if (irq < 0)
		return irq;
	return request_threaded_irq(irq, NULL, handler,
				     IRQF_TRIGGER_LOW | IRQF_ONESHOT,
				     name, data);
}

The problem here is that there is no way back to the 'dev' associated with the irq. To solve this requires a change to the function, or even better, just placing the function's code inline, in which case a two stage approach is required:- first 'inline' the function (from the common header file) to each of the PMIC's component drivers and second raise a patch for each component driver do the change as you suggest which will work because the correct 'dev' is available. Not that the second stage will have to wait until the first statge is actually in main-line.

Quoted 67 lines

 drivers/input/misc/da9052_onkey.c      | 4 +---
 drivers/input/touchscreen/da9052_tsi.c | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

drivers/input/misc/da9052_onkey.c +1 -1

index 184c8f2..6fc8243 100644
@@ -84,7 +84,7 @@ static int da9052_onkey_probe(struct platform_device
 *pdev)
 		return -EINVAL;
 	}
 
-	onkey = kzalloc(sizeof(*onkey), GFP_KERNEL);
+	onkey = devm_kzalloc(&pdev->dev, sizeof(*onkey), GFP_KERNEL);
 	input_dev = input_allocate_device();
 	if (!onkey || !input_dev) {

dev_err(&pdev->dev, "Failed to allocate memory\n"); @@ -
126,7 +126,6 @@ err_free_irq:

drivers/input/misc/da9052_onkey.c +0 -2

 	cancel_delayed_work_sync(&onkey->work);
 err_free_mem:
 	input_free_device(input_dev);
-	kfree(onkey);
 
 	return error;
 }

@@ -139,7 +138,6 @@ static int da9052_onkey_remove(struct
 platform_device *pdev)
 	cancel_delayed_work_sync(&onkey->work);
 
 	input_unregister_device(onkey->input);
-	kfree(onkey);
 
 	return 0;

}

drivers/input/touchscreen/da9052_tsi.c +1 -1

index ab64d58..dff6a2e 100644
@@ -238,7 +238,7 @@ static int da9052_ts_probe(struct platform_device
 *pdev)
 	if (!da9052)
 		return -EINVAL;
 
-	tsi = kzalloc(sizeof(struct da9052_tsi), GFP_KERNEL);
+	tsi = devm_kzalloc(&pdev->dev, sizeof(struct da9052_tsi),
 GFP_KERNEL);
 	input_dev = input_allocate_device();

drivers/input/touchscreen/da9052_tsi.c +0 -2

 	if (!tsi || !input_dev) {
 		error = -ENOMEM;

@@ -311,7 +311,6 @@ err_free_datardy_irq:
 err_free_pendwn_irq:
 	da9052_free_irq(tsi->da9052, DA9052_IRQ_PENDOWN, tsi);
 err_free_mem:
-	kfree(tsi);
 	input_free_device(input_dev);
 
 	return error;

@@ -327,7 +326,6 @@ static int  da9052_ts_remove(struct platform_device
 *pdev)
 	da9052_free_irq(tsi->da9052, DA9052_IRQ_PENDOWN, tsi);
 
 	input_unregister_device(tsi->dev);
-	kfree(tsi);
 
 	return 0;

Quoted 3 lines

}
--
1.9.1

for the content of this patch:

Acked-by Opensource [Anthony Olech] <[email protected]>