Re: [PATCH v4 1/5] rtl2832 ver. 0.4: removed signal statistics

[Antti Palosaari <crope@iki.fi>]

On 17.05.2012 23:45, Thomas Mair wrote:
> On 17.05.2012 22:41, Antti Palosaari wrote:
>> On 17.05.2012 23:27, poma wrote:
>>> On 05/17/2012 04:19 PM, Antti Palosaari wrote:
>>>> Moikka Thomas,
>>>>
>>>> Here is the review. See comments below.
>>>>
>>>> And conclusion is that it is ready for the Kernel merge. I did not see
>>>> any big functiuonality problems - only some small issues that are likely
>>>> considered as a coding style etc. Feel free to fix those and sent new
>>>> patc serie or just new patch top of that.
>>>>
>>>> Reviewed-by: Antti Palosaari<crope@iki.fi>
>>
>> [...]
>>
>>> rtl2832.c.diff:
>>> - static int ->   static const
>>> - struct ->   static const struct
>>> - newline between function call and error check ->   […]
>>> - 5 indications apropos 'spaces' regarding 'CodingStyle'- line 206
>>> (/usr/share/doc/kernel-doc-3.3.5/Documentation/CodingStyle)
>>> […]
>>> Use one space around (on each side of) most binary and ternary operators,
>>> such as any of these:
>>>
>>>           =  +  -<    >    *  /  %  |&    ^<=>=  ==  !=  ?  :
>>>
>>> […]
>>>
>>> grep '>>\|<<' v4-1-5-rtl2832-ver.-0.4-removed-signal-statistics.patch.orig
>>> +    len = (msb>>   3) + 1;
>>> +        reading_tmp |= reading[i]<<   ((len-1-i)*8);
>>> +    *val = (reading_tmp>>   lsb)&   mask;
>>> +    len = (msb>>   3) + 1;
>>> +        reading_tmp |= reading[i]<<   ((len-1-i)*8);
>>> +    writing_tmp = reading_tmp&   ~(mask<<   lsb);
>>> +    writing_tmp |= ((val&   mask)<<   lsb);
>>> +        writing[i] = (writing_tmp>>   ((len-1-i)*8))&   0xff;
>>> +    num = bw_mode<<   20;
>>>
>>> Bitshift operators seems to be OK.
>>> Something else?
>>
>> (len-1-i)*8
> I almost have a new corrected version of the patch series ready, fixing this issues and the
> other ones you mentioned.
>>
>>> - 1 indication apropos 'media/dvb/frontends/rtl2832_priv.h:30'
>>> Compared to 'rtl2830_priv.h' seems to be OK.
>>>
>>> ./checkpatch.pl --no-tree
>>> v4-1-5-rtl2832-ver.-0.4-removed-signal-statistics.patch.orig
>>> ERROR: Missing Signed-off-by: line(s)
>>>
>>> total: 1 errors, 0 warnings, 1177 lines checked
>>>
>>> v4-1-5-rtl2832-ver.-0.4-removed-signal-statistics.patch.orig has style
>>> problems, please review.  If any of these errors
>>> are false positives report them to the maintainer, see
>>> CHECKPATCH in MAINTAINERS.
>>>
>>> How do you produce this error:
>>> "ERROR: Macros with complex values should be enclosed in parenthesis…"?
>>
>> Just running checkpatch.pl --file foo
>>
>
> For me checkpath.pl also does not report the error you reported. It does seem
> strange to me, as the makros are the same as in rtl2830_priv.h

Are you using some old version of checkpatch.pl ?
Mine is:
commit c06a9ebdb7a4f4823d4225fe789d8c20a1d534eb
Author: Joe Perches <joe@perches.com>
Date:   Mon Apr 16 13:35:11 2012 -0600

If you are using older version then upgrade. checkpatch.pl is developed 
very rapidly and there is all the time new checks.

regards
Antti
-- 
http://palosaari.fi/