Re: [PATCH] Support for Asus MyCinema U3100Mini Plus

[Antti Palosaari <crope@iki.fi>]

On 09/20/2012 10:28 PM, Oliver Schinagl wrote:
> On 20-09-12 21:15, Antti Palosaari wrote:
>> On 09/20/2012 09:57 PM, oliver@schinagl.nl wrote:
>>> From: Oliver Schinagl <oliver@schinagl.nl>
>>>
>>> This is initial support for the Asus MyCinema U3100Mini Plus. The driver
>>> in its current form gets detected and loads properly.
>>>
>>> Scanning using dvbscan works without problems, Locking onto a channel
>>> using tzap also works fine. Only playback using tzap -r + mplayer was
>>> tested and was fully functional.
>>>
>>> It uses the af9035 USB Bridge chip, with an af9033 demodulator. The
>>> tuner
>>> used is the FCI FC2580.
>>>
>>> Signed-off-by: Oliver Schinagl <oliver@schinagl.nl>
>>
>> Acked-by: Antti Palosaari <crope@iki.fi>
>> Reviewed-by: Antti Palosaari <crope@iki.fi>
>>
>> It is OK. Mauro, please merge to the master.
> I do hope that it won't be a problem as I based it on your
> remotes/origin/for_v3.7-13
>>
>> @Oliver, you didn't fixed FC2580 useless braces as I requested.
>> Anyway, I will sent another patch to fix it later. Action not required.
> Ah, I did comment on that change in my reply on your comments; a
> re-paste from that:
>
> Checkpatch did not trigger on this. Which makes sense. Kernel
> CodingStyle is in very strong favor of K&R and from what I know from
> K&R, K&R strongly discourage not using braces as it is very likely to
> introduce bugs. Wikipedia has a small mention of this, then again
> wikipedia is wikipedia.

I am quite sure it says braces are not allowed for if () when it is 
single line.

> I will take it out of you really want it out, but with checkpatch not
> even complaining, I would think this as an improvement. :D

Seems like you are correct, it does not detect it from the patch for 
reason or the other. Maybe you could sent patch to fix checkpatch.pl :)

But it seems to find it when asked to check file correctness.

Anyway, my eyes seems to be again more careful than checkpatch ;-)

[crope@localhost linux]$ git show --format=email | ./scripts/checkpatch.pl -
total: 0 errors, 0 warnings, 141 lines checked

Your patch has no obvious style problems and is ready for submission.
[crope@localhost linux]$ ./scripts/checkpatch.pl --file 
drivers/media/tuners/fc2580.c
WARNING: braces {} are not necessary for single statement blocks
#501: FILE: media/tuners/fc2580.c:501:
+	if ((chip_id != 0x56) && (chip_id != 0x5a)) {
+		goto err;
+	}

total: 0 errors, 1 warnings, 525 lines checked

drivers/media/tuners/fc2580.c has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
[crope@localhost linux]$

Antti

-- 
http://palosaari.fi/