[PATCH 1/2] i2c-s3c2410: Convert to devm_kzalloc()

[PATCH 1/2] i2c-s3c2410: Convert to devm_kzalloc()

[Mark Brown]

Saves remembering to call kfree(). There's some kfree()s used by the
resource still, these will be removed in 3.3 using the newly added
devm_request_and_ioremap().

Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
---
 drivers/i2c/busses/i2c-s3c2410.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 2754cef..ed62a7f 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -889,7 +889,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 		}
 	}
 
-	i2c = kzalloc(sizeof(struct s3c24xx_i2c), GFP_KERNEL);
+	i2c = devm_kzalloc(&pdev->dev, sizeof(struct s3c24xx_i2c), GFP_KERNEL);
 	if (!i2c) {
 		dev_err(&pdev->dev, "no memory for state\n");
 		return -ENOMEM;
@@ -1034,7 +1034,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 	clk_put(i2c->clk);
 
  err_noclk:
-	kfree(i2c);
 	return ret;
 }
 
@@ -1060,7 +1059,6 @@ static int s3c24xx_i2c_remove(struct platform_device *pdev)
 	release_resource(i2c->ioarea);
 	s3c24xx_i2c_dt_gpio_free(i2c);
 	kfree(i2c->ioarea);
-	kfree(i2c);
 
 	return 0;
 }
-- 
1.7.7.3

[PATCH 2/2] i2c-s3c2410: Add stub runtime power management

[Mark Brown]

Add stub runtime_pm calls which go through the flow of enabling and
disabling but don't actually do anything with the device itself as
there's nothing useful we can do. This provides the core PM framework
with information about when the device is idle, enabling chip wide
power savings.

Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
---
 drivers/i2c/busses/i2c-s3c2410.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index ed62a7f..065c316 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -31,6 +31,7 @@
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/clk.h>
 #include <linux/cpufreq.h>
 #include <linux/slab.h>
@@ -563,6 +564,7 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
 	int retry;
 	int ret;
 
+	pm_runtime_get_sync(&adap->dev);
 	clk_enable(i2c->clk);
 
 	for (retry = 0; retry < adap->retries; retry++) {
@@ -571,6 +573,7 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
 
 		if (ret != -EAGAIN) {
 			clk_disable(i2c->clk);
+			pm_runtime_put_sync(&adap->dev);
 			return ret;
 		}
 
@@ -580,6 +583,7 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
 	}
 
 	clk_disable(i2c->clk);
+	pm_runtime_put_sync(&adap->dev);
 	return -EREMOTEIO;
 }
 
@@ -1012,6 +1016,9 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 	of_i2c_register_devices(&i2c->adap);
 	platform_set_drvdata(pdev, i2c);
 
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_enable(&i2c->adap.dev);
+
 	dev_info(&pdev->dev, "%s: S3C I2C adapter\n", dev_name(&i2c->adap.dev));
 	clk_disable(i2c->clk);
 	return 0;
@@ -1046,6 +1053,9 @@ static int s3c24xx_i2c_remove(struct platform_device *pdev)
 {
 	struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev);
 
+	pm_runtime_disable(&i2c->adap.dev);
+	pm_runtime_disable(&pdev->dev);
+
 	s3c24xx_i2c_deregister_cpufreq(i2c);
 
 	i2c_del_adapter(&i2c->adap);
-- 
1.7.7.3

Re: [PATCH 2/2] i2c-s3c2410: Add stub runtime power management

[Heiko Stübner]

Am Montag, 5. Dezember 2011, 15:06:06 schrieb Mark Brown:
> Add stub runtime_pm calls which go through the flow of enabling and
> disabling but don't actually do anything with the device itself as
> there's nothing useful we can do. This provides the core PM framework
> with information about when the device is idle, enabling chip wide
> power savings.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Acked-by: Heiko Stuebner <heiko@sntech.de>

Kgene, Jean, Ben: could we move this forward please?

Thanks
Heiko

> ---
>  drivers/i2c/busses/i2c-s3c2410.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c
> b/drivers/i2c/busses/i2c-s3c2410.c index ed62a7f..065c316 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -31,6 +31,7 @@
>  #include <linux/errno.h>
>  #include <linux/err.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/clk.h>
>  #include <linux/cpufreq.h>
>  #include <linux/slab.h>
> @@ -563,6 +564,7 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
>  	int retry;
>  	int ret;
> 
> +	pm_runtime_get_sync(&adap->dev);
>  	clk_enable(i2c->clk);
> 
>  	for (retry = 0; retry < adap->retries; retry++) {
> @@ -571,6 +573,7 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
> 
>  		if (ret != -EAGAIN) {
>  			clk_disable(i2c->clk);
> +			pm_runtime_put_sync(&adap->dev);
>  			return ret;
>  		}
> 
> @@ -580,6 +583,7 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
>  	}
> 
>  	clk_disable(i2c->clk);
> +	pm_runtime_put_sync(&adap->dev);
>  	return -EREMOTEIO;
>  }
> 
> @@ -1012,6 +1016,9 @@ static int s3c24xx_i2c_probe(struct platform_device
> *pdev) of_i2c_register_devices(&i2c->adap);
>  	platform_set_drvdata(pdev, i2c);
> 
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_enable(&i2c->adap.dev);
> +
>  	dev_info(&pdev->dev, "%s: S3C I2C adapter\n", dev_name(&i2c->adap.dev));
>  	clk_disable(i2c->clk);
>  	return 0;
> @@ -1046,6 +1053,9 @@ static int s3c24xx_i2c_remove(struct platform_device
> *pdev) {
>  	struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev);
> 
> +	pm_runtime_disable(&i2c->adap.dev);
> +	pm_runtime_disable(&pdev->dev);
> +
>  	s3c24xx_i2c_deregister_cpufreq(i2c);
> 
>  	i2c_del_adapter(&i2c->adap);

Re: [PATCH 2/2] i2c-s3c2410: Add stub runtime power management

[Jean Delvare]

On Tue, 13 Dec 2011 15:53:48 +0100, Heiko Stübner wrote:
> Am Montag, 5. Dezember 2011, 15:06:06 schrieb Mark Brown:
> > Add stub runtime_pm calls which go through the flow of enabling and
> > disabling but don't actually do anything with the device itself as
> > there's nothing useful we can do. This provides the core PM framework
> > with information about when the device is idle, enabling chip wide
> > power savings.
> > 
> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Acked-by: Heiko Stuebner <heiko@sntech.de>
> 
> Kgene, Jean, Ben: could we move this forward please?

The i2c-s3c2410 driver is in Ben's garden, not mine.

-- 
Jean Delvare

[PATCH 1/2] i2c-s3c2410: Convert to devm_kzalloc()

[Mark Brown]

Saves remembering to call kfree(). There's some kfree()s used by the
resource still, these will be removed in 3.3 using the newly added
devm_request_and_ioremap().

Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
---
 drivers/i2c/busses/i2c-s3c2410.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 4c17180..e6f982b 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -890,7 +890,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 		}
 	}
 
-	i2c = kzalloc(sizeof(struct s3c24xx_i2c), GFP_KERNEL);
+	i2c = devm_kzalloc(&pdev->dev, sizeof(struct s3c24xx_i2c), GFP_KERNEL);
 	if (!i2c) {
 		dev_err(&pdev->dev, "no memory for state\n");
 		return -ENOMEM;
@@ -1035,7 +1035,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 	clk_put(i2c->clk);
 
  err_noclk:
-	kfree(i2c);
 	return ret;
 }
 
@@ -1061,7 +1060,6 @@ static int s3c24xx_i2c_remove(struct platform_device *pdev)
 	release_resource(i2c->ioarea);
 	s3c24xx_i2c_dt_gpio_free(i2c);
 	kfree(i2c->ioarea);
-	kfree(i2c);
 
 	return 0;
 }
-- 
1.7.7.3

Re: [PATCH 1/2] i2c-s3c2410: Convert to devm_kzalloc()

[Sylwester Nawrocki]

On 01/21/2012 02:28 PM, Mark Brown wrote:
> Saves remembering to call kfree(). There's some kfree()s used by the
> resource still, these will be removed in 3.3 using the newly added
> devm_request_and_ioremap().
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

> ---
>   drivers/i2c/busses/i2c-s3c2410.c |    4 +---
>   1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index 4c17180..e6f982b 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -890,7 +890,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>   		}
>   	}
> 
> -	i2c = kzalloc(sizeof(struct s3c24xx_i2c), GFP_KERNEL);
> +	i2c = devm_kzalloc(&pdev->dev, sizeof(struct s3c24xx_i2c), GFP_KERNEL);
>   	if (!i2c) {
>   		dev_err(&pdev->dev, "no memory for state\n");
>   		return -ENOMEM;
> @@ -1035,7 +1035,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>   	clk_put(i2c->clk);
> 
>    err_noclk:
> -	kfree(i2c);
>   	return ret;
>   }
> 
> @@ -1061,7 +1060,6 @@ static int s3c24xx_i2c_remove(struct platform_device *pdev)
>   	release_resource(i2c->ioarea);
>   	s3c24xx_i2c_dt_gpio_free(i2c);
>   	kfree(i2c->ioarea);
> -	kfree(i2c);
> 
>   	return 0;
>   }

-- 

Thanks!
Sylwester

Re: [PATCH 1/2] i2c-s3c2410: Convert to devm_kzalloc()

[Barry Song]

2012/1/21 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> Saves remembering to call kfree(). There's some kfree()s used by the
> resource still, these will be removed in 3.3 using the newly added
> devm_request_and_ioremap().
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Reviewed-by: Barry Song <Baohua.Song@csr.com>

> ---
>  drivers/i2c/busses/i2c-s3c2410.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index 4c17180..e6f982b 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -890,7 +890,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>                }
>        }
>
> -       i2c = kzalloc(sizeof(struct s3c24xx_i2c), GFP_KERNEL);
> +       i2c = devm_kzalloc(&pdev->dev, sizeof(struct s3c24xx_i2c), GFP_KERNEL);
>        if (!i2c) {
>                dev_err(&pdev->dev, "no memory for state\n");
>                return -ENOMEM;
> @@ -1035,7 +1035,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>        clk_put(i2c->clk);
>
>  err_noclk:
> -       kfree(i2c);
>        return ret;
>  }
>
> @@ -1061,7 +1060,6 @@ static int s3c24xx_i2c_remove(struct platform_device *pdev)
>        release_resource(i2c->ioarea);
>        s3c24xx_i2c_dt_gpio_free(i2c);
>        kfree(i2c->ioarea);
> -       kfree(i2c);
>
>        return 0;
>  }
> --
> 1.7.7.3

Re: [PATCH 1/2] i2c-s3c2410: Convert to devm_kzalloc()

[Sylwester Nawrocki]

On 01/21/2012 02:28 PM, Mark Brown wrote:
> Saves remembering to call kfree(). There's some kfree()s used by the
> resource still, these will be removed in 3.3 using the newly added
> devm_request_and_ioremap().
> 
> Signed-off-by: Mark Brown Mark Brown<broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
> ---
>   drivers/i2c/busses/i2c-s3c2410.c |    4 +---
>   1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index 4c17180..e6f982b 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -890,7 +890,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>   		}
>   	}
> 
> -	i2c = kzalloc(sizeof(struct s3c24xx_i2c), GFP_KERNEL);
> +	i2c = devm_kzalloc(&pdev->dev, sizeof(struct s3c24xx_i2c), GFP_KERNEL);

Just in case you are resending the patch for any other reason, it might be worth
to change to sizeof(*i2c) for consistency.

Re: [PATCH 1/2] i2c-s3c2410: Convert to devm_kzalloc()

[Mark Brown]

On Sat, Jan 21, 2012 at 05:10:22PM +0100, Sylwester Nawrocki wrote:
> > -	i2c = kzalloc(sizeof(struct s3c24xx_i2c), GFP_KERNEL);
> > +	i2c = devm_kzalloc(&pdev->dev, sizeof(struct s3c24xx_i2c), GFP_KERNEL);

> Just in case you are resending the patch for any other reason, it might be worth
> to change to sizeof(*i2c) for consistency.

Consistency with...?

TBH I'd rather not change the coding style as well as presumably Ben
likes that the way it is.

Re: [PATCH 1/2] i2c-s3c2410: Convert to devm_kzalloc()

[Sylwester Nawrocki]

On 01/21/2012 06:57 PM, Mark Brown wrote:
> On Sat, Jan 21, 2012 at 05:10:22PM +0100, Sylwester Nawrocki wrote:
>>> -	i2c = kzalloc(sizeof(struct s3c24xx_i2c), GFP_KERNEL);
>>> +	i2c = devm_kzalloc(&pdev->dev, sizeof(struct s3c24xx_i2c), GFP_KERNEL);
> 
>> Just in case you are resending the patch for any other reason, it might be worth
>> to change to sizeof(*i2c) for consistency.
> 
> Consistency with...?

Consistency with the rest of code in i2c-s3c244.c, all other occurrences
of sizeof take name of a variable, rather than its type.
 
> TBH I'd rather not change the coding style as well as presumably Ben
> likes that the way it is.

If that's Ben's will then let it stay as it is. I don't even dare to propose
any change ;) It's rather a meaningless detail. I personally like more using 
variable's name, and I had a feeling it was preferred style in the kernel.

It's even mentioned in Documentation/CodingStyle:

"The kernel provides the following general purpose memory allocators:
kmalloc(), kzalloc(), kcalloc(), vmalloc(), and vzalloc().  Please refer to
the API documentation for further information about them.

The preferred form for passing a size of a struct is the following:

	p = kmalloc(sizeof(*p), ...);

The alternative form where struct name is spelled out hurts readability and
introduces an opportunity for a bug when the pointer variable type is changed
but the corresponding sizeof that is passed to a memory allocator is not."

Re: [PATCH 1/2] i2c-s3c2410: Convert to devm_kzalloc()

[Mark Brown]

On Sat, Jan 21, 2012 at 07:27:32PM +0100, Sylwester Nawrocki wrote:
> On 01/21/2012 06:57 PM, Mark Brown wrote:

> > TBH I'd rather not change the coding style as well as presumably Ben
> > likes that the way it is.

> If that's Ben's will then let it stay as it is. I don't even dare to propose
> any change ;) It's rather a meaningless detail. I personally like more using 
> variable's name, and I had a feeling it was preferred style in the kernel.

I tend to use the variable myself, but when editing existing code I'm
going to stick with what's there.

Re: [PATCH 1/2] i2c-s3c2410: Convert to devm_kzalloc()

[Ben Dooks]

On Sat, Jan 21, 2012 at 05:10:22PM +0100, Sylwester Nawrocki wrote:
> On 01/21/2012 02:28 PM, Mark Brown wrote:
> > Saves remembering to call kfree(). There's some kfree()s used by the
> > resource still, these will be removed in 3.3 using the newly added
> > devm_request_and_ioremap().
> > 
> > Signed-off-by: Mark Brown Mark Brown<broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
> > ---
> >   drivers/i2c/busses/i2c-s3c2410.c |    4 +---
> >   1 files changed, 1 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> > index 4c17180..e6f982b 100644
> > --- a/drivers/i2c/busses/i2c-s3c2410.c
> > +++ b/drivers/i2c/busses/i2c-s3c2410.c
> > @@ -890,7 +890,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
> >   		}
> >   	}
> > 
> > -	i2c = kzalloc(sizeof(struct s3c24xx_i2c), GFP_KERNEL);
> > +	i2c = devm_kzalloc(&pdev->dev, sizeof(struct s3c24xx_i2c), GFP_KERNEL);
> 
> Just in case you are resending the patch for any other reason, it might be worth
> to change to sizeof(*i2c) for consistency.

Yuck.

Re: [PATCH 1/2] i2c-s3c2410: Convert to devm_kzalloc()

[Shubhrajyoti Datta]

Hi Mark,

The changes look good to me.
Reviewed-by: Datta Shubhrajyoti <shubhrajyoti-l0cyMroinI0@public.gmane.org>
For your changes.

Some other doubts though not related to your patch.
Just curious.

On Sat, Jan 21, 2012 at 6:58 PM, Mark Brown
<broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> wrote:
> Saves remembering to call kfree(). There's some kfree()s used by the
> resource still, these will be removed in 3.3 using the newly added
> devm_request_and_ioremap().
>
> Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-s3c2410.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index 4c17180..e6f982b 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -890,7 +890,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>                }
>        }
>
> -       i2c = kzalloc(sizeof(struct s3c24xx_i2c), GFP_KERNEL);
> +       i2c = devm_kzalloc(&pdev->dev, sizeof(struct s3c24xx_i2c), GFP_KERNEL);
>        if (!i2c) {
>                dev_err(&pdev->dev, "no memory for state\n");

The comment looks confusing as I am not sure what is meant here by state.


>                return -ENOMEM;
> @@ -1035,7 +1035,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>        clk_put(i2c->clk);
>
>  err_noclk:
> -       kfree(i2c);
>        return ret;
>  }
>
> @@ -1061,7 +1060,6 @@ static int s3c24xx_i2c_remove(struct platform_device *pdev)
>        release_resource(i2c->ioarea);
>        s3c24xx_i2c_dt_gpio_free(i2c);
>        kfree(i2c->ioarea);
Do we need both  release_resource and kfree

> -       kfree(i2c);
>
>        return 0;
>  }
> --
> 1.7.7.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i2c-s3c2410: Convert to devm_kzalloc()

[Sylwester Nawrocki]

Hi Shubhrajyoti,

On 02/14/2012 02:03 PM, Shubhrajyoti Datta wrote:
> Hi Mark,
> 
> The changes look good to me.
> Reviewed-by: Datta Shubhrajyoti <shubhrajyoti@ti.com>
> For your changes.
> 
> Some other doubts though not related to your patch.
> Just curious.
> 
...
>> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
>> index 4c17180..e6f982b 100644
>> --- a/drivers/i2c/busses/i2c-s3c2410.c
>> +++ b/drivers/i2c/busses/i2c-s3c2410.c
...
>> @@ -1061,7 +1060,6 @@ static int s3c24xx_i2c_remove(struct platform_device *pdev)
>>        release_resource(i2c->ioarea);
>>        s3c24xx_i2c_dt_gpio_free(i2c);
>>        kfree(i2c->ioarea);
> Do we need both  release_resource and kfree

No, instead of release_resource() and kfree() you normally use
release_mem_region() to free resources acquired with request_mem_region().
See this patch for instance:

http://www.spinics.net/lists/linux-samsung-soc/msg08325.html

--
Thanks,
Sylwester