Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[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>

0 views
Skip to first unread message

t...@weberpafrica.com

unread,
Feb 16, 2010, 11:40:02 AM2/16/10
to
From: Tim Schofield <tim.scho...@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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Greg KH

unread,
Feb 18, 2010, 11:20:01 AM2/18/10
to
On Tue, Feb 16, 2010 at 04:35:16PM +0000, t...@weberpafrica.com wrote:
> From: Tim Schofield <tim.scho...@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

Florian Mickler

unread,
Feb 18, 2010, 1:00:02 PM2/18/10
to

hm... no.. seems to be correct... the comment get's ignored. But I
agree that {} would be nicer to the eye...

Greg KH

unread,
Feb 18, 2010, 1:00:02 PM2/18/10
to

wow, you are right, I just tested it out, learn something new every day.

But we should add braces...

thanks,

greg k-h

Dan Carpenter

unread,
Mar 3, 2010, 2:20:01 AM3/3/10
to

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) {

Matthew Wilcox

unread,
Mar 3, 2010, 7:20:02 AM3/3/10
to

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."

0 new messages