From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760000AbcHYQDE (ORCPT ); Thu, 25 Aug 2016 12:03:04 -0400 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:57042 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752013AbcHYQDC (ORCPT ); Thu, 25 Aug 2016 12:03:02 -0400 X-Sasl-enc: CVRtuuNFhqn8c3dE13k71xCNKvntVhywmtpg7vjT03DA 1472140116 Date: Thu, 25 Aug 2016 18:47:08 +0300 From: Andrey Utkin To: Anson Jacob Cc: gregkh@linuxfoundation.org, abbotti@mev.co.uk, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 6/6] staging: comedi: ni_at_a2150.c: Fix checkpatch warning Message-ID: <20160825154708.a55hsffnfdz2zqac@acer> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.2 (2016-06-11) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Aug 25, 2016 at 11:16:13AM -0400, Anson Jacob wrote: > Fix checkpatch.pl warning 'line over 80 characters' > > Signed-off-by: Anson Jacob > --- > drivers/staging/comedi/drivers/ni_at_a2150.c | 82 ++++++++++++++++------------ > 1 file changed, 46 insertions(+), 36 deletions(-) > > diff --git a/drivers/staging/comedi/drivers/ni_at_a2150.c b/drivers/staging/comedi/drivers/ni_at_a2150.c > index 957fb9f..3c00de1 100644 > --- a/drivers/staging/comedi/drivers/ni_at_a2150.c > +++ b/drivers/staging/comedi/drivers/ni_at_a2150.c > @@ -58,41 +58,49 @@ > > /* Registers and bits */ > #define CONFIG_REG 0x0 > -#define CHANNEL_BITS(x) ((x) & 0x7) > -#define CHANNEL_MASK 0x7 > -#define CLOCK_SELECT_BITS(x) (((x) & 0x3) << 3) You have lost this treeish look which carried information about thing being a register or a field. > +#define CHANNEL_BITS(x) ((x) & 0x7) > +#define CHANNEL_MASK 0x7 No uniform alignment. Please get everything in a row. If it's hard or this part of driver is expected to have a lot of changes in near future, then I'd remove any whitespace over single space between name and value. > +#define ENABLE0_BIT 0x80 /* enable (don't internally ground) > + * channels 0 and 1 > + */ > +#define ENABLE1_BIT 0x100 /* enable (don't internally ground) > + * channels 2 and 3 > + */ This commenting style is discouraged (Linus has even stated explicitly that he dislikes it, even for pieces of code which historically had this style everywhere). Opening "/*" should not be followed by text. Also, In my personal experience, it is more stable to have these comments on previous line, not at end of the line. This way you always have enough space for both a comment and a macro.