Re: [Outreachy kernel] [PATCH] staging: fbtft: fix spacing errors

[Jes Sorensen <jes.sorensen@gmail.com>]

On 02/17/15 12:48, Aya Mahfouz wrote:
> This patch fixes the following checkpatch.pl errors for
> the file fb_bd663474.c:
> 
> ERROR: space prohibited before that close parenthesis ')'
> ERROR: space prohibited after that open parenthesis '('
> ERROR: space required after that ','
> 
> Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> ---
> v1: I know that typically one patch should fix one type of
> checkpatch.pl error/warning but given that they are all 
> spacing issues, I merged them into one patch.
> 
>  drivers/staging/fbtft/fb_bd663474.c | 100 ++++++++++++++++++------------------
>  1 file changed, 50 insertions(+), 50 deletions(-)

The patch looks good - your reasoning for merging the patch is sound too
since you would otherwise have to modify the same lines 2-3 times.

The only issue is that you shouldn't put comments below the
signed-off-by line. The comment you added about merging the 3 rounds of
changes into one is valid, but I would do that in a cover letter, if you
don't feel that is relevant to go into the commit message.
Using '--cover-letter' with git format-patch will generate that for you.

Cheers,
Jes


> diff --git a/drivers/staging/fbtft/fb_bd663474.c b/drivers/staging/fbtft/fb_bd663474.c
> index 7e00c60..17a2162 100644
> --- a/drivers/staging/fbtft/fb_bd663474.c
> +++ b/drivers/staging/fbtft/fb_bd663474.c
> @@ -47,49 +47,49 @@ static int init_display(struct fbtft_par *par)
>  	/* Initialization sequence from Lib_UTFT */
>  
>  	/* oscillator start */
> -	write_reg(par, 0x000,0x0001);	/*oscillator 0: stop, 1: operation */
> +	write_reg(par, 0x000, 0x0001);	/*oscillator 0: stop, 1: operation */
>  	mdelay(10);
>  
>  	/* Power settings */
> -	write_reg(par, 0x100, 0x0000 ); /* power supply setup */
> -	write_reg(par, 0x101, 0x0000 );
> -	write_reg(par, 0x102, 0x3110 );
> -	write_reg(par, 0x103, 0xe200 );
> -	write_reg(par, 0x110, 0x009d );
> -	write_reg(par, 0x111, 0x0022 );
> -	write_reg(par, 0x100, 0x0120 );
> -	mdelay( 20 );
> -
> -	write_reg(par, 0x100, 0x3120 );
> -	mdelay( 80 );
> +	write_reg(par, 0x100, 0x0000); /* power supply setup */
> +	write_reg(par, 0x101, 0x0000);
> +	write_reg(par, 0x102, 0x3110);
> +	write_reg(par, 0x103, 0xe200);
> +	write_reg(par, 0x110, 0x009d);
> +	write_reg(par, 0x111, 0x0022);
> +	write_reg(par, 0x100, 0x0120);
> +	mdelay(20);
> +
> +	write_reg(par, 0x100, 0x3120);
> +	mdelay(80);
>  	/* Display control */
> -	write_reg(par, 0x001, 0x0100 );
> -	write_reg(par, 0x002, 0x0000 );
> -	write_reg(par, 0x003, 0x1230 );
> -	write_reg(par, 0x006, 0x0000 );
> -	write_reg(par, 0x007, 0x0101 );
> -	write_reg(par, 0x008, 0x0808 );
> -	write_reg(par, 0x009, 0x0000 );
> -	write_reg(par, 0x00b, 0x0000 );
> -	write_reg(par, 0x00c, 0x0000 );
> -	write_reg(par, 0x00d, 0x0018 );
> +	write_reg(par, 0x001, 0x0100);
> +	write_reg(par, 0x002, 0x0000);
> +	write_reg(par, 0x003, 0x1230);
> +	write_reg(par, 0x006, 0x0000);
> +	write_reg(par, 0x007, 0x0101);
> +	write_reg(par, 0x008, 0x0808);
> +	write_reg(par, 0x009, 0x0000);
> +	write_reg(par, 0x00b, 0x0000);
> +	write_reg(par, 0x00c, 0x0000);
> +	write_reg(par, 0x00d, 0x0018);
>  	/* LTPS control settings */
> -	write_reg(par, 0x012, 0x0000 );
> -	write_reg(par, 0x013, 0x0000 );
> -	write_reg(par, 0x018, 0x0000 );
> -	write_reg(par, 0x019, 0x0000 );
> -
> -	write_reg(par, 0x203, 0x0000 );
> -	write_reg(par, 0x204, 0x0000 );
> -
> -	write_reg(par, 0x210, 0x0000 );
> -	write_reg(par, 0x211, 0x00ef );
> -	write_reg(par, 0x212, 0x0000 );
> -	write_reg(par, 0x213, 0x013f );
> -	write_reg(par, 0x214, 0x0000 );
> -	write_reg(par, 0x215, 0x0000 );
> -	write_reg(par, 0x216, 0x0000 );
> -	write_reg(par, 0x217, 0x0000 );
> +	write_reg(par, 0x012, 0x0000);
> +	write_reg(par, 0x013, 0x0000);
> +	write_reg(par, 0x018, 0x0000);
> +	write_reg(par, 0x019, 0x0000);
> +
> +	write_reg(par, 0x203, 0x0000);
> +	write_reg(par, 0x204, 0x0000);
> +
> +	write_reg(par, 0x210, 0x0000);
> +	write_reg(par, 0x211, 0x00ef);
> +	write_reg(par, 0x212, 0x0000);
> +	write_reg(par, 0x213, 0x013f);
> +	write_reg(par, 0x214, 0x0000);
> +	write_reg(par, 0x215, 0x0000);
> +	write_reg(par, 0x216, 0x0000);
> +	write_reg(par, 0x217, 0x0000);
>  
>  	/* Gray scale settings */
>  	write_reg(par, 0x300, 0x5343);
> @@ -104,18 +104,18 @@ static int init_display(struct fbtft_par *par)
>  	write_reg(par, 0x309, 0x050a);
>  
>  	/* RAM access settings */
> -	write_reg(par, 0x400, 0x4027 );
> -	write_reg(par, 0x401, 0x0000 );
> -	write_reg(par, 0x402, 0x0000 );  /* First screen drive position (1) */
> -	write_reg(par, 0x403, 0x013f );  /* First screen drive position (2) */
> -	write_reg(par, 0x404, 0x0000 );
> -
> -	write_reg(par, 0x200, 0x0000 );
> -	write_reg(par, 0x201, 0x0000 );
> -	write_reg(par, 0x100, 0x7120 );
> -	write_reg(par, 0x007, 0x0103 );
> -	mdelay( 10 );
> -	write_reg(par, 0x007, 0x0113 );
> +	write_reg(par, 0x400, 0x4027);
> +	write_reg(par, 0x401, 0x0000);
> +	write_reg(par, 0x402, 0x0000);  /* First screen drive position (1) */
> +	write_reg(par, 0x403, 0x013f);  /* First screen drive position (2) */
> +	write_reg(par, 0x404, 0x0000);
> +
> +	write_reg(par, 0x200, 0x0000);
> +	write_reg(par, 0x201, 0x0000);
> +	write_reg(par, 0x100, 0x7120);
> +	write_reg(par, 0x007, 0x0103);
> +	mdelay(10);
> +	write_reg(par, 0x007, 0x0113);
>  
>  	return 0;
>  }
>