From: Greg KH <gregkh@linuxfoundation.org>
To: Giedrius Statkevicius <giedrius.statkevicius@gmail.com>
Cc: devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
fabio.falzoi84@gmail.com
Subject: Re: [PATCH v3] staging: rts5208: Clean up coding style in rtsx_chip.c
Date: Fri, 3 Oct 2014 09:35:59 -0700 [thread overview]
Message-ID: <20141003163559.GA12903@kroah.com> (raw)
In-Reply-To: <542ECBCD.3050209@gmail.com>
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@gmail.com>
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
prev parent reply other threads:[~2014-10-03 16:36 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-03 16:16 [PATCH v3] staging: rts5208: Clean up coding style in rtsx_chip.c Giedrius Statkevicius
2014-10-03 16:35 ` Greg KH [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141003163559.GA12903@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=devel@driverdev.osuosl.org \
--cc=fabio.falzoi84@gmail.com \
--cc=giedrius.statkevicius@gmail.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.