From: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: Manuel Lauss <manuel.lauss@googlemail.com>,
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: Mon, 30 Jan 2012 05:17:06 +0000 [thread overview]
Message-ID: <4F2627D2.8000005@gmx.de> (raw)
In-Reply-To: <alpine.DEB.2.02.1201211600580.1939@hadrien>
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;
> }
>
WARNING: multiple messages have this Message-ID (diff)
From: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: Manuel Lauss <manuel.lauss@googlemail.com>,
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: Mon, 30 Jan 2012 05:17:06 +0000 [thread overview]
Message-ID: <4F2627D2.8000005@gmx.de> (raw)
In-Reply-To: <alpine.DEB.2.02.1201211600580.1939@hadrien>
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;
> }
>
next prev parent reply other threads:[~2012-01-30 5:17 UTC|newest]
Thread overview: 24+ 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 ` 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 12:33 ` Mark Brown
2012-01-20 21:25 ` [PATCH 2/4] drivers/video/au*fb.c: " Julia Lawall
2012-01-20 21:25 ` Julia Lawall
2012-01-21 13:53 ` Florian Tobias Schandinat
2012-01-21 13:53 ` Florian Tobias Schandinat
2012-01-21 14:00 ` Manuel Lauss
2012-01-21 14:00 ` Manuel Lauss
2012-01-21 14:00 ` Manuel Lauss
2012-01-21 15:01 ` Julia Lawall
2012-01-21 15:01 ` Julia Lawall
2012-01-30 5:17 ` Florian Tobias Schandinat [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 ` Julia Lawall
2012-01-20 21:25 ` [PATCH 4/4] drivers/video/backlight/adp5520_bl.c: " Julia Lawall
2012-01-20 21:25 ` Julia Lawall
2012-01-23 8:32 ` Michael Hennerich
2012-01-23 8:32 ` Michael Hennerich
2012-01-21 14:02 ` [PATCH 0/4] " Florian Tobias Schandinat
2012-01-21 14:02 ` 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=4F2627D2.8000005@gmx.de \
--to=florianschandinat@gmx.de \
--cc=julia.lawall@lip6.fr \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.