staging: winbond: reg.c Coding style fixes.
in [Kernel]
|
Prev: [PATCH v2 0/6] hid: new driver for PicoLCD device
Next: [PATCH] scripts/kernel-doc: fix fatal error on function prototype
From: Dan Carpenter on 20 Mar 2010 12:30 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(). > - > + 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. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Lars Lindley on 20 Mar 2010 15:30 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.. [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(a)coyote.org> Signed-off-by: Lars Lindley <lindley(a)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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Pavel Machek on 21 Mar 2010 03:00 On Sat 2010-03-20 20:21:48, 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.. > [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(a)coyote.org> > > Signed-off-by: Lars Lindley <lindley(a)coyote.org> Ack. but you probably want to merge those two pieces together. > --- > 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)) -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Dan Carpenter on 21 Mar 2010 05:20 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(a)coyote.org> > > Signed-off-by: Lars Lindley <lindley(a)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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Pekka Enberg on 21 Mar 2010 08:50
Pavel Machek wrote: >> [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(a)coyote.org> >> >> Signed-off-by: Lars Lindley <lindley(a)coyote.org> > > Ack. but you probably want to merge those two pieces together. Yes. Like I said before, please _check_ the result before posting your patches. It's very easy to make silly mistakes in "trivial" patches like these. Remember, it's not about quantity, it's about quality of each individual patch no matter how trivial they might seem at first. Pekka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |