On 12/20/2011 05:26 PM, Mauro Carvalho Chehab wrote:
On 20-12-2011 12:52, Antti Palosaari wrote:
On 12/20/2011 03:39 PM, Mauro Carvalho Chehab wrote:
+ break;
+ case 2:
+ str_constellation = "QAM16";
+ c->modulation = QAM_16;
+ break;
+ case 3:
+ str_constellation = "QAM32";
+ c->modulation = QAM_32;
+ break;
+ case 4:
+ str_constellation = "QAM64";
+ c->modulation = QAM_64;
+ break;
Please, avoid magic numbers. Instead, use macros for each
value.
I disagree that. Those numbers are coming from demodulator
register value. Same way is used almost every driver that
supports reading current transmission params from the demod.
There are drivers that don't code it well, but it is always preferred
to use macros for register values. Good drivers have it.
I still disagree. Are we speaking same issue?
val = read_reg(rgister)
switch (val) {
case 2:
c->modulation = QAM_16;
break;
case 3:
c->modulation = QAM_32;
break;
}
Why I should define macros here?
Or do you mean I should define macros for the selecting correct bits
from the register?
Anyhow, for me that piece of code looks very clear. And it is used
similarly very many drivers.
After all as I see there is no big bugs. Those findings are mostly related
of missing DMB-TH API support (and was even commented clearly). And 1-2 CodingStyle issues.
One issue is pure CodingStyle. The other no-API related aren't.
As there is still few other DMB-TH drivers having similar issues already in
the master I don't see why not to add that too. Anyhow, if you see that must
be put to staging until DMB-TH is defined to API it is OK for me.
Please fix the non-API related issues. If you ack to provide us the API improvements
for DMB for 3.4, and get rid of "auto_mode = true" for all cases, I'm
ok on merging it after the fixes at drivers/media/dvb.
If I don't add DMB-TH support to API you will push that to the staging?
Adding those to API is not mission impossible. Interleaver is only new
parameter and all the rest are just extending values. But my time is
limited... and I really would like to finally got Anysee smart card
reader integrated to USB serial first.
regards
Antti
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
