Re: [PATCH] Staging: et131x: fix coding style issues

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

On Sat, May 26, 2012 at 11:44:15AM +0100, Mark Einon wrote:
> On 25 May 2012 19:32, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> > On Fri, May 25, 2012 at 06:56:40PM +0100, Adnan Ali wrote:
> >> This commit fixes coding style issues including braces
> >> position and line wrapping.
> >>
> >> Signed-off-by: Adnan Ali <adnan.ali@xxxxxxxxxxxxxxx>
> >> Reviewed-by: Jannis Pohlmann <jannis.pohlmann@xxxxxxxxxxxxxxx>
> >> ---
> >>  drivers/staging/et131x/et131x.c |   11 ++++-------
> >>  1 files changed, 4 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
> >> index 5b11c5e..cf02336 100644
> >> --- a/drivers/staging/et131x/et131x.c
> >> +++ b/drivers/staging/et131x/et131x.c
> >> @@ -85,8 +85,7 @@
> >>  MODULE_AUTHOR("Victor Soriano <vjsoriano@xxxxxxxxx>");
> >>  MODULE_AUTHOR("Mark Einon <mark.einon@xxxxxxxxx>");
> >>  MODULE_LICENSE("Dual BSD/GPL");
> >> -MODULE_DESCRIPTION("10/100/1000 Base-T Ethernet Driver "
> >> -                "for the ET1310 by Agere Systems");
> >> +MODULE_DESCRIPTION("10/100/1000 Base-T Ethernet Driver for the ET1310 by Agere Systems");
> >>
> >>  /* EEPROM defines */
> >>  #define MAX_NUM_REGISTER_POLLS          1000
> >> @@ -2967,11 +2966,10 @@ static struct rfd *nic_rx_pkts(struct et131x_adapter *adapter)
> >>               (ring_index == 0 &&
> >>               buff_index > rx_local->fbr[1]->num_entries - 1) ||
> >>               (ring_index == 1 &&
> >> -             buff_index > rx_local->fbr[0]->num_entries - 1))
> >> +             buff_index > rx_local->fbr[0]->num_entries - 1)) {
> >>  #else
> >> -     if (ring_index != 1 || buff_index > rx_local->fbr[0]->num_entries - 1)
> >> +     if (ring_index != 1 || buff_index > rx_local->fbr[0]->num_entries - 1) {
> >>  #endif
> >> -     {
> >
> > Mark, why do we have these nasty ifdefs?  It seems like this should
> > be an option at module load so that distros can support either way.
> > (But that is a stock response, I haven't looked at the code).
> 
> Hi Dan,
> 
> That particular define used for the ifdefs has always been defined
> since the earliest version of the driver I've encountered, so it can
> probably be removed at some point - It's on my TODO list, and I should
> update the in tree version at some point.
> 

Yeah.  Normally we would have removed it, but I saw that you fixed
it so that it worked with it undefined so I thought maybe you wanted
to keep it.

> Unfortunately due to recent changes, the driver has a few more issues
> that just coding style to fix at present. Startup is very
> hit-and-miss, for instance, and I've not had any external reports of
> these bugs - which leads me to suspect that no one else is testing the
> code on a real device... I'm slowly working my way through these.
> 

You mean changes in the kernel.org version?  We haven't been
applying very many patches against this driver.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel