# Re: \[PATCH RESEND 1/5\] staging: sm750fb: Use memset\_io instead of memset - List: linux-fbdev - 7 messages, 4 authors - Page: https://ratatoskr.run/linux-fbdev/2015/03/4453688/t ## Re: \[PATCH RESEND 1/5\] staging: sm750fb: Use memset\_io instead of memset - From: Vitaly Kuznetsov \ - Date: 2015-03-18 10:50:35 - Message: https://ratatoskr.run/linux-fbdev/2015/03/4453688.md ```text Lorenzo Stoakes [ref] writes: > This patch uses memset_io instead of memset when using memset on __iomem > qualified pointers. This fixes the following sparse warnings:- > > drivers/staging/sm750fb/sm750.c:489:17: warning: incorrect type in argument 1 (different address spaces) > drivers/staging/sm750fb/sm750.c:490:17: warning: incorrect type in argument 1 (different address spaces) > drivers/staging/sm750fb/sm750.c:501:17: warning: incorrect type in argument 1 (different address spaces) > drivers/staging/sm750fb/sm750.c:502:17: warning: incorrect type in argument 1 (different address spaces) > drivers/staging/sm750fb/sm750.c:833:5: warning: incorrect type in argument 1 (different address spaces) > drivers/staging/sm750fb/sm750.c:1154:9: warning: incorrect type in argument 1 (different address spaces) > > Signed-off-by: Lorenzo Stoakes > --- > drivers/staging/sm750fb/sm750.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c > index aa0888c..3e36b6a 100644 > --- a/drivers/staging/sm750fb/sm750.c > +++ b/drivers/staging/sm750fb/sm750.c > @@ -486,8 +486,8 @@ static int lynxfb_resume(struct pci_dev* pdev) > par = info->par; > crtc = &par->crtc; > cursor = &crtc->cursor; > - memset(cursor->vstart, 0x0, cursor->size); > - memset(crtc->vScreen,0x0,crtc->vidmem_size); > + memset_io(cursor->vstart, 0x0, cursor->size); > + memset_io(crtc->vScreen,0x0,crtc->vidmem_size); ERROR is reported by scripts/checkpatch.pl (spaces are missing after ','). This coding style problem was there before your patch but I don't think it makes sense to preserve it. > lynxfb_ops_set_par(info); > fb_set_suspend(info, 0); > } > @@ -498,8 +498,8 @@ static int lynxfb_resume(struct pci_dev* pdev) > par = info->par; > crtc = &par->crtc; > cursor = &crtc->cursor; > - memset(cursor->vstart, 0x0, cursor->size); > - memset(crtc->vScreen,0x0,crtc->vidmem_size); > + memset_io(cursor->vstart, 0x0, cursor->size); > + memset_io(crtc->vScreen,0x0,crtc->vidmem_size); the same > lynxfb_ops_set_par(info); > fb_set_suspend(info, 0); > } > @@ -830,7 +830,7 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index) > > crtc->cursor.share = share; > - memset(crtc->cursor.vstart, 0, crtc->cursor.size); > + memset_io(crtc->cursor.vstart, 0, crtc->cursor.size); WARNING: please, no spaces at the start of a line #137: FILE: drivers/staging/sm750fb/sm750.c:833: + memset_io(crtc->cursor.vstart, 0, crtc->cursor.size);$ > if(!g_hwcursor){ > lynxfb_ops.fb_cursor = NULL; > crtc->cursor.disable(&crtc->cursor); > @@ -1151,7 +1151,7 @@ static int lynxfb_pci_probe(struct pci_dev * pdev, > } > #endif > > - memset(share->pvMem,0,share->vidmem_size); > + memset_io(share->pvMem,0,share->vidmem_size); the same missing spaces > > pr_info("sm%3x mmio address = %p\n",share->devid,share->pvReg); > > -- > 2.3.2 > _______________________________________________ > devel mailing list > devel@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel -- Vitaly ``` ## Re: \[PATCH RESEND 1/5\] staging: sm750fb: Use memset\_io instead of memset - From: Lorenzo Stoakes \ - Date: 2015-03-18 11:12:43 - Message: https://ratatoskr.run/linux-fbdev/2015/03/4453707.md ```text On 18 March 2015 at 10:50, Vitaly Kuznetsov [ref] wrote: > ERROR is reported by scripts/checkpatch.pl (spaces are missing after > ','). This coding style problem was there before your patch but I don't > think it makes sense to preserve it. [snip] > WARNING: please, no spaces at the start of a line > #137: FILE: drivers/staging/sm750fb/sm750.c:833: [snip] Hi Vitaly, these style issues have vexed me and I was not sure whether to make changes or preserve all the obvious errors so as not to blend the two changes inappropriately, however it does indeed make sense to fix these on the lines I'm changing, will fix these! Best, -- Lorenzo Stoakes https:/ljs.io ``` ## Re: \[PATCH RESEND 1/5\] staging: sm750fb: Use memset\_io instead of memset - From: Dan Carpenter \ - Date: 2015-03-18 11:25:23 - Message: https://ratatoskr.run/linux-fbdev/2015/03/4453714.md ```text On Wed, Mar 18, 2015 at 11:12:20AM +0000, Lorenzo Stoakes wrote: > On 18 March 2015 at 10:50, Vitaly Kuznetsov [ref] wrote: > > ERROR is reported by scripts/checkpatch.pl (spaces are missing after > > ','). This coding style problem was there before your patch but I don't > > think it makes sense to preserve it. > > [snip] > > > WARNING: please, no spaces at the start of a line > > #137: FILE: drivers/staging/sm750fb/sm750.c:833: > > [snip] > > Hi Vitaly, these style issues have vexed me and I was not sure whether > to make changes or preserve all the obvious errors so as not to blend > the two changes inappropriately, however it does indeed make sense to > fix these on the lines I'm changing, will fix these! If it's a white space thing on the same line then it's generally ok to fix it. The "one thing per patch" is meant to make patches easier to review. If it's a trivial thing and it doesn't make it harder to review then we are reasonable people. Could you read your patches again and find other similar white space issues. + void __iomem * pbuffer,*pstart; Should be: + void __iomem *pbuffer, *pstart; regards, dan carpenter ``` ## Re: \[PATCH RESEND 1/5\] staging: sm750fb: Use memset\_io instead of memset - From: Sudip Mukherjee \ - Date: 2015-03-18 13:06:17 - Message: https://ratatoskr.run/linux-fbdev/2015/03/4453784.md ```text On Wed, Mar 18, 2015 at 02:25:09PM +0300, Dan Carpenter wrote: > On Wed, Mar 18, 2015 at 11:12:20AM +0000, Lorenzo Stoakes wrote: > > On 18 March 2015 at 10:50, Vitaly Kuznetsov [ref] wrote: > If it's a white space thing on the same line then it's generally ok to > fix it. The "one thing per patch" is meant to make patches easier to > review. If it's a trivial thing and it doesn't make it harder to review > then we are reasonable people. > but Greg K-H has explisitely mentiond not to do so. I did just that and fixed few whitespace things in the patch to fix the build failure. https://lkml.org/lkml/2015/3/10/685 regards sudip ``` ## Re: \[PATCH RESEND 1/5\] staging: sm750fb: Use memset\_io instead of memset - From: Dan Carpenter \ - Date: 2015-03-18 13:24:04 - Message: https://ratatoskr.run/linux-fbdev/2015/03/4453807.md ```text On Wed, Mar 18, 2015 at 06:36:07PM +0530, Sudip Mukherjee wrote: > On Wed, Mar 18, 2015 at 02:25:09PM +0300, Dan Carpenter wrote: > > On Wed, Mar 18, 2015 at 11:12:20AM +0000, Lorenzo Stoakes wrote: > > > On 18 March 2015 at 10:50, Vitaly Kuznetsov [ref] wrote: > > If it's a white space thing on the same line then it's generally ok to > > fix it. The "one thing per patch" is meant to make patches easier to > > review. If it's a trivial thing and it doesn't make it harder to review > > then we are reasonable people. > > > but Greg K-H has explisitely mentiond not to do so. > I did just that and fixed few whitespace things in the patch to fix the > build failure. > > https://lkml.org/lkml/2015/3/10/685 > You were making random white space changes and not on the same line. It was hard to review because you had to count how many u32 arguments there were (a million) and really look at it to see what the compile warning was. There was no compile warning in the end. Very annoying. regards, dan carpenter ``` ## Re: \[PATCH RESEND 1/5\] staging: sm750fb: Use memset\_io instead of memset - From: Sudip Mukherjee \ - Date: 2015-03-18 13:29:54 - Message: https://ratatoskr.run/linux-fbdev/2015/03/4453812.md ```text On Wed, Mar 18, 2015 at 04:23:39PM +0300, Dan Carpenter wrote: > > https://lkml.org/lkml/2015/3/10/685 > > > > You were making random white space changes and not on the same line. It > was hard to review because you had to count how many u32 arguments there > were (a million) and really look at it to see what the compile warning > was. There was no compile warning in the end. Very annoying. ok, now understood. Thanks. like you said once - combining different changes in a single patch is an art. :) regards sudip > > regards, > dan carpenter > ``` ## Re: \[PATCH RESEND 1/5\] staging: sm750fb: Use memset\_io instead of memset - From: Lorenzo Stoakes \ - Date: 2015-03-18 19:10:27 - Message: https://ratatoskr.run/linux-fbdev/2015/03/4454188.md ```text On 18 March 2015 at 11:25, Dan Carpenter [ref] wrote: > Could you read your patches again and find other similar white space > issues. Done. Best, -- Lorenzo Stoakes https:/ljs.io ```