All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: Lars Lindley <lindley@coyote.org>
Cc: gregkh@suse.de, greg@kroah.com, penberg@cs.helsinki.fi,
	pavel@ucw.cz, harvey.harrison@gmail.com,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: winbond: reg.c Coding style fixes.
Date: Sun, 21 Mar 2010 12:14:55 +0300	[thread overview]
Message-ID: <20100321091455.GH5331@bicker> (raw)
In-Reply-To: <4BA5204C.9050208@coyote.org>

On Sat, Mar 20, 2010 at 08:21:48PM +0100, Lars Lindley wrote:
> 
> 
> On 2010-03-20 17:24, Dan Carpenter wrote:
> > On Sat, Mar 20, 2010 at 04:14:11PM +0100, Lars Lindley wrote:
> >> I fixed all problems reported by checkpatch.pl except some
> >> (a lot of) long lines and some printk:s.
> >> I removed "commented away" code and version comments.
> >>
> > 
> > 	[snip]
> > 
> >> -		msleep(10); // Modify 20051221.1.b
> >> -		Wb35Reg_WriteSync( pHwData, 0x03d4, 0xb8 );// REG_ON RF_RSTN on, and
> >> -		msleep(10); // Modify 20051221.1.b
> > 
> > 	You removed this msleep().
> > 
> 
> Oops! I think I was fooled by the strange comment on the line before..
> 
> >> -
> >> +	if (pHwData->phy_type <= RF_MAXIM_V1)
> >> +		Wb35Reg_WriteSync(pHwData, 0x03d4, 0xffffff38);
> >> +	else {
> >> +		Wb35Reg_WriteSync(pHwData, 0x03f4, 0xFF5807FF);
> >> +		Wb35Reg_WriteSync(pHwData, 0x03d4, 0x80); /* regulator on only */
> >> +		msleep(10);
> >> +		Wb35Reg_WriteSync(pHwData, 0x03d4, 0xb8); /* REG_ON RF_RSTN on, and msleep(10); */
> > 
> > In the new version it's commented out.                                             ^^^^^^^^^^^^
> > 
> > I used a program to try verify that only white space changed.
> > http://lkml.indiana.edu/hypermail/linux/kernel/1003.1/01012.html
> > 
> > Then when it failed I grovelled through to find what changed.
> > 
> > Actually my tool complained about removed parenthesis as well.  I think 
> > this patch only removes one set of parenthesis.  Please don't do that.  
> > With a patch this size, it's nice to be able to use tools to verify that 
> > it doesn't change anything, instead of checking it by hand. Tabs, spaces
> > and newlines only, please.
> > 
> Point taken. Will keep it in mind from now on..
> 
> > Obviously in the next patch you can send the one liner that remove the
> > parenthesis.  We can eyeball one liners quite quickly.
> > 
> > regards,
> > dan carpenter
> > 
> 
> 
> Here's a patch to fix my screw up :)
> Now the .o doesn't differ before and after..
> 

Could you post your config?  I would think the curly brace change would
have made a difference even if all the line number changes didn't.

regards,
dan carpenter

> 
> [PATCH] staging: winbond: reg.c Fix accidental commenting of msleep(10)
> 
> I accidentaly commented out msleep() in one place.
> This applies after previous patch:
> Message-Id: <1269098051-6594-1-git-send-email-lindley@coyote.org>
> 
> Signed-off-by: Lars Lindley <lindley@coyote.org>
> ---
>  drivers/staging/winbond/reg.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/staging/winbond/reg.c b/drivers/staging/winbond/reg.c
> index 2bddf8f..d9a8128 100644
> --- a/drivers/staging/winbond/reg.c
> +++ b/drivers/staging/winbond/reg.c
> @@ -890,7 +890,8 @@ void Uxx_power_on_procedure(struct hw_data *pHwData)
>  		Wb35Reg_WriteSync(pHwData, 0x03f4, 0xFF5807FF);
>  		Wb35Reg_WriteSync(pHwData, 0x03d4, 0x80); /* regulator on only */
>  		msleep(10);
> -		Wb35Reg_WriteSync(pHwData, 0x03d4, 0xb8); /* REG_ON RF_RSTN on, and msleep(10); */
> +		Wb35Reg_WriteSync(pHwData, 0x03d4, 0xb8); /* REG_ON RF_RSTN on, and */
> +		msleep(10);
>  		ltmp = 0x4968;
>  		if ((pHwData->phy_type == RF_WB_242) ||
>  			(RF_WB_242_1 == pHwData->phy_type))
> -- 
> 1.7.0.2

  parent reply	other threads:[~2010-03-21  9:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-20 15:14 [PATCH] staging: winbond: reg.c Coding style fixes Lars Lindley
2010-03-20 16:24 ` Dan Carpenter
2010-03-20 19:21   ` Lars Lindley
2010-03-21  6:58     ` Pavel Machek
2010-03-21 12:39       ` Pekka Enberg
2010-03-21 15:40         ` Lars Lindley
2010-03-21  9:14     ` Dan Carpenter [this message]
2010-03-21 10:55     ` strip_whitespace.pl Dan Carpenter
2010-03-21 11:07       ` strip_whitespace.pl Dan Carpenter
2010-03-21 15:31         ` strip_whitespace.pl Lars Lindley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100321091455.GH5331@bicker \
    --to=error27@gmail.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=greg@kroah.com \
    --cc=gregkh@suse.de \
    --cc=harvey.harrison@gmail.com \
    --cc=lindley@coyote.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=penberg@cs.helsinki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.