i2c: fix two wrong mem release

PATCH In review Libo Chen 5 participants 10 messages May 9, 2013 – May 19, 2013 I2C subsystem host drivers

lore

Share & tools

lore

Libo Chen

author

2013-05-09 08:27 UTC #0 lore raw

v2 cover i2c: fix two wrong mem release

fix two wrong mem release

* Changelog from v1:
* exchange out_error_no_irq and out_error_pin_mux suggested by Sonic <[email protected]>
* add some new tag , make code cleanly

Libo Chen (2):
i2c: i2c-bfin-twi: don`t free peripheral before it init successful
i2c: pxa: no need release_mem_region before request_mem_region
successful

 drivers/i2c/busses/i2c-bfin-twi.c |    2 +-
 drivers/i2c/busses/i2c-pxa.c      |   13 ++++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

Libo Chen

author

2013-05-09 08:27 UTC #1 lore raw

PATCH v2 1/2 i2c: i2c-bfin-twi: don`t free peripheral before it init successful

down out_error_no_irq. When platform_get_irq fail, no need to free peripheral

Signed-off-by Libo Chen <[email protected]>

---
 drivers/i2c/busses/i2c-bfin-twi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/i2c/busses/i2c-bfin-twi.c +1 -1

index 05080c4..6617645 100644
@@ -709,9 +709,9 @@ static int i2c_bfin_twi_probe(struct platform_device *pdev)
 out_error_add_adapter:
 	free_irq(iface->irq, iface);
 out_error_req_irq:
-out_error_no_irq:
 	peripheral_free_list((unsigned short *)pdev->dev.platform_data);
 out_error_pin_mux:
+out_error_no_irq:
 	iounmap(iface->regs_base);
 out_error_ioremap:
 out_error_get_res:

Signature · 1 line

1.7.1

Zhang, Sonic

Acked-by on v2 1/2

2013-05-09 08:31 UTC #2 lore raw

RE: [PATCH v2 1/2] i2c: i2c-bfin-twi: don`t free peripheral before it init successful

Acked-by Sonic Zhang <[email protected]>

Quoted 35 lines

-----Original Message-----
From: Libo Chen [mailto:[email protected]]
Sent: Thursday, May 09, 2013 4:27 PM
To: Zhang, Sonic; wsa-z923LK4zBo2bacvFa/[email protected]
Cc: uclinux-dist-devel-ZG0+EudsQA8dtHy/[email protected]; [email protected]; linux- [email protected]; [email protected]; Libo Chen
Subject: [PATCH v2 1/2] i2c: i2c-bfin-twi: don`t free peripheral before it init
successful
down out_error_no_irq. When platform_get_irq fail, no need to free peripheral
Signed-off-by Libo Chen <[email protected]>
---
drivers/i2c/busses/i2c-bfin-twi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
drivers/i2c/busses/i2c-bfin-twi.c +0 -0
index 05080c4..6617645 100644
@@ -709,9 +709,9 @@ static int i2c_bfin_twi_probe(struct platform_device *pdev)
out_error_add_adapter:
      free_irq(iface->irq, iface);
out_error_req_irq:
-out_error_no_irq:
      peripheral_free_list((unsigned short *)pdev->dev.platform_data);
out_error_pin_mux:
+out_error_no_irq:
      iounmap(iface->regs_base);
out_error_ioremap:
out_error_get_res:
--
1.7.1

Libo Chen

2013-05-10 01:01 UTC #4 lore raw

Re: [PATCH v2 1/2] i2c: i2c-bfin-twi: don`t free peripheral before it init successful

Quoted 29 lines · Sonic

Hi Libo,
Quoted 27 lines
-----Original Message-----
From: Libo Chen [mailto:[email protected]]
Sent: Thursday, May 09, 2013 4:27 PM
To: Zhang, Sonic; wsa-z923LK4zBo2bacvFa/[email protected]
Cc: uclinux-dist-devel-ZG0+EudsQA8dtHy/[email protected]; [email protected]; linux- [email protected]; [email protected]; Libo Chen
Subject: [PATCH v2 1/2] i2c: i2c-bfin-twi: don`t free peripheral before it init
successful
down out_error_no_irq. When platform_get_irq fail, no need to free peripheral
Signed-off-by Libo Chen <[email protected]>
---
drivers/i2c/busses/i2c-bfin-twi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
drivers/i2c/busses/i2c-bfin-twi.c +0 -0
index 05080c4..6617645 100644
@@ -709,9 +709,9 @@ static int i2c_bfin_twi_probe(struct platform_device *pdev)
out_error_add_adapter:
      free_irq(iface->irq, iface);
out_error_req_irq:
-out_error_no_irq:
      peripheral_free_list((unsigned short *)pdev->dev.platform_data);
out_error_pin_mux:

Quoted 9 lines

Quoted 5 lines
+out_error_no_irq:
      iounmap(iface->regs_base);
out_error_ioremap:
out_error_get_res:
--
Sorry, but you didn't exchange as I suggested.
Should be

Hi Sonic,

Look at peripheral_request_list()

{
	u16 cnt;
	int ret;

	for (cnt = 0; per[cnt] != 0; cnt++) {

		ret = peripheral_request(per[cnt], label);

		if (ret < 0) {
			for ( ; cnt > 0; cnt--)
				peripheral_free(per[cnt - 1]);

			return ret;
		}
	}

	return 0;
}

When peripheral_request fail, it had called peripheral_free(). So there is no reason to call peripheral_free_list again.

Did I miss something else?

Quoted 14 lines

drivers/i2c/busses/i2c-bfin-twi.c static int i2c_bfin_twi_probe(struct platform_device *pdev) · line 714
Quoted 6 lines
out_error_req_irq:
drivers/i2c/busses/i2c-bfin-twi.c +2 -2
-out_error_no_irq:
+out_error_pin_mux:
      peripheral_free_list((unsigned short *)pdev->dev.platform_data);
-out_error_pin_mux:
+out_error_no_irq:
Regards,
Sonic

Zhang, Sonic

2013-05-13 08:52 UTC #5 lore raw

RE: [PATCH v2 1/2] i2c: i2c-bfin-twi: don`t free peripheral before it init successful

Hi Libo,

Quoted 77 lines

-----Original Message-----
From: Libo Chen [mailto:[email protected]]
Sent: Friday, May 10, 2013 9:02 AM
To: Zhang, Sonic
Cc: Libo Chen; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v2 1/2] i2c: i2c-bfin-twi: don`t free peripheral before it init successful
Quoted 34 lines · Sonic
Hi Libo,
Quoted 32 lines
-----Original Message-----
From: Libo Chen [mailto:[email protected]]
Sent: Thursday, May 09, 2013 4:27 PM
To: Zhang, Sonic; [email protected]
Cc: [email protected]; [email protected]; linux- [email protected]; [email protected]; Libo Chen
Subject: [PATCH v2 1/2] i2c: i2c-bfin-twi: don`t free peripheral before it init successful
down out_error_no_irq. When platform_get_irq fail, no need to free peripheral
Signed-off-by Libo Chen <[email protected]>
---
drivers/i2c/busses/i2c-bfin-twi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
drivers/i2c/busses/i2c-bfin-twi.c +0 -0
index 05080c4..6617645 100644
@@ -709,9 +709,9 @@ static int i2c_bfin_twi_probe(struct
 platform_device *pdev)
out_error_add_adapter:
      free_irq(iface->irq, iface);
out_error_req_irq:
-out_error_no_irq:
peripheral_free_list((unsigned short
*)pdev->dev.platform_data);
out_error_pin_mux:
Quoted 9 lines
Quoted 5 lines
+out_error_no_irq:
      iounmap(iface->regs_base);
out_error_ioremap:
out_error_get_res:
--
Sorry, but you didn't exchange as I suggested.
Should be
Hi Sonic,
Look at peripheral_request_list()
{
      u16 cnt;
      int ret;

      for (cnt = 0; per[cnt] != 0; cnt++) {

              ret = peripheral_request(per[cnt], label);

              if (ret < 0) {
                      for ( ; cnt > 0; cnt--)
                              peripheral_free(per[cnt - 1]);

                      return ret;
              }
      }

      return 0;
}

Quoted 6 lines

When peripheral_request fail, it had called peripheral_free(). So there is no reason to call peripheral_free_list again.
Did I miss something else?

Yes, you are right. They are freed in peripheral_request_list() if any one fails.

You have my ACK on this patch.

Regards,

Sonic

Zhang, Sonic

2013-05-09 08:34 UTC #6 lore raw

RE: [PATCH v2 1/2] i2c: i2c-bfin-twi: don`t free peripheral before it init successful

In reply to Libo Chen

Please ignore my Ack.

Sonic

Quoted 50 lines

-----Original Message-----
From: Zhang, Sonic
Sent: Thursday, May 09, 2013 4:31 PM
To: 'Libo Chen'; [email protected]
Cc: [email protected]; [email protected]; linux- [email protected]; [email protected]
Subject: RE: [PATCH v2 1/2] i2c: i2c-bfin-twi: don`t free peripheral before it init successful
Acked-by Sonic Zhang <[email protected]>
Quoted 39 lines
-----Original Message-----
From: Libo Chen [mailto:[email protected]]
Sent: Thursday, May 09, 2013 4:27 PM
To: Zhang, Sonic; [email protected]
Cc: [email protected]; [email protected]; linux- [email protected]; [email protected]; Libo Chen
Subject: [PATCH v2 1/2] i2c: i2c-bfin-twi: don`t free peripheral before it init successful
down out_error_no_irq. When platform_get_irq fail, no need to free peripheral
Signed-off-by Libo Chen <[email protected]>
---
drivers/i2c/busses/i2c-bfin-twi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
drivers/i2c/busses/i2c-bfin-twi.c +0 -0
index 05080c4..6617645 100644
@@ -709,9 +709,9 @@ static int i2c_bfin_twi_probe(struct
 platform_device *pdev)
out_error_add_adapter:
     free_irq(iface->irq, iface);
out_error_req_irq:
-out_error_no_irq:
     peripheral_free_list((unsigned short *)pdev->dev.platform_data);
out_error_pin_mux:
+out_error_no_irq:
     iounmap(iface->regs_base);
out_error_ioremap:
out_error_get_res:
--
1.7.1

Libo Chen

author

2013-05-09 08:27 UTC #7 lore raw

PATCH v2 2/2 i2c: pxa: no need release_mem_region before request_mem_region successful

In reply to Libo Chen

When kzalloc fail, there is no reason to release_mem_region

fix confuse tag, add new tag: emem, eirq, edt and make it cleanly

Signed-off-by Libo Chen <[email protected]>

---
 drivers/i2c/busses/i2c-pxa.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

drivers/i2c/busses/i2c-pxa.c +8 -5

index ea6d45d..a9da954 100644
@@ -1104,18 +1104,18 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	if (ret > 0)
 		ret = i2c_pxa_probe_pdata(dev, i2c, &i2c_type);
 	if (ret < 0)
-		goto eclk;
+		goto edt;
 
 	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(dev, 0);
 	if (res == NULL || irq < 0) {
 		ret = -ENODEV;
-		goto eclk;
+		goto eirq;
 	}
 
 	if (!request_mem_region(res->start, resource_size(res), res->name)) {
 		ret = -ENOMEM;
-		goto eclk;
+		goto emem;
 	}
 
 	i2c->adap.owner   = THIS_MODULE;

@@ -1207,9 +1207,12 @@ ereqirq:
 eremap:
 	clk_put(i2c->clk);
 eclk:
-	kfree(i2c);
-emalloc:
 	release_mem_region(res->start, resource_size(res));
+emem:
+eirq:
+edt:
+	kfree(i2c);
+emalloc:	
 	return ret;
 }

Signature · 1 line

1.7.1

Wolfram Sang

2013-05-17 09:41 UTC #8 lore raw

In reply to Libo Chen

Quoted 5 lines · Libo Chen

fix two wrong mem release
* Changelog from v1:
* exchange out_error_no_irq and out_error_pin_mux suggested by Sonic <[email protected]>
* add some new tag , make code cleanly

Thanks for the submission. I do think the proper solution would be to convert the drivers to use the devm_* mechanisms, though. You can check commit 857968434bb6dbda0911f38ec46b0c3d0c963771 ("i2c: davinci: update to devm_* API") as a reference.

Thanks,

Wolfram

Libo Chen

2013-05-19 02:12 UTC #9 lore raw

Quoted 6 lines · Wolfram Sang

Quoted 5 lines · Libo Chen
Quoted 5 lines
fix two wrong mem release
* Changelog from v1:
* exchange out_error_no_irq and out_error_pin_mux suggested by Sonic<[email protected]>
* add some new tag , make code cleanly

Quoted 8 lines

Thanks for the submission. I do think the proper solution would be to convert the drivers to use the devm_* mechanisms, though. You can check commit 857968434bb6dbda0911f38ec46b0c3d0c963771 ("i2c: davinci: update to devm_* API") as a reference.
Thanks,
Wolfram

Hi, Wolfram

It is a good idea. I will learn about it.

Thanks,
Libo