[PATCH 1/4] mfd: Remove obsolete cleanup for clientdata

[PATCH 1/4] mfd: Remove obsolete cleanup for clientdata

[Wolfram Sang]

A few new i2c-drivers came into the kernel which clear the clientdata-pointer
on exit or error. This is obsolete meanwhile, the core will do it.

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/mfd/wl1273-core.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/wl1273-core.c b/drivers/mfd/wl1273-core.c
index d2ecc24..6bb5136 100644
--- a/drivers/mfd/wl1273-core.c
+++ b/drivers/mfd/wl1273-core.c
@@ -38,7 +38,6 @@ static int wl1273_core_remove(struct i2c_client *client)
 	dev_dbg(&client->dev, "%s\n", __func__);
 
 	mfd_remove_devices(&client->dev);
-	i2c_set_clientdata(client, NULL);
 	kfree(core);
 
 	return 0;
@@ -104,7 +103,6 @@ static int __devinit wl1273_core_probe(struct i2c_client *client,
 	return 0;
 
 err:
-	i2c_set_clientdata(client, NULL);
 	pdata->free_resources();
 	kfree(core);
 
-- 
1.7.2.3

[PATCH 2/4] power: Remove obsolete cleanup for clientdata

[Wolfram Sang]

A few new i2c-drivers came into the kernel which clear the clientdata-pointer
on exit or error. This is obsolete meanwhile, the core will do it.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Anton Vorontsov <cbouatmailru@gmail.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/power/max17042_battery.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c
index c5c8805..09f7496 100644
--- a/drivers/power/max17042_battery.c
+++ b/drivers/power/max17042_battery.c
@@ -183,7 +183,6 @@ static int __devinit max17042_probe(struct i2c_client *client,
 	ret = power_supply_register(&client->dev, &chip->battery);
 	if (ret) {
 		dev_err(&client->dev, "failed: power supply register\n");
-		i2c_set_clientdata(client, NULL);
 		kfree(chip);
 		return ret;
 	}
@@ -202,7 +201,6 @@ static int __devexit max17042_remove(struct i2c_client *client)
 	struct max17042_chip *chip = i2c_get_clientdata(client);
 
 	power_supply_unregister(&chip->battery);
-	i2c_set_clientdata(client, NULL);
 	kfree(chip);
 	return 0;
 }
-- 
1.7.2.3

[PATCH 4/4] drivers/staging/lirc: Remove obsolete cleanup for clientdata

[Wolfram Sang]

A few new i2c-drivers came into the kernel which clear the clientdata-pointer
on exit or error. This is obsolete meanwhile, the core will do it. Simplify the
kfree() calls after that, the pointers don't need to be checked.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Andy Walls <awalls@md.metrocast.net>
Cc: Jarod Wilson <jarod@redhat.com>
Cc: Joe Perches <joe@perches.com>
---

Compile tested only.

 drivers/staging/lirc/lirc_zilog.c |   28 +++++++++-------------------
 1 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c
index 3fe5f41..df1055f 100644
--- a/drivers/staging/lirc/lirc_zilog.c
+++ b/drivers/staging/lirc/lirc_zilog.c
@@ -1214,15 +1214,12 @@ static int ir_remove(struct i2c_client *client)
 
 	/* Good-bye Rx */
 	destroy_rx_kthread(ir->rx);
-	if (ir->rx != NULL) {
-		if (ir->rx->buf.fifo_initialized)
-			lirc_buffer_free(&ir->rx->buf);
-		i2c_set_clientdata(ir->rx->c, NULL);
-		kfree(ir->rx);
-	}
+	if (ir->rx && ir->rx->buf.fifo_initialized)
+		lirc_buffer_free(&ir->rx->buf);
+
+	kfree(ir->rx);
 
 	/* Good-bye Tx */
-	i2c_set_clientdata(ir->tx->c, NULL);
 	kfree(ir->tx);
 
 	/* Good-bye IR */
@@ -1388,18 +1385,11 @@ out_unregister:
 out_free_thread:
 	destroy_rx_kthread(ir->rx);
 out_free_xx:
-	if (ir->rx != NULL) {
-		if (ir->rx->buf.fifo_initialized)
-			lirc_buffer_free(&ir->rx->buf);
-		if (ir->rx->c != NULL)
-			i2c_set_clientdata(ir->rx->c, NULL);
-		kfree(ir->rx);
-	}
-	if (ir->tx != NULL) {
-		if (ir->tx->c != NULL)
-			i2c_set_clientdata(ir->tx->c, NULL);
-		kfree(ir->tx);
-	}
+	if (ir->rx && ir->rx->buf.fifo_initialized)
+		lirc_buffer_free(&ir->rx->buf);
+
+	kfree(ir->rx);
+	kfree(ir->tx);
 out_free_ir:
 	del_ir_device(ir);
 	kfree(ir);
-- 
1.7.2.3

Re: [PATCH 4/4] drivers/staging/lirc: Remove obsolete cleanup for clientdata

[Jarod Wilson]

On Mon, Jan 24, 2011 at 11:44:30AM +0100, Wolfram Sang wrote:
> A few new i2c-drivers came into the kernel which clear the clientdata-pointer
> on exit or error. This is obsolete meanwhile, the core will do it. Simplify the
> kfree() calls after that, the pointers don't need to be checked.
> 
> Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Greg Kroah-Hartman <gregkh-l3A5Bk7waGM@public.gmane.org>
> Cc: Mauro Carvalho Chehab <mchehab-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Andy Walls <awalls-Xoej9cPu4Z+RGvkDC/A1pg@public.gmane.org>
> Cc: Jarod Wilson <jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
> ---
> 
> Compile tested only.

Looks sane to me.

Acked-by: Jarod Wilson <jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

-- 
Jarod Wilson
jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

Re: [PATCH 4/4] drivers/staging/lirc: Remove obsolete cleanup for clientdata

[Andy Walls]

On Mon, 2011-01-24 at 11:44 +0100, Wolfram Sang wrote:
> A few new i2c-drivers came into the kernel which clear the clientdata-pointer
> on exit or error. This is obsolete meanwhile, the core will do it. Simplify the
> kfree() calls after that, the pointers don't need to be checked.
> 
> Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Greg Kroah-Hartman <gregkh-l3A5Bk7waGM@public.gmane.org>
> Cc: Mauro Carvalho Chehab <mchehab-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Andy Walls <awalls-Xoej9cPu4Z+RGvkDC/A1pg@public.gmane.org>

Nak.

In a module where I plan to do more cleanup and add reference counting
to struct IR, IR_tx, and IR_rx objects, I'd like the explicit "NULL"s in
pointer checks to stay in for now.

Removing the explicit constant "NULL" makes searching for checks against
NULL pointers more difficult when auditing the code (No red syntax
highlighting and a /NULL search misses).

Regards,
Andy


> Cc: Jarod Wilson <jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
> ---
> 
> Compile tested only.
> 
>  drivers/staging/lirc/lirc_zilog.c |   28 +++++++++-------------------
>  1 files changed, 9 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c
> index 3fe5f41..df1055f 100644
> --- a/drivers/staging/lirc/lirc_zilog.c
> +++ b/drivers/staging/lirc/lirc_zilog.c
> @@ -1214,15 +1214,12 @@ static int ir_remove(struct i2c_client *client)
>  
>  	/* Good-bye Rx */
>  	destroy_rx_kthread(ir->rx);
> -	if (ir->rx != NULL) {
> -		if (ir->rx->buf.fifo_initialized)
> -			lirc_buffer_free(&ir->rx->buf);
> -		i2c_set_clientdata(ir->rx->c, NULL);
> -		kfree(ir->rx);
> -	}
> +	if (ir->rx && ir->rx->buf.fifo_initialized)
> +		lirc_buffer_free(&ir->rx->buf);
> +
> +	kfree(ir->rx);
>  
>  	/* Good-bye Tx */
> -	i2c_set_clientdata(ir->tx->c, NULL);
>  	kfree(ir->tx);
>  
>  	/* Good-bye IR */
> @@ -1388,18 +1385,11 @@ out_unregister:
>  out_free_thread:
>  	destroy_rx_kthread(ir->rx);
>  out_free_xx:
> -	if (ir->rx != NULL) {
> -		if (ir->rx->buf.fifo_initialized)
> -			lirc_buffer_free(&ir->rx->buf);
> -		if (ir->rx->c != NULL)
> -			i2c_set_clientdata(ir->rx->c, NULL);
> -		kfree(ir->rx);
> -	}
> -	if (ir->tx != NULL) {
> -		if (ir->tx->c != NULL)
> -			i2c_set_clientdata(ir->tx->c, NULL);
> -		kfree(ir->tx);
> -	}
> +	if (ir->rx && ir->rx->buf.fifo_initialized)
> +		lirc_buffer_free(&ir->rx->buf);
> +
> +	kfree(ir->rx);
> +	kfree(ir->tx);
>  out_free_ir:
>  	del_ir_device(ir);
>  	kfree(ir);

Re: [PATCH 4/4] drivers/staging/lirc: Remove obsolete cleanup for clientdata

[Jean Delvare]

On Mon, 24 Jan 2011 19:09:02 -0500, Andy Walls wrote:
> On Mon, 2011-01-24 at 11:44 +0100, Wolfram Sang wrote:
> > A few new i2c-drivers came into the kernel which clear the clientdata-pointer
> > on exit or error. This is obsolete meanwhile, the core will do it. Simplify the
> > kfree() calls after that, the pointers don't need to be checked.
> > 
> > Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > Cc: Greg Kroah-Hartman <gregkh-l3A5Bk7waGM@public.gmane.org>
> > Cc: Mauro Carvalho Chehab <mchehab-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Cc: Andy Walls <awalls-Xoej9cPu4Z+RGvkDC/A1pg@public.gmane.org>
> 
> Nak.
> 
> In a module where I plan to do more cleanup and add reference counting
> to struct IR, IR_tx, and IR_rx objects, I'd like the explicit "NULL"s in
> pointer checks to stay in for now.
> 
> Removing the explicit constant "NULL" makes searching for checks against
> NULL pointers more difficult when auditing the code (No red syntax
> highlighting and a /NULL search misses).

I agree that Wolfram was too generous with his clean-ups this time.
Wolfram, you should only be removing the calls to i2c_set_clientdata(x,
NULL), not changing the logic of the code.

Furthermore, lirc_zilog is currently undergoing heavy changes, I'd
rather let Andy and Jarod work on that staging driver, and clean up
what needs to be later.

-- 
Jean Delvare

Re: [PATCH 4/4] drivers/staging/lirc: Remove obsolete cleanup for clientdata

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1115 bytes --]

On Mon, Jan 24, 2011 at 07:09:02PM -0500, Andy Walls wrote:
> On Mon, 2011-01-24 at 11:44 +0100, Wolfram Sang wrote:
> > A few new i2c-drivers came into the kernel which clear the clientdata-pointer
> > on exit or error. This is obsolete meanwhile, the core will do it. Simplify the
> > kfree() calls after that, the pointers don't need to be checked.
> > 
> > Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > Cc: Greg Kroah-Hartman <gregkh-l3A5Bk7waGM@public.gmane.org>
> > Cc: Mauro Carvalho Chehab <mchehab-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Cc: Andy Walls <awalls-Xoej9cPu4Z+RGvkDC/A1pg@public.gmane.org>
> 
> Nak.
> 
> In a module where I plan to do more cleanup and add reference counting
> to struct IR, IR_tx, and IR_rx objects, I'd like the explicit "NULL"s in
> pointer checks to stay in for now.

Okay, I will ignore this driver until it comes out of staging.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH 3/4] staging/ste_rmi4: Remove obsolete cleanup for clientdata

[Wolfram Sang]

A few new i2c-drivers came into the kernel which clear the clientdata-pointer
on exit or error. This is obsolete meanwhile, the core will do it.

Signed-off-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Greg Kroah-Hartman <gregkh-l3A5Bk7waGM@public.gmane.org>
Cc: Naveen Kumar Gaddipati <naveen.gaddipati-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
Cc: Linus Walleij <linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>
---

Compile tested only.

 drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
index e8f047e..c9207de 100644
--- a/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
+++ b/drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
@@ -989,7 +989,7 @@ static int __devinit synaptics_rmi4_probe
 	retval = input_register_device(rmi4_data->input_dev);
 	if (retval) {
 		dev_err(&client->dev, "%s:input register failed\n", __func__);
-		goto err_input_register;
+		goto err_query_dev;
 	}
 
 	/* Clear interrupts */
@@ -1011,8 +1011,6 @@ static int __devinit synaptics_rmi4_probe
 err_request_irq:
 	free_irq(platformdata->irq_number, rmi4_data);
 	input_unregister_device(rmi4_data->input_dev);
-err_input_register:
-	i2c_set_clientdata(client, NULL);
 err_query_dev:
 	if (platformdata->regulator_en) {
 		regulator_disable(rmi4_data->regulator);
-- 
1.7.2.3

Re: [PATCH 1/4] mfd: Remove obsolete cleanup for clientdata

[Samuel Ortiz]

Hi _Wolfram_,

On Mon, Jan 24, 2011 at 11:44:27AM +0100, Wolfram Sang wrote:
> A few new i2c-drivers came into the kernel which clear the clientdata-pointer
> on exit or error. This is obsolete meanwhile, the core will do it.
Thanks a lot, patch applied now.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/