* [PATCH] Staging:rtl8192e: fix comments style issue in r8180_93cx6.c This is a patch to the r8180_93cx6.c file that fixes up the comments styling issues found by the checkpatch.pl tool Signed-off-by: Tim Schofield <tim@weberpafrica.com>
@ 2010-02-16 16:35 tim
2010-02-18 16:05 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: tim @ 2010-02-16 16:35 UTC (permalink / raw)
To: gregkh; +Cc: devel, linux-kernel, Tim Schofield
From: Tim Schofield <tim.schofield1960@googlemail.com>
---
drivers/staging/rtl8192e/r8180_93cx6.c | 22 ++++++++++++++--------
1 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/rtl8192e/r8180_93cx6.c b/drivers/staging/rtl8192e/r8180_93cx6.c
index 262ed5f..60fba80 100644
--- a/drivers/staging/rtl8192e/r8180_93cx6.c
+++ b/drivers/staging/rtl8192e/r8180_93cx6.c
@@ -23,12 +23,14 @@
static void eprom_cs(struct net_device *dev, short bit)
{
if (bit)
+ /* enable EPROM */
write_nic_byte(dev, EPROM_CMD,
(1<<EPROM_CS_SHIFT) | \
- read_nic_byte(dev, EPROM_CMD)); //enable EPROM
+ read_nic_byte(dev, EPROM_CMD));
else
+ /* disable EPROM */
write_nic_byte(dev, EPROM_CMD, read_nic_byte(dev, EPROM_CMD)\
- &~(1<<EPROM_CS_SHIFT)); //disable EPROM
+ &~(1<<EPROM_CS_SHIFT));
force_pci_posting(dev);
udelay(EPROM_DELAY);
@@ -96,7 +98,7 @@ u32 eprom_read(struct net_device *dev, u32 addr)
u32 ret;
ret = 0;
- //enable EPROM programming
+ /* enable EPROM programming */
write_nic_byte(dev, EPROM_CMD,
(EPROM_CMD_PROGRAM<<EPROM_CMD_OPERATING_MODE_SHIFT));
force_pci_posting(dev);
@@ -126,13 +128,17 @@ u32 eprom_read(struct net_device *dev, u32 addr)
eprom_send_bits_string(dev, read_cmd, 3);
eprom_send_bits_string(dev, addr_str, addr_len);
- //keep chip pin D to low state while reading.
- //I'm unsure if it is necessary, but anyway shouldn't hurt
+ /*
+ * keep chip pin D to low state while reading.
+ * I'm unsure if it is necessary, but anyway shouldn't hurt
+ */
eprom_w(dev, 0);
for (i = 0; i < 16; i++) {
- //eeprom needs a clk cycle between writing opcode&adr
- //and reading data. (eeprom outs a dummy 0)
+ /*
+ * eeprom needs a clk cycle between writing opcode&adr
+ * and reading data. (eeprom outs a dummy 0)
+ */
eprom_ck_cycle(dev);
ret |= (eprom_r(dev)<<(15-i));
}
@@ -140,7 +146,7 @@ u32 eprom_read(struct net_device *dev, u32 addr)
eprom_cs(dev, 0);
eprom_ck_cycle(dev);
- //disable EPROM programming
+ /* disable EPROM programming */
write_nic_byte(dev, EPROM_CMD,
(EPROM_CMD_NORMAL<<EPROM_CMD_OPERATING_MODE_SHIFT));
return ret;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Staging:rtl8192e: fix comments style issue in r8180_93cx6.c This is a patch to the r8180_93cx6.c file that fixes up the comments styling issues found by the checkpatch.pl tool Signed-off-by: Tim Schofield <tim@weberpafrica.com>
2010-02-16 16:35 [PATCH] Staging:rtl8192e: fix comments style issue in r8180_93cx6.c This is a patch to the r8180_93cx6.c file that fixes up the comments styling issues found by the checkpatch.pl tool Signed-off-by: Tim Schofield <tim@weberpafrica.com> tim
@ 2010-02-18 16:05 ` Greg KH
2010-02-18 17:31 ` Florian Mickler
2010-03-03 7:10 ` verifying whitespace patches don't change anything was Re: [PATCH] Staging:rtl8192e: fix comments Dan Carpenter
0 siblings, 2 replies; 6+ messages in thread
From: Greg KH @ 2010-02-18 16:05 UTC (permalink / raw)
To: tim; +Cc: gregkh, devel, Tim Schofield, linux-kernel
On Tue, Feb 16, 2010 at 04:35:16PM +0000, tim@weberpafrica.com wrote:
> From: Tim Schofield <tim.schofield1960@googlemail.com>
>
> ---
> drivers/staging/rtl8192e/r8180_93cx6.c | 22 ++++++++++++++--------
> 1 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/r8180_93cx6.c b/drivers/staging/rtl8192e/r8180_93cx6.c
> index 262ed5f..60fba80 100644
> --- a/drivers/staging/rtl8192e/r8180_93cx6.c
> +++ b/drivers/staging/rtl8192e/r8180_93cx6.c
> @@ -23,12 +23,14 @@
> static void eprom_cs(struct net_device *dev, short bit)
> {
> if (bit)
> + /* enable EPROM */
> write_nic_byte(dev, EPROM_CMD,
> (1<<EPROM_CS_SHIFT) | \
> - read_nic_byte(dev, EPROM_CMD)); //enable EPROM
> + read_nic_byte(dev, EPROM_CMD));
> else
> + /* disable EPROM */
> write_nic_byte(dev, EPROM_CMD, read_nic_byte(dev, EPROM_CMD)\
> - &~(1<<EPROM_CS_SHIFT)); //disable EPROM
> + &~(1<<EPROM_CS_SHIFT));
This does not do what you think it does (hint, you need {} if you want
to have more than one line in an if statement...)
Can you always verify that your coding style changes do not actually
break the code? A simple comparison of the .ko file before and after
should be sufficient.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Staging:rtl8192e: fix comments style issue in r8180_93cx6.c This is a patch to the r8180_93cx6.c file that fixes up the comments styling issues found by the checkpatch.pl tool Signed-off-by: Tim Schofield <tim@weberpafrica.com>
2010-02-18 16:05 ` Greg KH
@ 2010-02-18 17:31 ` Florian Mickler
2010-02-18 17:51 ` Greg KH
2010-03-03 7:10 ` verifying whitespace patches don't change anything was Re: [PATCH] Staging:rtl8192e: fix comments Dan Carpenter
1 sibling, 1 reply; 6+ messages in thread
From: Florian Mickler @ 2010-02-18 17:31 UTC (permalink / raw)
To: gregkh; +Cc: Tim Schofield, devel, linux-kernel, tim
On Thu, 18 Feb 2010 08:05:47 -0800
Greg KH <greg@kroah.com> wrote:
> On Tue, Feb 16, 2010 at 04:35:16PM +0000, tim@weberpafrica.com wrote:
> > From: Tim Schofield <tim.schofield1960@googlemail.com>
> >
> > ---
> > drivers/staging/rtl8192e/r8180_93cx6.c | 22 ++++++++++++++--------
> > 1 files changed, 14 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8192e/r8180_93cx6.c b/drivers/staging/rtl8192e/r8180_93cx6.c
> > index 262ed5f..60fba80 100644
> > --- a/drivers/staging/rtl8192e/r8180_93cx6.c
> > +++ b/drivers/staging/rtl8192e/r8180_93cx6.c
> > @@ -23,12 +23,14 @@
> > static void eprom_cs(struct net_device *dev, short bit)
> > {
> > if (bit)
> > + /* enable EPROM */
> > write_nic_byte(dev, EPROM_CMD,
> > (1<<EPROM_CS_SHIFT) | \
> > - read_nic_byte(dev, EPROM_CMD)); //enable EPROM
> > + read_nic_byte(dev, EPROM_CMD));
> > else
> > + /* disable EPROM */
> > write_nic_byte(dev, EPROM_CMD, read_nic_byte(dev, EPROM_CMD)\
> > - &~(1<<EPROM_CS_SHIFT)); //disable EPROM
> > + &~(1<<EPROM_CS_SHIFT));
>
> This does not do what you think it does (hint, you need {} if you want
> to have more than one line in an if statement...)
>
> Can you always verify that your coding style changes do not actually
> break the code? A simple comparison of the .ko file before and after
> should be sufficient.
>
> thanks,
>
> greg k-h
hm... no.. seems to be correct... the comment get's ignored. But I
agree that {} would be nicer to the eye...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Staging:rtl8192e: fix comments style issue in r8180_93cx6.c This is a patch to the r8180_93cx6.c file that fixes up the comments styling issues found by the checkpatch.pl tool Signed-off-by: Tim Schofield <tim@weberpafrica.com>
2010-02-18 17:31 ` Florian Mickler
@ 2010-02-18 17:51 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2010-02-18 17:51 UTC (permalink / raw)
To: Florian Mickler; +Cc: Tim Schofield, devel, linux-kernel, tim
On Thu, Feb 18, 2010 at 06:31:51PM +0100, Florian Mickler wrote:
> On Thu, 18 Feb 2010 08:05:47 -0800
> Greg KH <greg@kroah.com> wrote:
>
> > On Tue, Feb 16, 2010 at 04:35:16PM +0000, tim@weberpafrica.com wrote:
> > > From: Tim Schofield <tim.schofield1960@googlemail.com>
> > >
> > > ---
> > > drivers/staging/rtl8192e/r8180_93cx6.c | 22 ++++++++++++++--------
> > > 1 files changed, 14 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8192e/r8180_93cx6.c b/drivers/staging/rtl8192e/r8180_93cx6.c
> > > index 262ed5f..60fba80 100644
> > > --- a/drivers/staging/rtl8192e/r8180_93cx6.c
> > > +++ b/drivers/staging/rtl8192e/r8180_93cx6.c
> > > @@ -23,12 +23,14 @@
> > > static void eprom_cs(struct net_device *dev, short bit)
> > > {
> > > if (bit)
> > > + /* enable EPROM */
> > > write_nic_byte(dev, EPROM_CMD,
> > > (1<<EPROM_CS_SHIFT) | \
> > > - read_nic_byte(dev, EPROM_CMD)); //enable EPROM
> > > + read_nic_byte(dev, EPROM_CMD));
> > > else
> > > + /* disable EPROM */
> > > write_nic_byte(dev, EPROM_CMD, read_nic_byte(dev, EPROM_CMD)\
> > > - &~(1<<EPROM_CS_SHIFT)); //disable EPROM
> > > + &~(1<<EPROM_CS_SHIFT));
> >
> > This does not do what you think it does (hint, you need {} if you want
> > to have more than one line in an if statement...)
> >
> > Can you always verify that your coding style changes do not actually
> > break the code? A simple comparison of the .ko file before and after
> > should be sufficient.
> >
> > thanks,
> >
> > greg k-h
>
> hm... no.. seems to be correct... the comment get's ignored. But I
> agree that {} would be nicer to the eye...
wow, you are right, I just tested it out, learn something new every day.
But we should add braces...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* verifying whitespace patches don't change anything was Re: [PATCH] Staging:rtl8192e: fix comments
2010-02-18 16:05 ` Greg KH
2010-02-18 17:31 ` Florian Mickler
@ 2010-03-03 7:10 ` Dan Carpenter
2010-03-03 12:18 ` Matthew Wilcox
1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2010-03-03 7:10 UTC (permalink / raw)
To: Greg KH; +Cc: tim, gregkh, devel, Tim Schofield, linux-kernel, kernel-janitors
On Thu, Feb 18, 2010 at 08:05:47AM -0800, Greg KH wrote:
> On Tue, Feb 16, 2010 at 04:35:16PM +0000, tim@weberpafrica.com wrote:
> > From: Tim Schofield <tim.schofield1960@googlemail.com>
> >
> > ---
> > drivers/staging/rtl8192e/r8180_93cx6.c | 22 ++++++++++++++--------
> > 1 files changed, 14 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8192e/r8180_93cx6.c b/drivers/staging/rtl8192e/r8180_93cx6.c
> > index 262ed5f..60fba80 100644
> > --- a/drivers/staging/rtl8192e/r8180_93cx6.c
> > +++ b/drivers/staging/rtl8192e/r8180_93cx6.c
> > @@ -23,12 +23,14 @@
> > static void eprom_cs(struct net_device *dev, short bit)
> > {
> > if (bit)
> > + /* enable EPROM */
> > write_nic_byte(dev, EPROM_CMD,
> > (1<<EPROM_CS_SHIFT) | \
> > - read_nic_byte(dev, EPROM_CMD)); //enable EPROM
> > + read_nic_byte(dev, EPROM_CMD));
> > else
> > + /* disable EPROM */
> > write_nic_byte(dev, EPROM_CMD, read_nic_byte(dev, EPROM_CMD)\
> > - &~(1<<EPROM_CS_SHIFT)); //disable EPROM
> > + &~(1<<EPROM_CS_SHIFT));
>
> This does not do what you think it does (hint, you need {} if you want
> to have more than one line in an if statement...)
>
> Can you always verify that your coding style changes do not actually
> break the code? A simple comparison of the .ko file before and after
> should be sufficient.
>
That's what I thought too, but every debug macro has a __LINE__ in it so
if you add a new line at the start of the file it makes a ton of changes in
the final binary .ko file.
Is there a trick to this?
I hacked sparse to always use 12345 as the line, but if, for example, you
remove uneeded parenthesis that would still count as a code change.
regards,
dan carpenter
diff --git a/pre-process.c b/pre-process.c
index 34b21ff..d3ae7a0 100644
--- a/pre-process.c
+++ b/pre-process.c
@@ -154,7 +154,8 @@ static int expand_one_symbol(struct token **list)
return expand(list, sym);
}
if (token->ident == &__LINE___ident) {
- replace_with_integer(token, token->pos.line);
+// replace_with_integer(token, token->pos.line);
+ replace_with_integer(token, 12345);
} else if (token->ident == &__FILE___ident) {
replace_with_string(token, stream_name(token->pos.stream));
} else if (token->ident == &__DATE___ident) {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: verifying whitespace patches don't change anything was Re: [PATCH] Staging:rtl8192e: fix comments
2010-03-03 7:10 ` verifying whitespace patches don't change anything was Re: [PATCH] Staging:rtl8192e: fix comments Dan Carpenter
@ 2010-03-03 12:18 ` Matthew Wilcox
0 siblings, 0 replies; 6+ messages in thread
From: Matthew Wilcox @ 2010-03-03 12:18 UTC (permalink / raw)
To: Dan Carpenter, Greg KH, tim, gregkh, devel, Tim Schofield,
linux-kernel, kernel-janitors
On Wed, Mar 03, 2010 at 10:10:23AM +0300, Dan Carpenter wrote:
> On Thu, Feb 18, 2010 at 08:05:47AM -0800, Greg KH wrote:
> > On Tue, Feb 16, 2010 at 04:35:16PM +0000, tim@weberpafrica.com wrote:
> > > From: Tim Schofield <tim.schofield1960@googlemail.com>
> > >
> > > ---
> > > drivers/staging/rtl8192e/r8180_93cx6.c | 22 ++++++++++++++--------
> > > 1 files changed, 14 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8192e/r8180_93cx6.c b/drivers/staging/rtl8192e/r8180_93cx6.c
> > > index 262ed5f..60fba80 100644
> > > --- a/drivers/staging/rtl8192e/r8180_93cx6.c
> > > +++ b/drivers/staging/rtl8192e/r8180_93cx6.c
> > > @@ -23,12 +23,14 @@
> > > static void eprom_cs(struct net_device *dev, short bit)
> > > {
> > > if (bit)
> > > + /* enable EPROM */
> > > write_nic_byte(dev, EPROM_CMD,
> > > (1<<EPROM_CS_SHIFT) | \
> > > - read_nic_byte(dev, EPROM_CMD)); //enable EPROM
> > > + read_nic_byte(dev, EPROM_CMD));
> > > else
> > > + /* disable EPROM */
> > > write_nic_byte(dev, EPROM_CMD, read_nic_byte(dev, EPROM_CMD)\
> > > - &~(1<<EPROM_CS_SHIFT)); //disable EPROM
> > > + &~(1<<EPROM_CS_SHIFT));
> >
> > This does not do what you think it does (hint, you need {} if you want
> > to have more than one line in an if statement...)
Sorry, Greg, you're wrong in this case. /* */ does not count as a line.
Here's an example:
$ cat foo.c
#include <stdio.h>
int main(void)
{
if (0)
/* foo */
printf("Hello, World\n");
return 0;
}
$ gcc -o foo foo.c -W -Wall -O2 -g
$ ./foo
[no output]
I think a much more cogent criticism of this patch would be that the comment
is entirely unnecessary for this function with a simple change:
-static void eprom_cs(struct net_device *dev, short bit)
+static void eprom_cs(struct net_device *dev, short enable)
{
- if (bit)
+ if (enable)
write_nic_byte(dev, EPROM_CMD,
(1<<EPROM_CS_SHIFT) | \
- read_nic_byte(dev, EPROM_CMD)); //enable EPROM
+ read_nic_byte(dev, EPROM_CMD));
else
write_nic_byte(dev, EPROM_CMD, read_nic_byte(dev, EPROM_CMD)\
- &~(1<<EPROM_CS_SHIFT)); //disable EPROM
+ &~(1<<EPROM_CS_SHIFT));
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-03-03 12:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-16 16:35 [PATCH] Staging:rtl8192e: fix comments style issue in r8180_93cx6.c This is a patch to the r8180_93cx6.c file that fixes up the comments styling issues found by the checkpatch.pl tool Signed-off-by: Tim Schofield <tim@weberpafrica.com> tim
2010-02-18 16:05 ` Greg KH
2010-02-18 17:31 ` Florian Mickler
2010-02-18 17:51 ` Greg KH
2010-03-03 7:10 ` verifying whitespace patches don't change anything was Re: [PATCH] Staging:rtl8192e: fix comments Dan Carpenter
2010-03-03 12:18 ` Matthew Wilcox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox