2010-02-16 16:36:43

by Tim Schofield

Subject: [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 <[email protected]>

From: Tim Schofield <[email protected]>

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

2010-02-18 16:12:40

by Greg KH

Subject: 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 <[email protected]>

On Tue, Feb 16, 2010 at 04:35:16PM +0000, [email protected] wrote:
> From: Tim Schofield <[email protected]>
>
> ---
> 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

2010-02-18 17:51:15

by Florian Mickler

Subject: 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 <[email protected]>

On Thu, 18 Feb 2010 08:05:47 -0800
Greg KH <[email protected]> wrote:

> On Tue, Feb 16, 2010 at 04:35:16PM +0000, [email protected] wrote:
> > From: Tim Schofield <[email protected]>
> >
> > ---
> > 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...

2010-02-18 17:54:10

by Greg KH

Subject: 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 <[email protected]>

On Thu, Feb 18, 2010 at 06:31:51PM +0100, Florian Mickler wrote:
> On Thu, 18 Feb 2010 08:05:47 -0800
> Greg KH <[email protected]> wrote:
>
> > On Tue, Feb 16, 2010 at 04:35:16PM +0000, [email protected] wrote:
> > > From: Tim Schofield <[email protected]>
> > >
> > > ---
> > > 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