From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751533Ab2A3FRT (ORCPT ); Mon, 30 Jan 2012 00:17:19 -0500 Received: from mailout-de.gmx.net ([213.165.64.23]:45520 "HELO mailout-de.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751142Ab2A3FRR (ORCPT ); Mon, 30 Jan 2012 00:17:17 -0500 X-Authenticated: #10250065 X-Provags-ID: V01U2FsdGVkX1+v0M44a+avGIypgtOPfssEQlzE85P4cTDlF/tYYo 188byYqLASLX1p Message-ID: <4F2627D2.8000005@gmx.de> Date: Mon, 30 Jan 2012 05:17:06 +0000 From: Florian Tobias Schandinat User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.16) Gecko/20111110 Icedove/3.0.11 MIME-Version: 1.0 To: Julia Lawall CC: Manuel Lauss , kernel-janitors@vger.kernel.org, linux-fbdev@vger.kernel.org, LKML Subject: Re: [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions References: <1327094732-9050-1-git-send-email-Julia.Lawall@lip6.fr> <1327094732-9050-3-git-send-email-Julia.Lawall@lip6.fr> <4F1AC33E.90405@gmx.de> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Y-GMX-Trusted: 0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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; > } >