Re: [PATCH v3] staging: rts5208: Clean up coding style in rtsx_chip.c

[Greg KH]

On Fri, Oct 03, 2014 at 07:16:13PM +0300, Giedrius Statkevicius wrote:
> Make the second line of a divided line match the opening paranthesis.
> Combine two if's in form of 'if (a) if (b) { [...] }' into one to lower the indentation level.
> To further lower indentation level and make the code more concise use the ternary operator where possible and sensible.
> Make the names of labels all lower case to avoid Camel Case.
> Remove unnecessary parantheses around variables that are after a dereference operator i.e. &(a) => &a
> Switch the if condition around in 'if (0 == (value & (1 << bit))) {' to make it have the same form as the rest of the code.
> Remove unnecessary braces in rtsx_enable_aspm() around one statement
> 
> After this patch checkpatch.pl without --strict doesn't complain about rtsx_chip.c at all and with --strict it only complains about the unmatched parantheses.
> I am reluctant to move that code with unmatched parantheses into another function because I don't have the hardware needed to test this driver and don't have
> enough experience to do it properly. Doing so would just probably hide the other problems this code has. My patch should be purely about changing the code style without
> creating more functions and moving code to them. 
> 
> This patch is against Greg KH's staging-next tree.
> 
> Signed-off-by: Giedrius Statkevicius <giedrius.statkevicius@xxxxxxxxx>

That is a _lot_ to do in just one patch (also note you should wrap your
changelog text at 72 characters, like git asked you to...)

Please break this up into small, logical patches, each only doing one
thing.  Trying to review a large patch like this is almost impossible.

thanks,

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