LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/5] Convert release_resource to release_region/release_mem_region
@ 2011-02-13 12:12 Julia Lawall
  2011-02-13 12:12 ` [PATCH 1/5] drivers/i2c/busses/i2c-au1550.c: " Julia Lawall
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Julia Lawall @ 2011-02-13 12:12 UTC (permalink / raw)
  To: linux-kernel; +Cc: kernel-janitors

Request_region should be used with release_region, not release_resource.

The changes are somewhat complicated, so I have only done a few cases to
start with.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x,E;
@@
(
*x = request_region(...)
|
*x = request_mem_region(...)
)
... when != release_region(x)
    when != x = E
* release_resource(x);
// </smpl>

The complete set of files in which this semantic match finds problesm is as
follows:

arch/unicore32/kernel/rtc.c
arch/x86/pci/direct.c
drivers/char/hw_random/omap-rng.c
drivers/char/pcmcia/ipwireless/main.c
drivers/i2c/busses/i2c-au1550.c
drivers/i2c/busses/i2c-nuc900.c
drivers/i2c/busses/i2c-s3c2410.c
drivers/i2c/busses/i2c-sh7760.c
drivers/i2c/busses/i2c-simtec.c
drivers/media/video/s5p-fimc/fimc-core.c
drivers/mfd/sm501.c
drivers/misc/atmel_tclib.c
drivers/mmc/host/au1xmmc.c
drivers/mmc/host/davinci_mmc.c
drivers/mmc/host/mvsdio.c
drivers/mmc/host/pxamci.c
drivers/mmc/host/sdhci-s3c.c
drivers/mtd/maps/ceiva.c
drivers/net/a2065.c
drivers/net/ariadne.c
drivers/net/arm/ixp4xx_eth.c
drivers/net/ax88796.c
drivers/parport/parport_ax88796.c
drivers/parport/parport_pc.c
drivers/rtc/rtc-s3c.c
drivers/spi/au1550_spi.c
drivers/spi/spi_s3c24xx.c
drivers/tty/serial/sh-sci.c
drivers/tty/serial/vr41xx_siu.c
drivers/usb/gadget/s3c-hsotg.c
drivers/video/s3c-fb.c
drivers/video/s3c2410fb.c
drivers/video/sh7760fb.c
drivers/video/sm501fb.c
drivers/watchdog/davinci_wdt.c
drivers/watchdog/max63xx_wdt.c
drivers/watchdog/pnx4008_wdt.c
drivers/watchdog/s3c2410_wdt.c


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/5] drivers/i2c/busses/i2c-au1550.c: Convert release_resource to release_region/release_mem_region
  2011-02-13 12:12 [PATCH 0/5] Convert release_resource to release_region/release_mem_region Julia Lawall
@ 2011-02-13 12:12 ` Julia Lawall
  2011-02-13 12:12 ` [PATCH 2/5] drivers/char/hw_random/omap-rng.c: " Julia Lawall
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Julia Lawall @ 2011-02-13 12:12 UTC (permalink / raw)
  To: ben-linux; +Cc: kernel-janitors, linux-i2c, linux-kernel

Request_region should be used with release_region, not release_resource.

The result of request_mem_region is no longer stored.  Instead the field
ioarea is used to store a pointer to the resource structure that contains
the start address.  This is the information that is needed later in
i2c_au1550_remove to release the region.  The field ioarea is not used for
anything else.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x,E;
@@
(
*x = request_region(...)
|
*x = request_mem_region(...)
)
... when != release_region(x)
    when != x = E
* release_resource(x);
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
Not compiled due to incompatible architecture.

 drivers/i2c/busses/i2c-au1550.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-au1550.c b/drivers/i2c/busses/i2c-au1550.c
index 532828b..2ec78d7 100644
--- a/drivers/i2c/busses/i2c-au1550.c
+++ b/drivers/i2c/busses/i2c-au1550.c
@@ -389,13 +389,12 @@ i2c_au1550_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	priv->ioarea = request_mem_region(r->start, resource_size(r),
-					  pdev->name);
-	if (!priv->ioarea) {
+	if (!request_mem_region(r->start, resource_size(r), pdev->name)) {
 		ret = -EBUSY;
 		goto out_mem;
 	}
 
+	priv->ioarea = r;
 	priv->psc_base = CKSEG1ADDR(r->start);
 	priv->xfer_timeout = 200;
 	priv->ack_timeout = 200;
@@ -418,8 +417,7 @@ i2c_au1550_probe(struct platform_device *pdev)
 
 	i2c_au1550_disable(priv);
 
-	release_resource(priv->ioarea);
-	kfree(priv->ioarea);
+	release_mem_region(r->start, resource_size(r));
 out_mem:
 	kfree(priv);
 out:
@@ -430,12 +428,12 @@ static int __devexit
 i2c_au1550_remove(struct platform_device *pdev)
 {
 	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	struct resource *r = priv->ioarea;
 
 	platform_set_drvdata(pdev, NULL);
 	i2c_del_adapter(&priv->adap);
 	i2c_au1550_disable(priv);
-	release_resource(priv->ioarea);
-	kfree(priv->ioarea);
+	release_mem_region(r->start, resource_size(r));
 	kfree(priv);
 	return 0;
 }


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/5] drivers/char/hw_random/omap-rng.c: Convert release_resource to release_region/release_mem_region
  2011-02-13 12:12 [PATCH 0/5] Convert release_resource to release_region/release_mem_region Julia Lawall
  2011-02-13 12:12 ` [PATCH 1/5] drivers/i2c/busses/i2c-au1550.c: " Julia Lawall
@ 2011-02-13 12:12 ` Julia Lawall
  2011-02-14 22:42   ` Matt Mackall
  2011-02-13 12:12 ` [PATCH 3/5] drivers/char/pcmcia/ipwireless/main.c: " Julia Lawall
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Julia Lawall @ 2011-02-13 12:12 UTC (permalink / raw)
  To: Deepak Saxena; +Cc: kernel-janitors, Matt Mackall, Herbert Xu, linux-kernel

Request_region should be used with release_region, not release_resource.

The local variable mem, storing the result of request_mem_region, is
dropped and instead the pointer res is stored in the drvdata field of the
platform device.  This information is retrieved in omap_rng_remove to
release the region.  The drvdata field is not used elsewhere.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x,E;
@@
(
*x = request_region(...)
|
*x = request_mem_region(...)
)
... when != release_region(x)
    when != x = E
* release_resource(x);
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
Not compiled.

 drivers/char/hw_random/omap-rng.c |   14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index 06aad08..2cc755a 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -91,7 +91,7 @@ static struct hwrng omap_rng_ops = {
 
 static int __devinit omap_rng_probe(struct platform_device *pdev)
 {
-	struct resource *res, *mem;
+	struct resource *res;
 	int ret;
 
 	/*
@@ -116,14 +116,12 @@ static int __devinit omap_rng_probe(struct platform_device *pdev)
 	if (!res)
 		return -ENOENT;
 
-	mem = request_mem_region(res->start, resource_size(res),
-				 pdev->name);
-	if (mem == NULL) {
+	if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
 		ret = -EBUSY;
 		goto err_region;
 	}
 
-	dev_set_drvdata(&pdev->dev, mem);
+	dev_set_drvdata(&pdev->dev, res);
 	rng_base = ioremap(res->start, resource_size(res));
 	if (!rng_base) {
 		ret = -ENOMEM;
@@ -146,7 +144,7 @@ err_register:
 	iounmap(rng_base);
 	rng_base = NULL;
 err_ioremap:
-	release_resource(mem);
+	release_mem_region(res->start, resource_size(res));
 err_region:
 	if (cpu_is_omap24xx()) {
 		clk_disable(rng_ick);
@@ -157,7 +155,7 @@ err_region:
 
 static int __exit omap_rng_remove(struct platform_device *pdev)
 {
-	struct resource *mem = dev_get_drvdata(&pdev->dev);
+	struct resource *res = dev_get_drvdata(&pdev->dev);
 
 	hwrng_unregister(&omap_rng_ops);
 
@@ -170,7 +168,7 @@ static int __exit omap_rng_remove(struct platform_device *pdev)
 		clk_put(rng_ick);
 	}
 
-	release_resource(mem);
+	release_mem_region(res->start, resource_size(res));
 	rng_base = NULL;
 
 	return 0;


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/5] drivers/char/pcmcia/ipwireless/main.c: Convert release_resource to release_region/release_mem_region
  2011-02-13 12:12 [PATCH 0/5] Convert release_resource to release_region/release_mem_region Julia Lawall
  2011-02-13 12:12 ` [PATCH 1/5] drivers/i2c/busses/i2c-au1550.c: " Julia Lawall
  2011-02-13 12:12 ` [PATCH 2/5] drivers/char/hw_random/omap-rng.c: " Julia Lawall
@ 2011-02-13 12:12 ` Julia Lawall
  2011-02-16  7:21   ` Dominik Brodowski
  2011-02-13 12:12 ` [PATCH 4/5] arch/x86/pci/direct.c: " Julia Lawall
  2011-02-13 12:12 ` [PATCH 5/5] drivers/i2c/busses/i2c-nuc900.c: " Julia Lawall
  4 siblings, 1 reply; 13+ messages in thread
From: Julia Lawall @ 2011-02-13 12:12 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: kernel-janitors, David Sterba, linux-kernel

Request_region should be used with release_region, not release_resource.

This patch contains a number of changes, related to calls to request_region,
request_mem_region, and the associated error handling code.

1. For the call to request_region, the variable io_resource storing the
result is dropped.  The call to release_resource at the end of the function
is changed to a call to release_region with the first two arguments of
request_region as its arguments.  The same call to release_region is also
added to release_ipwireless.

2. The first call to request_mem_region is now tested and ret is set to
-EBUSY if the the call has failed.  This call was associated with the
initialization of ipw->attr_memory.  But the error handling code was
testing ipw->common_memory.  The definition of release_ipwireless also
suggests that this call should be associated with ipw->common_memory, not
ipw->attr_memory.

3. The second call to request_mem_region is now tested and ret is
set to -EBUSY if the the call has failed.

4. The various gotos to the error handling code is adjusted so that there
is no need for ifs.

5. Return the value stored in the ret variable rather than -1.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x,E;
@@
(
*x = request_region(...)
|
*x = request_mem_region(...)
)
... when != release_region(x)
    when != x = E
* release_resource(x);
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
Compiled, not tested.
In release_ipwireless, should the added call to release_region be protected
by some if?

 drivers/char/pcmcia/ipwireless/main.c |   52 ++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/drivers/char/pcmcia/ipwireless/main.c b/drivers/char/pcmcia/ipwireless/main.c
index 94b8eb4..444155a 100644
--- a/drivers/char/pcmcia/ipwireless/main.c
+++ b/drivers/char/pcmcia/ipwireless/main.c
@@ -78,7 +78,6 @@ static void signalled_reboot_callback(void *callback_data)
 static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data)
 {
 	struct ipw_dev *ipw = priv_data;
-	struct resource *io_resource;
 	int ret;
 
 	p_dev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
@@ -92,9 +91,12 @@ static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data)
 	if (ret)
 		return ret;
 
-	io_resource = request_region(p_dev->resource[0]->start,
-				resource_size(p_dev->resource[0]),
-				IPWIRELESS_PCCARD_NAME);
+	if (!request_region(p_dev->resource[0]->start,
+			    resource_size(p_dev->resource[0]),
+			    IPWIRELESS_PCCARD_NAME)) {
+		ret = -EBUSY;
+		goto exit;
+	}
 
 	p_dev->resource[2]->flags |=
 		WIN_DATA_WIDTH_16 | WIN_MEMORY_TYPE_CM | WIN_ENABLE;
@@ -105,22 +107,25 @@ static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data)
 
 	ret = pcmcia_map_mem_page(p_dev, p_dev->resource[2], p_dev->card_addr);
 	if (ret != 0)
-		goto exit2;
+		goto exit1;
 
 	ipw->is_v2_card = resource_size(p_dev->resource[2]) == 0x100;
 
-	ipw->attr_memory = ioremap(p_dev->resource[2]->start,
+	ipw->common_memory = ioremap(p_dev->resource[2]->start,
 				resource_size(p_dev->resource[2]));
-	request_mem_region(p_dev->resource[2]->start,
-			resource_size(p_dev->resource[2]),
-			IPWIRELESS_PCCARD_NAME);
+	if (!request_mem_region(p_dev->resource[2]->start,
+				resource_size(p_dev->resource[2]),
+				IPWIRELESS_PCCARD_NAME)) {
+		ret = -EBUSY;
+		goto exit2;
+	}
 
 	p_dev->resource[3]->flags |= WIN_DATA_WIDTH_16 | WIN_MEMORY_TYPE_AM |
 					WIN_ENABLE;
 	p_dev->resource[3]->end = 0; /* this used to be 0x1000 */
 	ret = pcmcia_request_window(p_dev, p_dev->resource[3], 0);
 	if (ret != 0)
-		goto exit2;
+		goto exit3;
 
 	ret = pcmcia_map_mem_page(p_dev, p_dev->resource[3], 0);
 	if (ret != 0)
@@ -128,23 +133,28 @@ static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data)
 
 	ipw->attr_memory = ioremap(p_dev->resource[3]->start,
 				resource_size(p_dev->resource[3]));
-	request_mem_region(p_dev->resource[3]->start,
-			resource_size(p_dev->resource[3]),
-			IPWIRELESS_PCCARD_NAME);
+	if (!request_mem_region(p_dev->resource[3]->start,
+				resource_size(p_dev->resource[3]),
+				IPWIRELESS_PCCARD_NAME)) {
+		ret = -EBUSY;
+		goto exit4;
+	}
 
 	return 0;
 
+exit4:
+	iounmap(ipw->attr_memory);
 exit3:
+	release_mem_region(p_dev->resource[2]->start,
+			resource_size(p_dev->resource[2]));
 exit2:
-	if (ipw->common_memory) {
-		release_mem_region(p_dev->resource[2]->start,
-				resource_size(p_dev->resource[2]));
-		iounmap(ipw->common_memory);
-	}
+	iounmap(ipw->common_memory);
 exit1:
-	release_resource(io_resource);
+	release_region(p_dev->resource[0]->start,
+		       resource_size(p_dev->resource[0]));
+exit:
 	pcmcia_disable_device(p_dev);
-	return -1;
+	return ret;
 }
 
 static int config_ipwireless(struct ipw_dev *ipw)
@@ -219,6 +229,8 @@ exit:
 
 static void release_ipwireless(struct ipw_dev *ipw)
 {
+	release_region(ipw->link->resource[0]->start,
+		       resource_size(ipw->link->resource[0]));
 	if (ipw->common_memory) {
 		release_mem_region(ipw->link->resource[2]->start,
 				resource_size(ipw->link->resource[2]));


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 4/5] arch/x86/pci/direct.c: Convert release_resource to release_region/release_mem_region
  2011-02-13 12:12 [PATCH 0/5] Convert release_resource to release_region/release_mem_region Julia Lawall
                   ` (2 preceding siblings ...)
  2011-02-13 12:12 ` [PATCH 3/5] drivers/char/pcmcia/ipwireless/main.c: " Julia Lawall
@ 2011-02-13 12:12 ` Julia Lawall
  2011-04-08 19:50   ` Jesse Barnes
  2011-02-13 12:12 ` [PATCH 5/5] drivers/i2c/busses/i2c-nuc900.c: " Julia Lawall
  4 siblings, 1 reply; 13+ messages in thread
From: Julia Lawall @ 2011-02-13 12:12 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: kernel-janitors, Ingo Molnar, H. Peter Anvin, x86, Jesse Barnes,
	linux-kernel

Request_region should be used with release_region, not release_resource.

The local variables region and region2 are dropped and the calls to
release_resource are replaced with calls to release_region, using the first
two arguments of the corresponding calls to request_region.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x,E;
@@
(
*x = request_region(...)
|
*x = request_mem_region(...)
)
... when != release_region(x)
    when != x = E
* release_resource(x);
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
Compiled, not tested.

 arch/x86/pci/direct.c |   17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/arch/x86/pci/direct.c b/arch/x86/pci/direct.c
index bd33620..e6fd847 100644
--- a/arch/x86/pci/direct.c
+++ b/arch/x86/pci/direct.c
@@ -280,12 +280,9 @@ void __init pci_direct_init(int type)
 
 int __init pci_direct_probe(void)
 {
-	struct resource *region, *region2;
-
 	if ((pci_probe & PCI_PROBE_CONF1) == 0)
 		goto type2;
-	region = request_region(0xCF8, 8, "PCI conf1");
-	if (!region)
+	if (!request_region(0xCF8, 8, "PCI conf1"))
 		goto type2;
 
 	if (pci_check_type1()) {
@@ -293,16 +290,14 @@ int __init pci_direct_probe(void)
 		port_cf9_safe = true;
 		return 1;
 	}
-	release_resource(region);
+	release_region(0xCF8, 8);
 
  type2:
 	if ((pci_probe & PCI_PROBE_CONF2) == 0)
 		return 0;
-	region = request_region(0xCF8, 4, "PCI conf2");
-	if (!region)
+	if (!request_region(0xCF8, 4, "PCI conf2"))
 		return 0;
-	region2 = request_region(0xC000, 0x1000, "PCI conf2");
-	if (!region2)
+	if (!request_region(0xC000, 0x1000, "PCI conf2"))
 		goto fail2;
 
 	if (pci_check_type2()) {
@@ -311,8 +306,8 @@ int __init pci_direct_probe(void)
 		return 2;
 	}
 
-	release_resource(region2);
+	release_region(0xC000, 0x1000);
  fail2:
-	release_resource(region);
+	release_region(0xCF8, 4);
 	return 0;
 }


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 5/5] drivers/i2c/busses/i2c-nuc900.c: Convert release_resource to release_region/release_mem_region
  2011-02-13 12:12 [PATCH 0/5] Convert release_resource to release_region/release_mem_region Julia Lawall
                   ` (3 preceding siblings ...)
  2011-02-13 12:12 ` [PATCH 4/5] arch/x86/pci/direct.c: " Julia Lawall
@ 2011-02-13 12:12 ` Julia Lawall
  2011-02-14 16:56   ` Marek Vasut
  4 siblings, 1 reply; 13+ messages in thread
From: Julia Lawall @ 2011-02-13 12:12 UTC (permalink / raw)
  To: ben-linux
  Cc: kernel-janitors, Marek Vasut, Baruch Siach, Dave Airlie,
	linux-i2c, linux-kernel

Request_region should be used with release_region, not release_resource.

The result of request_mem_region is no longer stored.  Instead the field
ioarea is used to store a pointer to the resource structure that contains
the start address.  This is the information that is needed later in
nuc900_i2c_remove to release the region.  The field ioarea is also printed
in some debugging code.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression x,E;
@@
(
*x = request_region(...)
|
*x = request_mem_region(...)
)
... when != release_region(x)
    when != x = E
* release_resource(x);
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
Not compiled due to incompatible architecture.

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

diff --git a/drivers/i2c/busses/i2c-nuc900.c b/drivers/i2c/busses/i2c-nuc900.c
index 7243426..97b16b7 100644
--- a/drivers/i2c/busses/i2c-nuc900.c
+++ b/drivers/i2c/busses/i2c-nuc900.c
@@ -568,15 +568,13 @@ static int __devinit nuc900_i2c_probe(struct platform_device *pdev)
 		goto err_clk;
 	}
 
-	i2c->ioarea = request_mem_region(res->start, resource_size(res),
-					 pdev->name);
-
-	if (i2c->ioarea == NULL) {
+	if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
 		dev_err(&pdev->dev, "cannot request IO\n");
 		ret = -ENXIO;
 		goto err_clk;
 	}
 
+	i2c->ioarea = res;
 	i2c->regs = ioremap(res->start, resource_size(res));
 
 	if (i2c->regs == NULL) {
@@ -645,8 +643,7 @@ static int __devinit nuc900_i2c_probe(struct platform_device *pdev)
 	iounmap(i2c->regs);
 
  err_ioarea:
-	release_resource(i2c->ioarea);
-	kfree(i2c->ioarea);
+	release_mem_region(res->start, resource_size(res));
 
  err_clk:
 	clk_disable(i2c->clk);
@@ -665,6 +662,7 @@ static int __devinit nuc900_i2c_probe(struct platform_device *pdev)
 static int __devexit nuc900_i2c_remove(struct platform_device *pdev)
 {
 	struct nuc900_i2c *i2c = platform_get_drvdata(pdev);
+	struct resource *res = i2c->ioarea;
 
 	i2c_del_adapter(&i2c->adap);
 	free_irq(i2c->irq, i2c);
@@ -674,8 +672,7 @@ static int __devexit nuc900_i2c_remove(struct platform_device *pdev)
 
 	iounmap(i2c->regs);
 
-	release_resource(i2c->ioarea);
-	kfree(i2c->ioarea);
+	release_mem_region(res->start, resource_size(res));
 	kfree(i2c);
 
 	return 0;


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/5] drivers/i2c/busses/i2c-nuc900.c: Convert release_resource to release_region/release_mem_region
  2011-02-13 12:12 ` [PATCH 5/5] drivers/i2c/busses/i2c-nuc900.c: " Julia Lawall
@ 2011-02-14 16:56   ` Marek Vasut
  2011-02-14 17:04     ` Julia Lawall
  0 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2011-02-14 16:56 UTC (permalink / raw)
  To: Julia Lawall
  Cc: ben-linux, kernel-janitors, Baruch Siach, Dave Airlie, linux-i2c,
	linux-kernel

On Sunday 13 February 2011 13:12:12 Julia Lawall wrote:
> Request_region should be used with release_region, not release_resource.
> 
> The result of request_mem_region is no longer stored.  Instead the field
> ioarea is used to store a pointer to the resource structure that contains
> the start address.  This is the information that is needed later in
> nuc900_i2c_remove to release the region.  The field ioarea is also printed
> in some debugging code.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression x,E;
> @@
> (
> *x = request_region(...)
> 
> *x = request_mem_region(...)
> )
> ... when != release_region(x)
>     when != x = E
> * release_resource(x);
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
> Not compiled due to incompatible architecture.
> 
>  drivers/i2c/busses/i2c-nuc900.c |   13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-nuc900.c
> b/drivers/i2c/busses/i2c-nuc900.c index 7243426..97b16b7 100644
> --- a/drivers/i2c/busses/i2c-nuc900.c
> +++ b/drivers/i2c/busses/i2c-nuc900.c
> @@ -568,15 +568,13 @@ static int __devinit nuc900_i2c_probe(struct
> platform_device *pdev) goto err_clk;
>  	}
> 
> -	i2c->ioarea = request_mem_region(res->start, resource_size(res),
> -					 pdev->name);
> -
> -	if (i2c->ioarea == NULL) {
> +	if (!request_mem_region(res->start, resource_size(res), pdev->name)) {

I prefer the original version here -- it's clearer and also, doesn't the new 
version violate line-over-80 principle ?

>  		dev_err(&pdev->dev, "cannot request IO\n");
>  		ret = -ENXIO;
>  		goto err_clk;
>  	}
> 
> +	i2c->ioarea = res;
>  	i2c->regs = ioremap(res->start, resource_size(res));
> 
>  	if (i2c->regs == NULL) {
> @@ -645,8 +643,7 @@ static int __devinit nuc900_i2c_probe(struct
> platform_device *pdev) iounmap(i2c->regs);
> 
>   err_ioarea:
> -	release_resource(i2c->ioarea);
> -	kfree(i2c->ioarea);
> +	release_mem_region(res->start, resource_size(res));
> 
>   err_clk:
>  	clk_disable(i2c->clk);
> @@ -665,6 +662,7 @@ static int __devinit nuc900_i2c_probe(struct
> platform_device *pdev) static int __devexit nuc900_i2c_remove(struct
> platform_device *pdev) {
>  	struct nuc900_i2c *i2c = platform_get_drvdata(pdev);
> +	struct resource *res = i2c->ioarea;

No need for this ...
> 
>  	i2c_del_adapter(&i2c->adap);
>  	free_irq(i2c->irq, i2c);
> @@ -674,8 +672,7 @@ static int __devexit nuc900_i2c_remove(struct
> platform_device *pdev)
> 
>  	iounmap(i2c->regs);
> 
> -	release_resource(i2c->ioarea);
> -	kfree(i2c->ioarea);

... the i2c pointer is still available here, why introducing struct resource 
*res. release_mem_region() looks like a sane change to me though.

> +	release_mem_region(res->start, resource_size(res));
>  	kfree(i2c);
> 
>  	return 0;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/5] drivers/i2c/busses/i2c-nuc900.c: Convert release_resource to release_region/release_mem_region
  2011-02-14 16:56   ` Marek Vasut
@ 2011-02-14 17:04     ` Julia Lawall
  0 siblings, 0 replies; 13+ messages in thread
From: Julia Lawall @ 2011-02-14 17:04 UTC (permalink / raw)
  To: Marek Vasut
  Cc: ben-linux, kernel-janitors, Baruch Siach, Dave Airlie, linux-i2c,
	linux-kernel

On Mon, 14 Feb 2011, Marek Vasut wrote:

> On Sunday 13 February 2011 13:12:12 Julia Lawall wrote:
> > Request_region should be used with release_region, not release_resource.
> > 
> > The result of request_mem_region is no longer stored.  Instead the field
> > ioarea is used to store a pointer to the resource structure that contains
> > the start address.  This is the information that is needed later in
> > nuc900_i2c_remove to release the region.  The field ioarea is also printed
> > in some debugging code.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @@
> > expression x,E;
> > @@
> > (
> > *x = request_region(...)
> > 
> > *x = request_mem_region(...)
> > )
> > ... when != release_region(x)
> >     when != x = E
> > * release_resource(x);
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> > Not compiled due to incompatible architecture.
> > 
> >  drivers/i2c/busses/i2c-nuc900.c |   13 +++++--------
> >  1 file changed, 5 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-nuc900.c
> > b/drivers/i2c/busses/i2c-nuc900.c index 7243426..97b16b7 100644
> > --- a/drivers/i2c/busses/i2c-nuc900.c
> > +++ b/drivers/i2c/busses/i2c-nuc900.c
> > @@ -568,15 +568,13 @@ static int __devinit nuc900_i2c_probe(struct
> > platform_device *pdev) goto err_clk;
> >  	}
> > 
> > -	i2c->ioarea = request_mem_region(res->start, resource_size(res),
> > -					 pdev->name);
> > -
> > -	if (i2c->ioarea == NULL) {
> > +	if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
> 
> I prefer the original version here -- it's clearer and also, doesn't the new 
> version violate line-over-80 principle ?

The line ends before column 80.

I'm not sure that the result of request_mem_region is the proper argment 
to give to release_region.  In any case, all of the other calls to 
request_mem_region that do use release_region do not use the return value.  
They just test it directly as I have done here.

julia

> >  		dev_err(&pdev->dev, "cannot request IO\n");
> >  		ret = -ENXIO;
> >  		goto err_clk;
> >  	}
> > 
> > +	i2c->ioarea = res;
> >  	i2c->regs = ioremap(res->start, resource_size(res));
> > 
> >  	if (i2c->regs == NULL) {
> > @@ -645,8 +643,7 @@ static int __devinit nuc900_i2c_probe(struct
> > platform_device *pdev) iounmap(i2c->regs);
> > 
> >   err_ioarea:
> > -	release_resource(i2c->ioarea);
> > -	kfree(i2c->ioarea);
> > +	release_mem_region(res->start, resource_size(res));
> > 
> >   err_clk:
> >  	clk_disable(i2c->clk);
> > @@ -665,6 +662,7 @@ static int __devinit nuc900_i2c_probe(struct
> > platform_device *pdev) static int __devexit nuc900_i2c_remove(struct
> > platform_device *pdev) {
> >  	struct nuc900_i2c *i2c = platform_get_drvdata(pdev);
> > +	struct resource *res = i2c->ioarea;
> 
> No need for this ...
> > 
> >  	i2c_del_adapter(&i2c->adap);
> >  	free_irq(i2c->irq, i2c);
> > @@ -674,8 +672,7 @@ static int __devexit nuc900_i2c_remove(struct
> > platform_device *pdev)
> > 
> >  	iounmap(i2c->regs);
> > 
> > -	release_resource(i2c->ioarea);
> > -	kfree(i2c->ioarea);
> 
> ... the i2c pointer is still available here, why introducing struct resource 
> *res. release_mem_region() looks like a sane change to me though.
> 
> > +	release_mem_region(res->start, resource_size(res));
> >  	kfree(i2c);
> > 
> >  	return 0;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/5] drivers/char/hw_random/omap-rng.c: Convert release_resource to release_region/release_mem_region
  2011-02-13 12:12 ` [PATCH 2/5] drivers/char/hw_random/omap-rng.c: " Julia Lawall
@ 2011-02-14 22:42   ` Matt Mackall
  2011-02-14 22:47     ` Herbert Xu
  0 siblings, 1 reply; 13+ messages in thread
From: Matt Mackall @ 2011-02-14 22:42 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Deepak Saxena, kernel-janitors, Herbert Xu, linux-kernel

On Sun, 2011-02-13 at 13:12 +0100, Julia Lawall wrote:
> Request_region should be used with release_region, not release_resource.
> 
> The local variable mem, storing the result of request_mem_region, is
> dropped and instead the pointer res is stored in the drvdata field of the
> platform device.  This information is retrieved in omap_rng_remove to
> release the region.  The drvdata field is not used elsewhere.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)

Looks ok to me, Herbert?

-- 
Mathematics is the supreme nostalgia of our time.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/5] drivers/char/hw_random/omap-rng.c: Convert release_resource to release_region/release_mem_region
  2011-02-14 22:42   ` Matt Mackall
@ 2011-02-14 22:47     ` Herbert Xu
  0 siblings, 0 replies; 13+ messages in thread
From: Herbert Xu @ 2011-02-14 22:47 UTC (permalink / raw)
  To: Matt Mackall; +Cc: Julia Lawall, Deepak Saxena, kernel-janitors, linux-kernel

On Mon, Feb 14, 2011 at 04:42:24PM -0600, Matt Mackall wrote:
> On Sun, 2011-02-13 at 13:12 +0100, Julia Lawall wrote:
> > Request_region should be used with release_region, not release_resource.
> > 
> > The local variable mem, storing the result of request_mem_region, is
> > dropped and instead the pointer res is stored in the drvdata field of the
> > platform device.  This information is retrieved in omap_rng_remove to
> > release the region.  The drvdata field is not used elsewhere.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> 
> Looks ok to me, Herbert?

Yes I will apply it today.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] drivers/char/pcmcia/ipwireless/main.c: Convert release_resource to release_region/release_mem_region
  2011-02-13 12:12 ` [PATCH 3/5] drivers/char/pcmcia/ipwireless/main.c: " Julia Lawall
@ 2011-02-16  7:21   ` Dominik Brodowski
  2011-02-16  8:44     ` Jiri Kosina
  0 siblings, 1 reply; 13+ messages in thread
From: Dominik Brodowski @ 2011-02-16  7:21 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Jiri Kosina, kernel-janitors, David Sterba, linux-kernel

Applied to the pcmcia treee, thanks.

On Sun, Feb 13, 2011 at 01:12:10PM +0100, Julia Lawall wrote:
> Request_region should be used with release_region, not release_resource.
> 
> This patch contains a number of changes, related to calls to request_region,
> request_mem_region, and the associated error handling code.
> 
> 1. For the call to request_region, the variable io_resource storing the
> result is dropped.  The call to release_resource at the end of the function
> is changed to a call to release_region with the first two arguments of
> request_region as its arguments.  The same call to release_region is also
> added to release_ipwireless.
> 
> 2. The first call to request_mem_region is now tested and ret is set to
> -EBUSY if the the call has failed.  This call was associated with the
> initialization of ipw->attr_memory.  But the error handling code was
> testing ipw->common_memory.  The definition of release_ipwireless also
> suggests that this call should be associated with ipw->common_memory, not
> ipw->attr_memory.
> 
> 3. The second call to request_mem_region is now tested and ret is
> set to -EBUSY if the the call has failed.
> 
> 4. The various gotos to the error handling code is adjusted so that there
> is no need for ifs.
> 
> 5. Return the value stored in the ret variable rather than -1.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression x,E;
> @@
> (
> *x = request_region(...)
> |
> *x = request_mem_region(...)
> )
> ... when != release_region(x)
>     when != x = E
> * release_resource(x);
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
> Compiled, not tested.
> In release_ipwireless, should the added call to release_region be protected
> by some if?
> 
>  drivers/char/pcmcia/ipwireless/main.c |   52 ++++++++++++++++++++--------------
>  1 file changed, 32 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/char/pcmcia/ipwireless/main.c b/drivers/char/pcmcia/ipwireless/main.c
> index 94b8eb4..444155a 100644
> --- a/drivers/char/pcmcia/ipwireless/main.c
> +++ b/drivers/char/pcmcia/ipwireless/main.c
> @@ -78,7 +78,6 @@ static void signalled_reboot_callback(void *callback_data)
>  static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data)
>  {
>  	struct ipw_dev *ipw = priv_data;
> -	struct resource *io_resource;
>  	int ret;
>  
>  	p_dev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
> @@ -92,9 +91,12 @@ static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data)
>  	if (ret)
>  		return ret;
>  
> -	io_resource = request_region(p_dev->resource[0]->start,
> -				resource_size(p_dev->resource[0]),
> -				IPWIRELESS_PCCARD_NAME);
> +	if (!request_region(p_dev->resource[0]->start,
> +			    resource_size(p_dev->resource[0]),
> +			    IPWIRELESS_PCCARD_NAME)) {
> +		ret = -EBUSY;
> +		goto exit;
> +	}
>  
>  	p_dev->resource[2]->flags |=
>  		WIN_DATA_WIDTH_16 | WIN_MEMORY_TYPE_CM | WIN_ENABLE;
> @@ -105,22 +107,25 @@ static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data)
>  
>  	ret = pcmcia_map_mem_page(p_dev, p_dev->resource[2], p_dev->card_addr);
>  	if (ret != 0)
> -		goto exit2;
> +		goto exit1;
>  
>  	ipw->is_v2_card = resource_size(p_dev->resource[2]) == 0x100;
>  
> -	ipw->attr_memory = ioremap(p_dev->resource[2]->start,
> +	ipw->common_memory = ioremap(p_dev->resource[2]->start,
>  				resource_size(p_dev->resource[2]));
> -	request_mem_region(p_dev->resource[2]->start,
> -			resource_size(p_dev->resource[2]),
> -			IPWIRELESS_PCCARD_NAME);
> +	if (!request_mem_region(p_dev->resource[2]->start,
> +				resource_size(p_dev->resource[2]),
> +				IPWIRELESS_PCCARD_NAME)) {
> +		ret = -EBUSY;
> +		goto exit2;
> +	}
>  
>  	p_dev->resource[3]->flags |= WIN_DATA_WIDTH_16 | WIN_MEMORY_TYPE_AM |
>  					WIN_ENABLE;
>  	p_dev->resource[3]->end = 0; /* this used to be 0x1000 */
>  	ret = pcmcia_request_window(p_dev, p_dev->resource[3], 0);
>  	if (ret != 0)
> -		goto exit2;
> +		goto exit3;
>  
>  	ret = pcmcia_map_mem_page(p_dev, p_dev->resource[3], 0);
>  	if (ret != 0)
> @@ -128,23 +133,28 @@ static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data)
>  
>  	ipw->attr_memory = ioremap(p_dev->resource[3]->start,
>  				resource_size(p_dev->resource[3]));
> -	request_mem_region(p_dev->resource[3]->start,
> -			resource_size(p_dev->resource[3]),
> -			IPWIRELESS_PCCARD_NAME);
> +	if (!request_mem_region(p_dev->resource[3]->start,
> +				resource_size(p_dev->resource[3]),
> +				IPWIRELESS_PCCARD_NAME)) {
> +		ret = -EBUSY;
> +		goto exit4;
> +	}
>  
>  	return 0;
>  
> +exit4:
> +	iounmap(ipw->attr_memory);
>  exit3:
> +	release_mem_region(p_dev->resource[2]->start,
> +			resource_size(p_dev->resource[2]));
>  exit2:
> -	if (ipw->common_memory) {
> -		release_mem_region(p_dev->resource[2]->start,
> -				resource_size(p_dev->resource[2]));
> -		iounmap(ipw->common_memory);
> -	}
> +	iounmap(ipw->common_memory);
>  exit1:
> -	release_resource(io_resource);
> +	release_region(p_dev->resource[0]->start,
> +		       resource_size(p_dev->resource[0]));
> +exit:
>  	pcmcia_disable_device(p_dev);
> -	return -1;
> +	return ret;
>  }
>  
>  static int config_ipwireless(struct ipw_dev *ipw)
> @@ -219,6 +229,8 @@ exit:
>  
>  static void release_ipwireless(struct ipw_dev *ipw)
>  {
> +	release_region(ipw->link->resource[0]->start,
> +		       resource_size(ipw->link->resource[0]));
>  	if (ipw->common_memory) {
>  		release_mem_region(ipw->link->resource[2]->start,
>  				resource_size(ipw->link->resource[2]));
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] drivers/char/pcmcia/ipwireless/main.c: Convert release_resource to release_region/release_mem_region
  2011-02-16  7:21   ` Dominik Brodowski
@ 2011-02-16  8:44     ` Jiri Kosina
  0 siblings, 0 replies; 13+ messages in thread
From: Jiri Kosina @ 2011-02-16  8:44 UTC (permalink / raw)
  To: Dominik Brodowski
  Cc: Julia Lawall, kernel-janitors, David Sterba, linux-kernel

On Wed, 16 Feb 2011, Dominik Brodowski wrote:

> Applied to the pcmcia treee, thanks.

FWIW

	Signed-off-by: Jiri Kosina <jkosina@suse.cz>

> On Sun, Feb 13, 2011 at 01:12:10PM +0100, Julia Lawall wrote:
> > Request_region should be used with release_region, not release_resource.
> > 
> > This patch contains a number of changes, related to calls to request_region,
> > request_mem_region, and the associated error handling code.
> > 
> > 1. For the call to request_region, the variable io_resource storing the
> > result is dropped.  The call to release_resource at the end of the function
> > is changed to a call to release_region with the first two arguments of
> > request_region as its arguments.  The same call to release_region is also
> > added to release_ipwireless.
> > 
> > 2. The first call to request_mem_region is now tested and ret is set to
> > -EBUSY if the the call has failed.  This call was associated with the
> > initialization of ipw->attr_memory.  But the error handling code was
> > testing ipw->common_memory.  The definition of release_ipwireless also
> > suggests that this call should be associated with ipw->common_memory, not
> > ipw->attr_memory.
> > 
> > 3. The second call to request_mem_region is now tested and ret is
> > set to -EBUSY if the the call has failed.
> > 
> > 4. The various gotos to the error handling code is adjusted so that there
> > is no need for ifs.
> > 
> > 5. Return the value stored in the ret variable rather than -1.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @@
> > expression x,E;
> > @@
> > (
> > *x = request_region(...)
> > |
> > *x = request_mem_region(...)
> > )
> > ... when != release_region(x)
> >     when != x = E
> > * release_resource(x);
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> > Compiled, not tested.
> > In release_ipwireless, should the added call to release_region be protected
> > by some if?
> > 
> >  drivers/char/pcmcia/ipwireless/main.c |   52 ++++++++++++++++++++--------------
> >  1 file changed, 32 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/char/pcmcia/ipwireless/main.c b/drivers/char/pcmcia/ipwireless/main.c
> > index 94b8eb4..444155a 100644
> > --- a/drivers/char/pcmcia/ipwireless/main.c
> > +++ b/drivers/char/pcmcia/ipwireless/main.c
> > @@ -78,7 +78,6 @@ static void signalled_reboot_callback(void *callback_data)
> >  static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data)
> >  {
> >  	struct ipw_dev *ipw = priv_data;
> > -	struct resource *io_resource;
> >  	int ret;
> >  
> >  	p_dev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH;
> > @@ -92,9 +91,12 @@ static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data)
> >  	if (ret)
> >  		return ret;
> >  
> > -	io_resource = request_region(p_dev->resource[0]->start,
> > -				resource_size(p_dev->resource[0]),
> > -				IPWIRELESS_PCCARD_NAME);
> > +	if (!request_region(p_dev->resource[0]->start,
> > +			    resource_size(p_dev->resource[0]),
> > +			    IPWIRELESS_PCCARD_NAME)) {
> > +		ret = -EBUSY;
> > +		goto exit;
> > +	}
> >  
> >  	p_dev->resource[2]->flags |=
> >  		WIN_DATA_WIDTH_16 | WIN_MEMORY_TYPE_CM | WIN_ENABLE;
> > @@ -105,22 +107,25 @@ static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data)
> >  
> >  	ret = pcmcia_map_mem_page(p_dev, p_dev->resource[2], p_dev->card_addr);
> >  	if (ret != 0)
> > -		goto exit2;
> > +		goto exit1;
> >  
> >  	ipw->is_v2_card = resource_size(p_dev->resource[2]) == 0x100;
> >  
> > -	ipw->attr_memory = ioremap(p_dev->resource[2]->start,
> > +	ipw->common_memory = ioremap(p_dev->resource[2]->start,
> >  				resource_size(p_dev->resource[2]));
> > -	request_mem_region(p_dev->resource[2]->start,
> > -			resource_size(p_dev->resource[2]),
> > -			IPWIRELESS_PCCARD_NAME);
> > +	if (!request_mem_region(p_dev->resource[2]->start,
> > +				resource_size(p_dev->resource[2]),
> > +				IPWIRELESS_PCCARD_NAME)) {
> > +		ret = -EBUSY;
> > +		goto exit2;
> > +	}
> >  
> >  	p_dev->resource[3]->flags |= WIN_DATA_WIDTH_16 | WIN_MEMORY_TYPE_AM |
> >  					WIN_ENABLE;
> >  	p_dev->resource[3]->end = 0; /* this used to be 0x1000 */
> >  	ret = pcmcia_request_window(p_dev, p_dev->resource[3], 0);
> >  	if (ret != 0)
> > -		goto exit2;
> > +		goto exit3;
> >  
> >  	ret = pcmcia_map_mem_page(p_dev, p_dev->resource[3], 0);
> >  	if (ret != 0)
> > @@ -128,23 +133,28 @@ static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data)
> >  
> >  	ipw->attr_memory = ioremap(p_dev->resource[3]->start,
> >  				resource_size(p_dev->resource[3]));
> > -	request_mem_region(p_dev->resource[3]->start,
> > -			resource_size(p_dev->resource[3]),
> > -			IPWIRELESS_PCCARD_NAME);
> > +	if (!request_mem_region(p_dev->resource[3]->start,
> > +				resource_size(p_dev->resource[3]),
> > +				IPWIRELESS_PCCARD_NAME)) {
> > +		ret = -EBUSY;
> > +		goto exit4;
> > +	}
> >  
> >  	return 0;
> >  
> > +exit4:
> > +	iounmap(ipw->attr_memory);
> >  exit3:
> > +	release_mem_region(p_dev->resource[2]->start,
> > +			resource_size(p_dev->resource[2]));
> >  exit2:
> > -	if (ipw->common_memory) {
> > -		release_mem_region(p_dev->resource[2]->start,
> > -				resource_size(p_dev->resource[2]));
> > -		iounmap(ipw->common_memory);
> > -	}
> > +	iounmap(ipw->common_memory);
> >  exit1:
> > -	release_resource(io_resource);
> > +	release_region(p_dev->resource[0]->start,
> > +		       resource_size(p_dev->resource[0]));
> > +exit:
> >  	pcmcia_disable_device(p_dev);
> > -	return -1;
> > +	return ret;
> >  }
> >  
> >  static int config_ipwireless(struct ipw_dev *ipw)
> > @@ -219,6 +229,8 @@ exit:
> >  
> >  static void release_ipwireless(struct ipw_dev *ipw)
> >  {
> > +	release_region(ipw->link->resource[0]->start,
> > +		       resource_size(ipw->link->resource[0]));
> >  	if (ipw->common_memory) {
> >  		release_mem_region(ipw->link->resource[2]->start,
> >  				resource_size(ipw->link->resource[2]));
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/5] arch/x86/pci/direct.c: Convert release_resource to release_region/release_mem_region
  2011-02-13 12:12 ` [PATCH 4/5] arch/x86/pci/direct.c: " Julia Lawall
@ 2011-04-08 19:50   ` Jesse Barnes
  0 siblings, 0 replies; 13+ messages in thread
From: Jesse Barnes @ 2011-04-08 19:50 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Thomas Gleixner, kernel-janitors, Ingo Molnar, H. Peter Anvin,
	x86, linux-kernel

On Sun, 13 Feb 2011 13:12:11 +0100
Julia Lawall <julia@diku.dk> wrote:

> Request_region should be used with release_region, not release_resource.
> 
> The local variables region and region2 are dropped and the calls to
> release_resource are replaced with calls to release_region, using the first
> two arguments of the corresponding calls to request_region.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)

Applied to linux-next, thanks.

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2011-04-08 19:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-13 12:12 [PATCH 0/5] Convert release_resource to release_region/release_mem_region Julia Lawall
2011-02-13 12:12 ` [PATCH 1/5] drivers/i2c/busses/i2c-au1550.c: " Julia Lawall
2011-02-13 12:12 ` [PATCH 2/5] drivers/char/hw_random/omap-rng.c: " Julia Lawall
2011-02-14 22:42   ` Matt Mackall
2011-02-14 22:47     ` Herbert Xu
2011-02-13 12:12 ` [PATCH 3/5] drivers/char/pcmcia/ipwireless/main.c: " Julia Lawall
2011-02-16  7:21   ` Dominik Brodowski
2011-02-16  8:44     ` Jiri Kosina
2011-02-13 12:12 ` [PATCH 4/5] arch/x86/pci/direct.c: " Julia Lawall
2011-04-08 19:50   ` Jesse Barnes
2011-02-13 12:12 ` [PATCH 5/5] drivers/i2c/busses/i2c-nuc900.c: " Julia Lawall
2011-02-14 16:56   ` Marek Vasut
2011-02-14 17:04     ` Julia Lawall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).