2013-08-08 17:03:54

by Paul McQuade

[permalink] [raw]

2013-08-08 17:23:43

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 2/2] Staging: BCM: Removed more whitespace/Errors

Please send your changes with an email client
that does not require attaching patches.

On Thu, 2013-08-08 at 18:03 +0100, Paul McQuade wrote:

> -#define MAXIMUM_USB_TCB 128
> -#define MAXIMUM_USB_RCB 128
> +#define MAXIMUM_USB_TCB 128
> +#define MAXIMUM_USB_RCB 128
>
> #define MAX_BUFFERS_PER_QUEUE 256
>
> #define MAX_DATA_BUFFER_SIZE 2048

If you're going to bother to change whitespace
here, please align these like the others.

There are also many style defect like:

+ } else {
+ BCM_DEBUG_PRINT(Adapter, DBG_TYPE_RX, RX_DPC, DBG_LVL_ALL, "Rx URB has got cancelled. status

so I stopped looking.

2013-08-08 20:45:10

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 2/2] Staging: BCM: Removed more whitespace/Errors

On Thu, Aug 08, 2013 at 06:03:49PM +0100, Paul McQuade wrote:
> -int InterfaceFileDownload( PVOID psIntfAdapter,
> - struct file *flp,
> - unsigned int on_chip_loc);
> +int InterfaceFileDownload(PVOID psIntfAdapter,
> + struct file *flp,
> + unsigned int on_chip_loc);


The very first line in this patch is totally wrong... :(

This is an automated patch. I'm not going to review any further.
Automated patches create a problem for reviews because they are
far harder to review than they are to generate. I'm not sure what
the solution is...

Anyway, the patch is very wrong.

regards,
dan carpenter

2013-08-08 21:03:11

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 2/2] Staging: BCM: Removed more whitespace/Errors

On Thu, 2013-08-08 at 23:44 +0300, Dan Carpenter wrote:
> On Thu, Aug 08, 2013 at 06:03:49PM +0100, Paul McQuade wrote:
> > -int InterfaceFileDownload( PVOID psIntfAdapter,
> > - struct file *flp,
> > - unsigned int on_chip_loc);
> > +int InterfaceFileDownload(PVOID psIntfAdapter,
> > + struct file *flp,
> > + unsigned int on_chip_loc);
>
>
> The very first line in this patch is totally wrong... :(
>
> This is an automated patch.

It is?

It doesn't look like an automated patch to me.
It looks like there are too many different style
choices for it to have been done by a machine.

Why do you think that?

Paul, how did you create this patch?

Dan, if you get a chance, could you try to use
checkpatch with the --fix option with specific
--types like SPACING and PARENTHESIS_ALIGNMENT
and tell me if you think it's OK?