| Message ID | 1425836411-15481-3-git-send-email-sudipm.mukherjee@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <sudip@vectorindia.org> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> 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 -- 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
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: > > 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 <sudip@vectorindia.org> > > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> > > 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 > -- 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
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: > > 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 <sudip@vectorindia.org> > > > > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > 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 -- 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
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: > > 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: > > > > 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 <sudip@vectorindia.org> > > > > > > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > > > 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 > -- 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
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: > > > > > > 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: > > > > 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 <sudip@vectorindia.org> > > > > > > > > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > > > > > 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 -- 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
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> > > > 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 > -- 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 --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,
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 <sudip@vectorindia.org> --- drivers/staging/sm7xxfb/sm7xxfb.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)