diff mbox

Handle efifb with no lfb_base better.

Message ID 1366730598-13942-1-git-send-email-pjones@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Jones April 23, 2013, 3:23 p.m. UTC
Right now we get a WARN from platform_device_unregister() because our
platform_device has no ->release function.  This is clearly wrong, but
we should be warning so we can figure out what happened, as this failure
results in bug reports.  So WARN() about the real problem, and use the
registration function that gives us a default release() function.

This fixes the tracback reported at
https://bugzilla.redhat.com/show_bug.cgi?id=840621 , though it does not
fix the actual /problem/ the user is seeing.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 drivers/video/efifb.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Comments

Josh Boyer June 25, 2013, 2:17 p.m. UTC | #1
On Tue, Apr 23, 2013 at 11:23:18AM -0400, Peter Jones wrote:
> Right now we get a WARN from platform_device_unregister() because our
> platform_device has no ->release function.  This is clearly wrong, but
> we should be warning so we can figure out what happened, as this failure
> results in bug reports.  So WARN() about the real problem, and use the
> registration function that gives us a default release() function.
> 
> This fixes the tracback reported at
> https://bugzilla.redhat.com/show_bug.cgi?id=840621 , though it does not
> fix the actual /problem/ the user is seeing.
> 
> Signed-off-by: Peter Jones <pjones@redhat.com>

Is anyone in the fdbdev camp looking at this?  It's been 2 months since
this was sent out without any comments and I don't see it in the tree
anywhere.

josh

> ---
>  drivers/video/efifb.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
> index 50fe668..390b61b 100644
> --- a/drivers/video/efifb.c
> +++ b/drivers/video/efifb.c
> @@ -285,6 +285,7 @@ static void efifb_destroy(struct fb_info *info)
>  {
>  	if (info->screen_base)
>  		iounmap(info->screen_base);
> +	fb_dealloc_cmap(&info->cmap);
>  	if (request_mem_succeeded)
>  		release_mem_region(info->apertures->ranges[0].base,
>  				   info->apertures->ranges[0].size);
> @@ -382,6 +383,8 @@ static int __init efifb_probe(struct platform_device *dev)
>  	if (!screen_info.pages)
>  		screen_info.pages = 1;
>  	if (!screen_info.lfb_base) {
> +		WARN(1, KERN_WARNING, "invalid framebuffer address for "
> +				      "device '%s'\n", dev_name(dev));
>  		printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
>  		return -ENODEV;
>  	}
> @@ -544,9 +547,7 @@ static struct platform_driver efifb_driver = {
>  	},
>  };
>  
> -static struct platform_device efifb_device = {
> -	.name	= "efifb",
> -};
> +static struct platform_device *efifb_device;
>  
>  static int __init efifb_init(void)
>  {
> @@ -571,9 +572,9 @@ static int __init efifb_init(void)
>  	if (!screen_info.lfb_linelength)
>  		return -ENODEV;
>  
> -	ret = platform_device_register(&efifb_device);
> -	if (ret)
> -		return ret;
> +	efifb_device = platform_device_register_simple("efifb", 0, NULL, 0);
> +	if (IS_ERROR(efifb_device))
> +		return PTR_ERR(efifb_device);
>  
>  	/*
>  	 * This is not just an optimization.  We will interfere
> @@ -582,7 +583,7 @@ static int __init efifb_init(void)
>  	 */
>  	ret = platform_driver_probe(&efifb_driver, efifb_probe);
>  	if (ret) {
> -		platform_device_unregister(&efifb_device);
> +		platform_device_unregister(efifb_device);
>  		return ret;
>  	}
>  
> -- 
> 1.8.1.4
> 
--
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
diff mbox

Patch

diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
index 50fe668..390b61b 100644
--- a/drivers/video/efifb.c
+++ b/drivers/video/efifb.c
@@ -285,6 +285,7 @@  static void efifb_destroy(struct fb_info *info)
 {
 	if (info->screen_base)
 		iounmap(info->screen_base);
+	fb_dealloc_cmap(&info->cmap);
 	if (request_mem_succeeded)
 		release_mem_region(info->apertures->ranges[0].base,
 				   info->apertures->ranges[0].size);
@@ -382,6 +383,8 @@  static int __init efifb_probe(struct platform_device *dev)
 	if (!screen_info.pages)
 		screen_info.pages = 1;
 	if (!screen_info.lfb_base) {
+		WARN(1, KERN_WARNING, "invalid framebuffer address for "
+				      "device '%s'\n", dev_name(dev));
 		printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
 		return -ENODEV;
 	}
@@ -544,9 +547,7 @@  static struct platform_driver efifb_driver = {
 	},
 };
 
-static struct platform_device efifb_device = {
-	.name	= "efifb",
-};
+static struct platform_device *efifb_device;
 
 static int __init efifb_init(void)
 {
@@ -571,9 +572,9 @@  static int __init efifb_init(void)
 	if (!screen_info.lfb_linelength)
 		return -ENODEV;
 
-	ret = platform_device_register(&efifb_device);
-	if (ret)
-		return ret;
+	efifb_device = platform_device_register_simple("efifb", 0, NULL, 0);
+	if (IS_ERROR(efifb_device))
+		return PTR_ERR(efifb_device);
 
 	/*
 	 * This is not just an optimization.  We will interfere
@@ -582,7 +583,7 @@  static int __init efifb_init(void)
 	 */
 	ret = platform_driver_probe(&efifb_driver, efifb_probe);
 	if (ret) {
-		platform_device_unregister(&efifb_device);
+		platform_device_unregister(efifb_device);
 		return ret;
 	}