Re: [PATCH] Add fc0011 tuner driver

[Antti Palosaari <crope@iki.fi>]

On 02.04.2012 20:20, Michael Büsch wrote:
> On Mon, 02 Apr 2012 19:56:50 +0300
> Antti Palosaari<crope@iki.fi>  wrote:
>
>> On 02.04.2012 19:14, Michael Büsch wrote:
>>> This adds support for the Fitipower fc0011 DVB-t tuner.
>>>
>>> Signed-off-by: Michael Buesch<m@bues.ch>
>>
>> Applied, thanks!
>> http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/af9035_experimental
>>
>> I looked it through quickly, no big issues. Anyhow, when I ran
>> checkpatch.pl and it complains rather much. All Kernel developers must
>> use checkpatch.pl before sent patches and fix findings if possible,
>> errors must be fixed and warnings too if there is no good reason to
>> leave as it is.
>
> Well, I _did_ run it on the patch.
> There is no error. Only (IMO bogus) warnings. Most of them
> are about the 80 char limit. Which isn't really a hard limit. And those lines
> only exceed the 80 char limit by a few chars (one, two or such). Splitting
> those line serves no readability purpose. In fact, it just worsens it.


git show --pretty=email b650f4d701859ccff76870208c9fd8093cc64209 | 
./scripts/checkpatch.pl -
WARNING: please write a paragraph that describes the config symbol fully
#37: FILE: drivers/media/common/tuners/Kconfig:207:
+config MEDIA_TUNER_FC0011

WARNING: msleep < 20ms can sleep for up to 20ms; see 
Documentation/timers/timers-howto.txt
+	msleep(10);
WARNING: quoted string split across lines
#334: FILE: drivers/media/common/tuners/fc0011.c:270:
+		dev_warn(&priv->i2c->dev, "Unsupported bandwidth %u kHz. "
+			 "Using 6000 kHz.\n",

WARNING: line over 80 characters
#397: FILE: drivers/media/common/tuners/fc0011.c:333:
+		err |= fc0011_writereg(priv, FC11_REG_RCCAL, regs[FC11_REG_RCCAL]);

WARNING: line over 80 characters
#420: FILE: drivers/media/common/tuners/fc0011.c:356:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: line over 80 characters
#430: FILE: drivers/media/common/tuners/fc0011.c:366:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: line over 80 characters
#439: FILE: drivers/media/common/tuners/fc0011.c:375:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: line over 80 characters
#449: FILE: drivers/media/common/tuners/fc0011.c:385:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: line over 80 characters
#456: FILE: drivers/media/common/tuners/fc0011.c:392:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: line over 80 characters
#468: FILE: drivers/media/common/tuners/fc0011.c:404:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: line over 80 characters
#478: FILE: drivers/media/common/tuners/fc0011.c:414:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: msleep < 20ms can sleep for up to 20ms; see 
Documentation/timers/timers-howto.txt
+	msleep(10);
WARNING: quoted string split across lines
#504: FILE: drivers/media/common/tuners/fc0011.c:440:
+	dev_dbg(&priv->i2c->dev, "Tuned to "
+		"fa=%02X fp=%02X xin=%02X%02X vco=%02X vcosel=%02X "

WARNING: please, no spaces at the start of a line
#620: FILE: drivers/media/common/tuners/fc0011.h:26:
+    defined(CONFIG_MEDIA_TUNER_FC0011_MODULE)$

total: 0 errors, 14 warnings, 598 lines checked

Your patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


hmmmm, I think Mauro will at least complain when I ask he to PULL that 
master. Personally I would like to see line len something more than 80 
chars, but as checkpatch.pl complains it I have shortened lines despite 
very few cases.

@Mauro what do you think are that kind of WARNINGs, those are not 
errors, allowed?

>> And one note about tuner driver, my AF9035 + FC0011 device founds only 1
>> mux of 4. Looks like some performance issues still to resolve.
>
> I have no idea what this means.
> So I have no remote idea of what could possibly be wrong here.
> Is this a bug on af903x or the tuner driver?

Likely tuner driver, or demod driver. But as demod tuner initialization 
tables are likely correct I suspect it is tuner issue at first hand. And 
secondly my other hardware with TUA9001 performs very well, better than 
old AF9015 sticks.


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