[PATCH 0/5] use devm_ functions

[PATCH 0/5] use devm_ functions

[Damien Cassou]

These patches introduce devm_ functions in some video drivers.

[PATCH 5/5] drivers/video/ep93xx-fb.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

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: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/ep93xx-fb.c |   17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/video/ep93xx-fb.c b/drivers/video/ep93xx-fb.c
index 345d962..69c89f2 100644
--- a/drivers/video/ep93xx-fb.c
+++ b/drivers/video/ep93xx-fb.c
@@ -529,7 +529,8 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
 	 * any of the framebuffer registers.
 	 */
 	fbi->res = res;
-	fbi->mmio_base = ioremap(res->start, resource_size(res));
+	fbi->mmio_base = devm_ioremap(&pdev->dev, res->start,
+				      resource_size(res));
 	if (!fbi->mmio_base) {
 		err = -ENXIO;
 		goto failed_resource;
@@ -553,20 +554,20 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
 	if (err = 0) {
 		dev_err(info->dev, "No suitable video mode found\n");
 		err = -EINVAL;
-		goto failed_mode;
+		goto failed_resource;
 	}
 
 	if (mach_info->setup) {
 		err = mach_info->setup(pdev);
 		if (err)
-			goto failed_mode;
+			goto failed_resource;
 	}
 
 	err = ep93xxfb_check_var(&info->var, info);
 	if (err)
 		goto failed_check;
 
-	fbi->clk = clk_get(info->dev, NULL);
+	fbi->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(fbi->clk)) {
 		err = PTR_ERR(fbi->clk);
 		fbi->clk = NULL;
@@ -578,19 +579,15 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
 
 	err = register_framebuffer(info);
 	if (err)
-		goto failed;
+		goto failed_check;
 
 	dev_info(info->dev, "registered. Mode = %dx%d-%d\n",
 		 info->var.xres, info->var.yres, info->var.bits_per_pixel);
 	return 0;
 
-failed:
-	clk_put(fbi->clk);
 failed_check:
 	if (fbi->mach_info->teardown)
 		fbi->mach_info->teardown(pdev);
-failed_mode:
-	iounmap(fbi->mmio_base);
 failed_resource:
 	ep93xxfb_dealloc_videomem(info);
 failed_videomem:
@@ -609,8 +606,6 @@ static int __devexit ep93xxfb_remove(struct platform_device *pdev)
 
 	unregister_framebuffer(info);
 	clk_disable(fbi->clk);
-	clk_put(fbi->clk);
-	iounmap(fbi->mmio_base);
 	ep93xxfb_dealloc_videomem(info);
 	fb_dealloc_cmap(&info->cmap);
 

Re: [PATCH 5/5] drivers/video/ep93xx-fb.c: use devm_ functions

[Florian Tobias Schandinat]

On 07/31/2012 01:54 PM, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> 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: Damien Cassou <damien.cassou@lifl.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
>  drivers/video/ep93xx-fb.c |   17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/video/ep93xx-fb.c b/drivers/video/ep93xx-fb.c
> index 345d962..69c89f2 100644
> --- a/drivers/video/ep93xx-fb.c
> +++ b/drivers/video/ep93xx-fb.c
> @@ -529,7 +529,8 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>  	 * any of the framebuffer registers.
>  	 */
>  	fbi->res = res;
> -	fbi->mmio_base = ioremap(res->start, resource_size(res));
> +	fbi->mmio_base = devm_ioremap(&pdev->dev, res->start,
> +				      resource_size(res));
>  	if (!fbi->mmio_base) {
>  		err = -ENXIO;
>  		goto failed_resource;
> @@ -553,20 +554,20 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>  	if (err = 0) {
>  		dev_err(info->dev, "No suitable video mode found\n");
>  		err = -EINVAL;
> -		goto failed_mode;
> +		goto failed_resource;
>  	}
>  
>  	if (mach_info->setup) {
>  		err = mach_info->setup(pdev);
>  		if (err)
> -			goto failed_mode;
> +			goto failed_resource;
>  	}
>  
>  	err = ep93xxfb_check_var(&info->var, info);
>  	if (err)
>  		goto failed_check;
>  
> -	fbi->clk = clk_get(info->dev, NULL);
> +	fbi->clk = devm_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(fbi->clk)) {
>  		err = PTR_ERR(fbi->clk);
>  		fbi->clk = NULL;
> @@ -578,19 +579,15 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>  
>  	err = register_framebuffer(info);
>  	if (err)
> -		goto failed;
> +		goto failed_check;
>  
>  	dev_info(info->dev, "registered. Mode = %dx%d-%d\n",
>  		 info->var.xres, info->var.yres, info->var.bits_per_pixel);
>  	return 0;
>  
> -failed:
> -	clk_put(fbi->clk);
>  failed_check:
>  	if (fbi->mach_info->teardown)
>  		fbi->mach_info->teardown(pdev);
> -failed_mode:
> -	iounmap(fbi->mmio_base);
>  failed_resource:
>  	ep93xxfb_dealloc_videomem(info);
>  failed_videomem:
> @@ -609,8 +606,6 @@ static int __devexit ep93xxfb_remove(struct platform_device *pdev)
>  
>  	unregister_framebuffer(info);
>  	clk_disable(fbi->clk);
> -	clk_put(fbi->clk);
> -	iounmap(fbi->mmio_base);
>  	ep93xxfb_dealloc_videomem(info);
>  	fb_dealloc_cmap(&info->cmap);
>  
> 
> 

[PATCH 4/5] drivers/video/da8xx-fb.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

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: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/da8xx-fb.c |   34 ++++++++--------------------------
 1 file changed, 8 insertions(+), 26 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 7ae9d53..0429431 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -947,12 +947,9 @@ static int __devexit fb_remove(struct platform_device *dev)
 				  par->p_palette_base);
 		dma_free_coherent(NULL, par->vram_size, par->vram_virt,
 				  par->vram_phys);
-		free_irq(par->irq, par);
 		clk_disable(par->lcdc_clk);
 		clk_put(par->lcdc_clk);
 		framebuffer_release(info);
-		iounmap((void __iomem *)da8xx_fb_reg_base);
-		release_mem_region(lcdc_regs->start, resource_size(lcdc_regs));
 
 	}
 	return 0;
@@ -1149,7 +1146,7 @@ static int __devinit fb_probe(struct platform_device *device)
 	struct fb_info *da8xx_fb_info;
 	struct clk *fb_clk = NULL;
 	struct da8xx_fb_par *par;
-	resource_size_t len;
+
 	int ret, i;
 	unsigned long ulcm;
 
@@ -1159,29 +1156,20 @@ static int __devinit fb_probe(struct platform_device *device)
 	}
 
 	lcdc_regs = platform_get_resource(device, IORESOURCE_MEM, 0);
-	if (!lcdc_regs) {
-		dev_err(&device->dev,
-			"Can not get memory resource for LCD controller\n");
-		return -ENOENT;
-	}
-
-	len = resource_size(lcdc_regs);
 
-	lcdc_regs = request_mem_region(lcdc_regs->start, len, lcdc_regs->name);
-	if (!lcdc_regs)
-		return -EBUSY;
-
-	da8xx_fb_reg_base = (resource_size_t)ioremap(lcdc_regs->start, len);
+	da8xx_fb_reg_base +		(resource_size_t) devm_request_and_ioremap(&device->dev,
+							   lcdc_regs);
 	if (!da8xx_fb_reg_base) {
 		ret = -EBUSY;
-		goto err_request_mem;
+		return ret;
 	}
 
 	fb_clk = clk_get(&device->dev, NULL);
 	if (IS_ERR(fb_clk)) {
 		dev_err(&device->dev, "Can not get device clock\n");
 		ret = -ENODEV;
-		goto err_ioremap;
+		return ret;
 	}
 	ret = clk_enable(fb_clk);
 	if (ret)
@@ -1359,8 +1347,8 @@ static int __devinit fb_probe(struct platform_device *device)
 	else
 		lcdc_irq_handler = lcdc_irq_handler_rev02;
 
-	ret = request_irq(par->irq, lcdc_irq_handler, 0,
-			DRIVER_NAME, par);
+	ret = devm_request_irq(&device->dev, par->irq, lcdc_irq_handler, 0,
+			       DRIVER_NAME, par);
 	if (ret)
 		goto irq_freq;
 	return 0;
@@ -1391,12 +1379,6 @@ err_clk_disable:
 err_clk_put:
 	clk_put(fb_clk);
 
-err_ioremap:
-	iounmap((void __iomem *)da8xx_fb_reg_base);
-
-err_request_mem:
-	release_mem_region(lcdc_regs->start, len);
-
 	return ret;
 }
 

Re: [PATCH 4/5] drivers/video/da8xx-fb.c: use devm_ functions

[Sachin Kamat]

On 31 July 2012 19:24, Damien Cassou <damien.cassou@lifl.fr> wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
>
> 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: Damien Cassou <damien.cassou@lifl.fr>
>
> ---
>  drivers/video/da8xx-fb.c |   34 ++++++++--------------------------
>  1 file changed, 8 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 7ae9d53..0429431 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -947,12 +947,9 @@ static int __devexit fb_remove(struct platform_device *dev)
>                                   par->p_palette_base);
>                 dma_free_coherent(NULL, par->vram_size, par->vram_virt,
>                                   par->vram_phys);
> -               free_irq(par->irq, par);
>                 clk_disable(par->lcdc_clk);
>                 clk_put(par->lcdc_clk);
>                 framebuffer_release(info);
> -               iounmap((void __iomem *)da8xx_fb_reg_base);
> -               release_mem_region(lcdc_regs->start, resource_size(lcdc_regs));
>
>         }
>         return 0;
> @@ -1149,7 +1146,7 @@ static int __devinit fb_probe(struct platform_device *device)
>         struct fb_info *da8xx_fb_info;
>         struct clk *fb_clk = NULL;
>         struct da8xx_fb_par *par;
> -       resource_size_t len;
> +
>         int ret, i;
>         unsigned long ulcm;
>
> @@ -1159,29 +1156,20 @@ static int __devinit fb_probe(struct platform_device *device)
>         }
>
>         lcdc_regs = platform_get_resource(device, IORESOURCE_MEM, 0);
> -       if (!lcdc_regs) {
> -               dev_err(&device->dev,
> -                       "Can not get memory resource for LCD controller\n");
> -               return -ENOENT;
> -       }
> -
> -       len = resource_size(lcdc_regs);
>
> -       lcdc_regs = request_mem_region(lcdc_regs->start, len, lcdc_regs->name);
> -       if (!lcdc_regs)
> -               return -EBUSY;
> -
> -       da8xx_fb_reg_base = (resource_size_t)ioremap(lcdc_regs->start, len);
> +       da8xx_fb_reg_base > +               (resource_size_t) devm_request_and_ioremap(&device->dev,
> +                                                          lcdc_regs);
>         if (!da8xx_fb_reg_base) {
>                 ret = -EBUSY;
> -               goto err_request_mem;
> +               return ret;

You can directly do
                  return -EBUSY;
and remove the braces.

>         }
>
>         fb_clk = clk_get(&device->dev, NULL);
>         if (IS_ERR(fb_clk)) {
>                 dev_err(&device->dev, "Can not get device clock\n");
>                 ret = -ENODEV;
> -               goto err_ioremap;
> +               return ret;

ditto

>         }
>         ret = clk_enable(fb_clk);
>         if (ret)
> @@ -1359,8 +1347,8 @@ static int __devinit fb_probe(struct platform_device *device)
>         else
>                 lcdc_irq_handler = lcdc_irq_handler_rev02;
>
> -       ret = request_irq(par->irq, lcdc_irq_handler, 0,
> -                       DRIVER_NAME, par);
> +       ret = devm_request_irq(&device->dev, par->irq, lcdc_irq_handler, 0,
> +                              DRIVER_NAME, par);
>         if (ret)
>                 goto irq_freq;
>         return 0;
> @@ -1391,12 +1379,6 @@ err_clk_disable:
>  err_clk_put:
>         clk_put(fb_clk);
>
> -err_ioremap:
> -       iounmap((void __iomem *)da8xx_fb_reg_base);
> -
> -err_request_mem:
> -       release_mem_region(lcdc_regs->start, len);
> -
>         return ret;
>  }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
With warm regards,
Sachin

Re: [PATCH 4/5] drivers/video/da8xx-fb.c: use devm_ functions

[Damien Cassou]

The whole patch is incorrect because of:

On Tue, Jul 31, 2012 at 5:47 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> -       ret = request_irq(par->irq, lcdc_irq_handler, 0,
>> -                       DRIVER_NAME, par);
>> +       ret = devm_request_irq(&device->dev, par->irq, lcdc_irq_handler, 0,
>> +                              DRIVER_NAME, par);

The following message explains why this is problematic:
http://marc.info/?l=kernel-janitors&m\x134374464625591&w=2

Please ignore this whole patch altogether.

Sorry about that.

-- 
Damien Cassou
http://damiencassou.seasidehosting.st

"Lambdas are relegated to relative obscurity until Java makes them
popular by not having them." James Iry

[PATCH 3/5] drivers/video/cobalt_lcdfb.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

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: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/cobalt_lcdfb.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/cobalt_lcdfb.c b/drivers/video/cobalt_lcdfb.c
index eae46f6..01a4ee7 100644
--- a/drivers/video/cobalt_lcdfb.c
+++ b/drivers/video/cobalt_lcdfb.c
@@ -348,7 +348,8 @@ static int __devinit cobalt_lcdfb_probe(struct platform_device *dev)
 	}
 
 	info->screen_size = resource_size(res);
-	info->screen_base = ioremap(res->start, info->screen_size);
+	info->screen_base = devm_ioremap(&dev->dev, res->start,
+					 info->screen_size);
 	info->fbops = &cobalt_lcd_fbops;
 	info->fix = cobalt_lcdfb_fix;
 	info->fix.smem_start = res->start;
@@ -359,7 +360,6 @@ static int __devinit cobalt_lcdfb_probe(struct platform_device *dev)
 
 	retval = register_framebuffer(info);
 	if (retval < 0) {
-		iounmap(info->screen_base);
 		framebuffer_release(info);
 		return retval;
 	}
@@ -380,7 +380,6 @@ static int __devexit cobalt_lcdfb_remove(struct platform_device *dev)
 
 	info = platform_get_drvdata(dev);
 	if (info) {
-		iounmap(info->screen_base);
 		unregister_framebuffer(info);
 		framebuffer_release(info);
 	}

Re: [PATCH 3/5] drivers/video/cobalt_lcdfb.c: use devm_ functions

[Florian Tobias Schandinat]

On 07/31/2012 01:54 PM, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> 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: Damien Cassou <damien.cassou@lifl.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
>  drivers/video/cobalt_lcdfb.c |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/cobalt_lcdfb.c b/drivers/video/cobalt_lcdfb.c
> index eae46f6..01a4ee7 100644
> --- a/drivers/video/cobalt_lcdfb.c
> +++ b/drivers/video/cobalt_lcdfb.c
> @@ -348,7 +348,8 @@ static int __devinit cobalt_lcdfb_probe(struct platform_device *dev)
>  	}
>  
>  	info->screen_size = resource_size(res);
> -	info->screen_base = ioremap(res->start, info->screen_size);
> +	info->screen_base = devm_ioremap(&dev->dev, res->start,
> +					 info->screen_size);
>  	info->fbops = &cobalt_lcd_fbops;
>  	info->fix = cobalt_lcdfb_fix;
>  	info->fix.smem_start = res->start;
> @@ -359,7 +360,6 @@ static int __devinit cobalt_lcdfb_probe(struct platform_device *dev)
>  
>  	retval = register_framebuffer(info);
>  	if (retval < 0) {
> -		iounmap(info->screen_base);
>  		framebuffer_release(info);
>  		return retval;
>  	}
> @@ -380,7 +380,6 @@ static int __devexit cobalt_lcdfb_remove(struct platform_device *dev)
>  
>  	info = platform_get_drvdata(dev);
>  	if (info) {
> -		iounmap(info->screen_base);
>  		unregister_framebuffer(info);
>  		framebuffer_release(info);
>  	}
> 
> 

[PATCH 2/5] drivers/video/bfin-t350mcqb-fb.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

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: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/bfin-t350mcqb-fb.c |   28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/video/bfin-t350mcqb-fb.c b/drivers/video/bfin-t350mcqb-fb.c
index 7a0c05f..43e0d1b 100644
--- a/drivers/video/bfin-t350mcqb-fb.c
+++ b/drivers/video/bfin-t350mcqb-fb.c
@@ -490,9 +490,10 @@ static int __devinit bfin_t350mcqb_probe(struct platform_device *pdev)
 	fbinfo->fbops = &bfin_t350mcqb_fb_ops;
 	fbinfo->flags = FBINFO_FLAG_DEFAULT;
 
-	info->fb_buffer = dma_alloc_coherent(NULL, fbinfo->fix.smem_len +
-				ACTIVE_VIDEO_MEM_OFFSET,
-				&info->dma_handle, GFP_KERNEL);
+	info->fb_buffer = dmam_alloc_coherent(&pdev->dev, NULL,
+					      fbinfo->fix.smem_len +
+					      ACTIVE_VIDEO_MEM_OFFSET,
+					      &info->dma_handle, GFP_KERNEL);
 
 	if (NULL = info->fb_buffer) {
 		printk(KERN_ERR DRIVER_NAME
@@ -514,7 +515,7 @@ static int __devinit bfin_t350mcqb_probe(struct platform_device *pdev)
 		       "Fail to allocate colormap (%d entries)\n",
 		       BFIN_LCD_NBR_PALETTE_ENTRIES);
 		ret = -EFAULT;
-		goto out4;
+		goto out3;
 	}
 
 	if (bfin_t350mcqb_request_ports(1)) {
@@ -529,8 +530,8 @@ static int __devinit bfin_t350mcqb_probe(struct platform_device *pdev)
 		goto out7;
 	}
 
-	ret = request_irq(info->irq, bfin_t350mcqb_irq_error, 0,
-			"PPI ERROR", info);
+	ret = devm_request_irq(&pdev->dev, info->irq, bfin_t350mcqb_irq_error,
+			       0, "PPI ERROR", info);
 	if (ret < 0) {
 		printk(KERN_ERR DRIVER_NAME
 		       ": unable to request PPI ERROR IRQ\n");
@@ -541,7 +542,7 @@ static int __devinit bfin_t350mcqb_probe(struct platform_device *pdev)
 		printk(KERN_ERR DRIVER_NAME
 		       ": unable to register framebuffer.\n");
 		ret = -EINVAL;
-		goto out8;
+		goto out7;
 	}
 #ifndef NO_BL_SUPPORT
 	memset(&props, 0, sizeof(struct backlight_properties));
@@ -554,7 +555,7 @@ static int __devinit bfin_t350mcqb_probe(struct platform_device *pdev)
 			": unable to register backlight.\n");
 		ret = -EINVAL;
 		unregister_framebuffer(fbinfo);
-		goto out8;
+		goto out7;
 	}
 
 	lcd_dev = lcd_device_register(DRIVER_NAME, NULL, &bfin_lcd_ops);
@@ -563,15 +564,10 @@ static int __devinit bfin_t350mcqb_probe(struct platform_device *pdev)
 
 	return 0;
 
-out8:
-	free_irq(info->irq, info);
 out7:
 	bfin_t350mcqb_request_ports(0);
 out6:
 	fb_dealloc_cmap(&fbinfo->cmap);
-out4:
-	dma_free_coherent(NULL, fbinfo->fix.smem_len + ACTIVE_VIDEO_MEM_OFFSET,
-			 info->fb_buffer, info->dma_handle);
 out3:
 	framebuffer_release(fbinfo);
 out2:
@@ -591,12 +587,6 @@ static int __devexit bfin_t350mcqb_remove(struct platform_device *pdev)
 	unregister_framebuffer(fbinfo);
 
 	free_dma(CH_PPI);
-	free_irq(info->irq, info);
-
-	if (info->fb_buffer != NULL)
-		dma_free_coherent(NULL, fbinfo->fix.smem_len +
-			ACTIVE_VIDEO_MEM_OFFSET, info->fb_buffer,
-			info->dma_handle);
 
 	fb_dealloc_cmap(&fbinfo->cmap);
 

Re: [PATCH 2/5] drivers/video/bfin-t350mcqb-fb.c: use devm_ functions

[Mike Frysinger]

On Tue, Jul 31, 2012 at 9:54 AM, Damien Cassou <damien.cassou@lifl.fr> 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.

Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike

Re: [PATCH 2/5] drivers/video/bfin-t350mcqb-fb.c: use devm_ functions

[Damien Cassou]

On Tue, Jul 31, 2012 at 3:57 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tue, Jul 31, 2012 at 9:54 AM, Damien Cassou <damien.cassou@lifl.fr> 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.
>
> Acked-by: Mike Frysinger <vapier@gentoo.org>

This whole patch is invalid, because of this part:

-	info->fb_buffer = dma_alloc_coherent(NULL, fbinfo->fix.smem_len +
-				ACTIVE_VIDEO_MEM_OFFSET,
-				&info->dma_handle, GFP_KERNEL);
+	info->fb_buffer = dmam_alloc_coherent(&pdev->dev, NULL,
+					      fbinfo->fix.smem_len +
+					      ACTIVE_VIDEO_MEM_OFFSET,
+					      &info->dma_handle, GFP_KERNEL);


dmam_alloc_coherent() is called with 5 arguments but only accepts 4.

Please ignore this whole patch altogether.

I am sorry about that.

-- 
Damien Cassou
http://damiencassou.seasidehosting.st

"Lambdas are relegated to relative obscurity until Java makes them
popular by not having them." James Iry

[PATCH 1/5] drivers/video/bf537-lq035.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

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: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/bf537-lq035.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/video/bf537-lq035.c b/drivers/video/bf537-lq035.c
index befbc80..7347aa1 100644
--- a/drivers/video/bf537-lq035.c
+++ b/drivers/video/bf537-lq035.c
@@ -760,18 +760,20 @@ static int __devinit bfin_lq035_probe(struct platform_device *pdev)
 	bfin_lq035_fb.flags = FBINFO_DEFAULT;
 
 
-	bfin_lq035_fb.pseudo_palette = kzalloc(sizeof(u32) * 16, GFP_KERNEL);
+	bfin_lq035_fb.pseudo_palette = devm_kzalloc(&pdev->dev,
+						    sizeof(u32) * 16,
+						    GFP_KERNEL);
 	if (bfin_lq035_fb.pseudo_palette = NULL) {
 		pr_err("failed to allocate pseudo_palette\n");
 		ret = -ENOMEM;
-		goto out_palette;
+		goto out_table;
 	}
 
 	if (fb_alloc_cmap(&bfin_lq035_fb.cmap, NBR_PALETTE, 0) < 0) {
 		pr_err("failed to allocate colormap (%d entries)\n",
 			NBR_PALETTE);
 		ret = -EFAULT;
-		goto out_cmap;
+		goto out_table;
 	}
 
 	if (register_framebuffer(&bfin_lq035_fb) < 0) {
@@ -804,9 +806,6 @@ out_lcd:
 	unregister_framebuffer(&bfin_lq035_fb);
 out_reg:
 	fb_dealloc_cmap(&bfin_lq035_fb.cmap);
-out_cmap:
-	kfree(bfin_lq035_fb.pseudo_palette);
-out_palette:
 out_table:
 	dma_free_coherent(NULL, TOTAL_VIDEO_MEM_SIZE, fb_buffer, 0);
 	fb_buffer = NULL;
@@ -834,7 +833,6 @@ static int __devexit bfin_lq035_remove(struct platform_device *pdev)
 	free_dma(CH_PPI);
 
 
-	kfree(bfin_lq035_fb.pseudo_palette);
 	fb_dealloc_cmap(&bfin_lq035_fb.cmap);
 
 

Re: [PATCH 1/5] drivers/video/bf537-lq035.c: use devm_ functions

[Florian Tobias Schandinat]

On 07/31/2012 01:54 PM, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> 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: Damien Cassou <damien.cassou@lifl.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
>  drivers/video/bf537-lq035.c |   12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/bf537-lq035.c b/drivers/video/bf537-lq035.c
> index befbc80..7347aa1 100644
> --- a/drivers/video/bf537-lq035.c
> +++ b/drivers/video/bf537-lq035.c
> @@ -760,18 +760,20 @@ static int __devinit bfin_lq035_probe(struct platform_device *pdev)
>  	bfin_lq035_fb.flags = FBINFO_DEFAULT;
>  
>  
> -	bfin_lq035_fb.pseudo_palette = kzalloc(sizeof(u32) * 16, GFP_KERNEL);
> +	bfin_lq035_fb.pseudo_palette = devm_kzalloc(&pdev->dev,
> +						    sizeof(u32) * 16,
> +						    GFP_KERNEL);
>  	if (bfin_lq035_fb.pseudo_palette = NULL) {
>  		pr_err("failed to allocate pseudo_palette\n");
>  		ret = -ENOMEM;
> -		goto out_palette;
> +		goto out_table;
>  	}
>  
>  	if (fb_alloc_cmap(&bfin_lq035_fb.cmap, NBR_PALETTE, 0) < 0) {
>  		pr_err("failed to allocate colormap (%d entries)\n",
>  			NBR_PALETTE);
>  		ret = -EFAULT;
> -		goto out_cmap;
> +		goto out_table;
>  	}
>  
>  	if (register_framebuffer(&bfin_lq035_fb) < 0) {
> @@ -804,9 +806,6 @@ out_lcd:
>  	unregister_framebuffer(&bfin_lq035_fb);
>  out_reg:
>  	fb_dealloc_cmap(&bfin_lq035_fb.cmap);
> -out_cmap:
> -	kfree(bfin_lq035_fb.pseudo_palette);
> -out_palette:
>  out_table:
>  	dma_free_coherent(NULL, TOTAL_VIDEO_MEM_SIZE, fb_buffer, 0);
>  	fb_buffer = NULL;
> @@ -834,7 +833,6 @@ static int __devexit bfin_lq035_remove(struct platform_device *pdev)
>  	free_dma(CH_PPI);
>  
>  
> -	kfree(bfin_lq035_fb.pseudo_palette);
>  	fb_dealloc_cmap(&bfin_lq035_fb.cmap);
>  
>  
> 
> 

[PATCH 0/5] use devm_ functions

[Damien Cassou]

These patches introduce devm_ functions in some video drivers.

[PATCH 3/5] drivers/video/mbx/mbxfb.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

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: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/mbx/mbxfb.c |   22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/video/mbx/mbxfb.c b/drivers/video/mbx/mbxfb.c
index 85e4f44..9229acf 100644
--- a/drivers/video/mbx/mbxfb.c
+++ b/drivers/video/mbx/mbxfb.c
@@ -939,8 +939,9 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
 	}
 	mfbi->reg_phys_addr = mfbi->reg_res->start;
 
-	mfbi->reg_virt_addr = ioremap_nocache(mfbi->reg_phys_addr,
-					      res_size(mfbi->reg_req));
+	mfbi->reg_virt_addr = devm_ioremap_nocache(&dev->dev,
+						   mfbi->reg_phys_addr,
+						   res_size(mfbi->reg_req));
 	if (!mfbi->reg_virt_addr) {
 		dev_err(&dev->dev, "failed to ioremap Marathon registers\n");
 		ret = -EINVAL;
@@ -948,12 +949,12 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
 	}
 	virt_base_2700 = mfbi->reg_virt_addr;
 
-	mfbi->fb_virt_addr = ioremap_nocache(mfbi->fb_phys_addr,
-					     res_size(mfbi->fb_req));
+	mfbi->fb_virt_addr = devm_ioremap_nocache(&dev->dev, mfbi->fb_phys_addr,
+						  res_size(mfbi->fb_req));
 	if (!mfbi->fb_virt_addr) {
 		dev_err(&dev->dev, "failed to ioremap frame buffer\n");
 		ret = -EINVAL;
-		goto err4;
+		goto err3;
 	}
 
 	fbi->screen_base = (char __iomem *)(mfbi->fb_virt_addr + 0x60000);
@@ -971,7 +972,7 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
 	if (ret < 0) {
 		dev_err(&dev->dev, "fb_alloc_cmap failed\n");
 		ret = -EINVAL;
-		goto err5;
+		goto err3;
 	}
 
 	platform_set_drvdata(dev, fbi);
@@ -996,10 +997,6 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
 
 err6:
 	fb_dealloc_cmap(&fbi->cmap);
-err5:
-	iounmap(mfbi->fb_virt_addr);
-err4:
-	iounmap(mfbi->reg_virt_addr);
 err3:
 	release_mem_region(mfbi->reg_res->start, res_size(mfbi->reg_res));
 err2:
@@ -1026,10 +1023,7 @@ static int __devexit mbxfb_remove(struct platform_device *dev)
 			if (mfbi->platform_remove)
 				mfbi->platform_remove(fbi);
 
-			if (mfbi->fb_virt_addr)
-				iounmap(mfbi->fb_virt_addr);
-			if (mfbi->reg_virt_addr)
-				iounmap(mfbi->reg_virt_addr);
+
 			if (mfbi->reg_req)
 				release_mem_region(mfbi->reg_req->start,
 						   res_size(mfbi->reg_req));

Re: [PATCH 3/5] drivers/video/mbx/mbxfb.c: use devm_ functions

[Florian Tobias Schandinat]

On 07/31/2012 04:39 PM, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> 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: Damien Cassou <damien.cassou@lifl.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
>  drivers/video/mbx/mbxfb.c |   22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/video/mbx/mbxfb.c b/drivers/video/mbx/mbxfb.c
> index 85e4f44..9229acf 100644
> --- a/drivers/video/mbx/mbxfb.c
> +++ b/drivers/video/mbx/mbxfb.c
> @@ -939,8 +939,9 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
>  	}
>  	mfbi->reg_phys_addr = mfbi->reg_res->start;
>  
> -	mfbi->reg_virt_addr = ioremap_nocache(mfbi->reg_phys_addr,
> -					      res_size(mfbi->reg_req));
> +	mfbi->reg_virt_addr = devm_ioremap_nocache(&dev->dev,
> +						   mfbi->reg_phys_addr,
> +						   res_size(mfbi->reg_req));
>  	if (!mfbi->reg_virt_addr) {
>  		dev_err(&dev->dev, "failed to ioremap Marathon registers\n");
>  		ret = -EINVAL;
> @@ -948,12 +949,12 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
>  	}
>  	virt_base_2700 = mfbi->reg_virt_addr;
>  
> -	mfbi->fb_virt_addr = ioremap_nocache(mfbi->fb_phys_addr,
> -					     res_size(mfbi->fb_req));
> +	mfbi->fb_virt_addr = devm_ioremap_nocache(&dev->dev, mfbi->fb_phys_addr,
> +						  res_size(mfbi->fb_req));
>  	if (!mfbi->fb_virt_addr) {
>  		dev_err(&dev->dev, "failed to ioremap frame buffer\n");
>  		ret = -EINVAL;
> -		goto err4;
> +		goto err3;
>  	}
>  
>  	fbi->screen_base = (char __iomem *)(mfbi->fb_virt_addr + 0x60000);
> @@ -971,7 +972,7 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
>  	if (ret < 0) {
>  		dev_err(&dev->dev, "fb_alloc_cmap failed\n");
>  		ret = -EINVAL;
> -		goto err5;
> +		goto err3;
>  	}
>  
>  	platform_set_drvdata(dev, fbi);
> @@ -996,10 +997,6 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
>  
>  err6:
>  	fb_dealloc_cmap(&fbi->cmap);
> -err5:
> -	iounmap(mfbi->fb_virt_addr);
> -err4:
> -	iounmap(mfbi->reg_virt_addr);
>  err3:
>  	release_mem_region(mfbi->reg_res->start, res_size(mfbi->reg_res));
>  err2:
> @@ -1026,10 +1023,7 @@ static int __devexit mbxfb_remove(struct platform_device *dev)
>  			if (mfbi->platform_remove)
>  				mfbi->platform_remove(fbi);
>  
> -			if (mfbi->fb_virt_addr)
> -				iounmap(mfbi->fb_virt_addr);
> -			if (mfbi->reg_virt_addr)
> -				iounmap(mfbi->reg_virt_addr);
> +
>  			if (mfbi->reg_req)
>  				release_mem_region(mfbi->reg_req->start,
>  						   res_size(mfbi->reg_req));
> 
> 

[PATCH 2/5] drivers/video/gbefb.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

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: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/gbefb.c |   15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/video/gbefb.c b/drivers/video/gbefb.c
index 7e7b7a9..9b79d38 100644
--- a/drivers/video/gbefb.c
+++ b/drivers/video/gbefb.c
@@ -1156,7 +1156,8 @@ static int __devinit gbefb_probe(struct platform_device *p_dev)
 		goto out_release_framebuffer;
 	}
 
-	gbe = (struct sgi_gbe *) ioremap(GBE_BASE, sizeof(struct sgi_gbe));
+	gbe = (struct sgi_gbe *) devm_ioremap(&p_dev->dev, GBE_BASE,
+					      sizeof(struct sgi_gbe));
 	if (!gbe) {
 		printk(KERN_ERR "gbefb: couldn't map mmio region\n");
 		ret = -ENXIO;
@@ -1170,12 +1171,13 @@ static int __devinit gbefb_probe(struct platform_device *p_dev)
 	if (!gbe_tiles.cpu) {
 		printk(KERN_ERR "gbefb: couldn't allocate tiles table\n");
 		ret = -ENOMEM;
-		goto out_unmap;
+		goto out_release_mem_region;
 	}
 
 	if (gbe_mem_phys) {
 		/* memory was allocated at boot time */
-		gbe_mem = ioremap_nocache(gbe_mem_phys, gbe_mem_size);
+		gbe_mem = devm_ioremap_nocache(&p_dev->dev, gbe_mem_phys,
+					       gbe_mem_size);
 		if (!gbe_mem) {
 			printk(KERN_ERR "gbefb: couldn't map framebuffer\n");
 			ret = -ENOMEM;
@@ -1241,13 +1243,9 @@ static int __devinit gbefb_probe(struct platform_device *p_dev)
 out_gbe_unmap:
 	if (gbe_dma_addr)
 		dma_free_coherent(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
-	else
-		iounmap(gbe_mem);
 out_tiles_free:
 	dma_free_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
 			  (void *)gbe_tiles.cpu, gbe_tiles.dma);
-out_unmap:
-	iounmap(gbe);
 out_release_mem_region:
 	release_mem_region(GBE_BASE, sizeof(struct sgi_gbe));
 out_release_framebuffer:
@@ -1264,12 +1262,9 @@ static int __devexit gbefb_remove(struct platform_device* p_dev)
 	gbe_turn_off();
 	if (gbe_dma_addr)
 		dma_free_coherent(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
-	else
-		iounmap(gbe_mem);
 	dma_free_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
 			  (void *)gbe_tiles.cpu, gbe_tiles.dma);
 	release_mem_region(GBE_BASE, sizeof(struct sgi_gbe));
-	iounmap(gbe);
 	gbefb_remove_sysfs(&p_dev->dev);
 	framebuffer_release(info);
 

Re: [PATCH 2/5] drivers/video/gbefb.c: use devm_ functions

[Florian Tobias Schandinat]

On 07/31/2012 04:39 PM, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> 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: Damien Cassou <damien.cassou@lifl.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
>  drivers/video/gbefb.c |   15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/video/gbefb.c b/drivers/video/gbefb.c
> index 7e7b7a9..9b79d38 100644
> --- a/drivers/video/gbefb.c
> +++ b/drivers/video/gbefb.c
> @@ -1156,7 +1156,8 @@ static int __devinit gbefb_probe(struct platform_device *p_dev)
>  		goto out_release_framebuffer;
>  	}
>  
> -	gbe = (struct sgi_gbe *) ioremap(GBE_BASE, sizeof(struct sgi_gbe));
> +	gbe = (struct sgi_gbe *) devm_ioremap(&p_dev->dev, GBE_BASE,
> +					      sizeof(struct sgi_gbe));
>  	if (!gbe) {
>  		printk(KERN_ERR "gbefb: couldn't map mmio region\n");
>  		ret = -ENXIO;
> @@ -1170,12 +1171,13 @@ static int __devinit gbefb_probe(struct platform_device *p_dev)
>  	if (!gbe_tiles.cpu) {
>  		printk(KERN_ERR "gbefb: couldn't allocate tiles table\n");
>  		ret = -ENOMEM;
> -		goto out_unmap;
> +		goto out_release_mem_region;
>  	}
>  
>  	if (gbe_mem_phys) {
>  		/* memory was allocated at boot time */
> -		gbe_mem = ioremap_nocache(gbe_mem_phys, gbe_mem_size);
> +		gbe_mem = devm_ioremap_nocache(&p_dev->dev, gbe_mem_phys,
> +					       gbe_mem_size);
>  		if (!gbe_mem) {
>  			printk(KERN_ERR "gbefb: couldn't map framebuffer\n");
>  			ret = -ENOMEM;
> @@ -1241,13 +1243,9 @@ static int __devinit gbefb_probe(struct platform_device *p_dev)
>  out_gbe_unmap:
>  	if (gbe_dma_addr)
>  		dma_free_coherent(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
> -	else
> -		iounmap(gbe_mem);
>  out_tiles_free:
>  	dma_free_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
>  			  (void *)gbe_tiles.cpu, gbe_tiles.dma);
> -out_unmap:
> -	iounmap(gbe);
>  out_release_mem_region:
>  	release_mem_region(GBE_BASE, sizeof(struct sgi_gbe));
>  out_release_framebuffer:
> @@ -1264,12 +1262,9 @@ static int __devexit gbefb_remove(struct platform_device* p_dev)
>  	gbe_turn_off();
>  	if (gbe_dma_addr)
>  		dma_free_coherent(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
> -	else
> -		iounmap(gbe_mem);
>  	dma_free_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
>  			  (void *)gbe_tiles.cpu, gbe_tiles.dma);
>  	release_mem_region(GBE_BASE, sizeof(struct sgi_gbe));
> -	iounmap(gbe);
>  	gbefb_remove_sysfs(&p_dev->dev);
>  	framebuffer_release(info);
>  
> 
> 

Re: [PATCH 2/5] drivers/video/gbefb.c: use devm_ functions

[Geert Uytterhoeven]

On Thu, Aug 23, 2012 at 10:37 PM, Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> wrote:
> On 07/31/2012 04:39 PM, Damien Cassou wrote:
>> From: Damien Cassou <damien.cassou@lifl.fr>
>>
>> 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: Damien Cassou <damien.cassou@lifl.fr>
>
> Applied.

>> diff --git a/drivers/video/gbefb.c b/drivers/video/gbefb.c
>> index 7e7b7a9..9b79d38 100644
>> --- a/drivers/video/gbefb.c
>> +++ b/drivers/video/gbefb.c
>> @@ -1156,7 +1156,8 @@ static int __devinit gbefb_probe(struct platform_device *p_dev)
>>               goto out_release_framebuffer;
>>       }
>>
>> -     gbe = (struct sgi_gbe *) ioremap(GBE_BASE, sizeof(struct sgi_gbe));
>> +     gbe = (struct sgi_gbe *) devm_ioremap(&p_dev->dev, GBE_BASE,
>> +                                           sizeof(struct sgi_gbe));

drivers/video/gbefb.c:1159:16: error: implicit declaration of function
'devm_ioremap' [-Werror=implicit-function-declaration]
drivers/video/gbefb.c:1179:3: error: implicit declaration of function
'devm_ioremap_nocache' [-Werror=implicit-function-declaration]

http://kisskb.ellerman.id.au/kisskb/buildresult/7187731/

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/5] drivers/video/gbefb.c: use devm_ functions

[Florian Tobias Schandinat]

Hi Geert,

On 09/13/2012 07:06 PM, Geert Uytterhoeven wrote:
> On Thu, Aug 23, 2012 at 10:37 PM, Florian Tobias Schandinat
> <FlorianSchandinat@gmx.de> wrote:
>> On 07/31/2012 04:39 PM, Damien Cassou wrote:
>>> From: Damien Cassou <damien.cassou@lifl.fr>
>>>
>>> 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: Damien Cassou <damien.cassou@lifl.fr>
>>
>> Applied.
> 
>>> diff --git a/drivers/video/gbefb.c b/drivers/video/gbefb.c
>>> index 7e7b7a9..9b79d38 100644
>>> --- a/drivers/video/gbefb.c
>>> +++ b/drivers/video/gbefb.c
>>> @@ -1156,7 +1156,8 @@ static int __devinit gbefb_probe(struct platform_device *p_dev)
>>>               goto out_release_framebuffer;
>>>       }
>>>
>>> -     gbe = (struct sgi_gbe *) ioremap(GBE_BASE, sizeof(struct sgi_gbe));
>>> +     gbe = (struct sgi_gbe *) devm_ioremap(&p_dev->dev, GBE_BASE,
>>> +                                           sizeof(struct sgi_gbe));
> 
> drivers/video/gbefb.c:1159:16: error: implicit declaration of function
> 'devm_ioremap' [-Werror=implicit-function-declaration]
> drivers/video/gbefb.c:1179:3: error: implicit declaration of function
> 'devm_ioremap_nocache' [-Werror=implicit-function-declaration]

Thanks for pointing this out. I guess the solution is to replace the
include asm/io.h by linux/io.h as Axel Lin did for some other driver in
this patch series.

@Damien:
Can you please do a patch as I suggested and preferable also recheck the
other drivers you touched?
And it probably would have been a good idea to mention that you didn't
compile-test it. I do some compile tests before pushing my branch but
that is far from complete due to arch and platform specific drivers.


Best regards,

Florian Tobias Schandinat

> 
> http://kisskb.ellerman.id.au/kisskb/buildresult/7187731/
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

[PATCH] gbefb: fix compile error

[Florian Tobias Schandinat]

The patch "drivers/video/gbefb.c: use devm_ functions" caused a
compile error.

drivers/video/gbefb.c:1159:16: error: implicit declaration of function
'devm_ioremap' [-Werror=implicit-function-declaration]
drivers/video/gbefb.c:1179:3: error: implicit declaration of function
'devm_ioremap_nocache' [-Werror=implicit-function-declaration]

Fix it by including linux/io.h which defines those functions.

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: Damien Cassou <damien.cassou@lifl.fr>
---
 drivers/video/gbefb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/gbefb.c b/drivers/video/gbefb.c
index 9b79d38..8dc1f7a 100644
--- a/drivers/video/gbefb.c
+++ b/drivers/video/gbefb.c
@@ -20,6 +20,7 @@
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/io.h>
 
 #ifdef CONFIG_X86
 #include <asm/mtrr.h>
@@ -28,7 +29,6 @@
 #include <asm/addrspace.h>
 #endif
 #include <asm/byteorder.h>
-#include <asm/io.h>
 #include <asm/tlbflush.h>
 
 #include <video/gbe.h>
-- 
1.7.10.4

[PATCH 1/5] drivers/video/fsl-diu-fb.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

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: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/fsl-diu-fb.c |   11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 458c006..19194c5 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1501,8 +1501,8 @@ static int __devinit fsl_diu_probe(struct platform_device *pdev)
 	unsigned int i;
 	int ret;

-	data = dma_alloc_coherent(&pdev->dev, sizeof(struct fsl_diu_data),
-				  &dma_addr, GFP_DMA | __GFP_ZERO);
+	data = dmam_alloc_coherent(&pdev->dev, sizeof(struct fsl_diu_data),
+				   &dma_addr, GFP_DMA | __GFP_ZERO);
 	if (!data)
 		return -ENOMEM;
 	data->dma_addr = dma_addr;
@@ -1628,9 +1628,6 @@ error:

 	iounmap(data->diu_reg);

-	dma_free_coherent(&pdev->dev, sizeof(struct fsl_diu_data), data,
-			  data->dma_addr);
-
 	return ret;
 }

@@ -1648,9 +1645,6 @@ static int fsl_diu_remove(struct platform_device *pdev)

 	iounmap(data->diu_reg);

-	dma_free_coherent(&pdev->dev, sizeof(struct fsl_diu_data), data,
-			  data->dma_addr);
-
 	return 0;
 }

Re: [PATCH 1/5] drivers/video/fsl-diu-fb.c: use devm_ functions

[Florian Tobias Schandinat]

On 07/31/2012 04:39 PM, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> 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: Damien Cassou <damien.cassou@lifl.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
>  drivers/video/fsl-diu-fb.c |   11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
> index 458c006..19194c5 100644
> --- a/drivers/video/fsl-diu-fb.c
> +++ b/drivers/video/fsl-diu-fb.c
> @@ -1501,8 +1501,8 @@ static int __devinit fsl_diu_probe(struct platform_device *pdev)
>  	unsigned int i;
>  	int ret;
> 
> -	data = dma_alloc_coherent(&pdev->dev, sizeof(struct fsl_diu_data),
> -				  &dma_addr, GFP_DMA | __GFP_ZERO);
> +	data = dmam_alloc_coherent(&pdev->dev, sizeof(struct fsl_diu_data),
> +				   &dma_addr, GFP_DMA | __GFP_ZERO);
>  	if (!data)
>  		return -ENOMEM;
>  	data->dma_addr = dma_addr;
> @@ -1628,9 +1628,6 @@ error:
> 
>  	iounmap(data->diu_reg);
> 
> -	dma_free_coherent(&pdev->dev, sizeof(struct fsl_diu_data), data,
> -			  data->dma_addr);
> -
>  	return ret;
>  }
> 
> @@ -1648,9 +1645,6 @@ static int fsl_diu_remove(struct platform_device *pdev)
> 
>  	iounmap(data->diu_reg);
> 
> -	dma_free_coherent(&pdev->dev, sizeof(struct fsl_diu_data), data,
> -			  data->dma_addr);
> -
>  	return 0;
>  }
> 

[PATCH 4/5] drivers/video/exynos/exynos_mipi_dsi.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

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: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/exynos/exynos_mipi_dsi.c |   41 ++++++++++++---------------------
 1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
index 4bc2b8a..c277a07 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -336,7 +336,8 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 	struct mipi_dsim_ddi *dsim_ddi;
 	int ret = -EINVAL;
 
-	dsim = kzalloc(sizeof(struct mipi_dsim_device), GFP_KERNEL);
+	dsim = devm_kzalloc(&pdev->dev, sizeof(struct mipi_dsim_device),
+			    GFP_KERNEL);
 	if (!dsim) {
 		dev_err(&pdev->dev, "failed to allocate dsim object.\n");
 		return -ENOMEM;
@@ -350,13 +351,13 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 	dsim_pd = (struct mipi_dsim_platform_data *)dsim->pd;
 	if (dsim_pd = NULL) {
 		dev_err(&pdev->dev, "failed to get platform data for dsim.\n");
-		goto err_clock_get;
+		goto err_platform_get_irq;
 	}
 	/* get mipi_dsim_config. */
 	dsim_config = dsim_pd->dsim_config;
 	if (dsim_config = NULL) {
 		dev_err(&pdev->dev, "failed to get dsim config data.\n");
-		goto err_clock_get;
+		goto err_platform_get_irq;
 	}
 
 	dsim->dsim_config = dsim_config;
@@ -367,13 +368,13 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 	ret = regulator_bulk_get(&pdev->dev, ARRAY_SIZE(supplies), supplies);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to get regulators: %d\n", ret);
-		goto err_clock_get;
+		goto err_platform_get_irq;
 	}
 
-	dsim->clock = clk_get(&pdev->dev, "dsim0");
+	dsim->clock = devm_clk_get(&pdev->dev, "dsim0");
 	if (IS_ERR(dsim->clock)) {
 		dev_err(&pdev->dev, "failed to get dsim clock source\n");
-		goto err_clock_get;
+		goto err_platform_get_irq;
 	}
 
 	clk_enable(dsim->clock);
@@ -384,15 +385,17 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 		goto err_platform_get;
 	}
 
-	dsim->res = request_mem_region(res->start, resource_size(res),
-					dev_name(&pdev->dev));
+	dsim->res = devm_request_mem_region(&pdev->dev, res->start,
+					    resource_size(res),
+					    dev_name(&pdev->dev));
 	if (!dsim->res) {
 		dev_err(&pdev->dev, "failed to request io memory region\n");
 		ret = -ENOMEM;
-		goto err_mem_region;
+		goto err_platform_get;
 	}
 
-	dsim->reg_base = ioremap(res->start, resource_size(res));
+	dsim->reg_base = devm_ioremap(&pdev->dev, res->start,
+				      resource_size(res));
 	if (!dsim->reg_base) {
 		dev_err(&pdev->dev, "failed to remap io region\n");
 		ret = -ENOMEM;
@@ -405,7 +408,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 	dsim_ddi = exynos_mipi_dsi_bind_lcd_ddi(dsim, dsim_pd->lcd_panel_name);
 	if (!dsim_ddi) {
 		dev_err(&pdev->dev, "mipi_dsim_ddi object not found.\n");
-		goto err_bind;
+		goto err_ioremap;
 	}
 
 	dsim->irq = platform_get_irq(pdev, 0);
@@ -424,7 +427,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 	if (ret != 0) {
 		dev_err(&pdev->dev, "failed to request dsim irq\n");
 		ret = -EINVAL;
-		goto err_bind;
+		goto err_ioremap;
 	}
 
 	/* enable interrupts */
@@ -466,20 +469,11 @@ done:
 
 	return 0;
 
-err_bind:
-	iounmap(dsim->reg_base);
-
 err_ioremap:
 	release_mem_region(dsim->res->start, resource_size(dsim->res));
 
-err_mem_region:
-	release_resource(dsim->res);
-
 err_platform_get:
 	clk_disable(dsim->clock);
-	clk_put(dsim->clock);
-err_clock_get:
-	kfree(dsim);
 
 err_platform_get_irq:
 	return ret;
@@ -491,12 +485,8 @@ static int __devexit exynos_mipi_dsi_remove(struct platform_device *pdev)
 	struct mipi_dsim_ddi *dsim_ddi, *next;
 	struct mipi_dsim_lcd_driver *dsim_lcd_drv;
 
-	iounmap(dsim->reg_base);
-
 	clk_disable(dsim->clock);
-	clk_put(dsim->clock);
 
-	release_resource(dsim->res);
 	release_mem_region(dsim->res->start, resource_size(dsim->res));
 
 	list_for_each_entry_safe(dsim_ddi, next, &dsim_ddi_list, list) {
@@ -514,7 +504,6 @@ static int __devexit exynos_mipi_dsi_remove(struct platform_device *pdev)
 	}
 
 	regulator_bulk_free(ARRAY_SIZE(supplies), supplies);
-	kfree(dsim);
 
 	return 0;
 }

[PATCH 5/5] drivers/video/exynos/exynos_dp_core.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

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: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/exynos/exynos_dp_core.c |   27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index c6c016a..00fe4f0 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -872,7 +872,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 
 	dp->dev = &pdev->dev;
 
-	dp->clock = clk_get(&pdev->dev, "dp");
+	dp->clock = devm_clk_get(&pdev->dev, "dp");
 	if (IS_ERR(dp->clock)) {
 		dev_err(&pdev->dev, "failed to get clock\n");
 		return PTR_ERR(dp->clock);
@@ -881,31 +881,24 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 	clk_enable(dp->clock);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "failed to get registers\n");
-		ret = -EINVAL;
-		goto err_clock;
-	}
 
 	dp->reg_base = devm_request_and_ioremap(&pdev->dev, res);
 	if (!dp->reg_base) {
 		dev_err(&pdev->dev, "failed to ioremap\n");
-		ret = -ENOMEM;
-		goto err_clock;
+		return -ENOMEM;
 	}
 
 	dp->irq = platform_get_irq(pdev, 0);
 	if (!dp->irq) {
 		dev_err(&pdev->dev, "failed to get irq\n");
-		ret = -ENODEV;
-		goto err_clock;
+		return -ENODEV;
 	}
 
 	ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
 				"exynos-dp", dp);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to request irq\n");
-		goto err_clock;
+		return ret;
 	}
 
 	dp->video_info = pdata->video_info;
@@ -917,7 +910,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 	ret = exynos_dp_detect_hpd(dp);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to detect hpd\n");
-		goto err_clock;
+		return ret;
 	}
 
 	exynos_dp_handle_edid(dp);
@@ -926,7 +919,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 				dp->video_info->link_rate);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to do link train\n");
-		goto err_clock;
+		return ret;
 	}
 
 	exynos_dp_enable_scramble(dp, 1);
@@ -940,17 +933,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 	ret = exynos_dp_config_video(dp, dp->video_info);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to config video\n");
-		goto err_clock;
+		return ret;
 	}
 
 	platform_set_drvdata(pdev, dp);
 
 	return 0;
-
-err_clock:
-	clk_put(dp->clock);
-
-	return ret;
 }
 
 static int __devexit exynos_dp_remove(struct platform_device *pdev)
@@ -962,7 +950,6 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
 		pdata->phy_exit();
 
 	clk_disable(dp->clock);
-	clk_put(dp->clock);
 
 	return 0;
 }

Re: [PATCH 5/5] drivers/video/exynos/exynos_dp_core.c: use devm_ functions

[Jingoo Han]

On Wednesday, August 01, 2012 1:39 AM Damien Cassou wrote:
> 
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> 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: Damien Cassou <damien.cassou@lifl.fr>
> 
> ---
>  drivers/video/exynos/exynos_dp_core.c |   27 +++++++--------------------
>  1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index c6c016a..00fe4f0 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -872,7 +872,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> 
>  	dp->dev = &pdev->dev;
> 
> -	dp->clock = clk_get(&pdev->dev, "dp");
> +	dp->clock = devm_clk_get(&pdev->dev, "dp");
>  	if (IS_ERR(dp->clock)) {
>  		dev_err(&pdev->dev, "failed to get clock\n");
>  		return PTR_ERR(dp->clock);
> @@ -881,31 +881,24 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>  	clk_enable(dp->clock);
> 
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res) {
> -		dev_err(&pdev->dev, "failed to get registers\n");
> -		ret = -EINVAL;
> -		goto err_clock;
> -	}

Why do you remove this return check?
If there is no reason, please, do it as follows:

  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
	if (!res) {
		dev_err(&pdev->dev, "failed to get registers\n");
-		ret = -EINVAL;
-		goto err_clock;
+		return -EINVAL;
	}


Best regards,
Jingoo Han


> 
>  	dp->reg_base = devm_request_and_ioremap(&pdev->dev, res);
>  	if (!dp->reg_base) {
>  		dev_err(&pdev->dev, "failed to ioremap\n");
> -		ret = -ENOMEM;
> -		goto err_clock;
> +		return -ENOMEM;
>  	}
> 
>  	dp->irq = platform_get_irq(pdev, 0);
>  	if (!dp->irq) {
>  		dev_err(&pdev->dev, "failed to get irq\n");
> -		ret = -ENODEV;
> -		goto err_clock;
> +		return -ENODEV;
>  	}
> 
>  	ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
>  				"exynos-dp", dp);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to request irq\n");
> -		goto err_clock;
> +		return ret;
>  	}
> 
>  	dp->video_info = pdata->video_info;
> @@ -917,7 +910,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>  	ret = exynos_dp_detect_hpd(dp);
>  	if (ret) {
>  		dev_err(&pdev->dev, "unable to detect hpd\n");
> -		goto err_clock;
> +		return ret;
>  	}
> 
>  	exynos_dp_handle_edid(dp);
> @@ -926,7 +919,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>  				dp->video_info->link_rate);
>  	if (ret) {
>  		dev_err(&pdev->dev, "unable to do link train\n");
> -		goto err_clock;
> +		return ret;
>  	}
> 
>  	exynos_dp_enable_scramble(dp, 1);
> @@ -940,17 +933,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>  	ret = exynos_dp_config_video(dp, dp->video_info);
>  	if (ret) {
>  		dev_err(&pdev->dev, "unable to config video\n");
> -		goto err_clock;
> +		return ret;
>  	}
> 
>  	platform_set_drvdata(pdev, dp);
> 
>  	return 0;
> -
> -err_clock:
> -	clk_put(dp->clock);
> -
> -	return ret;
>  }
> 
>  static int __devexit exynos_dp_remove(struct platform_device *pdev)
> @@ -962,7 +950,6 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
>  		pdata->phy_exit();
> 
>  	clk_disable(dp->clock);
> -	clk_put(dp->clock);
> 
>  	return 0;
>  }

Re: [PATCH 5/5] drivers/video/exynos/exynos_dp_core.c: use devm_ functions

[Sachin Kamat]

On 1 August 2012 04:51, Jingoo Han <jg1.han@samsung.com> wrote:
> On Wednesday, August 01, 2012 1:39 AM Damien Cassou wrote:
>>
>> From: Damien Cassou <damien.cassou@lifl.fr>
>>
>> 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: Damien Cassou <damien.cassou@lifl.fr>
>>
>> ---
>>  drivers/video/exynos/exynos_dp_core.c |   27 +++++++--------------------
>>  1 file changed, 7 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
>> index c6c016a..00fe4f0 100644
>> --- a/drivers/video/exynos/exynos_dp_core.c
>> +++ b/drivers/video/exynos/exynos_dp_core.c
>> @@ -872,7 +872,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>>
>>       dp->dev = &pdev->dev;
>>
>> -     dp->clock = clk_get(&pdev->dev, "dp");
>> +     dp->clock = devm_clk_get(&pdev->dev, "dp");
>>       if (IS_ERR(dp->clock)) {
>>               dev_err(&pdev->dev, "failed to get clock\n");
>>               return PTR_ERR(dp->clock);
>> @@ -881,31 +881,24 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>>       clk_enable(dp->clock);
>>
>>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -     if (!res) {
>> -             dev_err(&pdev->dev, "failed to get registers\n");
>> -             ret = -EINVAL;
>> -             goto err_clock;
>> -     }
>
> Why do you remove this return check?
> If there is no reason, please, do it as follows:
>
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         if (!res) {
>                 dev_err(&pdev->dev, "failed to get registers\n");
> -               ret = -EINVAL;
> -               goto err_clock;
> +               return -EINVAL;
>         }
>
>

devm_request_and_ioremap function checks the validity of res. Hence
this check above is redundant and can be removed.

Damien,
This patch only adds devm_clk_get() function. Hence you could make the
subject line more specific.




> Best regards,
> Jingoo Han
>
>
>>
>>       dp->reg_base = devm_request_and_ioremap(&pdev->dev, res);
>>       if (!dp->reg_base) {
>>               dev_err(&pdev->dev, "failed to ioremap\n");
>> -             ret = -ENOMEM;
>> -             goto err_clock;
>> +             return -ENOMEM;
>>       }
>>
>>       dp->irq = platform_get_irq(pdev, 0);
>>       if (!dp->irq) {
>>               dev_err(&pdev->dev, "failed to get irq\n");
>> -             ret = -ENODEV;
>> -             goto err_clock;
>> +             return -ENODEV;
>>       }
>>
>>       ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
>>                               "exynos-dp", dp);
>>       if (ret) {
>>               dev_err(&pdev->dev, "failed to request irq\n");
>> -             goto err_clock;
>> +             return ret;
>>       }
>>
>>       dp->video_info = pdata->video_info;
>> @@ -917,7 +910,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>>       ret = exynos_dp_detect_hpd(dp);
>>       if (ret) {
>>               dev_err(&pdev->dev, "unable to detect hpd\n");
>> -             goto err_clock;
>> +             return ret;
>>       }
>>
>>       exynos_dp_handle_edid(dp);
>> @@ -926,7 +919,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>>                               dp->video_info->link_rate);
>>       if (ret) {
>>               dev_err(&pdev->dev, "unable to do link train\n");
>> -             goto err_clock;
>> +             return ret;
>>       }
>>
>>       exynos_dp_enable_scramble(dp, 1);
>> @@ -940,17 +933,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>>       ret = exynos_dp_config_video(dp, dp->video_info);
>>       if (ret) {
>>               dev_err(&pdev->dev, "unable to config video\n");
>> -             goto err_clock;
>> +             return ret;
>>       }
>>
>>       platform_set_drvdata(pdev, dp);
>>
>>       return 0;
>> -
>> -err_clock:
>> -     clk_put(dp->clock);
>> -
>> -     return ret;
>>  }
>>
>>  static int __devexit exynos_dp_remove(struct platform_device *pdev)
>> @@ -962,7 +950,6 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
>>               pdata->phy_exit();
>>
>>       clk_disable(dp->clock);
>> -     clk_put(dp->clock);
>>
>>       return 0;
>>  }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
With warm regards,
Sachin

Re: [PATCH 5/5] drivers/video/exynos/exynos_dp_core.c: use devm_ functions

[Jingoo Han]

On Wednesday, August 01, 2012 1:00 PMSachin Kamat wrote:
> 
> On 1 August 2012 04:51, Jingoo Han <jg1.han@samsung.com> wrote:
> > On Wednesday, August 01, 2012 1:39 AM Damien Cassou wrote:
> >>
> >> From: Damien Cassou <damien.cassou@lifl.fr>
> >>
> >> 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: Damien Cassou <damien.cassou@lifl.fr>
> >>
> >> ---
> >>  drivers/video/exynos/exynos_dp_core.c |   27 +++++++--------------------
> >>  1 file changed, 7 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> >> index c6c016a..00fe4f0 100644
> >> --- a/drivers/video/exynos/exynos_dp_core.c
> >> +++ b/drivers/video/exynos/exynos_dp_core.c
> >> @@ -872,7 +872,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >>
> >>       dp->dev = &pdev->dev;
> >>
> >> -     dp->clock = clk_get(&pdev->dev, "dp");
> >> +     dp->clock = devm_clk_get(&pdev->dev, "dp");
> >>       if (IS_ERR(dp->clock)) {
> >>               dev_err(&pdev->dev, "failed to get clock\n");
> >>               return PTR_ERR(dp->clock);
> >> @@ -881,31 +881,24 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >>       clk_enable(dp->clock);
> >>
> >>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> -     if (!res) {
> >> -             dev_err(&pdev->dev, "failed to get registers\n");
> >> -             ret = -EINVAL;
> >> -             goto err_clock;
> >> -     }
> >
> > Why do you remove this return check?
> > If there is no reason, please, do it as follows:
> >
> >         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >         if (!res) {
> >                 dev_err(&pdev->dev, "failed to get registers\n");
> > -               ret = -EINVAL;
> > -               goto err_clock;
> > +               return -EINVAL;
> >         }
> >
> >
> 
> devm_request_and_ioremap function checks the validity of res. Hence
> this check above is redundant and can be removed.


I don't think so.
Even though function called next checks the NULL value,
for robustness, the return value of platform_get_resource() should be
checked.

It is possible that devm_request_and_ioremap() can be changed in the future,
as request_mem_region() & ioremap() were changed to devm_request_and_ioremap().


Best regards,
Jingoo Han


> 
> Damien,
> This patch only adds devm_clk_get() function. Hence you could make the
> subject line more specific.
> 
> 
> 
> 
> > Best regards,
> > Jingoo Han
> >
> >
> >>
> >>       dp->reg_base = devm_request_and_ioremap(&pdev->dev, res);
> >>       if (!dp->reg_base) {
> >>               dev_err(&pdev->dev, "failed to ioremap\n");
> >> -             ret = -ENOMEM;
> >> -             goto err_clock;
> >> +             return -ENOMEM;
> >>       }
> >>
> >>       dp->irq = platform_get_irq(pdev, 0);
> >>       if (!dp->irq) {
> >>               dev_err(&pdev->dev, "failed to get irq\n");
> >> -             ret = -ENODEV;
> >> -             goto err_clock;
> >> +             return -ENODEV;
> >>       }
> >>
> >>       ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
> >>                               "exynos-dp", dp);
> >>       if (ret) {
> >>               dev_err(&pdev->dev, "failed to request irq\n");
> >> -             goto err_clock;
> >> +             return ret;
> >>       }
> >>
> >>       dp->video_info = pdata->video_info;
> >> @@ -917,7 +910,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >>       ret = exynos_dp_detect_hpd(dp);
> >>       if (ret) {
> >>               dev_err(&pdev->dev, "unable to detect hpd\n");
> >> -             goto err_clock;
> >> +             return ret;
> >>       }
> >>
> >>       exynos_dp_handle_edid(dp);
> >> @@ -926,7 +919,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >>                               dp->video_info->link_rate);
> >>       if (ret) {
> >>               dev_err(&pdev->dev, "unable to do link train\n");
> >> -             goto err_clock;
> >> +             return ret;
> >>       }
> >>
> >>       exynos_dp_enable_scramble(dp, 1);
> >> @@ -940,17 +933,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >>       ret = exynos_dp_config_video(dp, dp->video_info);
> >>       if (ret) {
> >>               dev_err(&pdev->dev, "unable to config video\n");
> >> -             goto err_clock;
> >> +             return ret;
> >>       }
> >>
> >>       platform_set_drvdata(pdev, dp);
> >>
> >>       return 0;
> >> -
> >> -err_clock:
> >> -     clk_put(dp->clock);
> >> -
> >> -     return ret;
> >>  }
> >>
> >>  static int __devexit exynos_dp_remove(struct platform_device *pdev)
> >> @@ -962,7 +950,6 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
> >>               pdata->phy_exit();
> >>
> >>       clk_disable(dp->clock);
> >> -     clk_put(dp->clock);
> >>
> >>       return 0;
> >>  }
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 
> 
> 
> --
> With warm regards,
> Sachin

Re: [PATCH 5/5] drivers/video/exynos/exynos_dp_core.c: use devm_ functions

[Sachin Kamat]

On 1 August 2012 10:00, Jingoo Han <jg1.han@samsung.com> wrote:
> On Wednesday, August 01, 2012 1:00 PMSachin Kamat wrote:
>>
>> On 1 August 2012 04:51, Jingoo Han <jg1.han@samsung.com> wrote:
>> > On Wednesday, August 01, 2012 1:39 AM Damien Cassou wrote:
>> >>
>> >> From: Damien Cassou <damien.cassou@lifl.fr>
>> >>
>> >> 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: Damien Cassou <damien.cassou@lifl.fr>
>> >>
>> >> ---
>> >>  drivers/video/exynos/exynos_dp_core.c |   27 +++++++--------------------
>> >>  1 file changed, 7 insertions(+), 20 deletions(-)
>> >>
>> >> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
>> >> index c6c016a..00fe4f0 100644
>> >> --- a/drivers/video/exynos/exynos_dp_core.c
>> >> +++ b/drivers/video/exynos/exynos_dp_core.c
>> >> @@ -872,7 +872,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>> >>
>> >>       dp->dev = &pdev->dev;
>> >>
>> >> -     dp->clock = clk_get(&pdev->dev, "dp");
>> >> +     dp->clock = devm_clk_get(&pdev->dev, "dp");
>> >>       if (IS_ERR(dp->clock)) {
>> >>               dev_err(&pdev->dev, "failed to get clock\n");
>> >>               return PTR_ERR(dp->clock);
>> >> @@ -881,31 +881,24 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>> >>       clk_enable(dp->clock);
>> >>
>> >>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> >> -     if (!res) {
>> >> -             dev_err(&pdev->dev, "failed to get registers\n");
>> >> -             ret = -EINVAL;
>> >> -             goto err_clock;
>> >> -     }
>> >
>> > Why do you remove this return check?
>> > If there is no reason, please, do it as follows:
>> >
>> >         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> >         if (!res) {
>> >                 dev_err(&pdev->dev, "failed to get registers\n");
>> > -               ret = -EINVAL;
>> > -               goto err_clock;
>> > +               return -EINVAL;
>> >         }
>> >
>> >
>>
>> devm_request_and_ioremap function checks the validity of res. Hence
>> this check above is redundant and can be removed.
>
>
> I don't think so.
> Even though function called next checks the NULL value,
> for robustness, the return value of platform_get_resource() should be
> checked.
>
> It is possible that devm_request_and_ioremap() can be changed in the future,
> as request_mem_region() & ioremap() were changed to devm_request_and_ioremap().

They are not changed. They still exist.  devm_request_and_ioremap() is
an additional function provided for device managed resources.


>
>
> Best regards,
> Jingoo Han
>
>
>>
>> Damien,
>> This patch only adds devm_clk_get() function. Hence you could make the
>> subject line more specific.
>>
>>
>>
>>
>> > Best regards,
>> > Jingoo Han
>> >
>> >
>> >>
>> >>       dp->reg_base = devm_request_and_ioremap(&pdev->dev, res);
>> >>       if (!dp->reg_base) {
>> >>               dev_err(&pdev->dev, "failed to ioremap\n");
>> >> -             ret = -ENOMEM;
>> >> -             goto err_clock;
>> >> +             return -ENOMEM;
>> >>       }
>> >>
>> >>       dp->irq = platform_get_irq(pdev, 0);
>> >>       if (!dp->irq) {
>> >>               dev_err(&pdev->dev, "failed to get irq\n");
>> >> -             ret = -ENODEV;
>> >> -             goto err_clock;
>> >> +             return -ENODEV;
>> >>       }
>> >>
>> >>       ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
>> >>                               "exynos-dp", dp);
>> >>       if (ret) {
>> >>               dev_err(&pdev->dev, "failed to request irq\n");
>> >> -             goto err_clock;
>> >> +             return ret;
>> >>       }
>> >>
>> >>       dp->video_info = pdata->video_info;
>> >> @@ -917,7 +910,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>> >>       ret = exynos_dp_detect_hpd(dp);
>> >>       if (ret) {
>> >>               dev_err(&pdev->dev, "unable to detect hpd\n");
>> >> -             goto err_clock;
>> >> +             return ret;
>> >>       }
>> >>
>> >>       exynos_dp_handle_edid(dp);
>> >> @@ -926,7 +919,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>> >>                               dp->video_info->link_rate);
>> >>       if (ret) {
>> >>               dev_err(&pdev->dev, "unable to do link train\n");
>> >> -             goto err_clock;
>> >> +             return ret;
>> >>       }
>> >>
>> >>       exynos_dp_enable_scramble(dp, 1);
>> >> @@ -940,17 +933,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>> >>       ret = exynos_dp_config_video(dp, dp->video_info);
>> >>       if (ret) {
>> >>               dev_err(&pdev->dev, "unable to config video\n");
>> >> -             goto err_clock;
>> >> +             return ret;
>> >>       }
>> >>
>> >>       platform_set_drvdata(pdev, dp);
>> >>
>> >>       return 0;
>> >> -
>> >> -err_clock:
>> >> -     clk_put(dp->clock);
>> >> -
>> >> -     return ret;
>> >>  }
>> >>
>> >>  static int __devexit exynos_dp_remove(struct platform_device *pdev)
>> >> @@ -962,7 +950,6 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
>> >>               pdata->phy_exit();
>> >>
>> >>       clk_disable(dp->clock);
>> >> -     clk_put(dp->clock);
>> >>
>> >>       return 0;
>> >>  }
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> > Please read the FAQ at  http://www.tux.org/lkml/
>>
>>
>>
>> --
>> With warm regards,
>> Sachin
>



-- 
With warm regards,
Sachin

RE: [PATCH 5/5] drivers/video/exynos/exynos_dp_core.c: use devm_ functions

[Jingoo Han]

On Wednesday, August 01, 2012 1:38 PM Sachin Kamat wrote:
> 
> On 1 August 2012 10:00, Jingoo Han <jg1.han@samsung.com> wrote:
> > On Wednesday, August 01, 2012 1:00 PMSachin Kamat wrote:
> >>
> >> On 1 August 2012 04:51, Jingoo Han <jg1.han@samsung.com> wrote:
> >> > On Wednesday, August 01, 2012 1:39 AM Damien Cassou wrote:
> >> >>
> >> >> From: Damien Cassou <damien.cassou@lifl.fr>
> >> >>
> >> >> 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: Damien Cassou <damien.cassou@lifl.fr>
> >> >>
> >> >> ---
> >> >>  drivers/video/exynos/exynos_dp_core.c |   27 +++++++--------------------
> >> >>  1 file changed, 7 insertions(+), 20 deletions(-)
> >> >>
> >> >> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> >> >> index c6c016a..00fe4f0 100644
> >> >> --- a/drivers/video/exynos/exynos_dp_core.c
> >> >> +++ b/drivers/video/exynos/exynos_dp_core.c
> >> >> @@ -872,7 +872,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >> >>
> >> >>       dp->dev = &pdev->dev;
> >> >>
> >> >> -     dp->clock = clk_get(&pdev->dev, "dp");
> >> >> +     dp->clock = devm_clk_get(&pdev->dev, "dp");
> >> >>       if (IS_ERR(dp->clock)) {
> >> >>               dev_err(&pdev->dev, "failed to get clock\n");
> >> >>               return PTR_ERR(dp->clock);
> >> >> @@ -881,31 +881,24 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >> >>       clk_enable(dp->clock);
> >> >>
> >> >>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> >> -     if (!res) {
> >> >> -             dev_err(&pdev->dev, "failed to get registers\n");
> >> >> -             ret = -EINVAL;
> >> >> -             goto err_clock;
> >> >> -     }
> >> >
> >> > Why do you remove this return check?
> >> > If there is no reason, please, do it as follows:
> >> >
> >> >         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> >         if (!res) {
> >> >                 dev_err(&pdev->dev, "failed to get registers\n");
> >> > -               ret = -EINVAL;
> >> > -               goto err_clock;
> >> > +               return -EINVAL;
> >> >         }
> >> >
> >> >
> >>
> >> devm_request_and_ioremap function checks the validity of res. Hence
> >> this check above is redundant and can be removed.
> >
> >
> > I don't think so.
> > Even though function called next checks the NULL value,
> > for robustness, the return value of platform_get_resource() should be
> > checked.
> >
> > It is possible that devm_request_and_ioremap() can be changed in the future,
> > as request_mem_region() & ioremap() were changed to devm_request_and_ioremap().
> 
> They are not changed. They still exist.  devm_request_and_ioremap() is
> an additional function provided for device managed resources.


OK, I see. I accept it.
Anyway it is simpler.


> 
> 
> >
> >
> > Best regards,
> > Jingoo Han
> >
> >
> >>
> >> Damien,
> >> This patch only adds devm_clk_get() function. Hence you could make the
> >> subject line more specific.
> >>
> >>
> >>
> >>
> >> > Best regards,
> >> > Jingoo Han
> >> >
> >> >
> >> >>
> >> >>       dp->reg_base = devm_request_and_ioremap(&pdev->dev, res);
> >> >>       if (!dp->reg_base) {
> >> >>               dev_err(&pdev->dev, "failed to ioremap\n");
> >> >> -             ret = -ENOMEM;
> >> >> -             goto err_clock;
> >> >> +             return -ENOMEM;
> >> >>       }
> >> >>
> >> >>       dp->irq = platform_get_irq(pdev, 0);
> >> >>       if (!dp->irq) {
> >> >>               dev_err(&pdev->dev, "failed to get irq\n");
> >> >> -             ret = -ENODEV;
> >> >> -             goto err_clock;
> >> >> +             return -ENODEV;
> >> >>       }
> >> >>
> >> >>       ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
> >> >>                               "exynos-dp", dp);
> >> >>       if (ret) {
> >> >>               dev_err(&pdev->dev, "failed to request irq\n");
> >> >> -             goto err_clock;
> >> >> +             return ret;
> >> >>       }
> >> >>
> >> >>       dp->video_info = pdata->video_info;
> >> >> @@ -917,7 +910,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >> >>       ret = exynos_dp_detect_hpd(dp);
> >> >>       if (ret) {
> >> >>               dev_err(&pdev->dev, "unable to detect hpd\n");
> >> >> -             goto err_clock;
> >> >> +             return ret;
> >> >>       }
> >> >>
> >> >>       exynos_dp_handle_edid(dp);
> >> >> @@ -926,7 +919,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >> >>                               dp->video_info->link_rate);
> >> >>       if (ret) {
> >> >>               dev_err(&pdev->dev, "unable to do link train\n");
> >> >> -             goto err_clock;
> >> >> +             return ret;
> >> >>       }
> >> >>
> >> >>       exynos_dp_enable_scramble(dp, 1);
> >> >> @@ -940,17 +933,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >> >>       ret = exynos_dp_config_video(dp, dp->video_info);
> >> >>       if (ret) {
> >> >>               dev_err(&pdev->dev, "unable to config video\n");
> >> >> -             goto err_clock;
> >> >> +             return ret;
> >> >>       }
> >> >>
> >> >>       platform_set_drvdata(pdev, dp);
> >> >>
> >> >>       return 0;
> >> >> -
> >> >> -err_clock:
> >> >> -     clk_put(dp->clock);
> >> >> -
> >> >> -     return ret;
> >> >>  }
> >> >>
> >> >>  static int __devexit exynos_dp_remove(struct platform_device *pdev)
> >> >> @@ -962,7 +950,6 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
> >> >>               pdata->phy_exit();
> >> >>
> >> >>       clk_disable(dp->clock);
> >> >> -     clk_put(dp->clock);
> >> >>
> >> >>       return 0;
> >> >>  }
> >> >
> >> > --
> >> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >> > the body of a message to majordomo@vger.kernel.org
> >> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >> > Please read the FAQ at  http://www.tux.org/lkml/
> >>
> >>
> >>
> >> --
> >> With warm regards,
> >> Sachin
> >
> 
> 
> 
> --
> With warm regards,
> Sachin

RE: [PATCH 5/5] drivers/video/exynos/exynos_dp_core.c: use devm_ functions

[Julia Lawall]

On Wed, 1 Aug 2012, Jingoo Han wrote:

> On Wednesday, August 01, 2012 1:38 PM Sachin Kamat wrote:
>>
>> On 1 August 2012 10:00, Jingoo Han <jg1.han@samsung.com> wrote:
>>> On Wednesday, August 01, 2012 1:00 PMSachin Kamat wrote:
>>>>
>>>> On 1 August 2012 04:51, Jingoo Han <jg1.han@samsung.com> wrote:
>>>>> On Wednesday, August 01, 2012 1:39 AM Damien Cassou wrote:
>>>>>>
>>>>>> From: Damien Cassou <damien.cassou@lifl.fr>
>>>>>>
>>>>>> 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: Damien Cassou <damien.cassou@lifl.fr>
>>>>>>
>>>>>> ---
>>>>>>  drivers/video/exynos/exynos_dp_core.c |   27 +++++++--------------------
>>>>>>  1 file changed, 7 insertions(+), 20 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
>>>>>> index c6c016a..00fe4f0 100644
>>>>>> --- a/drivers/video/exynos/exynos_dp_core.c
>>>>>> +++ b/drivers/video/exynos/exynos_dp_core.c
>>>>>> @@ -872,7 +872,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>>>>>>
>>>>>>       dp->dev = &pdev->dev;
>>>>>>
>>>>>> -     dp->clock = clk_get(&pdev->dev, "dp");
>>>>>> +     dp->clock = devm_clk_get(&pdev->dev, "dp");
>>>>>>       if (IS_ERR(dp->clock)) {
>>>>>>               dev_err(&pdev->dev, "failed to get clock\n");
>>>>>>               return PTR_ERR(dp->clock);
>>>>>> @@ -881,31 +881,24 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>>>>>>       clk_enable(dp->clock);
>>>>>>
>>>>>>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>> -     if (!res) {
>>>>>> -             dev_err(&pdev->dev, "failed to get registers\n");
>>>>>> -             ret = -EINVAL;
>>>>>> -             goto err_clock;
>>>>>> -     }
>>>>>
>>>>> Why do you remove this return check?
>>>>> If there is no reason, please, do it as follows:
>>>>>
>>>>>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>         if (!res) {
>>>>>                 dev_err(&pdev->dev, "failed to get registers\n");
>>>>> -               ret = -EINVAL;
>>>>> -               goto err_clock;
>>>>> +               return -EINVAL;
>>>>>         }
>>>>>
>>>>>
>>>>
>>>> devm_request_and_ioremap function checks the validity of res. Hence
>>>> this check above is redundant and can be removed.
>>>
>>>
>>> I don't think so.
>>> Even though function called next checks the NULL value,
>>> for robustness, the return value of platform_get_resource() should be
>>> checked.
>>>
>>> It is possible that devm_request_and_ioremap() can be changed in the future,
>>> as request_mem_region() & ioremap() were changed to devm_request_and_ioremap().
>>
>> They are not changed. They still exist.  devm_request_and_ioremap() is
>> an additional function provided for device managed resources.
>
>
> OK, I see. I accept it.
> Anyway it is simpler.

This thread contains a discussion about the issue 
http://lkml.org/lkml/2012/1/28/10
Look for the comments by Wolfram Sang, who 
implemented devm_request_and_ioremap, and who suggests that the NULL test 
be removed.

I rather agree with the desire to be safe and uniform, but these 
initialization functions are really large, and with error handling code 
(although not in this case) there is always the danger of jumping to the 
wrong place, and thus making more of a mess.  It would be nice if the 
platform_get_resource could be merged with devm_request_and_ioremap, but I 
think that I looked once and there were not enough calls that were similar 
enough to make that compelling.

julia

RE: [PATCH 5/5] drivers/video/exynos/exynos_dp_core.c: use devm_ functions

[Jingoo Han]

On Wednesday, August 01, 2012 1:00 PM Sachin Kamat wrote:
> 
> On 1 August 2012 04:51, Jingoo Han <jg1.han@samsung.com> wrote:
> > On Wednesday, August 01, 2012 1:39 AM Damien Cassou wrote:
> >>
> >> From: Damien Cassou <damien.cassou@lifl.fr>
> >>
> >> 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: Damien Cassou <damien.cassou@lifl.fr>
> >>
> >> ---
> >>  drivers/video/exynos/exynos_dp_core.c |   27 +++++++--------------------
> >>  1 file changed, 7 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> >> index c6c016a..00fe4f0 100644
> >> --- a/drivers/video/exynos/exynos_dp_core.c
> >> +++ b/drivers/video/exynos/exynos_dp_core.c
> >> @@ -872,7 +872,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >>
> >>       dp->dev = &pdev->dev;
> >>
> >> -     dp->clock = clk_get(&pdev->dev, "dp");
> >> +     dp->clock = devm_clk_get(&pdev->dev, "dp");
> >>       if (IS_ERR(dp->clock)) {
> >>               dev_err(&pdev->dev, "failed to get clock\n");
> >>               return PTR_ERR(dp->clock);
> >> @@ -881,31 +881,24 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >>       clk_enable(dp->clock);
> >>
> >>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> -     if (!res) {
> >> -             dev_err(&pdev->dev, "failed to get registers\n");
> >> -             ret = -EINVAL;
> >> -             goto err_clock;
> >> -     }
> >
> > Why do you remove this return check?
> > If there is no reason, please, do it as follows:
> >
> >         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >         if (!res) {
> >                 dev_err(&pdev->dev, "failed to get registers\n");
> > -               ret = -EINVAL;
> > -               goto err_clock;
> > +               return -EINVAL;
> >         }
> >
> >
> 
> devm_request_and_ioremap function checks the validity of res. Hence
> this check above is redundant and can be removed.
> 
> Damien,
> This patch only adds devm_clk_get() function. Hence you could make the
> subject line more specific.

To Damien,
As Sachin Kamat mentioned, please change the subject more specific. For example,

    video: exynos_dp: use devm_clk_get function


Best regards,
Jingoo Han


> 
> 
> 
> 
> > Best regards,
> > Jingoo Han
> >
> >
> >>
> >>       dp->reg_base = devm_request_and_ioremap(&pdev->dev, res);
> >>       if (!dp->reg_base) {
> >>               dev_err(&pdev->dev, "failed to ioremap\n");
> >> -             ret = -ENOMEM;
> >> -             goto err_clock;
> >> +             return -ENOMEM;
> >>       }
> >>
> >>       dp->irq = platform_get_irq(pdev, 0);
> >>       if (!dp->irq) {
> >>               dev_err(&pdev->dev, "failed to get irq\n");
> >> -             ret = -ENODEV;
> >> -             goto err_clock;
> >> +             return -ENODEV;
> >>       }
> >>
> >>       ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
> >>                               "exynos-dp", dp);
> >>       if (ret) {
> >>               dev_err(&pdev->dev, "failed to request irq\n");
> >> -             goto err_clock;
> >> +             return ret;
> >>       }
> >>
> >>       dp->video_info = pdata->video_info;
> >> @@ -917,7 +910,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >>       ret = exynos_dp_detect_hpd(dp);
> >>       if (ret) {
> >>               dev_err(&pdev->dev, "unable to detect hpd\n");
> >> -             goto err_clock;
> >> +             return ret;
> >>       }
> >>
> >>       exynos_dp_handle_edid(dp);
> >> @@ -926,7 +919,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >>                               dp->video_info->link_rate);
> >>       if (ret) {
> >>               dev_err(&pdev->dev, "unable to do link train\n");
> >> -             goto err_clock;
> >> +             return ret;
> >>       }
> >>
> >>       exynos_dp_enable_scramble(dp, 1);
> >> @@ -940,17 +933,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
> >>       ret = exynos_dp_config_video(dp, dp->video_info);
> >>       if (ret) {
> >>               dev_err(&pdev->dev, "unable to config video\n");
> >> -             goto err_clock;
> >> +             return ret;
> >>       }
> >>
> >>       platform_set_drvdata(pdev, dp);
> >>
> >>       return 0;
> >> -
> >> -err_clock:
> >> -     clk_put(dp->clock);
> >> -
> >> -     return ret;
> >>  }
> >>
> >>  static int __devexit exynos_dp_remove(struct platform_device *pdev)
> >> @@ -962,7 +950,6 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
> >>               pdata->phy_exit();
> >>
> >>       clk_disable(dp->clock);
> >> -     clk_put(dp->clock);
> >>
> >>       return 0;
> >>  }
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 
> 
> 
> --
> With warm regards,
> Sachin
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] drivers/video/exynos/exynos_dp_core.c: use devm_ functions

[Damien Cassou]

On Wed, Aug 1, 2012 at 7:08 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> To Damien,
> As Sachin Kamat mentioned, please change the subject more specific. For example,
>
>     video: exynos_dp: use devm_clk_get function

ok, done in a new thread titled:

[PATCH v2] video: exynos_dp: use devm_clk_get function

Thank you for the feedback

-- 
Damien Cassou
http://damiencassou.seasidehosting.st

"Lambdas are relegated to relative obscurity until Java makes them
popular by not having them." James Iry

[PATCH 0/5] use devm_ functions

[Damien Cassou]

These patches introduce devm_ functions in some video drivers. They have been
generated using Coccinelle (http://coccinelle.lip6.fr/) and the following
semantic patch:

virtual after_start

virtual returned
virtual returnedDup
virtual arg
virtual argDup
virtual all_args
virtual get
virtual nonull

virtual report

// ---------------------------------------------------------------------
// 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 reporting = ref false

type ret      UseReturned | UseReturned2 of string | UseReturnedDup | UseReturnedNN
     | UseArg | UseArgDup | UseArgNN | UseAllArgs | 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;
   (match rule with
      UseReturned -> it#add_virtual_rule Returned
    | UseReturnedNN -> it#add_virtual_rule Returned; it#add_virtual_rule Nonull
    | UseReturnedDup -> it#add_virtual_rule ReturnedDup
    | UseReturned2(free) -> it#add_virtual_rule Returned;
      it#add_virtual_identifier Second_free free
    | UseArg -> it#add_virtual_rule Arg
    | UseArgNN -> it#add_virtual_rule Arg; it#add_virtual_rule Nonull
    | UseArgDup -> it#add_virtual_rule ArgDup
    | UseAllArgs -> it#add_virtual_rule All_args
    | UseGet -> it#add_virtual_rule Get);
   if !reporting then it#add_virtual_rule Report;
   if not (pfn="") then it#add_virtual_identifier Pfn pfn;
   if not (rfn="") then it#add_virtual_identifier Rfn rfn;
   if not (alloc="") then it#add_virtual_identifier Alloc alloc;
   if not (free="") then it#add_virtual_identifier Free free;
   if not (devm_alloc="") then it#add_virtual_identifier Devm_alloc devm_alloc;
   it#register()

@script:ocaml depends on report && !after_start@
@@
reporting := true

@script:ocaml@
pfn << plat.pfn;
rfn << plat.rfn;
p << plat.p;
@@

let file = (List.hd p).file in
add pfn rfn "kmalloc" "kfree" "devm_kzalloc" file UseReturned;
add pfn rfn "kzalloc" "kfree" "devm_kzalloc" file UseReturned;
add pfn rfn "ioremap" "iounmap" "devm_ioremap" file UseReturned;
add pfn rfn "ioremap_nocache" "iounmap" "devm_ioremap_nocache" file
   UseReturned;

add pfn rfn "clk_get" "clk_put" "devm_clk_get" file UseReturnedDup;
add pfn rfn "clk_get" "clk_put" "devm_clk_get_bad" file UseReturnedNN;

add pfn rfn "usb_get_phy" "usb_put_phy" "devm_usb_get_phy" file UseReturned;

add pfn rfn "pinctrl_get" "pinctrl_put" "devm_pinctrl_get" file UseReturned;
add pfn rfn "pinctrl_get_select_default" "pinctrl_put"
            "devm_pinctrl_get_select_default" file UseReturned;

add pfn rfn "regulator_get" "regulator_put" "devm_regulator_get"
            file UseReturned;
add pfn rfn "regulator_bulk_get" "regulator_bulk_free"
            "devm_regulator_bulk_get" file UseAllArgs;

add pfn rfn "gpio_request" "gpio_free" "devm_gpio_request" file UseArg;
add pfn rfn "gpio_request_one" "gpio_free" "devm_gpio_request_one" file UseArg;

(*
add pfn rfn "request_irq" "free_irq" "devm_request_irq" file UseArg;
add pfn rfn "request_threaded_irq" "free_irq" "devm_request_threaded_irq" file
  UseArg;
*)
add pfn rfn "dma_alloc_coherent" "dma_free_coherent" "dmam_alloc_coherent"
  file UseReturnedDup;
add pfn rfn "dma_alloc_noncoherent" "dma_free_noncoherent"
  "dmam_alloc_noncoherent" file UseReturnedDup;
add pfn rfn "dma_alloc_coherent" "dma_free_coherent" "dmam_alloc_coherent_bad"
  file UseReturnedNN;
add pfn rfn "dma_alloc_noncoherent" "dma_free_noncoherent"
  "dmam_alloc_noncoherent_bad" file UseReturnedNN;

(* several possibilities... *)
add pfn rfn "request_region" "release_region" "devm_request_region" file
  UseGet;
add pfn rfn "request_mem_region" "release_mem_region"
  "devm_request_mem_region" file UseGet;
add pfn rfn "request_region" "release_region" "devm_request_region" file
  UseArg;
add pfn rfn "request_mem_region" "release_mem_region"
  "devm_request_mem_region" file UseArg;
(* fix a bug at the same time *)
add pfn rfn "request_region" "release_resource" "devm_request_region" file
  (UseReturned2("kfree"));
add pfn rfn "request_mem_region" "release_resource"
  "devm_request_mem_region" file (UseReturned2("kfree"));
add pfn rfn "ioport_map" "ioport_unmap" "devm_ioport_map" file UseReturned

// ---------------------------------------------------------------------
// process the initial definition of the probe function

@preprobe@
identifier virtual.pfn;
position p;
@@

pfn@p(...) { ... }

@probe@
identifier pfn;
position preprobe.p;
@@

pfn@p(...) { ... }

@labelled_return@
identifier probe.pfn,l;
expression e;
@@

pfn(...) { <+... l: return e; ...+> }

// ---------------------------------------------------------------------
// transform functions where free uses the result

@prb depends on returned exists@
identifier probe.pfn,pdev,virtual.alloc,virtual.free,virtual.second_free;
expression x,y,e,a;
position p1,p2,p3;
type T1,T2,T3;
@@

pfn(struct platform_device *pdev) { ... when any
x = (T1)alloc@p1(a,...)
<... when strict
     when any
     when forall
(
y = x;
... when != y = e
    when != &y
free@p2(...,(T2)y,...);
... when != x
    when != y
second_free@p3(...,(T3)y,...);
|
y = x;
... when != y = e
    when != &y
free@p2(...,(T2)y,...);
|
free@p2(...,(T2)x,...);
... when != x
second_free@p3(...,(T3)x,...);
|
free@p2(...,(T2)x,...);
)
...>
}

@script:ocaml@
a << prb.a;
@@

if a = "NULL" then Coccilib.include_match false

@reme exists@
identifier virtual.rfn,virtual.free;
expression prb.x,prb.y;
type T;
@@

rfn(...) { ... free(...,(T)\(x\|y\),...); ... }

@rem depends on reme@
identifier virtual.rfn,virtual.free,virtual.second_free;
expression prb.x,prb.y;
position p4,p5;
type T,T1;
@@

rfn(...) {
<... when strict
(
free@p4(...,(T)\(x\|y\),...);
... when != x
second_free@p5(...,(T1)\(x\|y\),...);
|
free@p4(...,(T)\(x\|y\),...);
)
...>
}

@bad@
identifier virtual.free;
expression prb.x,prb.y;
position p != {prb.p2,rem.p4};
type T;
@@

free@p(...,(T)\(x\|y\),...)

@modif depends on rem && !bad && !report@
expression x;
identifier prb.pdev,virtual.alloc,virtual.free,virtual.devm_alloc;
identifier virtual.second_free;
expression list args;
position prb.p1,prb.p2,prb.p3,rem.p4,rem.p5;
type T;
@@

(
- free@p2(...);
|
- second_free@p3(...);
|
- free@p4(...);
|
- second_free@p5(...);
|
  x - alloc@p1(
+ devm_alloc(&pdev->dev,
    args)
|
  x - (T)alloc@p1(
+ (T)devm_alloc(&pdev->dev,
    args)
)

@script:python depends on rem && !bad && report@
p1 << prb.p1;
alloc << virtual.devm_alloc;
@@
msg = "WARNING opportunity for %s" % (alloc)
coccilib.report.print_report(p1[0], msg)

// ---------------------------------------------------------------------
// transform functions where free uses the result
// special case for clk where don't add &pdev->dev

@prbdup depends on returnedDup exists@
identifier probe.pfn,pdev,virtual.alloc,virtual.free,virtual.second_free;
expression x,y,e;
expression list args;
position p1,p2,p3;
type T1,T2,T3;
@@

pfn(struct platform_device *pdev) { ... when any
x = (T1)alloc@p1(&pdev->dev,args)
<... when strict
     when any
     when forall
(
y = x;
... when != y = e
    when != &y
free@p2(...,(T2)y,...);
... when != x
    when != y
second_free@p3(...,(T3)y,...);
|
y = x;
... when != y = e
    when != &y
free@p2(...,(T2)y,...);
|
free@p2(...,(T2)x,...);
... when != x
second_free@p3(...,(T3)x,...);
|
free@p2(...,(T2)x,...);
)
...>
}

@remdupe exists@
identifier virtual.rfn,virtual.free;
expression prbdup.x,prbdup.y;
type T;
@@

rfn(...) { ... free(...,(T)\(x\|y\),...); ... }

@remdup depends on remdupe@
identifier virtual.rfn,virtual.free,virtual.second_free;
expression prbdup.x,prbdup.y;
position p4,p5;
type T,T1;
@@

rfn(...) {
<... when strict
(
free@p4(...,(T)\(x\|y\),...);
... when != x
second_free@p5(...,(T1)\(x\|y\),...);
|
free@p4(...,(T)\(x\|y\),...);
)
...>
}

@baddup@
identifier virtual.free;
expression prbdup.x,prbdup.y;
position p != {prbdup.p2,remdup.p4};
type T;
@@

free@p(...,(T)\(x\|y\),...)

@modifdup depends on remdup && !baddup && !report@
expression x;
identifier virtual.alloc,virtual.free,virtual.devm_alloc;
identifier virtual.second_free;
expression list args;
position prbdup.p1,prbdup.p2,prbdup.p3,remdup.p4,remdup.p5;
type T;
@@

(
- free@p2(...);
|
- second_free@p3(...);
|
- free@p4(...);
|
- second_free@p5(...);
|
  x - alloc@p1
+ devm_alloc
    (args)
|
  x - (T)alloc@p1
+ (T)devm_alloc
    (args)
)

@script:python depends on remdup && !baddup && report@
p1 << prbdup.p1;
alloc << virtual.devm_alloc;
@@
msg = "WARNING opportunity for %s" % (alloc)
coccilib.report.print_report(p1[0], msg)

// ---------------------------------------------------------------------
// transform functions where free uses the first argument

@prbx depends on arg exists@
identifier probe.pfn,pdev,virtual.alloc,virtual.free;
expression x,y,e;
expression list args;
position p1,p2;
@@

pfn(struct platform_device *pdev) { ... when any
alloc@p1(x,args)
<... when strict
     when any
     when forall
(
y = x;
... when != y = e
    when != &y
free@p2(y,...);
|
free@p2(x,...)
)
...>
}

@script:ocaml@
x << prbx.x;
@@

if x = "NULL" then Coccilib.include_match false

@remxe exists@
identifier virtual.rfn, virtual.free;
expression prbx.x,prbx.y;
@@

rfn(...) { ... free(\(x\|y\),...); ... }

@remx depends on remxe@
identifier virtual.rfn, virtual.free;
expression prbx.x,prbx.y;
position p3;
@@

rfn(...) {
<... when strict
free@p3(\(x\|y\),...)
...>
}

@badx@
identifier virtual.free;
expression prbx.x,prbx.y;
position p != {prbx.p2,remx.p3};
@@

free@p(\(x\|y\),...)

@modifx depends on remx && !badx && !report@
expression x;
identifier prbx.pdev,virtual.alloc,virtual.free,virtual.devm_alloc;
expression list args;
position prbx.p1,prbx.p2,remx.p3;
@@

(
- free@p2(...);
|
- free@p3(...);
|
- alloc@p1(
+ devm_alloc(&pdev->dev,
   x,args)
)

@script:python depends on remx && !badx && report@
p1 << prbx.p1;
alloc << virtual.devm_alloc;
@@
msg = "WARNING opportunity for %s" % (alloc)
coccilib.report.print_report(p1[0], msg)

// ---------------------------------------------------------------------
// transform functions where free uses the first argument

@prbxdup depends on argDup exists@
identifier probe.pfn,pdev,virtual.alloc,virtual.free;
expression y,e;
expression list args;
position p1,p2;
@@

pfn(struct platform_device *pdev) { ... when any
alloc@p1(&pdev->dev,args)
<... when strict
     when any
     when forall
(
y = &pdev->dev;
... when != y = e
    when != &y
free@p2(y,...);
|
free@p2(&pdev->dev,...)
)
...>
}

@remxedup exists@
identifier virtual.rfn, virtual.free, prbxdup.pdev;
expression prbxdup.y;
@@

rfn(...) { ... free(\(&pdev->dev\|y\),...); ... }

@remxdup depends on remxedup@
identifier virtual.rfn, virtual.free, prbxdup.pdev;
expression prbxdup.y;
position p3;
@@

rfn(...) {
<... when strict
free@p3(\(&pdev->dev\|y\),...)
...>
}

@badxdup@
identifier virtual.free,prbxdup.pdev;
expression prbxdup.y;
position p != {prbxdup.p2,remxdup.p3};
@@

free@p(\(&pdev->dev\|y\),...)

@modifxdup depends on remxdup && !badxdup && !report@
identifier virtual.alloc,virtual.free,virtual.devm_alloc;
expression list args;
position prbxdup.p1,prbxdup.p2,remxdup.p3;
@@

(
- free@p2(...);
|
- free@p3(...);
|
- alloc@p1
+ devm_alloc
   (args)
)

@script:python depends on remxdup && !badxdup && report@
p1 << prbxdup.p1;
alloc << virtual.devm_alloc;
@@
msg = "WARNING opportunity for %s" % (alloc)
coccilib.report.print_report(p1[0], msg)

// ---------------------------------------------------------------------
// transform functions where free uses all arguments

@prbax depends on all_args exists@
identifier probe.pfn,pdev,virtual.alloc,virtual.free;
expression list x;
position p1,p2;
@@

pfn(struct platform_device *pdev) { ... when any
alloc@p1(x)
<... when strict
     when any
     when forall
free@p2(x)
...>
}

@remaxe exists@
identifier virtual.rfn, virtual.free;
expression list prbax.x;
@@

rfn(...) { ... free(x); ... }

@remax depends on remaxe@
identifier virtual.rfn, virtual.free;
expression list prbax.x;
position p3;
@@

rfn(...) {
<... when strict
free@p3(x)
...>
}

@badax@
identifier virtual.free;
expression list prbax.x;
position p != {prbax.p2,remax.p3};
@@

free@p(x)

@modifax depends on remax && !badax && !report@
identifier prbax.pdev,virtual.alloc,virtual.free,virtual.devm_alloc;
expression list x;
position prbax.p1,prbax.p2,remax.p3;
@@

(
- free@p2(...);
|
- free@p3(...);
|
- alloc@p1(
+ devm_alloc(&pdev->dev,
   x)
)

@script:python depends on remax && !badax && report@
p1 << prbax.p1;
alloc << virtual.devm_alloc;
@@
msg = "WARNING opportunity for %s" % (alloc)
coccilib.report.print_report(p1[0], msg)

// ---------------------------------------------------------------------
// transform functions where free uses the result of platform_get_resource

@prbg depends on get exists@
identifier probe.pfn,pdev,virtual.alloc,virtual.free;
expression x,y,e;
expression list args;
position p1,p2;
@@

pfn(struct platform_device *pdev) { ... when any
alloc@p1(x,args)
<... when strict
     when any
     when forall
(
y = x;
... when != y = e
    when != &y
free@p2(y,...);
|
free@p2(x,...)
)
...>
}

@remge exists@
identifier virtual.rfn, virtual.free;
identifier z;
identifier pdev;
expression e,n;
@@

rfn(struct platform_device *pdev) { ... when any
z = platform_get_resource(pdev, IORESOURCE_MEM, n)
... when != z = e
    when != &z
free(z->start,...)
...
}

@remg depends on remge@
identifier virtual.rfn, virtual.free;
identifier z;
identifier pdev;
position p3;
expression e,n;
@@

rfn(struct platform_device *pdev) {
<... when strict
z = platform_get_resource(pdev, IORESOURCE_MEM, n)
... when strict
    when != z = e
    when != &z
free@p3(z->start,...)
...>
}

@badg@
identifier virtual.free;
position p != {prbg.p2,remg.p3};
@@

free@p(...)

@modifg depends on remg && !badg && !report@
expression x;
identifier prbg.pdev,virtual.alloc,virtual.free,virtual.devm_alloc;
expression list args;
position prbg.p1,prbg.p2,remg.p3;
@@

(
- free@p2(...);
|
- free@p3(...);
|
- alloc@p1(
+ devm_alloc(&pdev->dev,
   x,args)
)

@script:python depends on remg && !badg && report@
p1 << prbg.p1;
alloc << virtual.devm_alloc;
@@
msg = "WARNING opportunity for %s" % (alloc)
coccilib.report.print_report(p1[0], msg)

// ---------------------------------------------------------------------
// cleanup, if the drvdata was only used to enable the free
// probably only relevant for kmalloc/kzalloc

@dclean depends on modif || modifdup || modifx || modifxdup || modifax || modifg@
identifier virtual.rfn, pdev, i;
type T;
@@

rfn(struct platform_device *pdev) { ...
(
- T i = platform_get_drvdata(pdev);
|
- T i = dev_get_drvdata(&pdev->drv);
|
- T i;
  ... when != i
(
- i = platform_get_drvdata(pdev);
|
- i = dev_get_drvdata(&pdev->drv);
)
)
... when != i
}

@rclean depends on modif || modifdup || modifx || modifxdup || modifax || modifg@
identifier virtual.rfn, pdev, i;
type T;
@@

rfn(struct platform_device *pdev) { ...
(
- T i = platform_get_resource(pdev,...);
|
- T i;
  ... when != i
- i = platform_get_resource(pdev,...);
)
... when != i
}

// ---------------------------------------------------------------------
// cleanup empty ifs, etc

@depends on modif || modifdup || modifx || modifxdup || modifax || modifg@
identifier probe.pfn;
@@

pfn(...) { <...
- if (...) {}
...> }

@depends on modif || modifdup || modifx || modifxdup || modifax || modifg@
identifier virtual.rfn;
@@

rfn(...) { <...
- if (...) {}
...> }

@depends on modif || modifdup || modifx || modifxdup || modifax || modifg@
identifier probe.pfn;
expression ret,e;
@@

pfn(...) { <...
+ return
- ret  e;
- return ret;
...> }

@depends on modif || modifdup || modifx || modifxdup || modifax || modifg@
identifier virtual.rfn;
expression ret,e;
@@

rfn(...) { <...
+ return
- ret  e;
- return ret;
...> }

// ---------------------------------------------------------------------

// this is likely to leave dead code, if l: is preceded by a return
// because we are control-flow based, there is no way to match on that
@depends on labelled_return && (modif || modifdup || modifx || modifxdup || modifax || modifg)@
identifier l,l1,l2;
expression e;
statement S;
identifier probe.pfn;
identifier i;
statement S1,S2;
@@

pfn(...) { <...
- goto l;
+ goto l2;
...
-l:
<... when != S
     when any
l1:
...>
l2:
(
 (<+...i...+>);
|
 if (...) S1 else S2
|
 while (...) S1
|
 for (...;...;...) S1
|
 return e;
)
...> }

@depends on !labelled_return && (modif || modifdup || modifx || modifxdup || modifax || modifg)@
identifier l,l1,l2;
expression e;
statement S;
identifier probe.pfn;
identifier i;
statement S1,S2;
@@

pfn(...) { <...
(
- goto l;
+ goto l2;
...
-l:
<... when != S
     when any
l1:
...>
l2:
(
 (<+...i...+>);
|
 if (...) S1 else S2
|
 while (...) S1
|
 for (...;...;...) S1
)
|
- goto l;
+ return e;
...
-l:
<... when != S
     when any
l1:
...>
return e;
)
...> }

@depends on !labelled_return && (modif || modifdup || modifx || modifxdup || modifax || modifg)@
expression e1,e2;
identifier probe.pfn;
@@

pfn(...) { <...
-e1 = e2;
-return e1;
+return e2;
...> }

[PATCH 1/5] drivers/video/epson1355fb.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

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: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/epson1355fb.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/video/epson1355fb.c b/drivers/video/epson1355fb.c
index 68b9b51..246da1e 100644
--- a/drivers/video/epson1355fb.c
+++ b/drivers/video/epson1355fb.c
@@ -592,12 +592,8 @@ static int epson1355fb_remove(struct platform_device *dev)

 	if (info) {
 		fb_dealloc_cmap(&info->cmap);
-		if (info->screen_base)
-			iounmap(info->screen_base);
 		framebuffer_release(info);
 	}
-	release_mem_region(EPSON1355FB_FB_PHYS, EPSON1355FB_FB_LEN);
-	release_mem_region(EPSON1355FB_REGS_PHYS, EPSON1355FB_REGS_LEN);
 	return 0;
 }

@@ -608,15 +604,18 @@ static int __devinit epson1355fb_probe(struct platform_device *dev)
 	u8 revision;
 	int rc = 0;

-	if (!request_mem_region(EPSON1355FB_REGS_PHYS, EPSON1355FB_REGS_LEN, "S1D13505 registers")) {
+	if (!devm_request_mem_region(&dev->dev, EPSON1355FB_REGS_PHYS,
+				     EPSON1355FB_REGS_LEN,
+				     "S1D13505 registers")) {
 		printk(KERN_ERR "epson1355fb: unable to reserve "
 		       "registers at 0x%0x\n", EPSON1355FB_REGS_PHYS);
 		rc = -EBUSY;
 		goto bail;
 	}

-	if (!request_mem_region(EPSON1355FB_FB_PHYS, EPSON1355FB_FB_LEN,
-				"S1D13505 framebuffer")) {
+	if (!devm_request_mem_region(&dev->dev, EPSON1355FB_FB_PHYS,
+				     EPSON1355FB_FB_LEN,
+				     "S1D13505 framebuffer")) {
 		printk(KERN_ERR "epson1355fb: unable to reserve "
 		       "framebuffer at 0x%0x\n", EPSON1355FB_FB_PHYS);
 		rc = -EBUSY;
@@ -638,7 +637,8 @@ static int __devinit epson1355fb_probe(struct platform_device *dev)
 	}
 	info->pseudo_palette = default_par->pseudo_palette;

-	info->screen_base = ioremap(EPSON1355FB_FB_PHYS, EPSON1355FB_FB_LEN);
+	info->screen_base = devm_ioremap(&dev->dev, EPSON1355FB_FB_PHYS,
+					 EPSON1355FB_FB_LEN);
 	if (!info->screen_base) {
 		printk(KERN_ERR "epson1355fb: unable to map framebuffer\n");
 		rc = -ENOMEM;

Re: [PATCH 1/5] drivers/video/epson1355fb.c: use devm_ functions

[Florian Tobias Schandinat]

On 08/03/2012 03:40 PM, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> 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: Damien Cassou <damien.cassou@lifl.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
>  drivers/video/epson1355fb.c |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/video/epson1355fb.c b/drivers/video/epson1355fb.c
> index 68b9b51..246da1e 100644
> --- a/drivers/video/epson1355fb.c
> +++ b/drivers/video/epson1355fb.c
> @@ -592,12 +592,8 @@ static int epson1355fb_remove(struct platform_device *dev)
> 
>  	if (info) {
>  		fb_dealloc_cmap(&info->cmap);
> -		if (info->screen_base)
> -			iounmap(info->screen_base);
>  		framebuffer_release(info);
>  	}
> -	release_mem_region(EPSON1355FB_FB_PHYS, EPSON1355FB_FB_LEN);
> -	release_mem_region(EPSON1355FB_REGS_PHYS, EPSON1355FB_REGS_LEN);
>  	return 0;
>  }
> 
> @@ -608,15 +604,18 @@ static int __devinit epson1355fb_probe(struct platform_device *dev)
>  	u8 revision;
>  	int rc = 0;
> 
> -	if (!request_mem_region(EPSON1355FB_REGS_PHYS, EPSON1355FB_REGS_LEN, "S1D13505 registers")) {
> +	if (!devm_request_mem_region(&dev->dev, EPSON1355FB_REGS_PHYS,
> +				     EPSON1355FB_REGS_LEN,
> +				     "S1D13505 registers")) {
>  		printk(KERN_ERR "epson1355fb: unable to reserve "
>  		       "registers at 0x%0x\n", EPSON1355FB_REGS_PHYS);
>  		rc = -EBUSY;
>  		goto bail;
>  	}
> 
> -	if (!request_mem_region(EPSON1355FB_FB_PHYS, EPSON1355FB_FB_LEN,
> -				"S1D13505 framebuffer")) {
> +	if (!devm_request_mem_region(&dev->dev, EPSON1355FB_FB_PHYS,
> +				     EPSON1355FB_FB_LEN,
> +				     "S1D13505 framebuffer")) {
>  		printk(KERN_ERR "epson1355fb: unable to reserve "
>  		       "framebuffer at 0x%0x\n", EPSON1355FB_FB_PHYS);
>  		rc = -EBUSY;
> @@ -638,7 +637,8 @@ static int __devinit epson1355fb_probe(struct platform_device *dev)
>  	}
>  	info->pseudo_palette = default_par->pseudo_palette;
> 
> -	info->screen_base = ioremap(EPSON1355FB_FB_PHYS, EPSON1355FB_FB_LEN);
> +	info->screen_base = devm_ioremap(&dev->dev, EPSON1355FB_FB_PHYS,
> +					 EPSON1355FB_FB_LEN);
>  	if (!info->screen_base) {
>  		printk(KERN_ERR "epson1355fb: unable to map framebuffer\n");
>  		rc = -ENOMEM;
> 

[PATCH 3/5] drivers/video/jz4740_fb.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

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: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/jz4740_fb.c |   22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
index de36693..7669770 100644
--- a/drivers/video/jz4740_fb.c
+++ b/drivers/video/jz4740_fb.c
@@ -659,25 +659,25 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
 	jzfb->pdata = pdata;
 	jzfb->mem = mem;

-	jzfb->ldclk = clk_get(&pdev->dev, "lcd");
+	jzfb->ldclk = devm_clk_get(&pdev->dev, "lcd");
 	if (IS_ERR(jzfb->ldclk)) {
 		ret = PTR_ERR(jzfb->ldclk);
 		dev_err(&pdev->dev, "Failed to get lcd clock: %d\n", ret);
 		goto err_framebuffer_release;
 	}

-	jzfb->lpclk = clk_get(&pdev->dev, "lcd_pclk");
+	jzfb->lpclk = devm_clk_get(&pdev->dev, "lcd_pclk");
 	if (IS_ERR(jzfb->lpclk)) {
 		ret = PTR_ERR(jzfb->lpclk);
 		dev_err(&pdev->dev, "Failed to get lcd pixel clock: %d\n", ret);
-		goto err_put_ldclk;
+		goto err_framebuffer_release;
 	}

-	jzfb->base = ioremap(mem->start, resource_size(mem));
+	jzfb->base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
 	if (!jzfb->base) {
 		dev_err(&pdev->dev, "Failed to ioremap register memory region\n");
 		ret = -EBUSY;
-		goto err_put_lpclk;
+		goto err_framebuffer_release;
 	}

 	platform_set_drvdata(pdev, jzfb);
@@ -693,7 +693,7 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
 	ret = jzfb_alloc_devmem(jzfb);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to allocate video memory\n");
-		goto err_iounmap;
+		goto err_framebuffer_release;
 	}

 	fb->fix = jzfb_fix;
@@ -734,12 +734,6 @@ err_free_devmem:

 	fb_dealloc_cmap(&fb->cmap);
 	jzfb_free_devmem(jzfb);
-err_iounmap:
-	iounmap(jzfb->base);
-err_put_lpclk:
-	clk_put(jzfb->lpclk);
-err_put_ldclk:
-	clk_put(jzfb->ldclk);
 err_framebuffer_release:
 	framebuffer_release(fb);
 err_release_mem_region:
@@ -756,7 +750,6 @@ static int __devexit jzfb_remove(struct platform_device *pdev)
 	jz_gpio_bulk_free(jz_lcd_ctrl_pins, jzfb_num_ctrl_pins(jzfb));
 	jz_gpio_bulk_free(jz_lcd_data_pins, jzfb_num_data_pins(jzfb));

-	iounmap(jzfb->base);
 	release_mem_region(jzfb->mem->start, resource_size(jzfb->mem));

 	fb_dealloc_cmap(&jzfb->fb->cmap);
@@ -764,9 +757,6 @@ static int __devexit jzfb_remove(struct platform_device *pdev)

 	platform_set_drvdata(pdev, NULL);

-	clk_put(jzfb->lpclk);
-	clk_put(jzfb->ldclk);
-
 	framebuffer_release(jzfb->fb);

 	return 0;

Re: [PATCH 3/5] drivers/video/jz4740_fb.c: use devm_ functions

[Florian Tobias Schandinat]

On 08/03/2012 03:40 PM, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> 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: Damien Cassou <damien.cassou@lifl.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
>  drivers/video/jz4740_fb.c |   22 ++++++----------------
>  1 file changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
> index de36693..7669770 100644
> --- a/drivers/video/jz4740_fb.c
> +++ b/drivers/video/jz4740_fb.c
> @@ -659,25 +659,25 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
>  	jzfb->pdata = pdata;
>  	jzfb->mem = mem;
> 
> -	jzfb->ldclk = clk_get(&pdev->dev, "lcd");
> +	jzfb->ldclk = devm_clk_get(&pdev->dev, "lcd");
>  	if (IS_ERR(jzfb->ldclk)) {
>  		ret = PTR_ERR(jzfb->ldclk);
>  		dev_err(&pdev->dev, "Failed to get lcd clock: %d\n", ret);
>  		goto err_framebuffer_release;
>  	}
> 
> -	jzfb->lpclk = clk_get(&pdev->dev, "lcd_pclk");
> +	jzfb->lpclk = devm_clk_get(&pdev->dev, "lcd_pclk");
>  	if (IS_ERR(jzfb->lpclk)) {
>  		ret = PTR_ERR(jzfb->lpclk);
>  		dev_err(&pdev->dev, "Failed to get lcd pixel clock: %d\n", ret);
> -		goto err_put_ldclk;
> +		goto err_framebuffer_release;
>  	}
> 
> -	jzfb->base = ioremap(mem->start, resource_size(mem));
> +	jzfb->base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
>  	if (!jzfb->base) {
>  		dev_err(&pdev->dev, "Failed to ioremap register memory region\n");
>  		ret = -EBUSY;
> -		goto err_put_lpclk;
> +		goto err_framebuffer_release;
>  	}
> 
>  	platform_set_drvdata(pdev, jzfb);
> @@ -693,7 +693,7 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
>  	ret = jzfb_alloc_devmem(jzfb);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to allocate video memory\n");
> -		goto err_iounmap;
> +		goto err_framebuffer_release;
>  	}
> 
>  	fb->fix = jzfb_fix;
> @@ -734,12 +734,6 @@ err_free_devmem:
> 
>  	fb_dealloc_cmap(&fb->cmap);
>  	jzfb_free_devmem(jzfb);
> -err_iounmap:
> -	iounmap(jzfb->base);
> -err_put_lpclk:
> -	clk_put(jzfb->lpclk);
> -err_put_ldclk:
> -	clk_put(jzfb->ldclk);
>  err_framebuffer_release:
>  	framebuffer_release(fb);
>  err_release_mem_region:
> @@ -756,7 +750,6 @@ static int __devexit jzfb_remove(struct platform_device *pdev)
>  	jz_gpio_bulk_free(jz_lcd_ctrl_pins, jzfb_num_ctrl_pins(jzfb));
>  	jz_gpio_bulk_free(jz_lcd_data_pins, jzfb_num_data_pins(jzfb));
> 
> -	iounmap(jzfb->base);
>  	release_mem_region(jzfb->mem->start, resource_size(jzfb->mem));
> 
>  	fb_dealloc_cmap(&jzfb->fb->cmap);
> @@ -764,9 +757,6 @@ static int __devexit jzfb_remove(struct platform_device *pdev)
> 
>  	platform_set_drvdata(pdev, NULL);
> 
> -	clk_put(jzfb->lpclk);
> -	clk_put(jzfb->ldclk);
> -
>  	framebuffer_release(jzfb->fb);
> 
>  	return 0;
> 

Re: [PATCH 3/5] drivers/video/jz4740_fb.c: use devm_ functions

[Lars-Peter Clausen]

On 08/23/2012 10:41 PM, Florian Tobias Schandinat wrote:
> On 08/03/2012 03:40 PM, Damien Cassou wrote:
>> From: Damien Cassou <damien.cassou@lifl.fr>
>>
>> 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: Damien Cassou <damien.cassou@lifl.fr>
> 
> Applied.
> 
> 
> Thanks,
> 
> Florian Tobias Schandinat
> 
>>
>> ---
>>  drivers/video/jz4740_fb.c |   22 ++++++----------------
>>  1 file changed, 6 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
>> index de36693..7669770 100644
>> --- a/drivers/video/jz4740_fb.c
>> +++ b/drivers/video/jz4740_fb.c
>> @@ -659,25 +659,25 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
>>  	jzfb->pdata = pdata;
>>  	jzfb->mem = mem;
>>
>> -	jzfb->ldclk = clk_get(&pdev->dev, "lcd");
>> +	jzfb->ldclk = devm_clk_get(&pdev->dev, "lcd");

I guess I'm a bit late, but we do not have devm_clk_get on jz4740 (yet), so
this patch breaks linking for this driver in next. I'll to to have this added
for the next release, but if I do not succeed we'll have to revert part of this
patch.

Also the driver does not include #include <linux/io.h>, since it is required
for devm_ioremap compilation is also broken. This one is easy to fix though,
will send a follow-up patch.

- Lars

[PATCH 2/5] drivers/video/bf54x-lq043fb.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

The various devm_ functions allocate memory that is released when a driver
detaches. This patch replaces the use of kzalloc by devm_kzalloc.

Signed-off-by: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/bf54x-lq043fb.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/video/bf54x-lq043fb.c b/drivers/video/bf54x-lq043fb.c
index dc2f004..47702ee 100644
--- a/drivers/video/bf54x-lq043fb.c
+++ b/drivers/video/bf54x-lq043fb.c
@@ -601,7 +601,8 @@ static int __devinit bfin_bf54x_probe(struct platform_device *pdev)

 	fbinfo->fbops = &bfin_bf54x_fb_ops;

-	fbinfo->pseudo_palette = kzalloc(sizeof(u32) * 16, GFP_KERNEL);
+	fbinfo->pseudo_palette = devm_kzalloc(&pdev->dev, sizeof(u32) * 16,
+					      GFP_KERNEL);
 	if (!fbinfo->pseudo_palette) {
 		printk(KERN_ERR DRIVER_NAME
 		       "Fail to allocate pseudo_palette\n");
@@ -616,7 +617,7 @@ static int __devinit bfin_bf54x_probe(struct platform_device *pdev)
 		       "Fail to allocate colormap (%d entries)\n",
 		       BFIN_LCD_NBR_PALETTE_ENTRIES);
 		ret = -EFAULT;
-		goto out5;
+		goto out4;
 	}

 	if (request_ports(info)) {
@@ -671,8 +672,6 @@ out7:
 	free_ports(info);
 out6:
 	fb_dealloc_cmap(&fbinfo->cmap);
-out5:
-	kfree(fbinfo->pseudo_palette);
 out4:
 	dma_free_coherent(NULL, fbinfo->fix.smem_len, info->fb_buffer,
 			  info->dma_handle);
@@ -699,7 +698,6 @@ static int __devexit bfin_bf54x_remove(struct platform_device *pdev)
 		dma_free_coherent(NULL, fbinfo->fix.smem_len, info->fb_buffer,
 				  info->dma_handle);

-	kfree(fbinfo->pseudo_palette);
 	fb_dealloc_cmap(&fbinfo->cmap);

 #ifndef NO_BL_SUPPORT

Re: [PATCH 2/5] drivers/video/bf54x-lq043fb.c: use devm_ functions

[Florian Tobias Schandinat]

On 08/03/2012 03:40 PM, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> The various devm_ functions allocate memory that is released when a driver
> detaches. This patch replaces the use of kzalloc by devm_kzalloc.
> 
> Signed-off-by: Damien Cassou <damien.cassou@lifl.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
>  drivers/video/bf54x-lq043fb.c |    8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/video/bf54x-lq043fb.c b/drivers/video/bf54x-lq043fb.c
> index dc2f004..47702ee 100644
> --- a/drivers/video/bf54x-lq043fb.c
> +++ b/drivers/video/bf54x-lq043fb.c
> @@ -601,7 +601,8 @@ static int __devinit bfin_bf54x_probe(struct platform_device *pdev)
> 
>  	fbinfo->fbops = &bfin_bf54x_fb_ops;
> 
> -	fbinfo->pseudo_palette = kzalloc(sizeof(u32) * 16, GFP_KERNEL);
> +	fbinfo->pseudo_palette = devm_kzalloc(&pdev->dev, sizeof(u32) * 16,
> +					      GFP_KERNEL);
>  	if (!fbinfo->pseudo_palette) {
>  		printk(KERN_ERR DRIVER_NAME
>  		       "Fail to allocate pseudo_palette\n");
> @@ -616,7 +617,7 @@ static int __devinit bfin_bf54x_probe(struct platform_device *pdev)
>  		       "Fail to allocate colormap (%d entries)\n",
>  		       BFIN_LCD_NBR_PALETTE_ENTRIES);
>  		ret = -EFAULT;
> -		goto out5;
> +		goto out4;
>  	}
> 
>  	if (request_ports(info)) {
> @@ -671,8 +672,6 @@ out7:
>  	free_ports(info);
>  out6:
>  	fb_dealloc_cmap(&fbinfo->cmap);
> -out5:
> -	kfree(fbinfo->pseudo_palette);
>  out4:
>  	dma_free_coherent(NULL, fbinfo->fix.smem_len, info->fb_buffer,
>  			  info->dma_handle);
> @@ -699,7 +698,6 @@ static int __devexit bfin_bf54x_remove(struct platform_device *pdev)
>  		dma_free_coherent(NULL, fbinfo->fix.smem_len, info->fb_buffer,
>  				  info->dma_handle);
> 
> -	kfree(fbinfo->pseudo_palette);
>  	fb_dealloc_cmap(&fbinfo->cmap);
> 
>  #ifndef NO_BL_SUPPORT
> 

[PATCH 5/5] drivers/video/msm/mddi_client_nt35399.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

The various devm_ functions allocate memory that is released when a driver
detaches. This patch replaces the use of kzalloc by devm_kzalloc.

Additionally, this patch fixes a memory leak: some memory was allocated for
'panel' but not released when the subsequent call to setup_vsync fails.

Signed-off-by: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/msm/mddi_client_nt35399.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/msm/mddi_client_nt35399.c b/drivers/video/msm/mddi_client_nt35399.c
index 7fcd67e..66b314e 100644
--- a/drivers/video/msm/mddi_client_nt35399.c
+++ b/drivers/video/msm/mddi_client_nt35399.c
@@ -189,8 +189,9 @@ static int mddi_nt35399_probe(struct platform_device *pdev)

 	int ret;

-	struct panel_info *panel = kzalloc(sizeof(struct panel_info),
-					   GFP_KERNEL);
+	struct panel_info *panel = devm_kzalloc(&pdev->dev,
+						sizeof(struct panel_info),
+						GFP_KERNEL);

 	printk(KERN_DEBUG "%s: enter.\n", __func__);

@@ -233,7 +234,6 @@ static int mddi_nt35399_remove(struct platform_device *pdev)
 	struct panel_info *panel = platform_get_drvdata(pdev);

 	setup_vsync(panel, 0);
-	kfree(panel);
 	return 0;
 }

Re: [PATCH 5/5] drivers/video/msm/mddi_client_nt35399.c: use devm_ functions

[Dan Carpenter]

On Fri, Aug 03, 2012 at 05:40:13PM +0200, Damien Cassou wrote:
> @@ -233,7 +234,6 @@ static int mddi_nt35399_remove(struct platform_device *pdev)
>  	struct panel_info *panel = platform_get_drvdata(pdev);
> 
>  	setup_vsync(panel, 0);
> -	kfree(panel);
>  	return 0;
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in

It's weird.  This patch doesn't apply for me unless I add a blank
line between the "}" and the "--".  I'm not sure if that line is
getting removed by your email client or by the kernel janitors email
list.

regards,
dan carpenter

Re: [PATCH 5/5] drivers/video/msm/mddi_client_nt35399.c: use devm_ functions

[David Brown]

On Fri, Aug 03, 2012 at 05:40:13PM +0200, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> The various devm_ functions allocate memory that is released when a driver
> detaches. This patch replaces the use of kzalloc by devm_kzalloc.
> 
> Additionally, this patch fixes a memory leak: some memory was allocated for
> 'panel' but not released when the subsequent call to setup_vsync fails.
> 
> Signed-off-by: Damien Cassou <damien.cassou@lifl.fr>

Acked-by: David Brown <davidb@codeaurora.org>

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 5/5] drivers/video/msm/mddi_client_nt35399.c: use devm_ functions

[Florian Tobias Schandinat]

On 08/03/2012 03:40 PM, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> The various devm_ functions allocate memory that is released when a driver
> detaches. This patch replaces the use of kzalloc by devm_kzalloc.
> 
> Additionally, this patch fixes a memory leak: some memory was allocated for
> 'panel' but not released when the subsequent call to setup_vsync fails.
> 
> Signed-off-by: Damien Cassou <damien.cassou@lifl.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
>  drivers/video/msm/mddi_client_nt35399.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/msm/mddi_client_nt35399.c b/drivers/video/msm/mddi_client_nt35399.c
> index 7fcd67e..66b314e 100644
> --- a/drivers/video/msm/mddi_client_nt35399.c
> +++ b/drivers/video/msm/mddi_client_nt35399.c
> @@ -189,8 +189,9 @@ static int mddi_nt35399_probe(struct platform_device *pdev)
> 
>  	int ret;
> 
> -	struct panel_info *panel = kzalloc(sizeof(struct panel_info),
> -					   GFP_KERNEL);
> +	struct panel_info *panel = devm_kzalloc(&pdev->dev,
> +						sizeof(struct panel_info),
> +						GFP_KERNEL);
> 
>  	printk(KERN_DEBUG "%s: enter.\n", __func__);
> 
> @@ -233,7 +234,6 @@ static int mddi_nt35399_remove(struct platform_device *pdev)
>  	struct panel_info *panel = platform_get_drvdata(pdev);
> 
>  	setup_vsync(panel, 0);
> -	kfree(panel);
>  	return 0;
>  }
> 

[PATCH 4/5] drivers/video/msm/mddi_client_dummy.c: use devm_ functions

[Damien Cassou]

From: Damien Cassou <damien.cassou@lifl.fr>

The various devm_ functions allocate memory that is released when a driver
detaches. This patch replaces the use of kzalloc by devm_kzalloc.

Signed-off-by: Damien Cassou <damien.cassou@lifl.fr>

---
 drivers/video/msm/mddi_client_dummy.c |   12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/video/msm/mddi_client_dummy.c b/drivers/video/msm/mddi_client_dummy.c
index d2a091c..4c31325 100644
--- a/drivers/video/msm/mddi_client_dummy.c
+++ b/drivers/video/msm/mddi_client_dummy.c
@@ -51,7 +51,7 @@ static int mddi_dummy_probe(struct platform_device *pdev)
 {
 	struct msm_mddi_client_data *client_data = pdev->dev.platform_data;
 	struct panel_info *panel -		kzalloc(sizeof(struct panel_info), GFP_KERNEL);
+		devm_kzalloc(&pdev->dev, sizeof(struct panel_info), GFP_KERNEL);
 	int ret;
 	if (!panel)
 		return -ENOMEM;
@@ -67,18 +67,11 @@ static int mddi_dummy_probe(struct platform_device *pdev)
 				      client_data->fb_resource, 1);
 	panel->panel_data.fb_data = client_data->private_client_data;
 	panel->pdev.dev.platform_data = &panel->panel_data;
-	ret = platform_device_register(&panel->pdev);
-	if (ret) {
-		kfree(panel);
-		return ret;
-	}
-	return 0;
+	return platform_device_register(&panel->pdev);
 }

 static int mddi_dummy_remove(struct platform_device *pdev)
 {
-	struct panel_info *panel = platform_get_drvdata(pdev);
-	kfree(panel);
 	return 0;
 }

Re: [PATCH 4/5] drivers/video/msm/mddi_client_dummy.c: use devm_ functions

[David Brown]

On Fri, Aug 03, 2012 at 05:40:14PM +0200, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> The various devm_ functions allocate memory that is released when a driver
> detaches. This patch replaces the use of kzalloc by devm_kzalloc.
> 
> Signed-off-by: Damien Cassou <damien.cassou@lifl.fr>
> 
> ---
>  drivers/video/msm/mddi_client_dummy.c |   12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)

Acked-by: David Brown <davidb@codeaurora.org>

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 4/5] drivers/video/msm/mddi_client_dummy.c: use devm_ functions

[David Brown]

On Fri, Aug 03, 2012 at 05:40:14PM +0200, Damien Cassou wrote:
> From: Damien Cassou <damien.cassou@lifl.fr>
> 
> The various devm_ functions allocate memory that is released when a driver
> detaches. This patch replaces the use of kzalloc by devm_kzalloc.
> 
> Signed-off-by: Damien Cassou <damien.cassou@lifl.fr>
> 
> ---
>  drivers/video/msm/mddi_client_dummy.c |   12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/video/msm/mddi_client_dummy.c b/drivers/video/msm/mddi_client_dummy.c
> index d2a091c..4c31325 100644
> --- a/drivers/video/msm/mddi_client_dummy.c
> +++ b/drivers/video/msm/mddi_client_dummy.c
> @@ -51,7 +51,7 @@ static int mddi_dummy_probe(struct platform_device *pdev)
>  {
>  	struct msm_mddi_client_data *client_data = pdev->dev.platform_data;
>  	struct panel_info *panel > -		kzalloc(sizeof(struct panel_info), GFP_KERNEL);
> +		devm_kzalloc(&pdev->dev, sizeof(struct panel_info), GFP_KERNEL);
>  	int ret;
>  	if (!panel)
>  		return -ENOMEM;
> @@ -67,18 +67,11 @@ static int mddi_dummy_probe(struct platform_device *pdev)
>  				      client_data->fb_resource, 1);
>  	panel->panel_data.fb_data = client_data->private_client_data;
>  	panel->pdev.dev.platform_data = &panel->panel_data;
> -	ret = platform_device_register(&panel->pdev);
> -	if (ret) {
> -		kfree(panel);
> -		return ret;
> -	}
> -	return 0;
> +	return platform_device_register(&panel->pdev);

Removing this block causes a warning:
kernel/drivers/video/msm/mddi_client_dummy.c: In function 'mddi_dummy_probe':
kernel/drivers/video/msm/mddi_client_dummy.c:55:6: warning: unused variable 'ret' [-Wunused-variable]

Please remove the 'int ret;' line above as well.

Thanks,
David Brown

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 4/5] drivers/video/msm/mddi_client_dummy.c: use devm_ functions

[Damien Cassou]

On Thu, Aug 9, 2012 at 7:57 PM, David Brown <davidb@codeaurora.org> wrote:
> Removing this block causes a warning:
> kernel/drivers/video/msm/mddi_client_dummy.c: In function 'mddi_dummy_probe':
> kernel/drivers/video/msm/mddi_client_dummy.c:55:6: warning: unused variable 'ret' [-Wunused-variable]
>
> Please remove the 'int ret;' line above as well.

Thank you for your feedback.
Please ignore this thread and patch. I've just sent a new email with
corrected patch.

-- 
Damien Cassou
http://damiencassou.seasidehosting.st

"Lambdas are relegated to relative obscurity until Java makes them
popular by not having them." James Iry