Re: [PATCH] Re: [PATCH] Multi protocol support (stage #1)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Trent Piepho wrote:
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?


Yes, it explicitly says that.

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.


No, since it is part of the spec and not having anything to do with the driver as it is. Some drivers return this explicitly as part of register settings , not all drivers do comply.


+ *     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.


ok, will add QAM_512 and QAM_1024


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?


The driver has to choose the relevant modulation. Modulation Auto is used in the case where the hardware can tell the driver whether it can support the same. Or the driver is capable. Current single standard (delivery system) drivers/hardware do not support this.

+/**
+ *     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'.

I have used the terms as to reflect the specs, yet maintain consistency. DVB-T uses constellation instead of modulation.
some changes have been used to avoid conflicts with the existing names.

+/**
+ *     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.


okay, so all the driver has to say that it supports only 2/3. If it supports only 2/3. Or the option is to remove fecrates from ATSC. IMHO fecrates in ATSC would be better for use at any later stage.

+
+/**
+ *     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.

If you see the structure, i have put this struct only for maintaining the naming consistency. If i put the pad[512] into the union straightaway, that consistency is lost.




_______________________________________________

linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

[Index of Archives]     [Linux Media]     [Video 4 Linux]     [Asterisk]     [Samba]     [Xorg]     [Xfree86]     [Linux USB]

  Powered by Linux