linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] use devm_ functions
@ 2012-01-20 21:25 Julia Lawall
  2012-01-20 21:25 ` [PATCH 1/4] drivers/video/backlight/wm831x_bl.c: " Julia Lawall
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Julia Lawall @ 2012-01-20 21:25 UTC (permalink / raw)
  To: Florian Tobias Schandinat; +Cc: kernel-janitors, linux-fbdev, linux-kernel

The semantic patch (http://coccinelle.lip6.fr/) used in generating this
patch is as follows.  Some manual cleanup was required.

// requires -in_place
virtual after_start
virtual returned
virtual arg
virtual get

virtual drop_labels

// ---------------------------------------------------------------------
// find functions

@plat depends on !after_start@
identifier i,pfn,rfn;
position p;
@@

struct platform_driver i@p = {
  .probe = pfn,
  .remove = (<+...rfn...+>),
};

// ---------------------------------------------------------------------
// set up iteration

@initialize:ocaml@

let drop_labels = ref false

type ret = UseReturned | UseReturned2 of string | UseArg | UseGet

let add pfn rfn alloc free devm_alloc file rule =
   let it = new iteration() in
   it#set_files [file];
   it#add_virtual_rule After_start;
   (if !drop_labels then it#add_virtual_rule Drop_labels);
   (match rule with
      UseReturned -> it#add_virtual_rule Returned
    | UseReturned2(free) -> it#add_virtual_rule Returned;
      it#add_virtual_identifier Second_free free
    | UseArg -> it#add_virtual_rule Arg
    | UseGet -> it#add_virtual_rule Get);
   it#add_virtual_identifier Pfn pfn;
   it#add_virtual_identifier Rfn rfn;
   it#add_virtual_identifier Alloc alloc;
   it#add_virtual_identifier Free free;
   it#add_virtual_identifier Devm_alloc devm_alloc;
   it#register()

@script:ocaml depends on drop_labels@
@@

drop_labels := true

@script:ocaml@
pfn << plat.pfn;
rfn << plat.rfn;
p << plat.p;
@@

let file = (List.hd p).file in
add pfn rfn "kmalloc" "kfree" "devm_kzalloc" file UseReturned;
add pfn rfn "kzalloc" "kfree" "devm_kzalloc" file UseReturned;
add pfn rfn "ioremap" "iounmap" "devm_ioremap" file UseReturned;
add pfn rfn "ioremap_nocache" "iounmap" "devm_ioremap_nocache" file
   UseReturned;

add pfn rfn "request_irq" "free_irq" "devm_request_irq" file UseArg;
add pfn rfn "request_threaded_irq" "free_irq" "devm_request_threaded_irq" file
  UseArg;
add pfn rfn "dma_alloc_coherent" "dma_free_coherent" "dmam_alloc_coherent"
  file UseArg;
add pfn rfn "dma_alloc_noncoherent" "dma_free_noncoherent"
  "dmam_alloc_noncoherent" file UseArg;

(* several possibilities... *)
add pfn rfn "request_region" "release_region" "devm_request_region" file
  UseGet;
add pfn rfn "request_mem_region" "release_mem_region"
  "devm_request_mem_region" file UseGet;
add pfn rfn "request_region" "release_region" "devm_request_region" file
  UseArg;
add pfn rfn "request_mem_region" "release_mem_region"
  "devm_request_mem_region" file UseArg;
(* fix a bug at the same time *)
add pfn rfn "request_region" "release_resource" "devm_request_region" file
  (UseReturned2("kfree"));
add pfn rfn "request_mem_region" "release_resource"
  "devm_request_mem_region" file (UseReturned2("kfree"));
add pfn rfn "ioport_map" "ioport_unmap" "devm_ioport_map" file UseReturned

// ---------------------------------------------------------------------
// transform functions where free uses the result

@prb depends on returned exists@
identifier virtual.pfn,pdev,virtual.alloc,virtual.free,virtual.second_free;
expression x;
expression list args;
position p1,p2,p3;
type T1,T2,T3;
@@

pfn(struct platform_device *pdev) { ... when any
x = (T1)alloc@p1(args)
<... when strict
     when any
     when forall
(
free@p2((T2)x,...);
... when != x
second_free@p3((T3)x,...);
|
free@p2((T2)x,...);
)
...>
}

@reme exists@
identifier virtual.rfn,virtual.free;
expression prb.x;
type T;
@@

rfn(...) { ... free((T)x,...); ... }

@rem depends on reme@
identifier virtual.rfn,virtual.free,virtual.second_free;
expression prb.x;
position p4,p5;
type T,T1;
@@

rfn(...) {
<... when strict
(
free@p4((T)x,...);
... when != x
second_free@p5((T1)x,...);
|
free@p4((T)x,...);
)
...>
}

@bad@
identifier virtual.free;
expression prb.x;
position p != {prb.p2,rem.p4};
type T;
@@

free@p((T)x,...)

@modif depends on rem && !bad@
expression x;
identifier prb.pdev,virtual.alloc,virtual.free,virtual.devm_alloc;
identifier virtual.second_free;
expression list args;
position prb.p1,prb.p2,prb.p3,rem.p4,rem.p5;
type T;
@@

(
- free@p2(...);
|
- second_free@p3(...);
|
- free@p4(...);
|
- second_free@p5(...);
|
  x =
- alloc@p1(
+ devm_alloc(&pdev->dev,
    args)
|
  x = 
- (T)alloc@p1(
+ (T)devm_alloc(&pdev->dev,
    args)
)

// ---------------------------------------------------------------------
// transform functions where free uses the first argument

@prbx depends on arg exists@
identifier virtual.pfn,pdev,virtual.alloc,virtual.free;
expression x;
expression list args;
position p1,p2;
@@

pfn(struct platform_device *pdev) { ... when any
alloc@p1(x,args)
<... when strict
     when any
     when forall
free@p2(x,...)
...>
}

@remxe exists@
identifier virtual.rfn, virtual.free;
expression prbx.x;
@@

rfn(...) { ... free(x,...); ... }

@remx depends on remxe@
identifier virtual.rfn, virtual.free;
expression prbx.x;
position p3;
@@

rfn(...) {
<... when strict
free@p3(x,...)
...>
}

@badx@
identifier virtual.free;
expression prbx.x;
position p != {prbx.p2,remx.p3};
@@

free@p(x,...)

@modifx depends on remx && !badx@
expression x;
identifier prbx.pdev,virtual.alloc,virtual.free,virtual.devm_alloc;
expression list args;
position prbx.p1,prbx.p2,remx.p3;
@@

(
- free@p2(...);
|
- free@p3(...);
|
- alloc@p1(
+ devm_alloc(&pdev->dev,
   x,args)
)

// ---------------------------------------------------------------------
// transform functions where free uses the result of platform_get_resource

@prbg depends on get exists@
identifier virtual.pfn,pdev,virtual.alloc,virtual.free;
expression x;
expression list args;
position p1,p2;
@@

pfn(struct platform_device *pdev) { ... when any
alloc@p1(x,args)
<... when strict
     when any
     when forall
free@p2(x,...)
...>
}

@remge exists@
identifier virtual.rfn, virtual.free;
identifier y;
identifier pdev;
@@

rfn(struct platform_device *pdev) { ... when any
y = platform_get_resource(pdev, IORESOURCE_MEM, 0)
...
free(y->start,...)
...
}

@remg depends on remge@
identifier virtual.rfn, virtual.free;
identifier y;
identifier pdev;
position p3;
@@

rfn(struct platform_device *pdev) {
<... when strict
y = platform_get_resource(pdev, IORESOURCE_MEM, 0)
... when strict
free@p3(y->start,...)
...>
}

@badg@
identifier virtual.free;
position p != {prbg.p2,remg.p3};
@@

free@p(...)

@modifg depends on remg && !badg@
expression x;
identifier prbg.pdev,virtual.alloc,virtual.free,virtual.devm_alloc;
expression list args;
position prbg.p1,prbg.p2,remg.p3;
@@

(
- free@p2(...);
|
- free@p3(...);
|
- alloc@p1(
+ devm_alloc(&pdev->dev,
   x,args)
)

// ---------------------------------------------------------------------
// cleanup, if the drvdata was only used to enable the free
// probably only relevant for kmalloc/kzalloc

@dclean depends on modif || modifx || modifg@
identifier virtual.rfn, pdev, i;
type T;
@@

rfn(struct platform_device *pdev) { ...
(
- T i = platform_get_drvdata(pdev);
|
- T i = dev_get_drvdata(&pdev->drv);
|
- T i;
  ... when != i
(
- i = platform_get_drvdata(pdev);
|
- i = dev_get_drvdata(&pdev->drv);
)
)
... when != i
}

@rclean depends on modif || modifx || modifg@
identifier virtual.rfn, pdev, i;
type T;
@@

rfn(struct platform_device *pdev) { ...
(
- T i = platform_get_resource(pdev,...);
|
- T i;
  ... when != i
- i = platform_get_resource(pdev,...);
)
... when != i
}

// ---------------------------------------------------------------------
// cleanup empty ifs, etc

@depends on modif || modifx || modifg@
identifier virtual.pfn;
@@

pfn(...) { <...
- if (...) {}
...> }

@depends on modif || modifx || modifg@
identifier virtual.rfn;
@@

rfn(...) { <...
- if (...) {}
...> }

@depends on modif || modifx || modifg@
identifier virtual.pfn;
expression ret,e;
@@

pfn(...) { <...
+ return
- ret =
 e;
- return ret;
...> }

@depends on modif || modifx || modifg@
identifier virtual.rfn;
expression ret,e;
@@

rfn(...) { <...
+ return
- ret =
 e;
- return ret;
...> }

// ---------------------------------------------------------------------

// this is likely to leave dead code, if l: is preceded by a return
// because we are control-flow based, there is no way to match on that
@r depends on drop_labels && (modif || modifx || modifg)@
identifier l,l1,l2;
expression e;
statement S;
identifier virtual.pfn;
identifier i;
statement S1,S2;
@@

pfn(...) { <...
(
- goto l;
+ goto l2;
...
-l:
<... when != S
     when any
l1:
...>
l2:
(
 (<+...i...+>);
|
 if (...) S1 else S2
|
 while (...) S1
|
 for (...;...;...) S1
)
|
- goto l;
+ return e;
...
-l:
<... when != S
     when any
l1:
...>
return e;
)
...> }


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

* [PATCH 1/4] drivers/video/backlight/wm831x_bl.c: use devm_ functions
  2012-01-20 21:25 [PATCH 0/4] use devm_ functions Julia Lawall
@ 2012-01-20 21:25 ` Julia Lawall
  2012-01-21 12:33   ` Mark Brown
  2012-01-20 21:25 ` [PATCH 2/4] drivers/video/au*fb.c: " Julia Lawall
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2012-01-20 21:25 UTC (permalink / raw)
  To: Mark Brown
  Cc: kernel-janitors, Ian Lartey, Dimitris Papastamos, Richard Purdie,
	Florian Tobias Schandinat, linux-fbdev, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/video/backlight/wm831x_bl.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/video/backlight/wm831x_bl.c b/drivers/video/backlight/wm831x_bl.c
index 4e915f5..5d365de 100644
--- a/drivers/video/backlight/wm831x_bl.c
+++ b/drivers/video/backlight/wm831x_bl.c
@@ -186,7 +186,7 @@ static int wm831x_backlight_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (data == NULL)
 		return -ENOMEM;
 
@@ -200,7 +200,6 @@ static int wm831x_backlight_probe(struct platform_device *pdev)
 				       &wm831x_backlight_ops, &props);
 	if (IS_ERR(bl)) {
 		dev_err(&pdev->dev, "failed to register backlight\n");
-		kfree(data);
 		return PTR_ERR(bl);
 	}
 
@@ -211,7 +210,6 @@ static int wm831x_backlight_probe(struct platform_device *pdev)
 	/* Disable the DCDC if it was started so we can bootstrap */
 	wm831x_set_bits(wm831x, WM831X_DCDC_ENABLE, WM831X_DC4_ENA, 0);
 
-
 	backlight_update_status(bl);
 
 	return 0;
@@ -220,10 +218,8 @@ static int wm831x_backlight_probe(struct platform_device *pdev)
 static int wm831x_backlight_remove(struct platform_device *pdev)
 {
 	struct backlight_device *bl = platform_get_drvdata(pdev);
-	struct wm831x_backlight_data *data = bl_get_data(bl);
 
 	backlight_device_unregister(bl);
-	kfree(data);
 	return 0;
 }
 


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

* [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions
  2012-01-20 21:25 [PATCH 0/4] use devm_ functions Julia Lawall
  2012-01-20 21:25 ` [PATCH 1/4] drivers/video/backlight/wm831x_bl.c: " Julia Lawall
@ 2012-01-20 21:25 ` Julia Lawall
  2012-01-21 13:53   ` Florian Tobias Schandinat
  2012-01-20 21:25 ` [PATCH 3/4] drivers/video/backlight: " Julia Lawall
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2012-01-20 21:25 UTC (permalink / raw)
  To: Florian Tobias Schandinat; +Cc: kernel-janitors, linux-fbdev, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
In the case of au1100fb.c, should the failed label return something other
than 0?
In the case of au1200fb.c, should the error-handling code under the new
call to dma_alloc_noncoherent jump to failed, like the other error handling
code?  I was not sure that there was actually anything for failed to do at
this point in the function.

 drivers/video/au1100fb.c |   30 +++++++++++-------------------
 drivers/video/au1200fb.c |    9 +--------
 2 files changed, 12 insertions(+), 27 deletions(-)

diff --git a/drivers/video/au1100fb.c b/drivers/video/au1100fb.c
index de9da67..8f7b7d7 100644
--- a/drivers/video/au1100fb.c
+++ b/drivers/video/au1100fb.c
@@ -477,7 +477,8 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
 	u32 sys_clksrc;
 
 	/* Allocate new device private */
-	fbdev = kzalloc(sizeof(struct au1100fb_device), GFP_KERNEL);
+	fbdev = devm_kzalloc(&dev->dev, sizeof(struct au1100fb_device),
+			     GFP_KERNEL);
 	if (!fbdev) {
 		print_err("fail to allocate device private record");
 		return -ENOMEM;
@@ -498,8 +499,9 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
 	au1100fb_fix.mmio_start = regs_res->start;
 	au1100fb_fix.mmio_len = resource_size(regs_res);
 
-	if (!request_mem_region(au1100fb_fix.mmio_start, au1100fb_fix.mmio_len,
-				DRIVER_NAME)) {
+	if (!devm_request_mem_region(au1100fb_fix.mmio_start,
+				     au1100fb_fix.mmio_len,
+				     DRIVER_NAME)) {
 		print_err("fail to lock memory region at 0x%08lx",
 				au1100fb_fix.mmio_start);
 		return -EBUSY;
@@ -514,8 +516,9 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
 	fbdev->fb_len = fbdev->panel->xres * fbdev->panel->yres *
 		  	(fbdev->panel->bpp >> 3) * AU1100FB_NBR_VIDEO_BUFFERS;
 
-	fbdev->fb_mem = dma_alloc_coherent(&dev->dev, PAGE_ALIGN(fbdev->fb_len),
-					&fbdev->fb_phys, GFP_KERNEL);
+	fbdev->fb_mem = dmam_alloc_coherent(&dev->dev, &dev->dev,
+					    PAGE_ALIGN(fbdev->fb_len),
+					    &fbdev->fb_phys, GFP_KERNEL);
 	if (!fbdev->fb_mem) {
 		print_err("fail to allocate frambuffer (size: %dK))",
 			  fbdev->fb_len / 1024);
@@ -557,14 +560,14 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
 	fbdev->info.fbops = &au1100fb_ops;
 	fbdev->info.fix = au1100fb_fix;
 
-	if (!(fbdev->info.pseudo_palette = kzalloc(sizeof(u32) * 16, GFP_KERNEL))) {
+	fbdev->info.pseudo_palette =
+		devm_kzalloc(&dev->dev, sizeof(u32) * 16, GFP_KERNEL);
+	if (!fbdev->info.pseudo_palette)
 		return -ENOMEM;
-	}
 
 	if (fb_alloc_cmap(&fbdev->info.cmap, AU1100_LCD_NBR_PALETTE_ENTRIES, 0) < 0) {
 		print_err("Fail to allocate colormap (%d entries)",
 			   AU1100_LCD_NBR_PALETTE_ENTRIES);
-		kfree(fbdev->info.pseudo_palette);
 		return -EFAULT;
 	}
 
@@ -582,9 +585,6 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
 	return 0;
 
 failed:
-	if (fbdev->regs) {
-		release_mem_region(fbdev->regs_phys, fbdev->regs_len);
-	}
 	if (fbdev->fb_mem) {
 		dma_free_noncoherent(&dev->dev, fbdev->fb_len, fbdev->fb_mem,
 				     fbdev->fb_phys);
@@ -592,7 +592,6 @@ failed:
 	if (fbdev->info.cmap.len != 0) {
 		fb_dealloc_cmap(&fbdev->info.cmap);
 	}
-	kfree(fbdev);
 	platform_set_drvdata(dev, NULL);
 
 	return 0;
@@ -615,14 +614,7 @@ int au1100fb_drv_remove(struct platform_device *dev)
 	/* Clean up all probe data */
 	unregister_framebuffer(&fbdev->info);
 
-	release_mem_region(fbdev->regs_phys, fbdev->regs_len);
-
-	dma_free_coherent(&dev->dev, PAGE_ALIGN(fbdev->fb_len), fbdev->fb_mem,
-			  fbdev->fb_phys);
-
 	fb_dealloc_cmap(&fbdev->info.cmap);
-	kfree(fbdev->info.pseudo_palette);
-	kfree((void*)fbdev);
 
 	return 0;
 }
diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
index 04e4479..3e9a773 100644
--- a/drivers/video/au1200fb.c
+++ b/drivers/video/au1200fb.c
@@ -1724,7 +1724,7 @@ static int __devinit au1200fb_drv_probe(struct platform_device *dev)
 		/* Allocate the framebuffer to the maximum screen size */
 		fbdev->fb_len = (win->w[plane].xres * win->w[plane].yres * bpp) / 8;
 
-		fbdev->fb_mem = dma_alloc_noncoherent(&dev->dev,
+		fbdev->fb_mem = dmam_alloc_noncoherent(&dev->dev, &dev->dev,
 				PAGE_ALIGN(fbdev->fb_len),
 				&fbdev->fb_phys, GFP_KERNEL);
 		if (!fbdev->fb_mem) {
@@ -1788,9 +1788,6 @@ static int __devinit au1200fb_drv_probe(struct platform_device *dev)
 
 failed:
 	/* NOTE: This only does the current plane/window that failed; others are still active */
-	if (fbdev->fb_mem)
-		dma_free_noncoherent(&dev->dev, PAGE_ALIGN(fbdev->fb_len),
-				fbdev->fb_mem, fbdev->fb_phys);
 	if (fbi) {
 		if (fbi->cmap.len != 0)
 			fb_dealloc_cmap(&fbi->cmap);
@@ -1817,10 +1814,6 @@ static int __devexit au1200fb_drv_remove(struct platform_device *dev)
 
 		/* Clean up all probe data */
 		unregister_framebuffer(fbi);
-		if (fbdev->fb_mem)
-			dma_free_noncoherent(&dev->dev,
-					PAGE_ALIGN(fbdev->fb_len),
-					fbdev->fb_mem, fbdev->fb_phys);
 		if (fbi->cmap.len != 0)
 			fb_dealloc_cmap(&fbi->cmap);
 		kfree(fbi->pseudo_palette);

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

* [PATCH 3/4] drivers/video/backlight: use devm_ functions
  2012-01-20 21:25 [PATCH 0/4] use devm_ functions Julia Lawall
  2012-01-20 21:25 ` [PATCH 1/4] drivers/video/backlight/wm831x_bl.c: " Julia Lawall
  2012-01-20 21:25 ` [PATCH 2/4] drivers/video/au*fb.c: " Julia Lawall
@ 2012-01-20 21:25 ` Julia Lawall
  2012-01-20 21:25 ` [PATCH 4/4] drivers/video/backlight/adp5520_bl.c: " Julia Lawall
  2012-01-21 14:02 ` [PATCH 0/4] " Florian Tobias Schandinat
  4 siblings, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2012-01-20 21:25 UTC (permalink / raw)
  To: Richard Purdie
  Cc: kernel-janitors, Florian Tobias Schandinat, linux-fbdev, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/video/backlight/88pm860x_bl.c        |    8 ++------
 drivers/video/backlight/aat2870_bl.c         |    9 ++++-----
 drivers/video/backlight/cr_bllcd.c           |    3 +--
 drivers/video/backlight/da903x_bl.c          |    6 +-----
 drivers/video/backlight/max8925_bl.c         |    7 ++-----
 drivers/video/backlight/omap1_bl.c           |    9 +++------
 drivers/video/backlight/pcf50633-backlight.c |   16 +++-------------
 drivers/video/backlight/pwm_bl.c             |    7 ++-----
 8 files changed, 18 insertions(+), 47 deletions(-)

diff --git a/drivers/video/backlight/88pm860x_bl.c b/drivers/video/backlight/88pm860x_bl.c
index a1376dc..915943a 100644
--- a/drivers/video/backlight/88pm860x_bl.c
+++ b/drivers/video/backlight/88pm860x_bl.c
@@ -187,7 +187,8 @@ static int pm860x_backlight_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	data = kzalloc(sizeof(struct pm860x_backlight_data), GFP_KERNEL);
+	data = devm_kzalloc(&pdev->dev, sizeof(struct pm860x_backlight_data),
+			    GFP_KERNEL);
 	if (data == NULL)
 		return -ENOMEM;
 	strncpy(name, res->name, MFD_NAME_SIZE);
@@ -200,7 +201,6 @@ static int pm860x_backlight_probe(struct platform_device *pdev)
 	data->port = pdata->flags;
 	if (data->port < 0) {
 		dev_err(&pdev->dev, "wrong platform data is assigned");
-		kfree(data);
 		return -EINVAL;
 	}
 
@@ -211,7 +211,6 @@ static int pm860x_backlight_probe(struct platform_device *pdev)
 					&pm860x_backlight_ops, &props);
 	if (IS_ERR(bl)) {
 		dev_err(&pdev->dev, "failed to register backlight\n");
-		kfree(data);
 		return PTR_ERR(bl);
 	}
 	bl->props.brightness = MAX_BRIGHTNESS;
@@ -247,17 +246,14 @@ static int pm860x_backlight_probe(struct platform_device *pdev)
 	return 0;
 out:
 	backlight_device_unregister(bl);
-	kfree(data);
 	return ret;
 }
 
 static int pm860x_backlight_remove(struct platform_device *pdev)
 {
 	struct backlight_device *bl = platform_get_drvdata(pdev);
-	struct pm860x_backlight_data *data = bl_get_data(bl);
 
 	backlight_device_unregister(bl);
-	kfree(data);
 	return 0;
 }
 
diff --git a/drivers/video/backlight/aat2870_bl.c b/drivers/video/backlight/aat2870_bl.c
index 331f1ef..7ff7522 100644
--- a/drivers/video/backlight/aat2870_bl.c
+++ b/drivers/video/backlight/aat2870_bl.c
@@ -145,7 +145,9 @@ static int aat2870_bl_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	aat2870_bl = kzalloc(sizeof(struct aat2870_bl_driver_data), GFP_KERNEL);
+	aat2870_bl = devm_kzalloc(&pdev->dev,
+				  sizeof(struct aat2870_bl_driver_data),
+				  GFP_KERNEL);
 	if (!aat2870_bl) {
 		dev_err(&pdev->dev,
 			"Failed to allocate memory for aat2870 backlight\n");
@@ -162,7 +164,7 @@ static int aat2870_bl_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev,
 			"Failed allocate memory for backlight device\n");
 		ret = PTR_ERR(bd);
-		goto out_kfree;
+		goto out;
 	}
 
 	aat2870_bl->pdev = pdev;
@@ -199,8 +201,6 @@ static int aat2870_bl_probe(struct platform_device *pdev)
 
 out_bl_dev_unregister:
 	backlight_device_unregister(bd);
-out_kfree:
-	kfree(aat2870_bl);
 out:
 	return ret;
 }
@@ -215,7 +215,6 @@ static int aat2870_bl_remove(struct platform_device *pdev)
 	backlight_update_status(bd);
 
 	backlight_device_unregister(bd);
-	kfree(aat2870_bl);
 
 	return 0;
 }
diff --git a/drivers/video/backlight/cr_bllcd.c b/drivers/video/backlight/cr_bllcd.c
index 6c8c540..22489eb 100644
--- a/drivers/video/backlight/cr_bllcd.c
+++ b/drivers/video/backlight/cr_bllcd.c
@@ -212,7 +212,7 @@ static int cr_backlight_probe(struct platform_device *pdev)
 			      &gpio_bar);
 	gpio_bar &= ~0x3F;
 
-	crp = kzalloc(sizeof(*crp), GFP_KERNEL);
+	crp = devm_kzalloc(&pdev->dev, sizeof(*crp), GFP_KERNEL);
 	if (!crp) {
 		lcd_device_unregister(ldp);
 		backlight_device_unregister(bdp);
@@ -243,7 +243,6 @@ static int cr_backlight_remove(struct platform_device *pdev)
 	backlight_device_unregister(crp->cr_backlight_device);
 	lcd_device_unregister(crp->cr_lcd_device);
 	pci_dev_put(lpc_dev);
-	kfree(crp);
 
 	return 0;
 }
diff --git a/drivers/video/backlight/da903x_bl.c b/drivers/video/backlight/da903x_bl.c
index abb4a06..30e1968 100644
--- a/drivers/video/backlight/da903x_bl.c
+++ b/drivers/video/backlight/da903x_bl.c
@@ -110,7 +110,7 @@ static int da903x_backlight_probe(struct platform_device *pdev)
 	struct backlight_properties props;
 	int max_brightness;
 
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (data == NULL)
 		return -ENOMEM;
 
@@ -124,7 +124,6 @@ static int da903x_backlight_probe(struct platform_device *pdev)
 	default:
 		dev_err(&pdev->dev, "invalid backlight device ID(%d)\n",
 				pdev->id);
-		kfree(data);
 		return -EINVAL;
 	}
 
@@ -143,7 +142,6 @@ static int da903x_backlight_probe(struct platform_device *pdev)
 				       &da903x_backlight_ops, &props);
 	if (IS_ERR(bl)) {
 		dev_err(&pdev->dev, "failed to register backlight\n");
-		kfree(data);
 		return PTR_ERR(bl);
 	}
 
@@ -157,10 +155,8 @@ static int da903x_backlight_probe(struct platform_device *pdev)
 static int da903x_backlight_remove(struct platform_device *pdev)
 {
 	struct backlight_device *bl = platform_get_drvdata(pdev);
-	struct da903x_backlight_data *data = bl_get_data(bl);
 
 	backlight_device_unregister(bl);
-	kfree(data);
 	return 0;
 }
 
diff --git a/drivers/video/backlight/max8925_bl.c b/drivers/video/backlight/max8925_bl.c
index c915e3b..e833ac7 100644
--- a/drivers/video/backlight/max8925_bl.c
+++ b/drivers/video/backlight/max8925_bl.c
@@ -129,7 +129,8 @@ static int __devinit max8925_backlight_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	data = kzalloc(sizeof(struct max8925_backlight_data), GFP_KERNEL);
+	data = devm_kzalloc(&pdev->dev, sizeof(struct max8925_backlight_data),
+			    GFP_KERNEL);
 	if (data == NULL)
 		return -ENOMEM;
 	strncpy(name, res->name, MAX8925_NAME_SIZE);
@@ -143,7 +144,6 @@ static int __devinit max8925_backlight_probe(struct platform_device *pdev)
 					&max8925_backlight_ops, &props);
 	if (IS_ERR(bl)) {
 		dev_err(&pdev->dev, "failed to register backlight\n");
-		kfree(data);
 		return PTR_ERR(bl);
 	}
 	bl->props.brightness = MAX_BRIGHTNESS;
@@ -165,17 +165,14 @@ static int __devinit max8925_backlight_probe(struct platform_device *pdev)
 	return 0;
 out:
 	backlight_device_unregister(bl);
-	kfree(data);
 	return ret;
 }
 
 static int __devexit max8925_backlight_remove(struct platform_device *pdev)
 {
 	struct backlight_device *bl = platform_get_drvdata(pdev);
-	struct max8925_backlight_data *data = bl_get_data(bl);
 
 	backlight_device_unregister(bl);
-	kfree(data);
 	return 0;
 }
 
diff --git a/drivers/video/backlight/omap1_bl.c b/drivers/video/backlight/omap1_bl.c
index d8cde27..0175bfb 100644
--- a/drivers/video/backlight/omap1_bl.c
+++ b/drivers/video/backlight/omap1_bl.c
@@ -141,7 +141,8 @@ static int omapbl_probe(struct platform_device *pdev)
 	if (!pdata)
 		return -ENXIO;
 
-	bl = kzalloc(sizeof(struct omap_backlight), GFP_KERNEL);
+	bl = devm_kzalloc(&pdev->dev, sizeof(struct omap_backlight),
+			  GFP_KERNEL);
 	if (unlikely(!bl))
 		return -ENOMEM;
 
@@ -150,10 +151,8 @@ static int omapbl_probe(struct platform_device *pdev)
 	props.max_brightness = OMAPBL_MAX_INTENSITY;
 	dev = backlight_device_register("omap-bl", &pdev->dev, bl, &omapbl_ops,
 					&props);
-	if (IS_ERR(dev)) {
-		kfree(bl);
+	if (IS_ERR(dev))
 		return PTR_ERR(dev);
-	}
 
 	bl->powermode = FB_BLANK_POWERDOWN;
 	bl->current_intensity = 0;
@@ -177,10 +176,8 @@ static int omapbl_probe(struct platform_device *pdev)
 static int omapbl_remove(struct platform_device *pdev)
 {
 	struct backlight_device *dev = platform_get_drvdata(pdev);
-	struct omap_backlight *bl = dev_get_drvdata(&dev->dev);
 
 	backlight_device_unregister(dev);
-	kfree(bl);
 
 	return 0;
 }
diff --git a/drivers/video/backlight/pcf50633-backlight.c b/drivers/video/backlight/pcf50633-backlight.c
index 13e88b7..c65853c 100644
--- a/drivers/video/backlight/pcf50633-backlight.c
+++ b/drivers/video/backlight/pcf50633-backlight.c
@@ -101,14 +101,13 @@ static const struct backlight_ops pcf50633_bl_ops = {
 
 static int __devinit pcf50633_bl_probe(struct platform_device *pdev)
 {
-	int ret;
 	struct pcf50633_bl *pcf_bl;
 	struct device *parent = pdev->dev.parent;
 	struct pcf50633_platform_data *pcf50633_data = parent->platform_data;
 	struct pcf50633_bl_platform_data *pdata = pcf50633_data->backlight_data;
 	struct backlight_properties bl_props;
 
-	pcf_bl = kzalloc(sizeof(*pcf_bl), GFP_KERNEL);
+	pcf_bl = devm_kzalloc(&pdev->dev, sizeof(*pcf_bl), GFP_KERNEL);
 	if (!pcf_bl)
 		return -ENOMEM;
 
@@ -129,10 +128,8 @@ static int __devinit pcf50633_bl_probe(struct platform_device *pdev)
 	pcf_bl->bl = backlight_device_register(pdev->name, &pdev->dev, pcf_bl,
 						&pcf50633_bl_ops, &bl_props);
 
-	if (IS_ERR(pcf_bl->bl)) {
-		ret = PTR_ERR(pcf_bl->bl);
-		goto err_free;
-	}
+	if (IS_ERR(pcf_bl->bl))
+		return PTR_ERR(pcf_bl->bl);
 
 	platform_set_drvdata(pdev, pcf_bl);
 
@@ -145,11 +142,6 @@ static int __devinit pcf50633_bl_probe(struct platform_device *pdev)
 	backlight_update_status(pcf_bl->bl);
 
 	return 0;
-
-err_free:
-	kfree(pcf_bl);
-
-	return ret;
 }
 
 static int __devexit pcf50633_bl_remove(struct platform_device *pdev)
@@ -160,8 +152,6 @@ static int __devexit pcf50633_bl_remove(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, NULL);
 
-	kfree(pcf_bl);
-
 	return 0;
 }
 
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 7496d04..342b7d7 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -102,7 +102,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	pb = kzalloc(sizeof(*pb), GFP_KERNEL);
+	pb = devm_kzalloc(&pdev->dev, sizeof(*pb), GFP_KERNEL);
 	if (!pb) {
 		dev_err(&pdev->dev, "no memory for state\n");
 		ret = -ENOMEM;
@@ -121,7 +121,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 	if (IS_ERR(pb->pwm)) {
 		dev_err(&pdev->dev, "unable to request PWM for backlight\n");
 		ret = PTR_ERR(pb->pwm);
-		goto err_pwm;
+		goto err_alloc;
 	} else
 		dev_dbg(&pdev->dev, "got pwm for backlight\n");
 
@@ -144,8 +144,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 
 err_bl:
 	pwm_free(pb->pwm);
-err_pwm:
-	kfree(pb);
 err_alloc:
 	if (data->exit)
 		data->exit(&pdev->dev);
@@ -162,7 +160,6 @@ static int pwm_backlight_remove(struct platform_device *pdev)
 	pwm_config(pb->pwm, 0, pb->period);
 	pwm_disable(pb->pwm);
 	pwm_free(pb->pwm);
-	kfree(pb);
 	if (data->exit)
 		data->exit(&pdev->dev);
 	return 0;


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

* [PATCH 4/4] drivers/video/backlight/adp5520_bl.c: use devm_ functions
  2012-01-20 21:25 [PATCH 0/4] use devm_ functions Julia Lawall
                   ` (2 preceding siblings ...)
  2012-01-20 21:25 ` [PATCH 3/4] drivers/video/backlight: " Julia Lawall
@ 2012-01-20 21:25 ` Julia Lawall
  2012-01-23  8:32   ` Michael Hennerich
  2012-01-21 14:02 ` [PATCH 0/4] " Florian Tobias Schandinat
  4 siblings, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2012-01-20 21:25 UTC (permalink / raw)
  To: Michael Hennerich
  Cc: kernel-janitors, Richard Purdie, Florian Tobias Schandinat,
	device-drivers-devel, linux-fbdev, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/video/backlight/adp5520_bl.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/video/backlight/adp5520_bl.c b/drivers/video/backlight/adp5520_bl.c
index 2e630bf..4911ea7 100644
--- a/drivers/video/backlight/adp5520_bl.c
+++ b/drivers/video/backlight/adp5520_bl.c
@@ -289,7 +289,7 @@ static int __devinit adp5520_bl_probe(struct platform_device *pdev)
 	struct adp5520_bl *data;
 	int ret = 0;
 
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (data == NULL)
 		return -ENOMEM;
 
@@ -298,7 +298,6 @@ static int __devinit adp5520_bl_probe(struct platform_device *pdev)
 
 	if (data->pdata  == NULL) {
 		dev_err(&pdev->dev, "missing platform data\n");
-		kfree(data);
 		return -ENODEV;
 	}
 
@@ -314,7 +313,6 @@ static int __devinit adp5520_bl_probe(struct platform_device *pdev)
 				       &adp5520_bl_ops, &props);
 	if (IS_ERR(bl)) {
 		dev_err(&pdev->dev, "failed to register backlight\n");
-		kfree(data);
 		return PTR_ERR(bl);
 	}
 
@@ -326,7 +324,6 @@ static int __devinit adp5520_bl_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register sysfs\n");
 		backlight_device_unregister(bl);
-		kfree(data);
 	}
 
 	platform_set_drvdata(pdev, bl);
@@ -348,7 +345,6 @@ static int __devexit adp5520_bl_remove(struct platform_device *pdev)
 				&adp5520_bl_attr_group);
 
 	backlight_device_unregister(bl);
-	kfree(data);
 
 	return 0;
 }


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

* Re: [PATCH 1/4] drivers/video/backlight/wm831x_bl.c: use devm_ functions
  2012-01-20 21:25 ` [PATCH 1/4] drivers/video/backlight/wm831x_bl.c: " Julia Lawall
@ 2012-01-21 12:33   ` Mark Brown
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2012-01-21 12:33 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors, Ian Lartey, Dimitris Papastamos, Richard Purdie,
	Florian Tobias Schandinat, linux-fbdev, linux-kernel

On Fri, Jan 20, 2012 at 10:25:29PM +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> The various devm_ functions allocate memory that is released when a driver
> detaches.  This patch uses these functions for data that is allocated in
> the probe function of a platform device and is only freed in the remove
> function.

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

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

* Re: [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions
  2012-01-20 21:25 ` [PATCH 2/4] drivers/video/au*fb.c: " Julia Lawall
@ 2012-01-21 13:53   ` Florian Tobias Schandinat
  2012-01-21 14:00     ` Manuel Lauss
  0 siblings, 1 reply; 12+ messages in thread
From: Florian Tobias Schandinat @ 2012-01-21 13:53 UTC (permalink / raw)
  To: Julia Lawall; +Cc: kernel-janitors, linux-fbdev, LKML, Manuel Lauss

[CC'ing Manuel Lauss as he knows those drivers better than I do]

On 01/20/2012 09:25 PM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> The various devm_ functions allocate memory that is released when a driver
> detaches.  This patch uses these functions for data that is allocated in
> the probe function of a platform device and is only freed in the remove
> function.
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> ---
> In the case of au1100fb.c, should the failed label return something other
> than 0?
> In the case of au1200fb.c, should the error-handling code under the new
> call to dma_alloc_noncoherent jump to failed, like the other error handling
> code?  I was not sure that there was actually anything for failed to do at
> this point in the function.

Maybe Manuel is able to answer these questions. I've queued this patch and if he
doesn't respond I'll apply it after some time.


Best regards,

Florian Tobias Schandinat

> 
>  drivers/video/au1100fb.c |   30 +++++++++++-------------------
>  drivers/video/au1200fb.c |    9 +--------
>  2 files changed, 12 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/video/au1100fb.c b/drivers/video/au1100fb.c
> index de9da67..8f7b7d7 100644
> --- a/drivers/video/au1100fb.c
> +++ b/drivers/video/au1100fb.c
> @@ -477,7 +477,8 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
>  	u32 sys_clksrc;
>  
>  	/* Allocate new device private */
> -	fbdev = kzalloc(sizeof(struct au1100fb_device), GFP_KERNEL);
> +	fbdev = devm_kzalloc(&dev->dev, sizeof(struct au1100fb_device),
> +			     GFP_KERNEL);
>  	if (!fbdev) {
>  		print_err("fail to allocate device private record");
>  		return -ENOMEM;
> @@ -498,8 +499,9 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
>  	au1100fb_fix.mmio_start = regs_res->start;
>  	au1100fb_fix.mmio_len = resource_size(regs_res);
>  
> -	if (!request_mem_region(au1100fb_fix.mmio_start, au1100fb_fix.mmio_len,
> -				DRIVER_NAME)) {
> +	if (!devm_request_mem_region(au1100fb_fix.mmio_start,
> +				     au1100fb_fix.mmio_len,
> +				     DRIVER_NAME)) {
>  		print_err("fail to lock memory region at 0x%08lx",
>  				au1100fb_fix.mmio_start);
>  		return -EBUSY;
> @@ -514,8 +516,9 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
>  	fbdev->fb_len = fbdev->panel->xres * fbdev->panel->yres *
>  		  	(fbdev->panel->bpp >> 3) * AU1100FB_NBR_VIDEO_BUFFERS;
>  
> -	fbdev->fb_mem = dma_alloc_coherent(&dev->dev, PAGE_ALIGN(fbdev->fb_len),
> -					&fbdev->fb_phys, GFP_KERNEL);
> +	fbdev->fb_mem = dmam_alloc_coherent(&dev->dev, &dev->dev,
> +					    PAGE_ALIGN(fbdev->fb_len),
> +					    &fbdev->fb_phys, GFP_KERNEL);
>  	if (!fbdev->fb_mem) {
>  		print_err("fail to allocate frambuffer (size: %dK))",
>  			  fbdev->fb_len / 1024);
> @@ -557,14 +560,14 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
>  	fbdev->info.fbops = &au1100fb_ops;
>  	fbdev->info.fix = au1100fb_fix;
>  
> -	if (!(fbdev->info.pseudo_palette = kzalloc(sizeof(u32) * 16, GFP_KERNEL))) {
> +	fbdev->info.pseudo_palette =
> +		devm_kzalloc(&dev->dev, sizeof(u32) * 16, GFP_KERNEL);
> +	if (!fbdev->info.pseudo_palette)
>  		return -ENOMEM;
> -	}
>  
>  	if (fb_alloc_cmap(&fbdev->info.cmap, AU1100_LCD_NBR_PALETTE_ENTRIES, 0) < 0) {
>  		print_err("Fail to allocate colormap (%d entries)",
>  			   AU1100_LCD_NBR_PALETTE_ENTRIES);
> -		kfree(fbdev->info.pseudo_palette);
>  		return -EFAULT;
>  	}
>  
> @@ -582,9 +585,6 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
>  	return 0;
>  
>  failed:
> -	if (fbdev->regs) {
> -		release_mem_region(fbdev->regs_phys, fbdev->regs_len);
> -	}
>  	if (fbdev->fb_mem) {
>  		dma_free_noncoherent(&dev->dev, fbdev->fb_len, fbdev->fb_mem,
>  				     fbdev->fb_phys);
> @@ -592,7 +592,6 @@ failed:
>  	if (fbdev->info.cmap.len != 0) {
>  		fb_dealloc_cmap(&fbdev->info.cmap);
>  	}
> -	kfree(fbdev);
>  	platform_set_drvdata(dev, NULL);
>  
>  	return 0;
> @@ -615,14 +614,7 @@ int au1100fb_drv_remove(struct platform_device *dev)
>  	/* Clean up all probe data */
>  	unregister_framebuffer(&fbdev->info);
>  
> -	release_mem_region(fbdev->regs_phys, fbdev->regs_len);
> -
> -	dma_free_coherent(&dev->dev, PAGE_ALIGN(fbdev->fb_len), fbdev->fb_mem,
> -			  fbdev->fb_phys);
> -
>  	fb_dealloc_cmap(&fbdev->info.cmap);
> -	kfree(fbdev->info.pseudo_palette);
> -	kfree((void*)fbdev);
>  
>  	return 0;
>  }
> diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
> index 04e4479..3e9a773 100644
> --- a/drivers/video/au1200fb.c
> +++ b/drivers/video/au1200fb.c
> @@ -1724,7 +1724,7 @@ static int __devinit au1200fb_drv_probe(struct platform_device *dev)
>  		/* Allocate the framebuffer to the maximum screen size */
>  		fbdev->fb_len = (win->w[plane].xres * win->w[plane].yres * bpp) / 8;
>  
> -		fbdev->fb_mem = dma_alloc_noncoherent(&dev->dev,
> +		fbdev->fb_mem = dmam_alloc_noncoherent(&dev->dev, &dev->dev,
>  				PAGE_ALIGN(fbdev->fb_len),
>  				&fbdev->fb_phys, GFP_KERNEL);
>  		if (!fbdev->fb_mem) {
> @@ -1788,9 +1788,6 @@ static int __devinit au1200fb_drv_probe(struct platform_device *dev)
>  
>  failed:
>  	/* NOTE: This only does the current plane/window that failed; others are still active */
> -	if (fbdev->fb_mem)
> -		dma_free_noncoherent(&dev->dev, PAGE_ALIGN(fbdev->fb_len),
> -				fbdev->fb_mem, fbdev->fb_phys);
>  	if (fbi) {
>  		if (fbi->cmap.len != 0)
>  			fb_dealloc_cmap(&fbi->cmap);
> @@ -1817,10 +1814,6 @@ static int __devexit au1200fb_drv_remove(struct platform_device *dev)
>  
>  		/* Clean up all probe data */
>  		unregister_framebuffer(fbi);
> -		if (fbdev->fb_mem)
> -			dma_free_noncoherent(&dev->dev,
> -					PAGE_ALIGN(fbdev->fb_len),
> -					fbdev->fb_mem, fbdev->fb_phys);
>  		if (fbi->cmap.len != 0)
>  			fb_dealloc_cmap(&fbi->cmap);
>  		kfree(fbi->pseudo_palette);
> 


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

* Re: [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions
  2012-01-21 13:53   ` Florian Tobias Schandinat
@ 2012-01-21 14:00     ` Manuel Lauss
  2012-01-21 15:01       ` Julia Lawall
  0 siblings, 1 reply; 12+ messages in thread
From: Manuel Lauss @ 2012-01-21 14:00 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Julia Lawall, kernel-janitors, linux-fbdev, LKML

Hello,

On Sat, Jan 21, 2012 at 2:53 PM, Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> wrote:
> [CC'ing Manuel Lauss as he knows those drivers better than I do]
>
> On 01/20/2012 09:25 PM, Julia Lawall wrote:
>> From: Julia Lawall <Julia.Lawall@lip6.fr>
>>
>> The various devm_ functions allocate memory that is released when a driver
>> detaches.  This patch uses these functions for data that is allocated in
>> the probe function of a platform device and is only freed in the remove
>> function.
>>
>> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>>
>> ---
>> In the case of au1100fb.c, should the failed label return something other
>> than 0?
>> In the case of au1200fb.c, should the error-handling code under the new
>> call to dma_alloc_noncoherent jump to failed, like the other error handling
>> code?  I was not sure that there was actually anything for failed to do at
>> this point in the function.
>
> Maybe Manuel is able to answer these questions. I've queued this patch and if he
> doesn't respond I'll apply it after some time.

au1100fb:  Yes, a -ENODEV instead of 0 in the error case is probably a
good idea.
au1200fb:  The error path(s) are buggy; they don't free all windows if
creation of
                      the 4th fails for example. It has been this way
since the driver appeared
                      but for all current use cases (demoboards and in
the field) it actually works.
                      I'm working on a rewrite which is however
progressing rather slowly.


>>  drivers/video/au1100fb.c |   30 +++++++++++-------------------
>>  drivers/video/au1200fb.c |    9 +--------
>>  2 files changed, 12 insertions(+), 27 deletions(-)

Acked-by: Manuel Lauss <manuel.lauss@googlemail.com>

[...]

Thanks!
      Manuel Lauss

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

* Re: [PATCH 0/4] use devm_ functions
  2012-01-20 21:25 [PATCH 0/4] use devm_ functions Julia Lawall
                   ` (3 preceding siblings ...)
  2012-01-20 21:25 ` [PATCH 4/4] drivers/video/backlight/adp5520_bl.c: " Julia Lawall
@ 2012-01-21 14:02 ` Florian Tobias Schandinat
  4 siblings, 0 replies; 12+ messages in thread
From: Florian Tobias Schandinat @ 2012-01-21 14:02 UTC (permalink / raw)
  To: Julia Lawall, Andrew Morton; +Cc: kernel-janitors, linux-fbdev, linux-kernel

Hi Andrew,

can you please take care of the backlight patches (1, 3, 4) of this series?


Thank you,

Florian Tobias Schandinat


On 01/20/2012 09:25 PM, Julia Lawall wrote:
> The semantic patch (http://coccinelle.lip6.fr/) used in generating this
> patch is as follows.  Some manual cleanup was required.
> 
> // requires -in_place
> virtual after_start
> virtual returned
> virtual arg
> virtual get
> 
> virtual drop_labels
> 
> // ---------------------------------------------------------------------
> // find functions
> 
> @plat depends on !after_start@
> identifier i,pfn,rfn;
> position p;
> @@
> 
> struct platform_driver i@p = {
>   .probe = pfn,
>   .remove = (<+...rfn...+>),
> };
> 
> // ---------------------------------------------------------------------
> // set up iteration
> 
> @initialize:ocaml@
> 
> let drop_labels = ref false
> 
> type ret = UseReturned | UseReturned2 of string | UseArg | UseGet
> 
> let add pfn rfn alloc free devm_alloc file rule =
>    let it = new iteration() in
>    it#set_files [file];
>    it#add_virtual_rule After_start;
>    (if !drop_labels then it#add_virtual_rule Drop_labels);
>    (match rule with
>       UseReturned -> it#add_virtual_rule Returned
>     | UseReturned2(free) -> it#add_virtual_rule Returned;
>       it#add_virtual_identifier Second_free free
>     | UseArg -> it#add_virtual_rule Arg
>     | UseGet -> it#add_virtual_rule Get);
>    it#add_virtual_identifier Pfn pfn;
>    it#add_virtual_identifier Rfn rfn;
>    it#add_virtual_identifier Alloc alloc;
>    it#add_virtual_identifier Free free;
>    it#add_virtual_identifier Devm_alloc devm_alloc;
>    it#register()
> 
> @script:ocaml depends on drop_labels@
> @@
> 
> drop_labels := true
> 
> @script:ocaml@
> pfn << plat.pfn;
> rfn << plat.rfn;
> p << plat.p;
> @@
> 
> let file = (List.hd p).file in
> add pfn rfn "kmalloc" "kfree" "devm_kzalloc" file UseReturned;
> add pfn rfn "kzalloc" "kfree" "devm_kzalloc" file UseReturned;
> add pfn rfn "ioremap" "iounmap" "devm_ioremap" file UseReturned;
> add pfn rfn "ioremap_nocache" "iounmap" "devm_ioremap_nocache" file
>    UseReturned;
> 
> add pfn rfn "request_irq" "free_irq" "devm_request_irq" file UseArg;
> add pfn rfn "request_threaded_irq" "free_irq" "devm_request_threaded_irq" file
>   UseArg;
> add pfn rfn "dma_alloc_coherent" "dma_free_coherent" "dmam_alloc_coherent"
>   file UseArg;
> add pfn rfn "dma_alloc_noncoherent" "dma_free_noncoherent"
>   "dmam_alloc_noncoherent" file UseArg;
> 
> (* several possibilities... *)
> add pfn rfn "request_region" "release_region" "devm_request_region" file
>   UseGet;
> add pfn rfn "request_mem_region" "release_mem_region"
>   "devm_request_mem_region" file UseGet;
> add pfn rfn "request_region" "release_region" "devm_request_region" file
>   UseArg;
> add pfn rfn "request_mem_region" "release_mem_region"
>   "devm_request_mem_region" file UseArg;
> (* fix a bug at the same time *)
> add pfn rfn "request_region" "release_resource" "devm_request_region" file
>   (UseReturned2("kfree"));
> add pfn rfn "request_mem_region" "release_resource"
>   "devm_request_mem_region" file (UseReturned2("kfree"));
> add pfn rfn "ioport_map" "ioport_unmap" "devm_ioport_map" file UseReturned
> 
> // ---------------------------------------------------------------------
> // transform functions where free uses the result
> 
> @prb depends on returned exists@
> identifier virtual.pfn,pdev,virtual.alloc,virtual.free,virtual.second_free;
> expression x;
> expression list args;
> position p1,p2,p3;
> type T1,T2,T3;
> @@
> 
> pfn(struct platform_device *pdev) { ... when any
> x = (T1)alloc@p1(args)
> <... when strict
>      when any
>      when forall
> (
> free@p2((T2)x,...);
> ... when != x
> second_free@p3((T3)x,...);
> |
> free@p2((T2)x,...);
> )
> ...>
> }
> 
> @reme exists@
> identifier virtual.rfn,virtual.free;
> expression prb.x;
> type T;
> @@
> 
> rfn(...) { ... free((T)x,...); ... }
> 
> @rem depends on reme@
> identifier virtual.rfn,virtual.free,virtual.second_free;
> expression prb.x;
> position p4,p5;
> type T,T1;
> @@
> 
> rfn(...) {
> <... when strict
> (
> free@p4((T)x,...);
> ... when != x
> second_free@p5((T1)x,...);
> |
> free@p4((T)x,...);
> )
> ...>
> }
> 
> @bad@
> identifier virtual.free;
> expression prb.x;
> position p != {prb.p2,rem.p4};
> type T;
> @@
> 
> free@p((T)x,...)
> 
> @modif depends on rem && !bad@
> expression x;
> identifier prb.pdev,virtual.alloc,virtual.free,virtual.devm_alloc;
> identifier virtual.second_free;
> expression list args;
> position prb.p1,prb.p2,prb.p3,rem.p4,rem.p5;
> type T;
> @@
> 
> (
> - free@p2(...);
> |
> - second_free@p3(...);
> |
> - free@p4(...);
> |
> - second_free@p5(...);
> |
>   x =
> - alloc@p1(
> + devm_alloc(&pdev->dev,
>     args)
> |
>   x = 
> - (T)alloc@p1(
> + (T)devm_alloc(&pdev->dev,
>     args)
> )
> 
> // ---------------------------------------------------------------------
> // transform functions where free uses the first argument
> 
> @prbx depends on arg exists@
> identifier virtual.pfn,pdev,virtual.alloc,virtual.free;
> expression x;
> expression list args;
> position p1,p2;
> @@
> 
> pfn(struct platform_device *pdev) { ... when any
> alloc@p1(x,args)
> <... when strict
>      when any
>      when forall
> free@p2(x,...)
> ...>
> }
> 
> @remxe exists@
> identifier virtual.rfn, virtual.free;
> expression prbx.x;
> @@
> 
> rfn(...) { ... free(x,...); ... }
> 
> @remx depends on remxe@
> identifier virtual.rfn, virtual.free;
> expression prbx.x;
> position p3;
> @@
> 
> rfn(...) {
> <... when strict
> free@p3(x,...)
> ...>
> }
> 
> @badx@
> identifier virtual.free;
> expression prbx.x;
> position p != {prbx.p2,remx.p3};
> @@
> 
> free@p(x,...)
> 
> @modifx depends on remx && !badx@
> expression x;
> identifier prbx.pdev,virtual.alloc,virtual.free,virtual.devm_alloc;
> expression list args;
> position prbx.p1,prbx.p2,remx.p3;
> @@
> 
> (
> - free@p2(...);
> |
> - free@p3(...);
> |
> - alloc@p1(
> + devm_alloc(&pdev->dev,
>    x,args)
> )
> 
> // ---------------------------------------------------------------------
> // transform functions where free uses the result of platform_get_resource
> 
> @prbg depends on get exists@
> identifier virtual.pfn,pdev,virtual.alloc,virtual.free;
> expression x;
> expression list args;
> position p1,p2;
> @@
> 
> pfn(struct platform_device *pdev) { ... when any
> alloc@p1(x,args)
> <... when strict
>      when any
>      when forall
> free@p2(x,...)
> ...>
> }
> 
> @remge exists@
> identifier virtual.rfn, virtual.free;
> identifier y;
> identifier pdev;
> @@
> 
> rfn(struct platform_device *pdev) { ... when any
> y = platform_get_resource(pdev, IORESOURCE_MEM, 0)
> ...
> free(y->start,...)
> ...
> }
> 
> @remg depends on remge@
> identifier virtual.rfn, virtual.free;
> identifier y;
> identifier pdev;
> position p3;
> @@
> 
> rfn(struct platform_device *pdev) {
> <... when strict
> y = platform_get_resource(pdev, IORESOURCE_MEM, 0)
> ... when strict
> free@p3(y->start,...)
> ...>
> }
> 
> @badg@
> identifier virtual.free;
> position p != {prbg.p2,remg.p3};
> @@
> 
> free@p(...)
> 
> @modifg depends on remg && !badg@
> expression x;
> identifier prbg.pdev,virtual.alloc,virtual.free,virtual.devm_alloc;
> expression list args;
> position prbg.p1,prbg.p2,remg.p3;
> @@
> 
> (
> - free@p2(...);
> |
> - free@p3(...);
> |
> - alloc@p1(
> + devm_alloc(&pdev->dev,
>    x,args)
> )
> 
> // ---------------------------------------------------------------------
> // cleanup, if the drvdata was only used to enable the free
> // probably only relevant for kmalloc/kzalloc
> 
> @dclean depends on modif || modifx || modifg@
> identifier virtual.rfn, pdev, i;
> type T;
> @@
> 
> rfn(struct platform_device *pdev) { ...
> (
> - T i = platform_get_drvdata(pdev);
> |
> - T i = dev_get_drvdata(&pdev->drv);
> |
> - T i;
>   ... when != i
> (
> - i = platform_get_drvdata(pdev);
> |
> - i = dev_get_drvdata(&pdev->drv);
> )
> )
> ... when != i
> }
> 
> @rclean depends on modif || modifx || modifg@
> identifier virtual.rfn, pdev, i;
> type T;
> @@
> 
> rfn(struct platform_device *pdev) { ...
> (
> - T i = platform_get_resource(pdev,...);
> |
> - T i;
>   ... when != i
> - i = platform_get_resource(pdev,...);
> )
> ... when != i
> }
> 
> // ---------------------------------------------------------------------
> // cleanup empty ifs, etc
> 
> @depends on modif || modifx || modifg@
> identifier virtual.pfn;
> @@
> 
> pfn(...) { <...
> - if (...) {}
> ...> }
> 
> @depends on modif || modifx || modifg@
> identifier virtual.rfn;
> @@
> 
> rfn(...) { <...
> - if (...) {}
> ...> }
> 
> @depends on modif || modifx || modifg@
> identifier virtual.pfn;
> expression ret,e;
> @@
> 
> pfn(...) { <...
> + return
> - ret =
>  e;
> - return ret;
> ...> }
> 
> @depends on modif || modifx || modifg@
> identifier virtual.rfn;
> expression ret,e;
> @@
> 
> rfn(...) { <...
> + return
> - ret =
>  e;
> - return ret;
> ...> }
> 
> // ---------------------------------------------------------------------
> 
> // this is likely to leave dead code, if l: is preceded by a return
> // because we are control-flow based, there is no way to match on that
> @r depends on drop_labels && (modif || modifx || modifg)@
> identifier l,l1,l2;
> expression e;
> statement S;
> identifier virtual.pfn;
> identifier i;
> statement S1,S2;
> @@
> 
> pfn(...) { <...
> (
> - goto l;
> + goto l2;
> ...
> -l:
> <... when != S
>      when any
> l1:
> ...>
> l2:
> (
>  (<+...i...+>);
> |
>  if (...) S1 else S2
> |
>  while (...) S1
> |
>  for (...;...;...) S1
> )
> |
> - goto l;
> + return e;
> ...
> -l:
> <... when != S
>      when any
> l1:
> ...>
> return e;
> )
> ...> }
> 
> 


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

* Re: [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions
  2012-01-21 14:00     ` Manuel Lauss
@ 2012-01-21 15:01       ` Julia Lawall
  2012-01-30  5:17         ` Florian Tobias Schandinat
  0 siblings, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2012-01-21 15:01 UTC (permalink / raw)
  To: Manuel Lauss
  Cc: Florian Tobias Schandinat, Julia Lawall, kernel-janitors,
	linux-fbdev, LKML

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

In au1100fb.c, the probe function now returns -ENODEV on failure.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
In the case of au1200fb.c, should the error-handling code under the new
call to dma_alloc_noncoherent jump to failed, like the other error handling
code?  I was not sure that there was actually anything for failed to do at
this point in the function.

Added return of -ENODEV.

  drivers/video/au1100fb.c |   32 ++++++++++++--------------------
  drivers/video/au1200fb.c |    9 +--------
  2 files changed, 13 insertions(+), 28 deletions(-)

diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
index 04e4479..3e9a773 100644
--- a/drivers/video/au1200fb.c
+++ b/drivers/video/au1200fb.c
@@ -1724,7 +1724,7 @@ static int __devinit au1200fb_drv_probe(struct platform_device *dev)
  		/* Allocate the framebuffer to the maximum screen size */
  		fbdev->fb_len = (win->w[plane].xres * win->w[plane].yres * bpp) / 8;

-		fbdev->fb_mem = dma_alloc_noncoherent(&dev->dev,
+		fbdev->fb_mem = dmam_alloc_noncoherent(&dev->dev, &dev->dev,
  				PAGE_ALIGN(fbdev->fb_len),
  				&fbdev->fb_phys, GFP_KERNEL);
  		if (!fbdev->fb_mem) {
@@ -1788,9 +1788,6 @@ static int __devinit au1200fb_drv_probe(struct platform_device *dev)

  failed:
  	/* NOTE: This only does the current plane/window that failed; others are still active */
-	if (fbdev->fb_mem)
-		dma_free_noncoherent(&dev->dev, PAGE_ALIGN(fbdev->fb_len),
-				fbdev->fb_mem, fbdev->fb_phys);
  	if (fbi) {
  		if (fbi->cmap.len != 0)
  			fb_dealloc_cmap(&fbi->cmap);
@@ -1817,10 +1814,6 @@ static int __devexit au1200fb_drv_remove(struct platform_device *dev)

  		/* Clean up all probe data */
  		unregister_framebuffer(fbi);
-		if (fbdev->fb_mem)
-			dma_free_noncoherent(&dev->dev,
-					PAGE_ALIGN(fbdev->fb_len),
-					fbdev->fb_mem, fbdev->fb_phys);
  		if (fbi->cmap.len != 0)
  			fb_dealloc_cmap(&fbi->cmap);
  		kfree(fbi->pseudo_palette);
diff --git a/drivers/video/au1100fb.c b/drivers/video/au1100fb.c
index de9da67..befcbd8 100644
--- a/drivers/video/au1100fb.c
+++ b/drivers/video/au1100fb.c
@@ -477,7 +477,8 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
  	u32 sys_clksrc;

  	/* Allocate new device private */
-	fbdev = kzalloc(sizeof(struct au1100fb_device), GFP_KERNEL);
+	fbdev = devm_kzalloc(&dev->dev, sizeof(struct au1100fb_device),
+			     GFP_KERNEL);
  	if (!fbdev) {
  		print_err("fail to allocate device private record");
  		return -ENOMEM;
@@ -498,8 +499,9 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
  	au1100fb_fix.mmio_start = regs_res->start;
  	au1100fb_fix.mmio_len = resource_size(regs_res);

-	if (!request_mem_region(au1100fb_fix.mmio_start, au1100fb_fix.mmio_len,
-				DRIVER_NAME)) {
+	if (!devm_request_mem_region(au1100fb_fix.mmio_start,
+				     au1100fb_fix.mmio_len,
+				     DRIVER_NAME)) {
  		print_err("fail to lock memory region at 0x%08lx",
  				au1100fb_fix.mmio_start);
  		return -EBUSY;
@@ -514,8 +516,9 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
  	fbdev->fb_len = fbdev->panel->xres * fbdev->panel->yres *
  		  	(fbdev->panel->bpp >> 3) * AU1100FB_NBR_VIDEO_BUFFERS;

-	fbdev->fb_mem = dma_alloc_coherent(&dev->dev, PAGE_ALIGN(fbdev->fb_len),
-					&fbdev->fb_phys, GFP_KERNEL);
+	fbdev->fb_mem = dmam_alloc_coherent(&dev->dev, &dev->dev,
+					    PAGE_ALIGN(fbdev->fb_len),
+					    &fbdev->fb_phys, GFP_KERNEL);
  	if (!fbdev->fb_mem) {
  		print_err("fail to allocate frambuffer (size: %dK))",
  			  fbdev->fb_len / 1024);
@@ -557,14 +560,14 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
  	fbdev->info.fbops = &au1100fb_ops;
  	fbdev->info.fix = au1100fb_fix;

-	if (!(fbdev->info.pseudo_palette = kzalloc(sizeof(u32) * 16, GFP_KERNEL))) {
+	fbdev->info.pseudo_palette =
+		devm_kzalloc(&dev->dev, sizeof(u32) * 16, GFP_KERNEL);
+	if (!fbdev->info.pseudo_palette)
  		return -ENOMEM;
-	}

  	if (fb_alloc_cmap(&fbdev->info.cmap, AU1100_LCD_NBR_PALETTE_ENTRIES, 0) < 0) {
  		print_err("Fail to allocate colormap (%d entries)",
  			   AU1100_LCD_NBR_PALETTE_ENTRIES);
-		kfree(fbdev->info.pseudo_palette);
  		return -EFAULT;
  	}

@@ -582,9 +585,6 @@ static int __devinit au1100fb_drv_probe(struct platform_device *dev)
  	return 0;

  failed:
-	if (fbdev->regs) {
-		release_mem_region(fbdev->regs_phys, fbdev->regs_len);
-	}
  	if (fbdev->fb_mem) {
  		dma_free_noncoherent(&dev->dev, fbdev->fb_len, fbdev->fb_mem,
  				     fbdev->fb_phys);
@@ -592,10 +592,9 @@ failed:
  	if (fbdev->info.cmap.len != 0) {
  		fb_dealloc_cmap(&fbdev->info.cmap);
  	}
-	kfree(fbdev);
  	platform_set_drvdata(dev, NULL);

-	return 0;
+	return -ENODEV;
  }

  int au1100fb_drv_remove(struct platform_device *dev)
@@ -615,14 +614,7 @@ int au1100fb_drv_remove(struct platform_device *dev)
  	/* Clean up all probe data */
  	unregister_framebuffer(&fbdev->info);

-	release_mem_region(fbdev->regs_phys, fbdev->regs_len);
-
-	dma_free_coherent(&dev->dev, PAGE_ALIGN(fbdev->fb_len), fbdev->fb_mem,
-			  fbdev->fb_phys);
-
  	fb_dealloc_cmap(&fbdev->info.cmap);
-	kfree(fbdev->info.pseudo_palette);
-	kfree((void*)fbdev);

  	return 0;
  }

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

* Re: [PATCH 4/4] drivers/video/backlight/adp5520_bl.c: use devm_ functions
  2012-01-20 21:25 ` [PATCH 4/4] drivers/video/backlight/adp5520_bl.c: " Julia Lawall
@ 2012-01-23  8:32   ` Michael Hennerich
  0 siblings, 0 replies; 12+ messages in thread
From: Michael Hennerich @ 2012-01-23  8:32 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors, Richard Purdie, Florian Tobias Schandinat,
	device-drivers-devel, linux-fbdev, linux-kernel

On 01/20/2012 10:25 PM, Julia Lawall wrote:
> From: Julia Lawall<Julia.Lawall@lip6.fr>
>
> The various devm_ functions allocate memory that is released when a driver
> detaches.  This patch uses these functions for data that is allocated in
> the probe function of a platform device and is only freed in the remove
> function.
>
> Signed-off-by: Julia Lawall<Julia.Lawall@lip6.fr>
Acked-by: Michael Hennerich <michael.hennerich@analog.com>
> ---
>   drivers/video/backlight/adp5520_bl.c |    6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/video/backlight/adp5520_bl.c b/drivers/video/backlight/adp5520_bl.c
> index 2e630bf..4911ea7 100644
> --- a/drivers/video/backlight/adp5520_bl.c
> +++ b/drivers/video/backlight/adp5520_bl.c
> @@ -289,7 +289,7 @@ static int __devinit adp5520_bl_probe(struct platform_device *pdev)
>   	struct adp5520_bl *data;
>   	int ret = 0;
>
> -	data = kzalloc(sizeof(*data), GFP_KERNEL);
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>   	if (data == NULL)
>   		return -ENOMEM;
>
> @@ -298,7 +298,6 @@ static int __devinit adp5520_bl_probe(struct platform_device *pdev)
>
>   	if (data->pdata  == NULL) {
>   		dev_err(&pdev->dev, "missing platform data\n");
> -		kfree(data);
>   		return -ENODEV;
>   	}
>
> @@ -314,7 +313,6 @@ static int __devinit adp5520_bl_probe(struct platform_device *pdev)
>   				&adp5520_bl_ops,&props);
>   	if (IS_ERR(bl)) {
>   		dev_err(&pdev->dev, "failed to register backlight\n");
> -		kfree(data);
>   		return PTR_ERR(bl);
>   	}
>
> @@ -326,7 +324,6 @@ static int __devinit adp5520_bl_probe(struct platform_device *pdev)
>   	if (ret) {
>   		dev_err(&pdev->dev, "failed to register sysfs\n");
>   		backlight_device_unregister(bl);
> -		kfree(data);
>   	}
>
>   	platform_set_drvdata(pdev, bl);
> @@ -348,7 +345,6 @@ static int __devexit adp5520_bl_remove(struct platform_device *pdev)
>   				&adp5520_bl_attr_group);
>
>   	backlight_device_unregister(bl);
> -	kfree(data);
>
>   	return 0;
>   }
>
>


-- 
Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif



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

* Re: [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions
  2012-01-21 15:01       ` Julia Lawall
@ 2012-01-30  5:17         ` Florian Tobias Schandinat
  0 siblings, 0 replies; 12+ messages in thread
From: Florian Tobias Schandinat @ 2012-01-30  5:17 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Manuel Lauss, kernel-janitors, linux-fbdev, LKML

On 01/21/2012 03:01 PM, Julia Lawall wrote:
> The various devm_ functions allocate memory that is released when a driver
> detaches.  This patch uses these functions for data that is allocated in
> the probe function of a platform device and is only freed in the remove
> function.
> 
> In au1100fb.c, the probe function now returns -ENODEV on failure.
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
> In the case of au1200fb.c, should the error-handling code under the new
> call to dma_alloc_noncoherent jump to failed, like the other error handling
> code?  I was not sure that there was actually anything for failed to do at
> this point in the function.
> 
> Added return of -ENODEV.
> 
>  drivers/video/au1100fb.c |   32 ++++++++++++--------------------
>  drivers/video/au1200fb.c |    9 +--------
>  2 files changed, 13 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
> index 04e4479..3e9a773 100644
> --- a/drivers/video/au1200fb.c
> +++ b/drivers/video/au1200fb.c
> @@ -1724,7 +1724,7 @@ static int __devinit au1200fb_drv_probe(struct
> platform_device *dev)
>          /* Allocate the framebuffer to the maximum screen size */
>          fbdev->fb_len = (win->w[plane].xres * win->w[plane].yres * bpp)
> / 8;
> 
> -        fbdev->fb_mem = dma_alloc_noncoherent(&dev->dev,
> +        fbdev->fb_mem = dmam_alloc_noncoherent(&dev->dev, &dev->dev,
>                  PAGE_ALIGN(fbdev->fb_len),
>                  &fbdev->fb_phys, GFP_KERNEL);
>          if (!fbdev->fb_mem) {
> @@ -1788,9 +1788,6 @@ static int __devinit au1200fb_drv_probe(struct
> platform_device *dev)
> 
>  failed:
>      /* NOTE: This only does the current plane/window that failed;
> others are still active */
> -    if (fbdev->fb_mem)
> -        dma_free_noncoherent(&dev->dev, PAGE_ALIGN(fbdev->fb_len),
> -                fbdev->fb_mem, fbdev->fb_phys);
>      if (fbi) {
>          if (fbi->cmap.len != 0)
>              fb_dealloc_cmap(&fbi->cmap);
> @@ -1817,10 +1814,6 @@ static int __devexit au1200fb_drv_remove(struct
> platform_device *dev)
> 
>          /* Clean up all probe data */
>          unregister_framebuffer(fbi);
> -        if (fbdev->fb_mem)
> -            dma_free_noncoherent(&dev->dev,
> -                    PAGE_ALIGN(fbdev->fb_len),
> -                    fbdev->fb_mem, fbdev->fb_phys);
>          if (fbi->cmap.len != 0)
>              fb_dealloc_cmap(&fbi->cmap);
>          kfree(fbi->pseudo_palette);
> diff --git a/drivers/video/au1100fb.c b/drivers/video/au1100fb.c
> index de9da67..befcbd8 100644
> --- a/drivers/video/au1100fb.c
> +++ b/drivers/video/au1100fb.c
> @@ -477,7 +477,8 @@ static int __devinit au1100fb_drv_probe(struct
> platform_device *dev)
>      u32 sys_clksrc;
> 
>      /* Allocate new device private */
> -    fbdev = kzalloc(sizeof(struct au1100fb_device), GFP_KERNEL);
> +    fbdev = devm_kzalloc(&dev->dev, sizeof(struct au1100fb_device),
> +                 GFP_KERNEL);
>      if (!fbdev) {
>          print_err("fail to allocate device private record");
>          return -ENOMEM;
> @@ -498,8 +499,9 @@ static int __devinit au1100fb_drv_probe(struct
> platform_device *dev)
>      au1100fb_fix.mmio_start = regs_res->start;
>      au1100fb_fix.mmio_len = resource_size(regs_res);
> 
> -    if (!request_mem_region(au1100fb_fix.mmio_start,
> au1100fb_fix.mmio_len,
> -                DRIVER_NAME)) {
> +    if (!devm_request_mem_region(au1100fb_fix.mmio_start,
> +                     au1100fb_fix.mmio_len,
> +                     DRIVER_NAME)) {
>          print_err("fail to lock memory region at 0x%08lx",
>                  au1100fb_fix.mmio_start);
>          return -EBUSY;
> @@ -514,8 +516,9 @@ static int __devinit au1100fb_drv_probe(struct
> platform_device *dev)
>      fbdev->fb_len = fbdev->panel->xres * fbdev->panel->yres *
>                (fbdev->panel->bpp >> 3) * AU1100FB_NBR_VIDEO_BUFFERS;
> 
> -    fbdev->fb_mem = dma_alloc_coherent(&dev->dev,
> PAGE_ALIGN(fbdev->fb_len),
> -                    &fbdev->fb_phys, GFP_KERNEL);
> +    fbdev->fb_mem = dmam_alloc_coherent(&dev->dev, &dev->dev,
> +                        PAGE_ALIGN(fbdev->fb_len),
> +                        &fbdev->fb_phys, GFP_KERNEL);
>      if (!fbdev->fb_mem) {
>          print_err("fail to allocate frambuffer (size: %dK))",
>                fbdev->fb_len / 1024);
> @@ -557,14 +560,14 @@ static int __devinit au1100fb_drv_probe(struct
> platform_device *dev)
>      fbdev->info.fbops = &au1100fb_ops;
>      fbdev->info.fix = au1100fb_fix;
> 
> -    if (!(fbdev->info.pseudo_palette = kzalloc(sizeof(u32) * 16,
> GFP_KERNEL))) {
> +    fbdev->info.pseudo_palette =
> +        devm_kzalloc(&dev->dev, sizeof(u32) * 16, GFP_KERNEL);
> +    if (!fbdev->info.pseudo_palette)
>          return -ENOMEM;
> -    }
> 
>      if (fb_alloc_cmap(&fbdev->info.cmap,
> AU1100_LCD_NBR_PALETTE_ENTRIES, 0) < 0) {
>          print_err("Fail to allocate colormap (%d entries)",
>                 AU1100_LCD_NBR_PALETTE_ENTRIES);
> -        kfree(fbdev->info.pseudo_palette);
>          return -EFAULT;
>      }
> 
> @@ -582,9 +585,6 @@ static int __devinit au1100fb_drv_probe(struct
> platform_device *dev)
>      return 0;
> 
>  failed:
> -    if (fbdev->regs) {
> -        release_mem_region(fbdev->regs_phys, fbdev->regs_len);
> -    }
>      if (fbdev->fb_mem) {
>          dma_free_noncoherent(&dev->dev, fbdev->fb_len, fbdev->fb_mem,
>                       fbdev->fb_phys);
> @@ -592,10 +592,9 @@ failed:
>      if (fbdev->info.cmap.len != 0) {
>          fb_dealloc_cmap(&fbdev->info.cmap);
>      }
> -    kfree(fbdev);
>      platform_set_drvdata(dev, NULL);
> 
> -    return 0;
> +    return -ENODEV;
>  }
> 
>  int au1100fb_drv_remove(struct platform_device *dev)
> @@ -615,14 +614,7 @@ int au1100fb_drv_remove(struct platform_device *dev)
>      /* Clean up all probe data */
>      unregister_framebuffer(&fbdev->info);
> 
> -    release_mem_region(fbdev->regs_phys, fbdev->regs_len);
> -
> -    dma_free_coherent(&dev->dev, PAGE_ALIGN(fbdev->fb_len), fbdev->fb_mem,
> -              fbdev->fb_phys);
> -
>      fb_dealloc_cmap(&fbdev->info.cmap);
> -    kfree(fbdev->info.pseudo_palette);
> -    kfree((void*)fbdev);
> 
>      return 0;
>  }
> 


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

end of thread, other threads:[~2012-01-30  5:17 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-20 21:25 [PATCH 0/4] use devm_ functions Julia Lawall
2012-01-20 21:25 ` [PATCH 1/4] drivers/video/backlight/wm831x_bl.c: " Julia Lawall
2012-01-21 12:33   ` Mark Brown
2012-01-20 21:25 ` [PATCH 2/4] drivers/video/au*fb.c: " Julia Lawall
2012-01-21 13:53   ` Florian Tobias Schandinat
2012-01-21 14:00     ` Manuel Lauss
2012-01-21 15:01       ` Julia Lawall
2012-01-30  5:17         ` Florian Tobias Schandinat
2012-01-20 21:25 ` [PATCH 3/4] drivers/video/backlight: " Julia Lawall
2012-01-20 21:25 ` [PATCH 4/4] drivers/video/backlight/adp5520_bl.c: " Julia Lawall
2012-01-23  8:32   ` Michael Hennerich
2012-01-21 14:02 ` [PATCH 0/4] " Florian Tobias Schandinat

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).