Thread (9 messages) 9 messages, 2 authors, 2015-03-11

Re: [PATCH 3/3] staging: sm7xxfb: change return of sm7xx_vga_setup

From: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Date: 2015-03-09 13:53:52
Also in: linux-fbdev

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help