Re: [linux-dvb] First patch for Freecom DVB-T (with RTL2831U, usb id 14aa:0160)
On Sat, 15 Mar 2008 23:23:51 +0100 Jan Hoogenraad [EMAIL PROTECTED] wrote: Due to several issues I've noticed at the driver, I opted, for now, to add it as a separate tree. This way, we can fix things there, without affecting the staging tree. I've made it available at: http://linuxtv.org/hg/~mchehab/rtl2831 Thanks a lot. This way, the people involved have a place to focus on. Now, I need to find a way to synchronise my tree with this directory. I'll do some reading on the mercurial. Kdiff3 has a very nice algorithm to see the differences on a code with different CodingStyles. Anyway, this could be a pain. I have some scripts to convert things I receive from Realtek. I'll add the new directory names and Lindent at least. Also, I noticed that nobody, on RealTek signed it. It would be interesting if someone there could send us a SOB for the first changeset: http://linuxtv.org/hg/~mchehab/rtl2831/rev/bb7749446173 Please explain the abbrreviation SOB, and if possible send the text. SOB - Signed-off-by: It should be useful for you to take a look on README.patches [1]. [1] http://linuxtv.org/hg/v4l-dvb/raw-file/tip/README.patches Would you like to have a paper copy, or is e-mail confirmation to you sufficient ? Just an e-mail with their SOBs. The text I added in the header is vetted by people from Realtek. They can change the text. The only requirement is that the code should be licensed with GPLv2. They are eager to work together, and willing to learn. This is very good! With time we will learn how to cope together. Unfortunately, I myself am completely new to linux development. It shouldn't take much time to learn ;) It is a completely different environment than what you'll have inside a company, since it is a community-driven work. So, all members of the community are freed to cope, comment and help with your work, sending you newer patches. Also, since kernel internal API's change, we need to handle patches that will change the API, testing they. I'll add those specific cases to my import script; I think that script should (due to the nature of the driver) get a central role, as to keep updates automated. Yes, this seems to be the easiest way. 3) Name convention. Names are generally in lower case. Since we try to have all lines with maxsize=80, the better is trying to have shorter names. I don't think that it would be a good idea to replace all names inside the driver, since this will make your life harder, when receiving patches from Realtek. Anyway, please consider this if you need to touch on some var name. There are other comments I want to do, about the integration with the tree. I intend to do it later, after having a better understanding on how the driver works and what can be done to avoid code duplication with dvb core and to allow the usage of the tuners by other drivers. Some of us have studied this already, and communicated with Realtek on this, For example, they have an improved handling of the mt2060 tuner. Decoupling the front end, setting this temporatily up as a new driver (would the naming there be something like mt2060_for_rtl2831 ?) and then integration have been on our wish list already. We did this kind of things already (for example, with saa711x). While this is not the ideal way, we can handle this. I suspect, however, that it is too late to merge the driver for 2.6.26 (the window for newer drivers should be opening soon, and the driver reviewing should happen before the opening of the windows). Considering this, we'll have a timeframe of about 8-10 weeks before the next merge window (for 2.6.27). Maybe this timeframe is enough to merge the required newer features on mt2060 and reuse it, instead of adding a newer one. My suggestion is to work with the separate tree and see the progress. Also, I'll need help from other developers on this large task ;) I at least have a lot of people interested already for testing. I've cc-ed them. Great. There are also the other guys from DVB ML that could also help on this. Cheers, Mauro ___ linux-dvb mailing list linux-dvb@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
Re: [linux-dvb] First patch for Freecom DVB-T (with RTL2831U, usb id 14aa:0160)
Mauro: Most of the mail understood. Can you put us into contact with the people who did the decoupling to the tuner for for example, with saa711x ? We could re-use some of their work, and learn from them. A similar thing goes for the IR devices. Currently, these sticks are shipped with 3 different IR devices, with different (and sometimes conflicting) IR codes. I see that there are two different mechanisms used in the drivers. The cxusb.c driver shows how to switch based on the USB ID of the stick. The hauppage driver, with ir-kbd-i2c.c and ir-keymaps.c shosw how to switch based on module parameters (mechanism similar to the debug=1). Can you tell me if there is a roadmap for this (e.g. decoupling the mapping from the stick ID and putting the rc_query subroutines together) ? In the next patch, I'll add a file /linux/Documentation/dvb/README.rtl2831 with some data I got from Realtek. Mauro Carvalho Chehab wrote: On Sat, 15 Mar 2008 23:23:51 +0100 Jan Hoogenraad [EMAIL PROTECTED] wrote: I have some scripts to convert things I receive from Realtek. I'll add the new directory names and Lindent at least. There are other comments I want to do, about the integration with the tree. I intend to do it later, after having a better understanding on how the driver works and what can be done to avoid code duplication with dvb core and to allow the usage of the tuners by other drivers. Some of us have studied this already, and communicated with Realtek on this, For example, they have an improved handling of the mt2060 tuner. Decoupling the front end, setting this temporatily up as a new driver (would the naming there be something like mt2060_for_rtl2831 ?) and then integration have been on our wish list already. We did this kind of things already (for example, with saa711x). While this is not the ideal way, we can handle this. Also, I'll need help from other developers on this large task ;) I at least have a lot of people interested already for testing. I've cc-ed them. Great. There are also the other guys from DVB ML that could also help on this. Cheers, Mauro -- Jan Hoogenraad Hoogenraad Interface Services Postbus 2717 3500 GS Utrecht ___ linux-dvb mailing list linux-dvb@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
Re: [linux-dvb] First patch for Freecom DVB-T (with RTL2831U, usb id 14aa:0160)
On Mon, 17 Mar 2008 20:36:12 +0100
Jan Hoogenraad [EMAIL PROTECTED] wrote:
Mauro:
Most of the mail understood.
Can you put us into contact with the people who did the decoupling to
the tuner for for example, with saa711x ?
We could re-use some of their work, and learn from them.
Several developers ;)
First of all, there are two different behaviors, userspace and kernelspace
API's being one for analog and another for digital. This is due to historic
reasons.
The decoupling is via I2C interface. Basically, each i2c driver is independent.
Kernel Documentation/i2c explains the usage of I2C.
In the case of tuner and saa711x, those are two independent drivers, used
(mostly) by analog mode.
tuner.ko module contains tuner-core.c. There are several other tuner drivers
that can be bound to tuner.ko or to a frontend driver.
On i2c normal way for analog TV, composite/svideo and FM, it probes I2C bus for
the presence of I2C devices by sending a read message with 0 bytes. If some
answer is given, this means that an I2C device is present there. When something
is detected, an attach callback is called. For example, attach_inform(), on
cx88-i2c. The attach_inform() then configures that device, if needed.
A newer method for probing is starting to be implemented, on ivtv driver.
Both tuner and saa711x implements a command interface. The command is meant
to receive requests for tuning and other things, like changing video
standard. All I2C devices listen to the I2C command. So, when you change a
video standard, tuner may set the proper IF, while saa711x can send the proper
initialization codes for that video standard.
In order to better support hybrid devices, tuner now is implemented on separate
files.
tuner.ko is just the core, meant to be use by analog/svideo/composite or radio
mode.
The tuner, itself, can work with both V4L and DVB API's. This is warranted by
the usage of callbacks.
For digital TV, there are several callbacks. In the case of a tuner, for
example xc2028, the driver fills an structure with the callbacks:
static const struct dvb_tuner_ops xc2028_dvb_tuner_ops = {
.info = {
.name = Xceive XC3028,
.frequency_min = 4200,
.frequency_max = 86400,
.frequency_step = 5,
},
.set_config= xc2028_set_config,
.set_analog_params = xc2028_set_analog_freq,
.release = xc2028_dvb_release,
.get_frequency = xc2028_get_frequency,
.get_rf_strength = xc2028_signal,
.set_params= xc2028_set_params,
.sleep = xc2028_sleep,
#if 0
int (*get_bandwidth)(struct dvb_frontend *fe, u32 *bandwidth);
int (*get_status)(struct dvb_frontend *fe, u32 *status);
#endif
};
The above struct contains both callbacks for analog and digital.
dvb core will call the proper callbacks when needed. For example, set_params
select the proper DVB mode, and tune to an specific frequency.
There are similar callbacks also for DVB frontend (a frontend is a tuner plus
a demod - and, for DVB-S, the LNBf control interface).
A similar thing goes for the IR devices.
Currently, these sticks are shipped with 3 different IR devices, with
different (and sometimes conflicting) IR codes.
IR scan tables can be inside drivers/media/common/ir-keymaps. The hanlding for
gpio RC5 is provided at ir-common.ko.
For I2C, there's a special module for handling this (ir_i2c_kbd). This is also
a generic module. The attach callback is used to setup the specific parameters
for each IR.
I see that there are two different mechanisms used in the drivers.
The cxusb.c driver shows how to switch based on the USB ID of the stick.
The hauppage driver, with ir-kbd-i2c.c and ir-keymaps.c shosw how to
switch based on module parameters (mechanism similar to the debug=1).
The usage of module parameters should be used only when other mechanisms aren't
possible. The preferred way is to detect board characteristics via reading the
board eeprom, if available. Hauppauge detection is very good, for most things,
due to tveeprom.ko. There's a parser there to read their info inside the eeprom.
This saves a lot of space inside kernel to handle all differences between the
boards.
If this is not possible, the next option is to use USB ID or PCI ID. However,
sometimes, unfortunately, a cheap board doesn't have an unique PCI ID. So, the
only way is to provide module options.
Can you tell me if there is a roadmap for this (e.g. decoupling the
mapping from the stick ID and putting the rc_query subroutines together) ?
What do you mean?
In the next patch, I'll add a file
/linux/Documentation/dvb/README.rtl2831
with some data I got from Realtek.
Good.
Cheers,
Mauro
___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
Re: [linux-dvb] First patch for Freecom DVB-T (with RTL2831U, usb id 14aa:0160)
Summary of this mail:
There is activity on the Freecom / Conceptronic / Realtek card on:
http://linuxtv.org/hg/~mchehab/rtl2831
Please help us improve.
Mauro Carvalho Chehab wrote:
On Fri, 14 Mar 2008 22:24:46 +0100
Jan Hoogenraad [EMAIL PROTECTED] wrote:
Dear v4l maintainer:
Please, just call me by my name ;)
OK. From the previous mail I could not discern if this was the same ...
I have created a first version of the patch for the Freecom stick, based
on the latest sources I received today from RealTek.
Due to several issues I've noticed at the driver, I opted, for now, to add it
as a separate tree. This way, we can fix things there, without affecting the
staging tree. I've made it available at:
http://linuxtv.org/hg/~mchehab/rtl2831
Thanks a lot. This way, the people involved have a place to focus on.
Now, I need to find a way to synchronise my tree with this directory.
I'll do some reading on the mercurial.
I have some scripts to convert things I receive from Realtek.
I'll add the new directory names and Lindent at least.
Also, I noticed that nobody, on RealTek signed it. It would be interesting if
someone there could send us a SOB for the first changeset:
http://linuxtv.org/hg/~mchehab/rtl2831/rev/bb7749446173
Please explain the abbrreviation SOB, and if possible send the text.
Would you like to have a paper copy, or is e-mail confirmation to you
sufficient ?
The text I added in the header is vetted by people from Realtek.
They are eager to work together, and willing to learn.
Unfortunately, I myself am completely new to linux development.
Ah, by convention, we name directories in lowercase. So, I've did an sed
before applying your patch, as I've explained at the changeset comments.
thanks
I have received several updates per week from them during the fixing
time, so I expect some updates later on.
Ok.
Mauro:
Thanks for the help.
I agree that this is probably the best thing to do.
Ok, this is the Lindent changeset:
http://linuxtv.org/hg/~mchehab/rtl2831/rev/698c1894a3fd
Unfortunately, Lindent does not fix errors that
make checkpatch
reports like the two below.
tuner_mxl5005s.h: In '// I2C birdge module demod argument setting':
tuner_mxl5005s.h:531: ERROR: do not use C99 // comments
I tried to quickfix this with a small perl script, like:
for i in *.c *.h; do perl -ne \
'if (s|//\s*(.*)\n|/* \1 */\n|) { s|/\*\s*\*/||; } print $_' \
$i /tmp/tmp; mv -f /tmp/tmp $i; done
However, this failed, since there are some comments with // inside. It doesn't
seem to be hard to fix this by a close script.
So I found as well, using a similar sed code.
tuner_mxl5005s.h: In 'void
mxl5005s_SetI2cBridgeModuleTunerArg(TUNER_MODULE * pTuner);':
tuner_mxl5005s.h:532: ERROR: foo * bar should be foo *bar
True. Yet, this shows another thing that is forbidden on Linux CodingStyle:
the
usage of typedef. While this is valid on a few cases, on most cases we prefer
to use things like struct foo *foo;.
---
Due to the size of the driver, and the nature of it (a port from other OS), it
is natural that we will have a large amount of issues. Before visiting the
code
to check everything, maybe the better approach would be to do some general
comments.
I'll start commenting some things about CodingStyle. The better is if you
could
read kernel Documentation/CodingStyle.
1) Kernel already defines several types. Please use the already defined
typedefs.
For example:
+typedef unsigned char U8Data;
use, instead __u8
+typedef unsigned int UData_t; /* type must be at least 32 bits */
use, instead __u32
+typedef int SData_t; /* type must be at least 32 bits */
use, instead __s32
+typedef void *Handle_t; /* memory pointer type
Just use void *
I'll add those specific cases to my import script;
I think that script should (due to the nature of the driver) get a
central role, as to keep updates automated.
2) We don't use typedef struct foo. Instead, just declare struct foo and
replace all foo * to struct foo *:
+typedef struct {
+ UData_t nAS_Algorithm;
+ UData_t f_ref;
+ UData_t f_in;
+ UData_t f_LO1;
+ UData_t f_if1_Center;
+ UData_t f_if1_Request;
+ UData_t f_if1_bw;
+ UData_t f_LO2;
+ UData_t f_out;
+ UData_t f_out_bw;
+ UData_t f_LO1_Step;
+ UData_t f_LO2_Step;
+ UData_t f_LO1_FracN_Avoid;
+ UData_t f_LO2_FracN_Avoid;
+ UData_t f_zif_bw;
+ UData_t f_min_LO_Separation;
+ UData_t maxH1;
+ UData_t maxH2;
+ UData_t bSpurPresent;
+ UData_t bSpurAvoided;
+ UData_t nSpursFound;
+ UData_t nZones;
+ struct MT_ExclZone_t *freeZones;
+ struct MT_ExclZone_t *usedZones;
+ struct MT_ExclZone_t MT_ExclZones[MAX_ZONES];
} MT_AvoidSpursData_t;
again, same thing; should be scriptable.
3) Name convention. Names are generally in lower case. Since we try to have
all
lines with maxsize=80, the better is trying to have shorter names.
I don't think that it would be a good
