Linux Framebuffer Layer development
 help / color / mirror / Atom feed
From: Julia Lawall <julia.lawall@lip6.fr>
To: Manuel Lauss <manuel.lauss@googlemail.com>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>,
	Julia Lawall <Julia.Lawall@lip6.fr>,
	kernel-janitors@vger.kernel.org, linux-fbdev@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions
Date: Sat, 21 Jan 2012 15:01:58 +0000	[thread overview]
Message-ID: <alpine.DEB.2.02.1201211600580.1939@hadrien> (raw)
In-Reply-To: <CAOLZvyGujvpnCFxe7x82Y91kRRNM1-P165jMXXWryFqY3g-RXw@mail.gmail.com>

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

  reply	other threads:[~2012-01-21 15:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.02.1201211600580.1939@hadrien \
    --to=julia.lawall@lip6.fr \
    --cc=FlorianSchandinat@gmx.de \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manuel.lauss@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox