All of lore.kernel.org
 help / color / mirror / Atom feed
* + fbdev-section-cleanup-in-vesafb.patch added to -mm tree
@ 2010-03-31 21:44 akpm
  2010-04-02 12:28   ` Henrik Kretzschmar
  0 siblings, 1 reply; 5+ messages in thread
From: akpm @ 2010-03-31 21:44 UTC (permalink / raw)
  To: mm-commits; +Cc: henne, arnaud.patard, pjones


The patch titled
     fbdev: vesafb: fix section mismatches
has been added to the -mm tree.  Its filename is
     fbdev-section-cleanup-in-vesafb.patch

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/SubmitChecklist when testing your code ***

See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
out what to do about this

The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/

------------------------------------------------------
Subject: fbdev: vesafb: fix section mismatches
From: Henrik Kretzschmar <henne@nachtwindheim.de>

Fix section mismatches in the vesafb driver.

WARNING: drivers/video/built-in.o(.devinit.text+0x19495): Section mismatch in reference from the function vesafb_probe() to the variable .init.data:vesafb_fix
The function __devinit vesafb_probe() references
a variable __initdata vesafb_fix.
If vesafb_fix is only used by vesafb_probe then
annotate vesafb_fix with a matching annotation.

WARNING: drivers/video/built-in.o(.devinit.text+0x194a5): Section mismatch in reference from the function vesafb_probe() to the (unknown reference) .init.data:(unknown)
The function __devinit vesafb_probe() references
a (unknown reference) __initdata (unknown).
If (unknown) is only used by vesafb_probe then
annotate (unknown) with a matching annotation.

WARNING: drivers/video/built-in.o(.devinit.text+0x194d5): Section mismatch in reference from the function vesafb_probe() to the variable .init.data:vesafb_defined
The function __devinit vesafb_probe() references
a variable __initdata vesafb_defined.
If vesafb_defined is only used by vesafb_probe then
annotate vesafb_defined with a matching annotation.

...
fbdev: section cleanup in vesafb

This patch cleans up the sections in the vesafb driver.

WARNING: drivers/video/built-in.o(.devinit.text+0x19495): Section mismatch in reference from the function vesafb_probe() to the variable .init.data:vesafb_fix
The function __devinit vesafb_probe() references
a variable __initdata vesafb_fix.
If vesafb_fix is only used by vesafb_probe then
annotate vesafb_fix with a matching annotation.

WARNING: drivers/video/built-in.o(.devinit.text+0x194a5): Section mismatch in reference from the function vesafb_probe() to the (unknown reference) .init.data:(unknown)
The function __devinit vesafb_probe() references
a (unknown reference) __initdata (unknown).
If (unknown) is only used by vesafb_probe then
annotate (unknown) with a matching annotation.

WARNING: drivers/video/built-in.o(.devinit.text+0x194d5): Section mismatch in reference from the function vesafb_probe() to the variable .init.data:vesafb_defined
The function __devinit vesafb_probe() references
a variable __initdata vesafb_defined.
If vesafb_defined is only used by vesafb_probe then
annotate vesafb_defined with a matching annotation.

...

Signed-off-by: Henrik Kretzschmar <henne@nachtwindheim.de>
Cc: Peter Jones <pjones@redhat.com>
Cc: Arnaud Patard <arnaud.patard@rtp-net.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

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

diff -puN drivers/video/vesafb.c~fbdev-section-cleanup-in-vesafb drivers/video/vesafb.c
--- a/drivers/video/vesafb.c~fbdev-section-cleanup-in-vesafb
+++ a/drivers/video/vesafb.c
@@ -29,7 +29,7 @@
 
 /* --------------------------------------------------------------------- */
 
-static struct fb_var_screeninfo vesafb_defined __initdata = {
+static struct fb_var_screeninfo vesafb_defined __devinitdata = {
 	.activate	= FB_ACTIVATE_NOW,
 	.height		= -1,
 	.width		= -1,
@@ -40,7 +40,7 @@ static struct fb_var_screeninfo vesafb_d
 	.vmode		= FB_VMODE_NONINTERLACED,
 };
 
-static struct fb_fix_screeninfo vesafb_fix __initdata = {
+static struct fb_fix_screeninfo vesafb_fix __devinitdata = {
 	.id	= "VESA VGA",
 	.type	= FB_TYPE_PACKED_PIXELS,
 	.accel	= FB_ACCEL_NONE,
@@ -48,8 +48,8 @@ static struct fb_fix_screeninfo vesafb_f
 
 static int   inverse    __read_mostly;
 static int   mtrr       __read_mostly;		/* disable mtrr */
-static int   vram_remap __initdata;		/* Set amount of memory to be used */
-static int   vram_total __initdata;		/* Set total amount of memory */
+static int   vram_remap __devinitdata;		/* Set amount of memory to be used */
+static int   vram_total __devinitdata;		/* Set total amount of memory */
 static int   pmi_setpal __read_mostly = 1;	/* pmi for palette changes ??? */
 static int   ypan       __read_mostly;		/* 0..nothing, 1..ypan, 2..ywrap */
 static void  (*pmi_start)(void) __read_mostly;
_

Patches currently in -mm which might be from henne@nachtwindheim.de are

linux-next.patch
fbdev-section-cleanup-in-arcfb.patch
fbdev-section-cleanup-in-efifb.patch
fbdev-section-cleanup-in-hgafb.patch
fbdev-section-cleanup-in-vesafb.patch
fbdev-section-cleanup-in-vfb.patch
fbdev-section-cleanup-in-vga16fb.patch
fbdev-section-cleanup-in-w100fb.patch
auxdisplay-section-cleanup-in-cfag12864bfb-driver.patch


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: + fbdev-section-cleanup-in-vesafb.patch added to -mm tree
  2010-03-31 21:44 + fbdev-section-cleanup-in-vesafb.patch added to -mm tree akpm
@ 2010-04-02 12:28   ` Henrik Kretzschmar
  0 siblings, 0 replies; 5+ messages in thread
From: Henrik Kretzschmar @ 2010-04-02 12:28 UTC (permalink / raw)
  To: akpm; +Cc: linux-fbdev, linux-kernel, JBeulich, ben.collins

Hi Andrew,

I've rewritten the changelogs of five of patches from me,
which you have already in your tree and cc'ed you.

The patches in your tree are: 

fbdev-section-cleanup-in-arcfb.patch
fbdev-section-cleanup-in-vfb.patch
fbdev-section-cleanup-in-vga16fb.patch
fbdev-section-cleanup-in-w100fb.patch
fbdev-section-cleanup-in-hgafb.patch

Please also drop from you tree the fbdev-section-cleanup-in-vesafb.patch

I wrote it in the believe that it's a module, which is not the case in mainline.
So my patch would make the section mismatch warnings go away,
but hides that theres more work than that to do.

Jan Beulich wrote on linux-fbdev-ml a patch which assumes that vesafb is builtin (1st patch).
Ben Collins from Canonical had patched their lucid-kernel that vesafb is a module instead (2nd patch).
So the first question is if it makes sense to have the vesafb driver as a module,
before deciding what comes in which section.

Does someone have an idea about that?



From: Jan Beulich <JBeulich@novell.com>
Date: Fri, Mar 26, 2010 at 17:48
Subject: [PATCH] vesafb: use platform_driver_probe() instead of
platform_driver_register()
To: u.kleine-koenig@pengutronix.de, gregkh@suse.de
Cc: linux-kernel@vger.kernel.org


Commit c2e13037e6794bd0d9de3f9ecabf5615f15c160b introduced a huge
amount of section mismatch warnings in vesafb code. Rather than
converting all of the annotations, do the obvious and revert the
__init -> __devinit change, and use the recommended (in that patch)
alternative to calling platform_driver_register(): vesafb depends on
information obtained from by kernel at boot time, cannot be a module,
and no post-boot devices can ever show up.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

---
 drivers/video/vesafb.c |   26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

--- linux-2.6.34-rc2/drivers/video/vesafb.c
+++ 2.6.34-rc2-vesafb-init/drivers/video/vesafb.c
@@ -226,7 +226,7 @@ static int __init vesafb_setup(char *opt
       return 0;
 }

-static int __devinit vesafb_probe(struct platform_device *dev)
+static int __init vesafb_probe(struct platform_device *dev)
 {
       struct fb_info *info;
       int i, err;
@@ -477,7 +477,6 @@ err:
 }

 static struct platform_driver vesafb_driver = {
-       .probe  = vesafb_probe,
       .driver = {
               .name   = "vesafb",
       },
@@ -493,20 +492,21 @@ static int __init vesafb_init(void)
       /* ignore error return of fb_get_options */
       fb_get_options("vesafb", &option);
       vesafb_setup(option);
-       ret = platform_driver_register(&vesafb_driver);

+       vesafb_device = platform_device_alloc("vesafb", 0);
+       if (!vesafb_device)
+               return -ENOMEM;
+
+       ret = platform_device_add(vesafb_device);
       if (!ret) {
-               vesafb_device = platform_device_alloc("vesafb", 0);
+               ret = platform_driver_probe(&vesafb_driver, vesafb_probe);
+               if (ret)
+                       platform_device_del(vesafb_device);
+       }

-               if (vesafb_device)
-                       ret = platform_device_add(vesafb_device);
-               else
-                       ret = -ENOMEM;
-
-               if (ret) {
-                       platform_device_put(vesafb_device);
-                       platform_driver_unregister(&vesafb_driver);
-               }
+       if (ret) {
+               platform_device_put(vesafb_device);
+               vesafb_device = NULL;
       }

       return ret;



--

commit 0afa62be6d93770801581d915667d89f38a60d37
Author: Ben Collins <ben.collins@canonical.com>
Date:   Tue Oct 14 11:30:10 2008 -0400

    UBUNTU: SAUCE: (no-up) Modularize vesafb
    
    Signed-off-by: Ben Collins <ben.collins@canonical.com>

diff --git a/drivers/video/vesafb.c b/drivers/video/vesafb.c
index bd37ee1..0bcae3c 100644
--- a/drivers/video/vesafb.c
+++ b/drivers/video/vesafb.c
@@ -28,6 +28,12 @@
 #define dac_reg	(0x3c8)
 #define dac_val	(0x3c9)
 
+struct vesafb_info
+{
+	u32 pseudo_palette[256];
+	int mtrr_hdl;
+};
+
 /* --------------------------------------------------------------------- */
 
 static struct fb_var_screeninfo vesafb_defined __initdata = {
@@ -47,16 +53,37 @@ static struct fb_fix_screeninfo vesafb_fix __initdata = {
 	.accel	= FB_ACCEL_NONE,
 };
 
+#ifndef MODULE
 static int   inverse    __read_mostly;
+#endif
 static int   mtrr       __read_mostly;		/* disable mtrr */
 static int   vram_remap __initdata;		/* Set amount of memory to be used */
 static int   vram_total __initdata;		/* Set total amount of memory */
 static int   pmi_setpal __read_mostly = 1;	/* pmi for palette changes ??? */
+static int	redraw		__read_mostly;
 static int   ypan       __read_mostly;		/* 0..nothing, 1..ypan, 2..ywrap */
+static int	ywrap		__read_mostly;
 static void  (*pmi_start)(void) __read_mostly;
 static void  (*pmi_pal)  (void) __read_mostly;
 static int   depth      __read_mostly;
 static int   vga_compat __read_mostly;
+
+module_param(redraw, bool, 0);
+module_param(ypan, bool, 0);
+module_param(ywrap, bool, 0);
+module_param_named(vgapal, pmi_setpal, invbool, 0);
+MODULE_PARM_DESC(vgapal, "Use VGA for setting palette (default)");
+module_param_named(pmipal, pmi_setpal, bool, 0);
+MODULE_PARM_DESC(pmipal, "Use PMI for setting palette");
+module_param(mtrr, bool, 0);
+MODULE_PARM_DESC(mtrr, "Enable MTRR support (default)");
+module_param_named(nomtrr, mtrr, invbool, 0);
+MODULE_PARM_DESC(nomtrr, "Disable MTRR support");
+module_param(vram_remap, int, 0);
+MODULE_PARM_DESC(vram_remap, "Set total amount of memory to be used");
+module_param(vram_total, int, 0);
+MODULE_PARM_DESC(vram_total, "Total amount of memory");
+
 /* --------------------------------------------------------------------- */
 
 static int vesafb_pan_display(struct fb_var_screeninfo *var,
@@ -192,6 +219,7 @@ static struct fb_ops vesafb_ops = {
 	.fb_imageblit	= cfb_imageblit,
 };
 
+#ifndef MODULE
 static int __init vesafb_setup(char *options)
 {
 	char *this_opt;
@@ -225,6 +253,7 @@ static int __init vesafb_setup(char *options)
 	}
 	return 0;
 }
+#endif
 
 static int __init vesafb_probe(struct platform_device *dev)
 {
@@ -476,8 +505,28 @@ err:
 	return err;
 }
 
+static int __exit vesafb_remove(struct platform_device *device)
+{
+	struct fb_info *info = dev_get_drvdata(&device->dev);
+
+	unregister_framebuffer(info);
+#ifdef CONFIG_MTRR
+	{
+		struct vesafb_info *vfb_info = (struct vesafb_info *) info->par;
+		if (vfb_info->mtrr_hdl >= 0)
+			mtrr_del(vfb_info->mtrr_hdl, 0, 0);
+	}
+#endif
+	iounmap(info->screen_base);
+	framebuffer_release(info);
+	release_mem_region(vesafb_fix.smem_start, vesafb_fix.smem_len);
+
+	return 0;
+}
+
 static struct platform_driver vesafb_driver = {
 	.probe	= vesafb_probe,
+	.remove = vesafb_remove,
 	.driver	= {
 		.name	= "vesafb",
 	},
@@ -488,11 +537,18 @@ static struct platform_device *vesafb_device;
 static int __init vesafb_init(void)
 {
 	int ret;
+#ifndef MODULE
 	char *option = NULL;
 
 	/* ignore error return of fb_get_options */
 	fb_get_options("vesafb", &option);
 	vesafb_setup(option);
+#else
+	if (redraw)
+		ypan = 0;
+	if (ywrap)
+		ypan = 2;
+#endif
 	ret = platform_driver_register(&vesafb_driver);
 
 	if (!ret) {
@@ -511,6 +567,14 @@ static int __init vesafb_init(void)
 
 	return ret;
 }
+
+static void __exit vesafb_exit(void)
+{
+	platform_device_unregister(vesafb_device);
+	platform_driver_unregister(&vesafb_driver);
+}
+
 module_init(vesafb_init);
+module_exit(vesafb_exit);
 
 MODULE_LICENSE("GPL");





^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: + fbdev-section-cleanup-in-vesafb.patch added to -mm tree
@ 2010-04-02 12:28   ` Henrik Kretzschmar
  0 siblings, 0 replies; 5+ messages in thread
From: Henrik Kretzschmar @ 2010-04-02 12:28 UTC (permalink / raw)
  To: akpm; +Cc: linux-fbdev, linux-kernel, JBeulich, ben.collins

Hi Andrew,

I've rewritten the changelogs of five of patches from me,
which you have already in your tree and cc'ed you.

The patches in your tree are: 

fbdev-section-cleanup-in-arcfb.patch
fbdev-section-cleanup-in-vfb.patch
fbdev-section-cleanup-in-vga16fb.patch
fbdev-section-cleanup-in-w100fb.patch
fbdev-section-cleanup-in-hgafb.patch

Please also drop from you tree the fbdev-section-cleanup-in-vesafb.patch

I wrote it in the believe that it's a module, which is not the case in mainline.
So my patch would make the section mismatch warnings go away,
but hides that theres more work than that to do.

Jan Beulich wrote on linux-fbdev-ml a patch which assumes that vesafb is builtin (1st patch).
Ben Collins from Canonical had patched their lucid-kernel that vesafb is a module instead (2nd patch).
So the first question is if it makes sense to have the vesafb driver as a module,
before deciding what comes in which section.

Does someone have an idea about that?



From: Jan Beulich <JBeulich@novell.com>
Date: Fri, Mar 26, 2010 at 17:48
Subject: [PATCH] vesafb: use platform_driver_probe() instead of
platform_driver_register()
To: u.kleine-koenig@pengutronix.de, gregkh@suse.de
Cc: linux-kernel@vger.kernel.org


Commit c2e13037e6794bd0d9de3f9ecabf5615f15c160b introduced a huge
amount of section mismatch warnings in vesafb code. Rather than
converting all of the annotations, do the obvious and revert the
__init -> __devinit change, and use the recommended (in that patch)
alternative to calling platform_driver_register(): vesafb depends on
information obtained from by kernel at boot time, cannot be a module,
and no post-boot devices can ever show up.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

---
 drivers/video/vesafb.c |   26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

--- linux-2.6.34-rc2/drivers/video/vesafb.c
+++ 2.6.34-rc2-vesafb-init/drivers/video/vesafb.c
@@ -226,7 +226,7 @@ static int __init vesafb_setup(char *opt
       return 0;
 }

-static int __devinit vesafb_probe(struct platform_device *dev)
+static int __init vesafb_probe(struct platform_device *dev)
 {
       struct fb_info *info;
       int i, err;
@@ -477,7 +477,6 @@ err:
 }

 static struct platform_driver vesafb_driver = {
-       .probe  = vesafb_probe,
       .driver = {
               .name   = "vesafb",
       },
@@ -493,20 +492,21 @@ static int __init vesafb_init(void)
       /* ignore error return of fb_get_options */
       fb_get_options("vesafb", &option);
       vesafb_setup(option);
-       ret = platform_driver_register(&vesafb_driver);

+       vesafb_device = platform_device_alloc("vesafb", 0);
+       if (!vesafb_device)
+               return -ENOMEM;
+
+       ret = platform_device_add(vesafb_device);
       if (!ret) {
-               vesafb_device = platform_device_alloc("vesafb", 0);
+               ret = platform_driver_probe(&vesafb_driver, vesafb_probe);
+               if (ret)
+                       platform_device_del(vesafb_device);
+       }

-               if (vesafb_device)
-                       ret = platform_device_add(vesafb_device);
-               else
-                       ret = -ENOMEM;
-
-               if (ret) {
-                       platform_device_put(vesafb_device);
-                       platform_driver_unregister(&vesafb_driver);
-               }
+       if (ret) {
+               platform_device_put(vesafb_device);
+               vesafb_device = NULL;
       }

       return ret;



--

commit 0afa62be6d93770801581d915667d89f38a60d37
Author: Ben Collins <ben.collins@canonical.com>
Date:   Tue Oct 14 11:30:10 2008 -0400

    UBUNTU: SAUCE: (no-up) Modularize vesafb
    
    Signed-off-by: Ben Collins <ben.collins@canonical.com>

diff --git a/drivers/video/vesafb.c b/drivers/video/vesafb.c
index bd37ee1..0bcae3c 100644
--- a/drivers/video/vesafb.c
+++ b/drivers/video/vesafb.c
@@ -28,6 +28,12 @@
 #define dac_reg	(0x3c8)
 #define dac_val	(0x3c9)
 
+struct vesafb_info
+{
+	u32 pseudo_palette[256];
+	int mtrr_hdl;
+};
+
 /* --------------------------------------------------------------------- */
 
 static struct fb_var_screeninfo vesafb_defined __initdata = {
@@ -47,16 +53,37 @@ static struct fb_fix_screeninfo vesafb_fix __initdata = {
 	.accel	= FB_ACCEL_NONE,
 };
 
+#ifndef MODULE
 static int   inverse    __read_mostly;
+#endif
 static int   mtrr       __read_mostly;		/* disable mtrr */
 static int   vram_remap __initdata;		/* Set amount of memory to be used */
 static int   vram_total __initdata;		/* Set total amount of memory */
 static int   pmi_setpal __read_mostly = 1;	/* pmi for palette changes ??? */
+static int	redraw		__read_mostly;
 static int   ypan       __read_mostly;		/* 0..nothing, 1..ypan, 2..ywrap */
+static int	ywrap		__read_mostly;
 static void  (*pmi_start)(void) __read_mostly;
 static void  (*pmi_pal)  (void) __read_mostly;
 static int   depth      __read_mostly;
 static int   vga_compat __read_mostly;
+
+module_param(redraw, bool, 0);
+module_param(ypan, bool, 0);
+module_param(ywrap, bool, 0);
+module_param_named(vgapal, pmi_setpal, invbool, 0);
+MODULE_PARM_DESC(vgapal, "Use VGA for setting palette (default)");
+module_param_named(pmipal, pmi_setpal, bool, 0);
+MODULE_PARM_DESC(pmipal, "Use PMI for setting palette");
+module_param(mtrr, bool, 0);
+MODULE_PARM_DESC(mtrr, "Enable MTRR support (default)");
+module_param_named(nomtrr, mtrr, invbool, 0);
+MODULE_PARM_DESC(nomtrr, "Disable MTRR support");
+module_param(vram_remap, int, 0);
+MODULE_PARM_DESC(vram_remap, "Set total amount of memory to be used");
+module_param(vram_total, int, 0);
+MODULE_PARM_DESC(vram_total, "Total amount of memory");
+
 /* --------------------------------------------------------------------- */
 
 static int vesafb_pan_display(struct fb_var_screeninfo *var,
@@ -192,6 +219,7 @@ static struct fb_ops vesafb_ops = {
 	.fb_imageblit	= cfb_imageblit,
 };
 
+#ifndef MODULE
 static int __init vesafb_setup(char *options)
 {
 	char *this_opt;
@@ -225,6 +253,7 @@ static int __init vesafb_setup(char *options)
 	}
 	return 0;
 }
+#endif
 
 static int __init vesafb_probe(struct platform_device *dev)
 {
@@ -476,8 +505,28 @@ err:
 	return err;
 }
 
+static int __exit vesafb_remove(struct platform_device *device)
+{
+	struct fb_info *info = dev_get_drvdata(&device->dev);
+
+	unregister_framebuffer(info);
+#ifdef CONFIG_MTRR
+	{
+		struct vesafb_info *vfb_info = (struct vesafb_info *) info->par;
+		if (vfb_info->mtrr_hdl >= 0)
+			mtrr_del(vfb_info->mtrr_hdl, 0, 0);
+	}
+#endif
+	iounmap(info->screen_base);
+	framebuffer_release(info);
+	release_mem_region(vesafb_fix.smem_start, vesafb_fix.smem_len);
+
+	return 0;
+}
+
 static struct platform_driver vesafb_driver = {
 	.probe	= vesafb_probe,
+	.remove = vesafb_remove,
 	.driver	= {
 		.name	= "vesafb",
 	},
@@ -488,11 +537,18 @@ static struct platform_device *vesafb_device;
 static int __init vesafb_init(void)
 {
 	int ret;
+#ifndef MODULE
 	char *option = NULL;
 
 	/* ignore error return of fb_get_options */
 	fb_get_options("vesafb", &option);
 	vesafb_setup(option);
+#else
+	if (redraw)
+		ypan = 0;
+	if (ywrap)
+		ypan = 2;
+#endif
 	ret = platform_driver_register(&vesafb_driver);
 
 	if (!ret) {
@@ -511,6 +567,14 @@ static int __init vesafb_init(void)
 
 	return ret;
 }
+
+static void __exit vesafb_exit(void)
+{
+	platform_device_unregister(vesafb_device);
+	platform_driver_unregister(&vesafb_driver);
+}
+
 module_init(vesafb_init);
+module_exit(vesafb_exit);
 
 MODULE_LICENSE("GPL");





^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: + fbdev-section-cleanup-in-vesafb.patch added to -mm tree
  2010-04-02 12:28   ` Henrik Kretzschmar
@ 2010-04-02 13:03     ` Florian Tobias Schandinat
  -1 siblings, 0 replies; 5+ messages in thread
From: Florian Tobias Schandinat @ 2010-04-02 13:03 UTC (permalink / raw)
  To: Henrik Kretzschmar; +Cc: akpm, linux-fbdev, linux-kernel, JBeulich, ben.collins

Hi,

Henrik Kretzschmar schrieb:
> Please also drop from you tree the fbdev-section-cleanup-in-vesafb.patch
> 
> I wrote it in the believe that it's a module, which is not the case in mainline.
> So my patch would make the section mismatch warnings go away,
> but hides that theres more work than that to do.
> 
> Jan Beulich wrote on linux-fbdev-ml a patch which assumes that vesafb is builtin (1st patch).
> Ben Collins from Canonical had patched their lucid-kernel that vesafb is a module instead (2nd patch).
> So the first question is if it makes sense to have the vesafb driver as a module,
> before deciding what comes in which section.
> 
> Does someone have an idea about that?

I don't know VESA too well but if I understand it right its as the 
header of vesafb states:
"switching to graphics mode happens at boot time (while running in real 
mode, see arch/i386/boot/video.S)."
that means that most of the work is done during boot and that's why a 
module based on this probably won't work. Was the Ubuntu module tested 
successfully?
That's also the reason for vesafb not supporting mode switching as it 
can only select the mode during boot as long as the VESA BIOS is 
available. There is an alternative uvesafb in the kernel which doesn't 
have these limitations but requires some userspace tools to work (AFAIK 
they execute/emulate the VESA BIOS there)


Regards,

Florian Tobias Schandinat

> From: Jan Beulich <JBeulich@novell.com>
> Date: Fri, Mar 26, 2010 at 17:48
> Subject: [PATCH] vesafb: use platform_driver_probe() instead of
> platform_driver_register()
> To: u.kleine-koenig@pengutronix.de, gregkh@suse.de
> Cc: linux-kernel@vger.kernel.org
> 
> 
> Commit c2e13037e6794bd0d9de3f9ecabf5615f15c160b introduced a huge
> amount of section mismatch warnings in vesafb code. Rather than
> converting all of the annotations, do the obvious and revert the
> __init -> __devinit change, and use the recommended (in that patch)
> alternative to calling platform_driver_register(): vesafb depends on
> information obtained from by kernel at boot time, cannot be a module,
> and no post-boot devices can ever show up.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> ---
>  drivers/video/vesafb.c |   26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> --- linux-2.6.34-rc2/drivers/video/vesafb.c
> +++ 2.6.34-rc2-vesafb-init/drivers/video/vesafb.c
> @@ -226,7 +226,7 @@ static int __init vesafb_setup(char *opt
>        return 0;
>  }
> 
> -static int __devinit vesafb_probe(struct platform_device *dev)
> +static int __init vesafb_probe(struct platform_device *dev)
>  {
>        struct fb_info *info;
>        int i, err;
> @@ -477,7 +477,6 @@ err:
>  }
> 
>  static struct platform_driver vesafb_driver = {
> -       .probe  = vesafb_probe,
>        .driver = {
>                .name   = "vesafb",
>        },
> @@ -493,20 +492,21 @@ static int __init vesafb_init(void)
>        /* ignore error return of fb_get_options */
>        fb_get_options("vesafb", &option);
>        vesafb_setup(option);
> -       ret = platform_driver_register(&vesafb_driver);
> 
> +       vesafb_device = platform_device_alloc("vesafb", 0);
> +       if (!vesafb_device)
> +               return -ENOMEM;
> +
> +       ret = platform_device_add(vesafb_device);
>        if (!ret) {
> -               vesafb_device = platform_device_alloc("vesafb", 0);
> +               ret = platform_driver_probe(&vesafb_driver, vesafb_probe);
> +               if (ret)
> +                       platform_device_del(vesafb_device);
> +       }
> 
> -               if (vesafb_device)
> -                       ret = platform_device_add(vesafb_device);
> -               else
> -                       ret = -ENOMEM;
> -
> -               if (ret) {
> -                       platform_device_put(vesafb_device);
> -                       platform_driver_unregister(&vesafb_driver);
> -               }
> +       if (ret) {
> +               platform_device_put(vesafb_device);
> +               vesafb_device = NULL;
>        }
> 
>        return ret;
> 
> 
> 
> --
> 
> commit 0afa62be6d93770801581d915667d89f38a60d37
> Author: Ben Collins <ben.collins@canonical.com>
> Date:   Tue Oct 14 11:30:10 2008 -0400
> 
>     UBUNTU: SAUCE: (no-up) Modularize vesafb
>     
>     Signed-off-by: Ben Collins <ben.collins@canonical.com>
> 
> diff --git a/drivers/video/vesafb.c b/drivers/video/vesafb.c
> index bd37ee1..0bcae3c 100644
> --- a/drivers/video/vesafb.c
> +++ b/drivers/video/vesafb.c
> @@ -28,6 +28,12 @@
>  #define dac_reg	(0x3c8)
>  #define dac_val	(0x3c9)
>  
> +struct vesafb_info
> +{
> +	u32 pseudo_palette[256];
> +	int mtrr_hdl;
> +};
> +
>  /* --------------------------------------------------------------------- */
>  
>  static struct fb_var_screeninfo vesafb_defined __initdata = {
> @@ -47,16 +53,37 @@ static struct fb_fix_screeninfo vesafb_fix __initdata = {
>  	.accel	= FB_ACCEL_NONE,
>  };
>  
> +#ifndef MODULE
>  static int   inverse    __read_mostly;
> +#endif
>  static int   mtrr       __read_mostly;		/* disable mtrr */
>  static int   vram_remap __initdata;		/* Set amount of memory to be used */
>  static int   vram_total __initdata;		/* Set total amount of memory */
>  static int   pmi_setpal __read_mostly = 1;	/* pmi for palette changes ??? */
> +static int	redraw		__read_mostly;
>  static int   ypan       __read_mostly;		/* 0..nothing, 1..ypan, 2..ywrap */
> +static int	ywrap		__read_mostly;
>  static void  (*pmi_start)(void) __read_mostly;
>  static void  (*pmi_pal)  (void) __read_mostly;
>  static int   depth      __read_mostly;
>  static int   vga_compat __read_mostly;
> +
> +module_param(redraw, bool, 0);
> +module_param(ypan, bool, 0);
> +module_param(ywrap, bool, 0);
> +module_param_named(vgapal, pmi_setpal, invbool, 0);
> +MODULE_PARM_DESC(vgapal, "Use VGA for setting palette (default)");
> +module_param_named(pmipal, pmi_setpal, bool, 0);
> +MODULE_PARM_DESC(pmipal, "Use PMI for setting palette");
> +module_param(mtrr, bool, 0);
> +MODULE_PARM_DESC(mtrr, "Enable MTRR support (default)");
> +module_param_named(nomtrr, mtrr, invbool, 0);
> +MODULE_PARM_DESC(nomtrr, "Disable MTRR support");
> +module_param(vram_remap, int, 0);
> +MODULE_PARM_DESC(vram_remap, "Set total amount of memory to be used");
> +module_param(vram_total, int, 0);
> +MODULE_PARM_DESC(vram_total, "Total amount of memory");
> +
>  /* --------------------------------------------------------------------- */
>  
>  static int vesafb_pan_display(struct fb_var_screeninfo *var,
> @@ -192,6 +219,7 @@ static struct fb_ops vesafb_ops = {
>  	.fb_imageblit	= cfb_imageblit,
>  };
>  
> +#ifndef MODULE
>  static int __init vesafb_setup(char *options)
>  {
>  	char *this_opt;
> @@ -225,6 +253,7 @@ static int __init vesafb_setup(char *options)
>  	}
>  	return 0;
>  }
> +#endif
>  
>  static int __init vesafb_probe(struct platform_device *dev)
>  {
> @@ -476,8 +505,28 @@ err:
>  	return err;
>  }
>  
> +static int __exit vesafb_remove(struct platform_device *device)
> +{
> +	struct fb_info *info = dev_get_drvdata(&device->dev);
> +
> +	unregister_framebuffer(info);
> +#ifdef CONFIG_MTRR
> +	{
> +		struct vesafb_info *vfb_info = (struct vesafb_info *) info->par;
> +		if (vfb_info->mtrr_hdl >= 0)
> +			mtrr_del(vfb_info->mtrr_hdl, 0, 0);
> +	}
> +#endif
> +	iounmap(info->screen_base);
> +	framebuffer_release(info);
> +	release_mem_region(vesafb_fix.smem_start, vesafb_fix.smem_len);
> +
> +	return 0;
> +}
> +
>  static struct platform_driver vesafb_driver = {
>  	.probe	= vesafb_probe,
> +	.remove = vesafb_remove,
>  	.driver	= {
>  		.name	= "vesafb",
>  	},
> @@ -488,11 +537,18 @@ static struct platform_device *vesafb_device;
>  static int __init vesafb_init(void)
>  {
>  	int ret;
> +#ifndef MODULE
>  	char *option = NULL;
>  
>  	/* ignore error return of fb_get_options */
>  	fb_get_options("vesafb", &option);
>  	vesafb_setup(option);
> +#else
> +	if (redraw)
> +		ypan = 0;
> +	if (ywrap)
> +		ypan = 2;
> +#endif
>  	ret = platform_driver_register(&vesafb_driver);
>  
>  	if (!ret) {
> @@ -511,6 +567,14 @@ static int __init vesafb_init(void)
>  
>  	return ret;
>  }
> +
> +static void __exit vesafb_exit(void)
> +{
> +	platform_device_unregister(vesafb_device);
> +	platform_driver_unregister(&vesafb_driver);
> +}
> +
>  module_init(vesafb_init);
> +module_exit(vesafb_exit);
>  
>  MODULE_LICENSE("GPL");
> 
> 
> 
> 
> --
> 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
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: + fbdev-section-cleanup-in-vesafb.patch added to -mm tree
@ 2010-04-02 13:03     ` Florian Tobias Schandinat
  0 siblings, 0 replies; 5+ messages in thread
From: Florian Tobias Schandinat @ 2010-04-02 13:03 UTC (permalink / raw)
  To: Henrik Kretzschmar; +Cc: akpm, linux-fbdev, linux-kernel, JBeulich, ben.collins

Hi,

Henrik Kretzschmar schrieb:
> Please also drop from you tree the fbdev-section-cleanup-in-vesafb.patch
> 
> I wrote it in the believe that it's a module, which is not the case in mainline.
> So my patch would make the section mismatch warnings go away,
> but hides that theres more work than that to do.
> 
> Jan Beulich wrote on linux-fbdev-ml a patch which assumes that vesafb is builtin (1st patch).
> Ben Collins from Canonical had patched their lucid-kernel that vesafb is a module instead (2nd patch).
> So the first question is if it makes sense to have the vesafb driver as a module,
> before deciding what comes in which section.
> 
> Does someone have an idea about that?

I don't know VESA too well but if I understand it right its as the 
header of vesafb states:
"switching to graphics mode happens at boot time (while running in real 
mode, see arch/i386/boot/video.S)."
that means that most of the work is done during boot and that's why a 
module based on this probably won't work. Was the Ubuntu module tested 
successfully?
That's also the reason for vesafb not supporting mode switching as it 
can only select the mode during boot as long as the VESA BIOS is 
available. There is an alternative uvesafb in the kernel which doesn't 
have these limitations but requires some userspace tools to work (AFAIK 
they execute/emulate the VESA BIOS there)


Regards,

Florian Tobias Schandinat

> From: Jan Beulich <JBeulich@novell.com>
> Date: Fri, Mar 26, 2010 at 17:48
> Subject: [PATCH] vesafb: use platform_driver_probe() instead of
> platform_driver_register()
> To: u.kleine-koenig@pengutronix.de, gregkh@suse.de
> Cc: linux-kernel@vger.kernel.org
> 
> 
> Commit c2e13037e6794bd0d9de3f9ecabf5615f15c160b introduced a huge
> amount of section mismatch warnings in vesafb code. Rather than
> converting all of the annotations, do the obvious and revert the
> __init -> __devinit change, and use the recommended (in that patch)
> alternative to calling platform_driver_register(): vesafb depends on
> information obtained from by kernel at boot time, cannot be a module,
> and no post-boot devices can ever show up.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> ---
>  drivers/video/vesafb.c |   26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> --- linux-2.6.34-rc2/drivers/video/vesafb.c
> +++ 2.6.34-rc2-vesafb-init/drivers/video/vesafb.c
> @@ -226,7 +226,7 @@ static int __init vesafb_setup(char *opt
>        return 0;
>  }
> 
> -static int __devinit vesafb_probe(struct platform_device *dev)
> +static int __init vesafb_probe(struct platform_device *dev)
>  {
>        struct fb_info *info;
>        int i, err;
> @@ -477,7 +477,6 @@ err:
>  }
> 
>  static struct platform_driver vesafb_driver = {
> -       .probe  = vesafb_probe,
>        .driver = {
>                .name   = "vesafb",
>        },
> @@ -493,20 +492,21 @@ static int __init vesafb_init(void)
>        /* ignore error return of fb_get_options */
>        fb_get_options("vesafb", &option);
>        vesafb_setup(option);
> -       ret = platform_driver_register(&vesafb_driver);
> 
> +       vesafb_device = platform_device_alloc("vesafb", 0);
> +       if (!vesafb_device)
> +               return -ENOMEM;
> +
> +       ret = platform_device_add(vesafb_device);
>        if (!ret) {
> -               vesafb_device = platform_device_alloc("vesafb", 0);
> +               ret = platform_driver_probe(&vesafb_driver, vesafb_probe);
> +               if (ret)
> +                       platform_device_del(vesafb_device);
> +       }
> 
> -               if (vesafb_device)
> -                       ret = platform_device_add(vesafb_device);
> -               else
> -                       ret = -ENOMEM;
> -
> -               if (ret) {
> -                       platform_device_put(vesafb_device);
> -                       platform_driver_unregister(&vesafb_driver);
> -               }
> +       if (ret) {
> +               platform_device_put(vesafb_device);
> +               vesafb_device = NULL;
>        }
> 
>        return ret;
> 
> 
> 
> --
> 
> commit 0afa62be6d93770801581d915667d89f38a60d37
> Author: Ben Collins <ben.collins@canonical.com>
> Date:   Tue Oct 14 11:30:10 2008 -0400
> 
>     UBUNTU: SAUCE: (no-up) Modularize vesafb
>     
>     Signed-off-by: Ben Collins <ben.collins@canonical.com>
> 
> diff --git a/drivers/video/vesafb.c b/drivers/video/vesafb.c
> index bd37ee1..0bcae3c 100644
> --- a/drivers/video/vesafb.c
> +++ b/drivers/video/vesafb.c
> @@ -28,6 +28,12 @@
>  #define dac_reg	(0x3c8)
>  #define dac_val	(0x3c9)
>  
> +struct vesafb_info
> +{
> +	u32 pseudo_palette[256];
> +	int mtrr_hdl;
> +};
> +
>  /* --------------------------------------------------------------------- */
>  
>  static struct fb_var_screeninfo vesafb_defined __initdata = {
> @@ -47,16 +53,37 @@ static struct fb_fix_screeninfo vesafb_fix __initdata = {
>  	.accel	= FB_ACCEL_NONE,
>  };
>  
> +#ifndef MODULE
>  static int   inverse    __read_mostly;
> +#endif
>  static int   mtrr       __read_mostly;		/* disable mtrr */
>  static int   vram_remap __initdata;		/* Set amount of memory to be used */
>  static int   vram_total __initdata;		/* Set total amount of memory */
>  static int   pmi_setpal __read_mostly = 1;	/* pmi for palette changes ??? */
> +static int	redraw		__read_mostly;
>  static int   ypan       __read_mostly;		/* 0..nothing, 1..ypan, 2..ywrap */
> +static int	ywrap		__read_mostly;
>  static void  (*pmi_start)(void) __read_mostly;
>  static void  (*pmi_pal)  (void) __read_mostly;
>  static int   depth      __read_mostly;
>  static int   vga_compat __read_mostly;
> +
> +module_param(redraw, bool, 0);
> +module_param(ypan, bool, 0);
> +module_param(ywrap, bool, 0);
> +module_param_named(vgapal, pmi_setpal, invbool, 0);
> +MODULE_PARM_DESC(vgapal, "Use VGA for setting palette (default)");
> +module_param_named(pmipal, pmi_setpal, bool, 0);
> +MODULE_PARM_DESC(pmipal, "Use PMI for setting palette");
> +module_param(mtrr, bool, 0);
> +MODULE_PARM_DESC(mtrr, "Enable MTRR support (default)");
> +module_param_named(nomtrr, mtrr, invbool, 0);
> +MODULE_PARM_DESC(nomtrr, "Disable MTRR support");
> +module_param(vram_remap, int, 0);
> +MODULE_PARM_DESC(vram_remap, "Set total amount of memory to be used");
> +module_param(vram_total, int, 0);
> +MODULE_PARM_DESC(vram_total, "Total amount of memory");
> +
>  /* --------------------------------------------------------------------- */
>  
>  static int vesafb_pan_display(struct fb_var_screeninfo *var,
> @@ -192,6 +219,7 @@ static struct fb_ops vesafb_ops = {
>  	.fb_imageblit	= cfb_imageblit,
>  };
>  
> +#ifndef MODULE
>  static int __init vesafb_setup(char *options)
>  {
>  	char *this_opt;
> @@ -225,6 +253,7 @@ static int __init vesafb_setup(char *options)
>  	}
>  	return 0;
>  }
> +#endif
>  
>  static int __init vesafb_probe(struct platform_device *dev)
>  {
> @@ -476,8 +505,28 @@ err:
>  	return err;
>  }
>  
> +static int __exit vesafb_remove(struct platform_device *device)
> +{
> +	struct fb_info *info = dev_get_drvdata(&device->dev);
> +
> +	unregister_framebuffer(info);
> +#ifdef CONFIG_MTRR
> +	{
> +		struct vesafb_info *vfb_info = (struct vesafb_info *) info->par;
> +		if (vfb_info->mtrr_hdl >= 0)
> +			mtrr_del(vfb_info->mtrr_hdl, 0, 0);
> +	}
> +#endif
> +	iounmap(info->screen_base);
> +	framebuffer_release(info);
> +	release_mem_region(vesafb_fix.smem_start, vesafb_fix.smem_len);
> +
> +	return 0;
> +}
> +
>  static struct platform_driver vesafb_driver = {
>  	.probe	= vesafb_probe,
> +	.remove = vesafb_remove,
>  	.driver	= {
>  		.name	= "vesafb",
>  	},
> @@ -488,11 +537,18 @@ static struct platform_device *vesafb_device;
>  static int __init vesafb_init(void)
>  {
>  	int ret;
> +#ifndef MODULE
>  	char *option = NULL;
>  
>  	/* ignore error return of fb_get_options */
>  	fb_get_options("vesafb", &option);
>  	vesafb_setup(option);
> +#else
> +	if (redraw)
> +		ypan = 0;
> +	if (ywrap)
> +		ypan = 2;
> +#endif
>  	ret = platform_driver_register(&vesafb_driver);
>  
>  	if (!ret) {
> @@ -511,6 +567,14 @@ static int __init vesafb_init(void)
>  
>  	return ret;
>  }
> +
> +static void __exit vesafb_exit(void)
> +{
> +	platform_device_unregister(vesafb_device);
> +	platform_driver_unregister(&vesafb_driver);
> +}
> +
>  module_init(vesafb_init);
> +module_exit(vesafb_exit);
>  
>  MODULE_LICENSE("GPL");
> 
> 
> 
> 
> --
> 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
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-04-02 13:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-31 21:44 + fbdev-section-cleanup-in-vesafb.patch added to -mm tree akpm
2010-04-02 12:28 ` Henrik Kretzschmar
2010-04-02 12:28   ` Henrik Kretzschmar
2010-04-02 13:03   ` Florian Tobias Schandinat
2010-04-02 13:03     ` Florian Tobias Schandinat

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.