[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1393581227.57507.YahooMailNeo@web164006.mail.gq1.yahoo.com>
Date: Fri, 28 Feb 2014 01:53:47 -0800 (PST)
From: Chase Southwood <chase.southwood@...oo.com>
To: Dan Carpenter <dan.carpenter@...cle.com>
Cc: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"abbotti@....co.uk" <abbotti@....co.uk>,
"hsweeten@...ionengravers.com" <hsweeten@...ionengravers.com>,
"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2 v2] Staging: comedi: fix lines that are over 80 characters
>On Friday, February 28, 2014 3:42 AM, Dan Carpenter <dan.carpenter@...cle.com> wrote:
>>On Fri, Feb 28, 2014 at 03:15:45AM -0600, Chase Southwood wrote:
>> This patch introduces a simple helper function, outl_1564_timer(), to
>> allow several lines which violate the character limit to be shortened.
>> A handful of other lines that are too long are appropriately split as
>> well.
>>
>> Cc: Dan Carpenter <dan.carpenter@...cle.com>
>> Signed-off-by: Chase Southwood <chase.southwood@...oo.com>
>> ---
>> 2: introduced outl_1564_timer() at the suggestion of Dan.
>> .../comedi/drivers/addi-data/hwdrv_apci1564.c | 83 +++++++++++++---------
>> 1 file changed, 49 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
>> index 2b47fa1..d958d3c 100644
>> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
>> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
>> @@ -99,6 +99,9 @@ This program is distributed in the hope that it will be useful, but WITHOUT ANY
>> #define APCI1564_TCW_WARN_TIMEVAL 24
>> #define APCI1564_TCW_WARN_TIMEBASE 28
>>
>> +/* Helper functions */
>
>Obvious comment is too obvious.
>
>> +static void outl_1564_timer(struct addi_private *, unsigned int, unsigned int);
>> +
>
>Avoid forward declarations. Put the function itself here.
>
>Do a function for APCI1564_DIGITAL_IP as well.
>
>outl_1564_digital_ip()
>inl_1564_digital_ip()
>
>To be honest, I'm not sure how much the 1564 adds to the name. The
>names should match of course.
>
>I wonder if this line is buggy?:
>outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_MODE1); /* Disable the >and/or interrupt */
>Should it be?
>outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP + >APCI1564_DIGITAL_IP_INTERRUPT_MODE1);
>
>You would need to read the specs to be sure.
>
>Anyway, take some more time with this and play with different ways to
>clean it up.
>
>regards,
>
>dan carpenter
Hey Dan!
Thanks a lot, I really appreciate all of the really good feedback. It's getting late now, but tomorrow I'll work on tidying everything up based on all of these comments and double check that hardware spec as well.
Thanks,
Chase
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists