[PATCH 8/15] stifb: check fb_alloc_cmap return value and handle failure properly

[PATCH 8/15] stifb: check fb_alloc_cmap return value and handle failure properly

[Andres Salomon]

Signed-off-by: Andres Salomon <dilinger@debian.org>
---
 drivers/video/stifb.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/video/stifb.c b/drivers/video/stifb.c
index 1664814..eabaad7 100644
--- a/drivers/video/stifb.c
+++ b/drivers/video/stifb.c
@@ -1262,24 +1262,25 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
 	info->flags = FBINFO_DEFAULT;
 	info->pseudo_palette = &fb->pseudo_palette;
 
-	/* This has to been done !!! */
-	fb_alloc_cmap(&info->cmap, NR_PALETTE, 0);
+	/* This has to be done !!! */
+	if (fb_alloc_cmap(&info->cmap, NR_PALETTE, 0))
+		goto out_err1;
 	stifb_init_display(fb);
 
 	if (!request_mem_region(fix->smem_start, fix->smem_len, "stifb fb")) {
 		printk(KERN_ERR "stifb: cannot reserve fb region 0x%04lx-0x%04lx\n",
 				fix->smem_start, fix->smem_start+fix->smem_len);
-		goto out_err1;
+		goto out_err2;
 	}
 		
 	if (!request_mem_region(fix->mmio_start, fix->mmio_len, "stifb mmio")) {
 		printk(KERN_ERR "stifb: cannot reserve sti mmio region 0x%04lx-0x%04lx\n",
 				fix->mmio_start, fix->mmio_start+fix->mmio_len);
-		goto out_err2;
+		goto out_err3;
 	}
 
 	if (register_framebuffer(&fb->info) < 0)
-		goto out_err3;
+		goto out_err4;
 
 	sti->info = info; /* save for unregister_framebuffer() */
 
@@ -1297,13 +1298,14 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
 	return 0;
 
 
-out_err3:
+out_err4:
 	release_mem_region(fix->mmio_start, fix->mmio_len);
-out_err2:
+out_err3:
 	release_mem_region(fix->smem_start, fix->smem_len);
+out_err2:
+	fb_dealloc_cmap(&info->cmap);
 out_err1:
 	iounmap(info->screen_base);
-	fb_dealloc_cmap(&info->cmap);
 out_err0:
 	kfree(fb);
 	return -ENXIO;
-- 
1.5.6.5


------------------------------------------------------------------------------
Create and Deploy Rich Internet Apps outside the browser with Adobe(R)AIR(TM)
software. With Adobe AIR, Ajax developers can use existing skills and code to
build responsive, highly engaging applications that combine the power of local
resources and data with the reach of the web. Download the Adobe AIR SDK and
Ajax docs to start building applications today-http://p.sf.net/sfu/adobe-com

Re: [Linux-fbdev-devel] [PATCH 8/15] stifb: check fb_alloc_cmap return value and handle failure properly

[Krzysztof Helt]

On Sat, 7 Feb 2009 12:15:06 -0500
Andres Salomon <dilinger@queued.net> wrote:

> 
> Signed-off-by: Andres Salomon <dilinger@debian.org>
> ---

Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>

I feel something is not right with the driver if memory 
region (info->screen_base) is io mapped first then
reserved with request_mem_region().

Regards,
Krzysztof

>  drivers/video/stifb.c |   18 ++++++++++--------
>  1 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/video/stifb.c b/drivers/video/stifb.c
> index 1664814..eabaad7 100644
> --- a/drivers/video/stifb.c
> +++ b/drivers/video/stifb.c
> @@ -1262,24 +1262,25 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
>  	info->flags = FBINFO_DEFAULT;
>  	info->pseudo_palette = &fb->pseudo_palette;
>  
> -	/* This has to been done !!! */
> -	fb_alloc_cmap(&info->cmap, NR_PALETTE, 0);
> +	/* This has to be done !!! */
> +	if (fb_alloc_cmap(&info->cmap, NR_PALETTE, 0))
> +		goto out_err1;
>  	stifb_init_display(fb);
>  
>  	if (!request_mem_region(fix->smem_start, fix->smem_len, "stifb fb")) {
>  		printk(KERN_ERR "stifb: cannot reserve fb region 0x%04lx-0x%04lx\n",
>  				fix->smem_start, fix->smem_start+fix->smem_len);
> -		goto out_err1;
> +		goto out_err2;
>  	}
>  		
>  	if (!request_mem_region(fix->mmio_start, fix->mmio_len, "stifb mmio")) {
>  		printk(KERN_ERR "stifb: cannot reserve sti mmio region 0x%04lx-0x%04lx\n",
>  				fix->mmio_start, fix->mmio_start+fix->mmio_len);
> -		goto out_err2;
> +		goto out_err3;
>  	}
>  
>  	if (register_framebuffer(&fb->info) < 0)
> -		goto out_err3;
> +		goto out_err4;
>  
>  	sti->info = info; /* save for unregister_framebuffer() */
>  
> @@ -1297,13 +1298,14 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
>  	return 0;
>  
>  
> -out_err3:
> +out_err4:
>  	release_mem_region(fix->mmio_start, fix->mmio_len);
> -out_err2:
> +out_err3:
>  	release_mem_region(fix->smem_start, fix->smem_len);
> +out_err2:
> +	fb_dealloc_cmap(&info->cmap);
>  out_err1:
>  	iounmap(info->screen_base);
> -	fb_dealloc_cmap(&info->cmap);
>  out_err0:
>  	kfree(fb);
>  	return -ENXIO;
> -- 
> 1.5.6.5
> 
> 
> ------------------------------------------------------------------------------
> Create and Deploy Rich Internet Apps outside the browser with Adobe(R)AIR(TM)
> software. With Adobe AIR, Ajax developers can use existing skills and code to
> build responsive, highly engaging applications that combine the power of local
> resources and data with the reach of the web. Download the Adobe AIR SDK and
> Ajax docs to start building applications today-http://p.sf.net/sfu/adobe-com
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
> 

Re: [Linux-fbdev-devel] [PATCH 8/15] stifb: check fb_alloc_cmap return value and handle failure properly

[Andres Salomon]

On Mon, 9 Feb 2009 18:05:49 +0100
Krzysztof Helt <krzysztof.h1@wp.pl> wrote:

> On Sat, 7 Feb 2009 12:15:06 -0500
> Andres Salomon <dilinger@queued.net> wrote:
> 
> > 
> > Signed-off-by: Andres Salomon <dilinger@debian.org>
> > ---
> 
> Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> I feel something is not right with the driver if memory 
> region (info->screen_base) is io mapped first then
> reserved with request_mem_region().
> 
> Regards,
> Krzysztof

Yeah, I noticed quite a few other bugs while going through cmap brokenness,
and I'm not even done w/ the cmap stuff yet.  I figured I'd do one thing at
a time.

Thanks for the Acks!