[PATCH 1/3] staging: sm7xxfb: use module init and exit

use module_init() and module_exit() instead of module_pci_driver
and at the same time make way for use of kernel boot parameters.

Signed-off-by: Sudip Mukherjee <redacted>
---
 drivers/staging/sm7xxfb/sm7xxfb.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c b/drivers/staging/sm7xxfb/sm7xxfb.c
index ebd9536..17b3917 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c

@@ -113,6 +113,8 @@ static struct vesa_mode vesa_mode_table[] = {
 
 static struct screen_info smtc_scr_info;
 
+static char *mode_option;
+
 /* process command line options, get vga parameter */
 static int __init sm7xx_vga_setup(char *options)
 {

@@ -1017,7 +1019,29 @@ static struct pci_driver smtcfb_driver = {
 	.driver.pm  = SM7XX_PM_OPS,
 };
 
-module_pci_driver(smtcfb_driver);
+static int __init sm712fb_init(void)
+{
+#ifndef MODULE
+	char *option = NULL;
+
+	if (fb_get_options("sm712fb", &option))
+		return -ENODEV;
+	if (option && *option)
+		mode_option = option;
+#endif
+	sm7xx_vga_setup(mode_option);
+
+	return pci_register_driver(&smtcfb_driver);
+}
+
+module_init(sm712fb_init);
+
+static void __exit sm712fb_exit(void)
+{
+	pci_unregister_driver(&smtcfb_driver);
+}
+
+module_exit(sm712fb_exit);
 
 MODULE_AUTHOR("Siliconmotion ");
 MODULE_DESCRIPTION("Framebuffer driver for SMI Graphic Cards");

-- 
1.9.1

change return type of sm7xx_vga_setup() to void as we are not using
the return type anywhere. if this function fails we fall back to the
default configuration.

Signed-off-by: Sudip Mukherjee <redacted>
---
 drivers/staging/sm7xxfb/sm7xxfb.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c b/drivers/staging/sm7xxfb/sm7xxfb.c
index e93563f..abdb021 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c

@@ -116,12 +116,12 @@ static struct screen_info smtc_scr_info;
 static char *mode_option;
 
 /* process command line options, get vga parameter */
-static int __init sm7xx_vga_setup(char *options)
+static void __init sm7xx_vga_setup(char *options)
 {
 	int i;
 
 	if (!options || !*options)
-		return -EINVAL;
+		return;
 
 	smtc_scr_info.lfb_width = 0;
 	smtc_scr_info.lfb_height = 0;

@@ -135,11 +135,9 @@ static int __init sm7xx_vga_setup(char *options)
 			smtc_scr_info.lfb_height  						vesa_mode_table[i].lfb_height;
 			smtc_scr_info.lfb_depth  = vesa_mode_table[i].lfb_depth;
-			return 0;
+			return;
 		}
 	}
-
-	return -1;
 }
 
 static void sm712_setpalette(int regno, unsigned red, unsigned green,

-- 
1.9.1

as we start to use kernel boot parameters and fb_get_options()
we donot need to use __setup() any more.

Signed-off-by: Sudip Mukherjee <redacted>
---
 drivers/staging/sm7xxfb/sm7xxfb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c b/drivers/staging/sm7xxfb/sm7xxfb.c
index 17b3917..e93563f 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c

@@ -141,7 +141,6 @@ static int __init sm7xx_vga_setup(char *options)
 
 	return -1;
 }
-__setup("vga=", sm7xx_vga_setup);
 
 static void sm712_setpalette(int regno, unsigned red, unsigned green,
 			     unsigned blue, struct fb_info *info)

-- 
1.9.1

On Sun, Mar 08, 2015 at 11:10:11PM +0530, Sudip Mukherjee wrote:

change return type of sm7xx_vga_setup() to void as we are not using
the return type anywhere. if this function fails we fall back to the
default configuration.

Signed-off-by: Sudip Mukherjee <redacted>

Reviewed-by: Dan Carpenter <redacted>

This patch is fine but if you wanted you could send a follow on which
set it to SCREEN_X_RES, SCREEN_Y_RES, SCREEN_BPP if there were no
options.  Then remove the "if (smtc_scr_info.lfb_width != 0) " condition
in smtcfb_pci_probe().

regards,
dan carpenter

On Mon, Mar 09, 2015 at 04:13:43PM +0300, Dan Carpenter wrote:

On Sun, Mar 08, 2015 at 11:10:11PM +0530, Sudip Mukherjee wrote:

quoted

change return type of sm7xx_vga_setup() to void as we are not using
the return type anywhere. if this function fails we fall back to the
default configuration.

Signed-off-by: Sudip Mukherjee <redacted>

Reviewed-by: Dan Carpenter <redacted>

This patch is fine but if you wanted you could send a follow on which
set it to SCREEN_X_RES, SCREEN_Y_RES, SCREEN_BPP if there were no
options.  Then remove the "if (smtc_scr_info.lfb_width != 0) " condition
in smtcfb_pci_probe().

yes, i could have. But actually I am thinking to totally remove this function and use more conventional fb_get_mode() in its places. But if i do so then the boot parameters will change. Currently it is like vga=sm712fb:0x301 but if i change it to use fb_get_mode() then the boot parameters will change to vga=sm712fb:640x480@60-8 , and that is why I am hesitating. But it will be more conventional to what all framebuffer drivers follows.

regards
sudip

regards,
dan carpenter

On Mon, Mar 09, 2015 at 07:00:33PM +0530, Sudip Mukherjee wrote:

On Mon, Mar 09, 2015 at 04:13:43PM +0300, Dan Carpenter wrote:

quoted

On Sun, Mar 08, 2015 at 11:10:11PM +0530, Sudip Mukherjee wrote:

quoted

change return type of sm7xx_vga_setup() to void as we are not using
the return type anywhere. if this function fails we fall back to the
default configuration.

Signed-off-by: Sudip Mukherjee <redacted>

Reviewed-by: Dan Carpenter <redacted>

This patch is fine but if you wanted you could send a follow on which
set it to SCREEN_X_RES, SCREEN_Y_RES, SCREEN_BPP if there were no
options.  Then remove the "if (smtc_scr_info.lfb_width != 0) " condition
in smtcfb_pci_probe().

yes, i could have. But actually I am thinking to totally remove this function and use more conventional fb_get_mode() in its places. But if i do so then the boot parameters will change. Currently it is like vga=sm712fb:0x301 but if i change it to use fb_get_mode() then the boot parameters will change to vga=sm712fb:640x480@60-8 , and that is why I am hesitating. But it will be more conventional to what all framebuffer drivers follows.

Hm...  That's a good question.

I suspect we should just go with fb_get_mode() and do it quickly.  This
driver is so new in the kernel that there are no users to get upset.

regards,
dan carpenter

On Mon, Mar 09, 2015 at 04:38:05PM +0300, Dan Carpenter wrote:

On Mon, Mar 09, 2015 at 07:00:33PM +0530, Sudip Mukherjee wrote:

quoted

On Mon, Mar 09, 2015 at 04:13:43PM +0300, Dan Carpenter wrote:

quoted

On Sun, Mar 08, 2015 at 11:10:11PM +0530, Sudip Mukherjee wrote:

quoted

change return type of sm7xx_vga_setup() to void as we are not using
the return type anywhere. if this function fails we fall back to the
default configuration.

Signed-off-by: Sudip Mukherjee <redacted>

Reviewed-by: Dan Carpenter <redacted>

This patch is fine but if you wanted you could send a follow on which
set it to SCREEN_X_RES, SCREEN_Y_RES, SCREEN_BPP if there were no
options.  Then remove the "if (smtc_scr_info.lfb_width != 0) " condition
in smtcfb_pci_probe().

yes, i could have. But actually I am thinking to totally remove this function and use more conventional fb_get_mode() in its places. But if i do so then the boot parameters will change. Currently it is like vga=sm712fb:0x301 but if i change it to use fb_get_mode() then the boot parameters will change to vga=sm712fb:640x480@60-8 , and that is why I am hesitating. But it will be more conventional to what all framebuffer drivers follows.

Hm...  That's a good question.

I suspect we should just go with fb_get_mode() and do it quickly.  This
driver is so new in the kernel that there are no users to get upset.

i am afraid that is not correct. this driver was in staging from long back, and Greg removed it from staging as no one was working with it. But after that removal Lemote Yeeloong laptop of Steven Rostedt stopped working. (https://lkml.org/lkml/2014/11/20/698) , so i guess there will be many such users.

regards
sudip

regards,
dan carpenter

On Mon, Mar 09, 2015 at 07:23:43PM +0530, Sudip Mukherjee wrote:

On Mon, Mar 09, 2015 at 04:38:05PM +0300, Dan Carpenter wrote:

quoted

On Mon, Mar 09, 2015 at 07:00:33PM +0530, Sudip Mukherjee wrote:

quoted

On Mon, Mar 09, 2015 at 04:13:43PM +0300, Dan Carpenter wrote:

quoted

On Sun, Mar 08, 2015 at 11:10:11PM +0530, Sudip Mukherjee wrote:

quoted

change return type of sm7xx_vga_setup() to void as we are not using
the return type anywhere. if this function fails we fall back to the
default configuration.

Signed-off-by: Sudip Mukherjee <redacted>

Reviewed-by: Dan Carpenter <redacted>

This patch is fine but if you wanted you could send a follow on which
set it to SCREEN_X_RES, SCREEN_Y_RES, SCREEN_BPP if there were no
options.  Then remove the "if (smtc_scr_info.lfb_width != 0) " condition
in smtcfb_pci_probe().

yes, i could have. But actually I am thinking to totally remove this function and use more conventional fb_get_mode() in its places. But if i do so then the boot parameters will change. Currently it is like vga=sm712fb:0x301 but if i change it to use fb_get_mode() then the boot parameters will change to vga=sm712fb:640x480@60-8 , and that is why I am hesitating. But it will be more conventional to what all framebuffer drivers follows.

Hm...  That's a good question.

I suspect we should just go with fb_get_mode() and do it quickly.  This
driver is so new in the kernel that there are no users to get upset.

i am afraid that is not correct. this driver was in staging from long
back, and Greg removed it from staging as no one was working with it.
But after that removal Lemote Yeeloong laptop of Steven Rostedt
stopped working. (https://lkml.org/lkml/2014/11/20/698) , so i guess
there will be many such users.

Hm...  I suspect we should still change it so it matches the other FB
drivers but I don't know.

Perhaps Greg has an opinion?

regards,
dan carpenter

On Mon, Mar 09, 2015 at 05:04:14PM +0300, Dan Carpenter wrote:

On Mon, Mar 09, 2015 at 07:23:43PM +0530, Sudip Mukherjee wrote:

<snip>

quoted

quoted

Hm...  That's a good question.

I suspect we should just go with fb_get_mode() and do it quickly.  This
driver is so new in the kernel that there are no users to get upset.

i am afraid that is not correct. this driver was in staging from long
back, and Greg removed it from staging as no one was working with it.
But after that removal Lemote Yeeloong laptop of Steven Rostedt
stopped working. (https://lkml.org/lkml/2014/11/20/698) , so i guess
there will be many such users.

Hm...  I suspect we should still change it so it matches the other FB
drivers but I don't know.

Perhaps Greg has an opinion?

maybe, i should make the change and then we can request Steven Rostedt
to test it on his Lemote Yeeloong. if nothing breaks, then alright,
else revert the patch.

regards
sudip

regards,
dan carpenter