lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Mon, 30 Jan 2012 05:17:06 +0000
From:	Florian Tobias Schandinat <FlorianSchandinat@....de>
To:	Julia Lawall <julia.lawall@...6.fr>
CC:	Manuel Lauss <manuel.lauss@...glemail.com>,
	kernel-janitors@...r.kernel.org, linux-fbdev@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/4] drivers/video/au*fb.c: use devm_ functions

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@...6.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;
>  }
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ