Skip to contentSkip to conversation

i2c: pxa: no need release_mem_region before request_mem_region successful

PATCH In review Libo Chen 2 participants 3 messages May 6, 2013 – May 7, 2013 I2C subsystem host drivers

lore
Share & tools
lore

Libo Chen

author
#0 lore raw

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

move release_mem_region above kfree(i2c) && below clk_put(i2c->clk)

Signed-off-by Libo Chen <[email protected]>
---
 drivers/i2c/busses/i2c-pxa.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-pxa.c +3 -2
index ea6d45d..3188cd0 100644
@@ -1129,7 +1129,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	i2c->clk = clk_get(&dev->dev, NULL);
 	if (IS_ERR(i2c->clk)) {
 		ret = PTR_ERR(i2c->clk);
-		goto eclk;
+		goto emm;
 	}
 
 	i2c->reg_base = ioremap(res->start, resource_size(res));

@@ -1206,10 +1206,11 @@ ereqirq:
 	iounmap(i2c->reg_base);
 eremap:
 	clk_put(i2c->clk);
+emm:
+	release_mem_region(res->start, resource_size(res));
 eclk:
 	kfree(i2c);
 emalloc:
-	release_mem_region(res->start, resource_size(res));
 	return ret;
 }
Signature · 1 line
1.7.1
#1 lore raw

Hi libo,

Quoted 13 lines · Libo Chen

move release_mem_region above kfree(i2c) && below clk_put(i2c->clk)

Signed-off-by Libo Chen <[email protected]>
---
 drivers/i2c/busses/i2c-pxa.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-pxa.c +0 -0
index ea6d45d..3188cd0 100644

drivers/i2c/busses/i2c-pxa.c static int i2c_pxa_probe(struct platform_device *dev) · line 1132

Quoted 5 lines
drivers/i2c/busses/i2c-pxa.c +1 -1
@@ -1129,7 +1129,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	i2c->clk = clk_get(&dev->dev, NULL);
 	if (IS_ERR(i2c->clk)) {
 		ret = PTR_ERR(i2c->clk);
-		goto eclk;
+		goto emm;

The new tag 'emm' seems not suitable here, keep the original 'eclk' here, modify the upper 'eclk' tags instead:
...

	ret = i2c_pxa_probe_dt(dev, i2c, &i2c_type);
	if (ret > 0)
		ret = i2c_pxa_probe_pdata(dev, i2c, &i2c_type);
	if (ret < 0)

- goto eclk;
+ goto etype;

	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
	irq = platform_get_irq(dev, 0);
	if (res == NULL || irq < 0) {
		ret = -ENODEV;
-		goto eclk;
+		goto edev;
	}

	if (!request_mem_region(res->start, resource_size(res), res->name)) {
		ret = -ENOMEM;
-		goto eclk;
+		goto emem;
	}

...

drivers/i2c/busses/i2c-pxa.c ereqirq: · line 1212

Quoted 11 lines
drivers/i2c/busses/i2c-pxa.c +2 -0
 	}
 
 	i2c->reg_base = ioremap(res->start, resource_size(res));

@@ -1206,10 +1206,11 @@ ereqirq:
 	iounmap(i2c->reg_base);
 eremap:
 	clk_put(i2c->clk);
+emm:
+	release_mem_region(res->start, resource_size(res));
 eclk:
 	kfree(i2c);

eclk:

	release_mem_region(res->start, resource_size(res));

etype:
edev:
emem:
kfree(i2c);

drivers/i2c/busses/i2c-pxa.c ereqirq: · line 1216

Quoted 5 lines
drivers/i2c/busses/i2c-pxa.c +0 -1
 emalloc:
-	release_mem_region(res->start, resource_size(res));
 	return ret;
 }

Thanks,
Gu

author
#2 lore raw
Quoted 36 lines · Gu Zheng

Hi libo,

Quoted 13 lines · Libo Chen

move release_mem_region above kfree(i2c) && below clk_put(i2c->clk)

Signed-off-by Libo Chen <[email protected]>
---
 drivers/i2c/busses/i2c-pxa.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-pxa.c +0 -0
index ea6d45d..3188cd0 100644

drivers/i2c/busses/i2c-pxa.c static int i2c_pxa_probe(struct platform_device *dev) · line 1132

Quoted 5 lines
drivers/i2c/busses/i2c-pxa.c +1 -1
@@ -1129,7 +1129,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	i2c->clk = clk_get(&dev->dev, NULL);
 	if (IS_ERR(i2c->clk)) {
 		ret = PTR_ERR(i2c->clk);
-		goto eclk;
+		goto emm;

The new tag 'emm' seems not suitable here, keep the original 'eclk' here, modify the upper 'eclk' tags instead:
...

	ret = i2c_pxa_probe_dt(dev, i2c, &i2c_type);
	if (ret > 0)
		ret = i2c_pxa_probe_pdata(dev, i2c, &i2c_type);
	if (ret < 0)
(quoted diff) +1 -1
-		goto eclk;
+		goto etype;

	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
	irq = platform_get_irq(dev, 0);
	if (res == NULL || irq < 0) {
		ret = -ENODEV;

Review

Quoted 2 lines
(quoted diff) +1 -1
-		goto eclk;
+		goto edev;

Thanks a lot. Yes, it is . Original lable name seems not suitable. but lable:edev is redundant, useless .

I concern this patch is usefull for your work? However I will change it later.

Thanks,
Libo

Quoted 40 lines
	}

	if (!request_mem_region(res->start, resource_size(res), res->name)) {
		ret = -ENOMEM;
(quoted diff) +1 -1
-		goto eclk;
+		goto emem;

	}

...

drivers/i2c/busses/i2c-pxa.c ereqirq: · line 1212

Quoted 11 lines
drivers/i2c/busses/i2c-pxa.c +2 -0
 	}
 
 	i2c->reg_base = ioremap(res->start, resource_size(res));

@@ -1206,10 +1206,11 @@ ereqirq:
 	iounmap(i2c->reg_base);
 eremap:
 	clk_put(i2c->clk);
+emm:
+	release_mem_region(res->start, resource_size(res));
 eclk:
 	kfree(i2c);

   eclk:
	release_mem_region(res->start, resource_size(res));
   etype:
   edev:
   emem:

kfree(i2c);

drivers/i2c/busses/i2c-pxa.c ereqirq: · line 1216

Quoted 5 lines
drivers/i2c/busses/i2c-pxa.c +0 -1
 emalloc:
-	release_mem_region(res->start, resource_size(res));
 	return ret;
 }

Thanks,
Gu

.