| Message ID | 1425886506-8643-4-git-send-email-sudipm.mukherjee@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 09, 2015 at 01:05:06PM +0530, Sudip Mukherjee wrote: > kbuild test robot reported that for microblaze-allyesconfig > chan_to_field() and lynxfb_ops_set_par() were not defined. These two > functions were defined under CONFIG_PM, so for any archtecture if > CONFIG_PM is not defined we will have this error. > > while moving the lynxfb_suspend() function some very obvious > checkpatch errors, like space after comma, space after if, space > before opening brace, were taken care of. I have a script to review patches moving functions around but these white space changes break my script so I have to review it by hand. Sucks. > static int lynxfb_ops_set_par(struct fb_info * info) > { > struct lynxfb_par * par; > @@ -369,7 +313,6 @@ static int lynxfb_ops_set_par(struct fb_info * info) > struct fb_fix_screeninfo * fix; > int ret; > unsigned int line_length; > - > > if(!info) > return -EINVAL; > @@ -441,6 +384,7 @@ static int lynxfb_ops_set_par(struct fb_info * info) > ret = output->proc_setMode(output,var,fix); > return ret; > } > + > static inline unsigned int chan_to_field(unsigned int chan,struct fb_bitfield * bf) > { > chan &= 0xffff; These white space changes are not related. 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 03:42:22PM +0300, Dan Carpenter wrote: > On Mon, Mar 09, 2015 at 01:05:06PM +0530, Sudip Mukherjee wrote: > > kbuild test robot reported that for microblaze-allyesconfig > > chan_to_field() and lynxfb_ops_set_par() were not defined. These two > > functions were defined under CONFIG_PM, so for any archtecture if > > CONFIG_PM is not defined we will have this error. > > > > while moving the lynxfb_suspend() function some very obvious > > checkpatch errors, like space after comma, space after if, space > > before opening brace, were taken care of. > > I have a script to review patches moving functions around but these > white space changes break my script so I have to review it by hand. > Sucks. oops . sorry .. > > > static int lynxfb_ops_set_par(struct fb_info * info) <snip> > > static inline unsigned int chan_to_field(unsigned int chan,struct fb_bitfield * bf) > > { > > chan &= 0xffff; > > These white space changes are not related. if you want i can break it into multiple patches, so that reviewing can be easy and your script will not break :) . Actually I thought, since this is a vendor crude driver there will be many such changes, so if i can combine some changes together then atleast the number of patches can be kept low and also i thought of clubbing these changes together as Joe Perches once told me "Don't get carried away with patch type separation" (reference: https://lkml.org/lkml/2015/1/1/2). 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 06:25:03PM +0530, Sudip Mukherjee wrote:
> Actually I thought, since this is a vendor crude driver there will be many such changes, so if i can combine some changes together then atleast the number of patches can be kept low and also i thought of clubbing these changes together as Joe Perches once told me "Don't get carried away with patch type separation" (reference: https://lkml.org/lkml/2015/1/1/2).
How to break up patches is more art than science. I wouldn't have
complained about the minor extra white space changes except that I was
already going to email you about breaking my scripts.
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/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c index 753869e..476dc5c 100644 --- a/drivers/staging/sm750fb/sm750.c +++ b/drivers/staging/sm750fb/sm750.c @@ -303,62 +303,6 @@ static int lynxfb_ops_pan_display(struct fb_var_screeninfo *var, return ret; } - - - -#ifdef CONFIG_PM -static int lynxfb_suspend(struct pci_dev * pdev,pm_message_t mesg) -{ - struct fb_info * info; - struct lynx_share * share; - int ret; - - - if(mesg.event == pdev->dev.power.power_state.event) - return 0; - - ret = 0; - share = pci_get_drvdata(pdev); - switch (mesg.event) { - case PM_EVENT_FREEZE: - case PM_EVENT_PRETHAW: - pdev->dev.power.power_state = mesg; - return 0; - } - - console_lock(); - if (mesg.event & PM_EVENT_SLEEP) { - info = share->fbinfo[0]; - if(info) - fb_set_suspend(info, 1);/* 1 means do suspend*/ - - info = share->fbinfo[1]; - if(info) - fb_set_suspend(info, 1);/* 1 means do suspend*/ - - ret = pci_save_state(pdev); - if(ret){ - pr_err("error:%d occured in pci_save_state\n",ret); - return ret; - } - - /* set chip to sleep mode */ - if(share->suspend) - (*share->suspend)(share); - - pci_disable_device(pdev); - ret = pci_set_power_state(pdev,pci_choose_state(pdev,mesg)); - if(ret){ - pr_err("error:%d occured in pci_set_power_state\n",ret); - return ret; - } - } - - pdev->dev.power.power_state = mesg; - console_unlock(); - return ret; -} - static int lynxfb_ops_set_par(struct fb_info * info) { struct lynxfb_par * par; @@ -369,7 +313,6 @@ static int lynxfb_ops_set_par(struct fb_info * info) struct fb_fix_screeninfo * fix; int ret; unsigned int line_length; - if(!info) return -EINVAL; @@ -441,6 +384,7 @@ static int lynxfb_ops_set_par(struct fb_info * info) ret = output->proc_setMode(output,var,fix); return ret; } + static inline unsigned int chan_to_field(unsigned int chan,struct fb_bitfield * bf) { chan &= 0xffff; @@ -448,6 +392,58 @@ static inline unsigned int chan_to_field(unsigned int chan,struct fb_bitfield * return chan << bf->offset; } +#ifdef CONFIG_PM +static int lynxfb_suspend(struct pci_dev *pdev, pm_message_t mesg) +{ + struct fb_info *info; + struct lynx_share *share; + int ret; + + if (mesg.event == pdev->dev.power.power_state.event) + return 0; + + ret = 0; + share = pci_get_drvdata(pdev); + switch (mesg.event) { + case PM_EVENT_FREEZE: + case PM_EVENT_PRETHAW: + pdev->dev.power.power_state = mesg; + return 0; + } + + console_lock(); + if (mesg.event & PM_EVENT_SLEEP) { + info = share->fbinfo[0]; + if (info) + /* 1 means do suspend*/ + fb_set_suspend(info, 1); + info = share->fbinfo[1]; + if (info) + /* 1 means do suspend*/ + fb_set_suspend(info, 1); + + ret = pci_save_state(pdev); + if (ret) { + pr_err("error:%d occurred in pci_save_state\n", ret); + return ret; + } + + /* set chip to sleep mode*/ + if (share->suspend) + (*share->suspend)(share); + + pci_disable_device(pdev); + ret = pci_set_power_state(pdev, pci_choose_state(pdev, mesg)); + if (ret) { + pr_err("error:%d occurred in pci_set_power_state\n", ret); + return ret; + } + } + + pdev->dev.power.power_state = mesg; + console_unlock(); + return ret; +} static int lynxfb_resume(struct pci_dev* pdev) {
kbuild test robot reported that for microblaze-allyesconfig chan_to_field() and lynxfb_ops_set_par() were not defined. These two functions were defined under CONFIG_PM, so for any archtecture if CONFIG_PM is not defined we will have this error. while moving the lynxfb_suspend() function some very obvious checkpatch errors, like space after comma, space after if, space before opening brace, were taken care of. Reported-by: kbuild test robot <fengguang.wu@intel.com> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> --- v2: some extra spaces in the comments were removed. drivers/staging/sm750fb/sm750.c | 110 +++++++++++++++++++--------------------- 1 file changed, 53 insertions(+), 57 deletions(-)