On Sun, 30 Apr 2006, Manu Abraham wrote: > Updated the patch again, keeping in mind all the comments. Thanks to all > who provided feedback to improve it further. > Attached is the latest changes. The modcod stuff.. +struct modcod_table { +struct modcod_table dvbs2_modcod_lookup[] = { + { FE_MODCOD_DUMMY_PLFRAME, FE_MOD_NONE, FE_FECRATE_NONE }, +int decode_dvbs2_modcod(struct dvb_frontend *fe, enum fe_modcod modcod) It looks like the assignement of certain values to modulation-fec rate combinations is specific one demodulator's registers. Is there anything in the standard that says the value 3 corrisponds to QPSK and 2/5? IMHO, the modcod_table stuff should go in the driver for the hardware that uses it. If a new drivers appears that can make use of the same code, then it can be moved into the core at that time. The modcod enum doesn't belong in the public header file since nothing in the public interface uses it. + * FE_MOD_AUTO, Some devices/drivers, might support + * auto detecting the modulation type, depending on + * some other settings and or hardware features. + */ +enum fe_modulations { + FE_MOD_IGNORE = (0 << 0), + FE_MOD_NONE = (1 << 0), + FE_MOD_BPSK = (1 << 1), First, how about adding QAM_512 and QAM_1024 in the expected spot. There is already DOCSIS cable modem cardware than can use these modulations. You might want to add VSB_16 too, it's part of the ATSC spec. Second, what's FE_MOD_NONE for? Is it used for input to the kernel, output from the kernel, or both? Third, FE_MOD_AUTO isn't enough. Take the or51132 demodulator. It can demodulate VSB-16, QAM-64, and QAM-256. It can automatically detect QAM-64 vs QAM-256, but not QAM vs VSB. If an app sets the demodulator to MOD_AUTO, what firmware should it load, qam or vsb? There is no way for the driver to know. There is no way for an app to know that it can't just say 'AUTO', it needs to choose QAM or VSB. Maybe it would be enough to add QAM_AUTO and PSK_AUTO demodulations? + * NOTE: All current delivery systems need inversion in some + * way or the other, due to hardware implementation specifics. + * and/or others. I thought inversion was caused the hardware design and not by the transmitter? It was something that the card drivers should set, not user apps? +struct dvbs2_params { + enum fe_fecrates fecrate; + enum fe_fecrates coderate_HP; + enum fe_fecrates coderate_LP; What is the difference between fecrate and coderate? +/** + * DVB-T parameters + */ +struct dvbt_params { + enum fe_modulations constellation; + enum fe_bandwidths bandwidth; + enum fe_fecrates code_rate_HP; + enum fe_fecrates code_rate_LP; This uses 'constellation' where everywhere else 'modulation' is used. Also 'code_rate' vs 'fecrate' or 'coderate'. +/** + * ATSC parameters + */ +struct atsc_params { + enum fe_modulations modulation; + enum fe_fecrates fecrate; +}; I don't think ATSC supports multiple fec rates? It only allows for 2/3. + +/** + * Padding to handle future binary compatibility issues. + * + * NOTE: The padding is a dummy parameter ! + */ +struct pad_params { + __u8 pad[512]; +}; You could just make __u8 pad[512] one of members of the delsys union, then you wouldn't have to define this struct at all. _______________________________________________ linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb