Re: [PATCH 02/12] Staging: sm750fb: sm750_help.h: Insert spaces after commas.

Re: [PATCH 02/12] Staging: sm750fb: sm750_help.h: Insert spaces after commas.

[Sudip Mukherjee]

On Sun, May 31, 2015 at 10:01:03PM -0700, Isaac Assegai wrote:
> Insert Spaces after commas to rectify the
> following checkpatch errors in sm750_help.h:
> ERROR: space required after that ','
> 
> Signed-off-by: Isaac Assegai <isaac.a.travers@gmail.com>
> ---
>  drivers/staging/sm750fb/sm750_help.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750_help.h b/drivers/staging/sm750fb/sm750_help.h
> index e0128d2..3940359 100644
> --- a/drivers/staging/sm750fb/sm750_help.h
> +++ b/drivers/staging/sm750fb/sm750_help.h
> @@ -10,11 +10,11 @@
>  
>  #define RAW_MASK(f)         (0xFFFFFFFF >> (32 - _COUNT(f)))
>  #define GET_MASK(f)         (RAW_MASK(f) << _LSB(f))
> -#define GET_FIELD(d,f)      (((d) >> _LSB(f)) & RAW_MASK(f))
> -#define TEST_FIELD(d,f,v)   (GET_FIELD(d,f) == f ## _ ## v)
> -#define SET_FIELD(d,f,v)    (((d) & ~GET_MASK(f)) | \
> +#define GET_FIELD(d, f)     (((d) >> _LSB(f)) & RAW_MASK(f))
> +#define TEST_FIELD(d, f, v) (GET_FIELD(d,f) == f ## _ ## v)
					  ^^^
i guess you missed this one.

regards
sudip

Re: [PATCH 03/12] Staging: sm750fb: sm750.h: Insert spaces after commas.

[Sudip Mukherjee]

On Sun, May 31, 2015 at 10:01:04PM -0700, Isaac Assegai wrote:
> Insert Spaces after commas to rectify the
> following checkpatch errors in sm750.h:
> ERROR: space required after that ','
> 
> Signed-off-by: Isaac Assegai <isaac.a.travers@gmail.com>
> ---
>  drivers/staging/sm750fb/sm750.h | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> index 5528912..5f86077 100644
> --- a/drivers/staging/sm750fb/sm750.h
> +++ b/drivers/staging/sm750fb/sm750.h
> @@ -12,7 +12,7 @@
>  #define MB(x) ((x)<<20)
>  #define MHZ(x) ((x) * 1000000)
>  /* align should be 2,4,8,16 */
> -#define PADDING(align,data) (((data)+(align)-1)&(~((align) -1)))
> +#define PADDING(align, data) (((data)+(align)-1)&(~((align) -1)))
>  extern int smi_indent;
>  
>  
> @@ -27,15 +27,16 @@ struct lynx_accel{
>  
>  
> -	int (*de_imageblit)(struct lynx_accel *,const char *,u32,u32,u32,
> -						u32,u32,u32,u32,u32,u32,u32,u32,u32);
> +	int (*de_imageblit)(struct lynx_accel *, const char *, u32, u32, u32, u32, 
										^^^^^
oops, trailing whitespace errors :(
> +							       u32, u32, u32, u32, 
										^^^^^
here also
please test your patch with checkpatch before sending

regards
sudip

Re: [PATCH 10/12] Staging: sm750fb: Insert spaces after commas in two files.

[Sudip Mukherjee]

On Sun, May 31, 2015 at 10:01:11PM -0700, Isaac Assegai wrote:
> Insert Spaces after commas to rectify the following
> checkpatch errors in ddk750_help.c and ddk750_mode.c:
> ERROR: space required after that ','
> 
> Signed-off-by: Isaac Assegai <isaac.a.travers@gmail.com>
> ---
>  
<snip>
>  
> -		ulTmpValue = FIELD_VALUE(0,CRT_DISPLAY_CTRL,VSYNC_PHASE,pModeParam->vertical_sync_polarity)|
> -					  FIELD_VALUE(0,CRT_DISPLAY_CTRL,HSYNC_PHASE,pModeParam->horizontal_sync_polarity)|
> -					  FIELD_SET(0,CRT_DISPLAY_CTRL,TIMING,ENABLE)|
> +		ulTmpValue = FIELD_VALUE(0, CRT_DISPLAY_CTRL,VSYNC_PHASE, pModeParam->vertical_sync_polarity)|
							^^^^^^^^
missed this
> +					  FIELD_VALUE(0, CRT_DISPLAY_CTRL, HSYNC_PHASE, pModeParam->horizontal_sync_polarity)|
> +					  FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING,ENABLE)|
									     ^^^^^
this also

regards
sudip

Re: [PATCH 12/12] Staging: sm750fb: Insert spaces after commas in three files.

[Sudip Mukherjee]

On Sun, May 31, 2015 at 10:01:13PM -0700, Isaac Assegai wrote:
> Insert Spaces after commas to rectify the following checkpatch
> errors in ddk750_mode.c, sm750_accel.c and sm750_help.h:
> ERROR: space required after that ','
> 
> Signed-off-by: Isaac Assegai <isaac.a.travers@gmail.com>
> ---
>  drivers/staging/sm750fb/ddk750_mode.c | 46 +++++++++++++++++------------------
>  drivers/staging/sm750fb/sm750_accel.c |  2 +-
>  drivers/staging/sm750fb/sm750_help.h  |  2 +-
>  3 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c
> index 4e252fb..a054747 100644
> --- a/drivers/staging/sm750fb/ddk750_mode.c
> +++ b/drivers/staging/sm750fb/ddk750_mode.c
> @@ -101,29 +101,29 @@ static int programModeRegisters(mode_parameter_t * pModeParam, pll_value_t * pll
>              | FIELD_VALUE(0, CRT_VERTICAL_SYNC, START, pModeParam->vertical_sync_start - 1));
>  
>  
> -		ulTmpValue = FIELD_VALUE(0, CRT_DISPLAY_CTRL,VSYNC_PHASE, pModeParam->vertical_sync_polarity)|
> +		ulTmpValue = FIELD_VALUE(0, CRT_DISPLAY_CTRL, VSYNC_PHASE, pModeParam->vertical_sync_polarity)|
>  					  FIELD_VALUE(0, CRT_DISPLAY_CTRL, HSYNC_PHASE, pModeParam->horizontal_sync_polarity)|
> -					  FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING,ENABLE)|
> -					  FIELD_SET(0,CRT_DISPLAY_CTRL,PLANE,ENABLE);
> +					  FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING, ENABLE)|
> +					  FIELD_SET(0, CRT_DISPLAY_CTRL, PLANE, ENABLE);
I am getting confused now.
This part you have modified in patch 10/12 and i just replied that
you have missed two modifications.

regards
sudip

Re: [PATCH 12/12] Staging: sm750fb: Insert spaces after commas in three files.

[Isaac Assegai]

On Tue, Jun 02, 2015 at 12:18:06PM +0530, Sudip Mukherjee wrote:
> On Sun, May 31, 2015 at 10:01:13PM -0700, Isaac Assegai wrote:
> > Insert Spaces after commas to rectify the following checkpatch
> > errors in ddk750_mode.c, sm750_accel.c and sm750_help.h:
> > ERROR: space required after that ','
> > 
> > Signed-off-by: Isaac Assegai <isaac.a.travers@gmail.com>
> > ---
> >  drivers/staging/sm750fb/ddk750_mode.c | 46 +++++++++++++++++------------------
> >  drivers/staging/sm750fb/sm750_accel.c |  2 +-
> >  drivers/staging/sm750fb/sm750_help.h  |  2 +-
> >  3 files changed, 25 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c
> > index 4e252fb..a054747 100644
> > --- a/drivers/staging/sm750fb/ddk750_mode.c
> > +++ b/drivers/staging/sm750fb/ddk750_mode.c
> > @@ -101,29 +101,29 @@ static int programModeRegisters(mode_parameter_t * pModeParam, pll_value_t * pll
> >              | FIELD_VALUE(0, CRT_VERTICAL_SYNC, START, pModeParam->vertical_sync_start - 1));
> >  
> >  
> > -		ulTmpValue = FIELD_VALUE(0, CRT_DISPLAY_CTRL,VSYNC_PHASE, pModeParam->vertical_sync_polarity)|
> > +		ulTmpValue = FIELD_VALUE(0, CRT_DISPLAY_CTRL, VSYNC_PHASE, pModeParam->vertical_sync_polarity)|
> >  					  FIELD_VALUE(0, CRT_DISPLAY_CTRL, HSYNC_PHASE, pModeParam->horizontal_sync_polarity)|
> > -					  FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING,ENABLE)|
> > -					  FIELD_SET(0,CRT_DISPLAY_CTRL,PLANE,ENABLE);
> > +					  FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING, ENABLE)|
> > +					  FIELD_SET(0, CRT_DISPLAY_CTRL, PLANE, ENABLE);
> I am getting confused now.
> This part you have modified in patch 10/12 and i just replied that
> you have missed two modifications.
> 
> regards
> sudip
Thanks for your feedback Sudip, I have a few questions:
1. After applying the first 11 patches I ran checkpatch again and noticed that
I missed three warnings so I ran a 12th patch fixing them. That's why you
see the 12th patch modifying a file that another patch already touched.
What should be done in this type of situation?

2. On Patch 3 you pointed out two trailing whitespace errors.
I ran checkpatch on all the patches before I sent them to
make sure I didn't introduce any new errors, however
there are *many* warnings and errors showing up from
the poor styling already present and I missed these
whitespace errors in the mess.
Can I make checkpatch suppress any errors that might have already
been present in the code and only show me those issues introduced
by the patch itself? If not, how did you identify it?

Thanks,
Isaac

Re: [PATCH 12/12] Staging: sm750fb: Insert spaces after commas in three files.

[Sudip Mukherjee]

On Tue, Jun 02, 2015 at 12:55:22AM -0700, Isaac Assegai wrote:
> On Tue, Jun 02, 2015 at 12:18:06PM +0530, Sudip Mukherjee wrote:
> > On Sun, May 31, 2015 at 10:01:13PM -0700, Isaac Assegai wrote:
<snip>
> > > +					  FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING, ENABLE)|
> > > +					  FIELD_SET(0, CRT_DISPLAY_CTRL, PLANE, ENABLE);
> > I am getting confused now.
> > This part you have modified in patch 10/12 and i just replied that
> > you have missed two modifications.
> > 
> Thanks for your feedback Sudip, I have a few questions:
> 1. After applying the first 11 patches I ran checkpatch again and noticed that
> I missed three warnings so I ran a 12th patch fixing them. That's why you
> see the 12th patch modifying a file that another patch already touched.
> What should be done in this type of situation?
you have to start from the beginning again. :(
I have faced such similar situation also, but starting for the beginnig
is the only way.
> 
> 2. On Patch 3 you pointed out two trailing whitespace errors.
> I ran checkpatch on all the patches before I sent them to
> make sure I didn't introduce any new errors,
but it looks like they were introduced in your patch.
> -     int (*de_imageblit)(struct lynx_accel *,const char *,u32,u32,u32,
> -                                             u32,u32,u32,u32,u32,u32,u32,u32,u32);
> +     int (*de_imageblit)(struct lynx_accel *, const char *, u32, u32, u32, u32,
the original code was having three u32, i think when you moved one more
u32 from the following line you forgot to remove the space after that.

>however
> there are *many* warnings and errors showing up from
> the poor styling already present and I missed these
> whitespace errors in the mess.
> Can I make checkpatch suppress any errors that might have already
> been present in the code and only show me those issues introduced
> by the patch itself? If not, how did you identify it?
i dont think checkpatch can work like that way.
identification was easy. i saw checkpatch giving error and noticed you
have modified the code. So i checked the original code if trailing
whitespace was there or not.

regards
sudip

Re: [PATCH 12/12] Staging: sm750fb: Insert spaces after commas in three files.

[Isaac Assegai]

On Tue, Jun 02, 2015 at 01:55:56PM +0530, Sudip Mukherjee wrote:
> On Tue, Jun 02, 2015 at 12:55:22AM -0700, Isaac Assegai wrote:
> > On Tue, Jun 02, 2015 at 12:18:06PM +0530, Sudip Mukherjee wrote:
> > > On Sun, May 31, 2015 at 10:01:13PM -0700, Isaac Assegai wrote:
> <snip>
> > > > +					  FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING, ENABLE)|
> > > > +					  FIELD_SET(0, CRT_DISPLAY_CTRL, PLANE, ENABLE);
> > > I am getting confused now.
> > > This part you have modified in patch 10/12 and i just replied that
> > > you have missed two modifications.
> > > 
> > Thanks for your feedback Sudip, I have a few questions:
> > 1. After applying the first 11 patches I ran checkpatch again and noticed that
> > I missed three warnings so I ran a 12th patch fixing them. That's why you
> > see the 12th patch modifying a file that another patch already touched.
> > What should be done in this type of situation?
> you have to start from the beginning again. :(
> I have faced such similar situation also, but starting for the beginnig
> is the only way.
> > 
> > 2. On Patch 3 you pointed out two trailing whitespace errors.
> > I ran checkpatch on all the patches before I sent them to
> > make sure I didn't introduce any new errors,
> but it looks like they were introduced in your patch.
> > -     int (*de_imageblit)(struct lynx_accel *,const char *,u32,u32,u32,
> > -                                             u32,u32,u32,u32,u32,u32,u32,u32,u32);
> > +     int (*de_imageblit)(struct lynx_accel *, const char *, u32, u32, u32, u32,
> the original code was having three u32, i think when you moved one more
> u32 from the following line you forgot to remove the space after that.
> 
> >however
> > there are *many* warnings and errors showing up from
> > the poor styling already present and I missed these
> > whitespace errors in the mess.
> > Can I make checkpatch suppress any errors that might have already
> > been present in the code and only show me those issues introduced
> > by the patch itself? If not, how did you identify it?
> i dont think checkpatch can work like that way.
> identification was easy. i saw checkpatch giving error and noticed you
> have modified the code. So i checked the original code if trailing
> whitespace was there or not.
> 
> regards
> sudip
Okay,

I'll re-run this and send a new version in the morning.

Thanks,
Isaac