use devm_ functions

Started by Julia Lawall on Jan 20, 2012 · 12 messages from 5 authors · Last reply Jan 30, 2012

Series

4 patches, 12 files

Subsystems (6)

Show 3 more

Maintainers (7)

Show 3 more
STALE5401d

open the first message on its own page

  1. [PATCH 1/4] drivers/video/backlight/wm831x_bl.c: use devm_ functions
    Related patches
  2. [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions
    Related patches
  3. [PATCH 3/4] drivers/video/backlight: use devm_ functions
    Related patches
  4. [PATCH 4/4] drivers/video/backlight/adp5520_bl.c: use devm_ functions
    Related patches

Julia Lawall

Message details and related content
Open message details and related content
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;

405 more lines

[PATCH 1/4] drivers/video/backlight/wm831x_bl.c: use devm_ functions

Julia Lawall replied to Julia Lawall

Message details and related content
Open message details and related content
From: Julia Lawall <redacted>

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 <redacted>

---
 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;
 }

[PATCH 2/4] drivers/video/au*fb.c: use devm_ functions

Julia Lawall replied to Julia Lawall

Message details and related content
Open message details and related content
From: Julia Lawall <redacted>

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 <redacted>

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

[PATCH 3/4] drivers/video/backlight: use devm_ functions

Julia Lawall replied to Julia Lawall

Message details and related content
Open message details and related content
From: Julia Lawall <redacted>

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 <redacted>

---
 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;

[PATCH 4/4] drivers/video/backlight/adp5520_bl.c: use devm_ functions

Julia Lawall replied to Julia Lawall

Message details and related content
Open message details and related content
From: Julia Lawall <redacted>

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 <redacted>

---
 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;
 }

Re: [PATCH 1/4] drivers/video/backlight/wm831x_bl.c: use devm_ functions

Mark Brown replied to Julia Lawall

Message details and related content
Open message details and related content
On Fri, Jan 20, 2012 at 10:25:29PM +0100, Julia Lawall wrote:
From: Julia Lawall <redacted>

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 <redacted>

Re: [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions

Florian Tobias Schandinat replied to Julia Lawall

Message details and related content
Open message details and related content
[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 <redacted>

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 <redacted>

---
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
quoted hunk
 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);

88 more quoted lines






  


Re: [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions
    

      
      Manuel Lauss      replied to Florian Tobias Schandinat      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
Hello,

On Sat, Jan 21, 2012 at 2:53 PM, Florian Tobias Schandinat
[off-list ref] wrote:
[CC'ing Manuel Lauss as he knows those drivers better than I do]

On 01/20/2012 09:25 PM, Julia Lawall wrote:
quoted
From: Julia Lawall <redacted>

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 <redacted>

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


quoted
 drivers/video/au1100fb.c |   30 +++++++++++-------------------
 drivers/video/au1200fb.c |    9 +--------
 2 files changed, 12 insertions(+), 27 deletions(-)
Acked-by: Manuel Lauss <redacted>

[...]

Thanks!
      Manuel Lauss





  

    

      
      Florian Tobias Schandinat      replied to Julia Lawall      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
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;
406 more quoted lines





  


Re: [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions
    

      
      Julia Lawall      replied to Manuel Lauss      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
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 <redacted>

---
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;
  }





  


Re: [PATCH 4/4] drivers/video/backlight/adp5520_bl.c: use devm_ functions
    

      
      Michael Hennerich      replied to Julia Lawall      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
On 01/20/2012 10:25 PM, Julia Lawall wrote:
From: Julia Lawall<redacted>

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<redacted>
Acked-by: Michael Hennerich <michael.hennerich@analog.com>
quoted hunk
---
  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);
  	}
11 more quoted lines

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







  


Re: [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions
    

      
      Florian Tobias Schandinat      replied to Julia Lawall      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
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 <redacted>
Applied.


Thanks,

Florian Tobias Schandinat

quoted hunk
---
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);
114 more quoted lines










    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread / next series-diff position

          
kprevious message in thread / previous series-diff position

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help