[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-dvb
Subject:    Re: [linux-dvb] [PATCH] add DVB-S2 support to frontend.h
From:       Andreas Oberritter <obi () linuxtv ! org>
Date:       2006-02-28 19:01:29
Message-ID: 1141153289.4178.40.camel () ip6-localhost
[Download RAW message or body]

Hi Marcel,

On Mon, 2006-02-27 at 22:59 +0100, Marcel Siegert wrote:
> +/*! describes the available modulation types. */
> +enum dvb_fe_modulation {
> +	DVB_FE_MOD_QPSK		= (1 << 0),
> +	DVB_FE_MOD_QAM_16	= (1 << 1),
> +	DVB_FE_MOD_QAM_32	= (1 << 2),
> +	DVB_FE_MOD_QAM_64	= (1 << 3),
> +	DVB_FE_MOD_QAM_128	= (1 << 4),
> +	DVB_FE_MOD_QAM_256	= (1 << 5),
> +	DVB_FE_MOD_QAM_AUTO	= (1 << 6),
> +	DVB_FE_MOD_2_VSB	= (1 << 7),
> +	DVB_FE_MOD_4_VSB	= (1 << 8),
> +	DVB_FE_MOD_8_VSB	= (1 << 9),
> +	DVB_FE_MOD_16_VSB	= (1 << 10),
> +	DVB_FE_MOD_8_PSK	= (1 << 11),
> +	DVB_FE_MOD_BPSK		= (1 << 12),
> +	DVB_FE_MOD_MOD_UNKNOWN	= (1 << 31)
> +};

why do we need DVB_FE_MOD_QAM_AUTO and DVB_FE_MOD_MOD_UNKNOWN? I suggest
replacing them with DVB_FE_MOD_AUTO. Maybe MOD_UNKNOWN was meant to be
returned by FE_GET_FRONTEND, but if the modulation is unknown, then it
is very unlikely that it was possible to lock the frontend and therefore
calling FE_GET_FRONTEND in such a case seems to be useless. I think
FE_GET_FRONTEND should only replace the known bits of the tuning
parameters as it does today.

I also wonder why QAM and VSB values are sorted by size, but PSK values
are sorted alphanumerically.

> +/*! describes common supported frontend capabilities. */
> +enum dvb_fe_common_cap {
> +	DVB_FE_CAN_INVERSION_AUTO   = (1 << 0), /*!< fixme */
> +	DVB_FE_CAN_RECOVER          = (1 << 1), /*!< frontend can recover from a cable \
> unplug automatically */ +	DVB_FE_CAN_MUTE_TS          = (1 << 2), /*!< frontend can \
> stop spurious TS data output */ +	DVB_FE_SUP_HIGH_LNB_VOLTAGE = (1 << 31), /*!< \
> fixme, frontend can deliver higher lnb voltages */ +};

Why are there FIXMEs?

> +/*! describes other available, frontend type dependent capabilities. */
> +enum dvb_fe_other_cap {
> +	DVB_FE_CAN_TRANSMISSION_MODE_AUTO = (1 << 0), /*!< fixme (DVB-T specific) */
> +	DVB_FE_CAN_BANDWIDTH_AUTO         = (1 << 1), /*!< fixme (DVB-T specific) */
> +	DVB_FE_CAN_GUARD_INTERVAL_AUTO    = (1 << 2), /*!< fixme (DVB-T specific) */
> +	DVB_FE_CAN_HIERARCHY_AUTO         = (1 << 31), /*!< fixme (DVB-T specific) */
> +};

Same here. Well, I know these are specific to DVB-T and proably DVB-H,
but how are these FIXMEs supposed to get fixed?

> @@ -126,7 +194,8 @@
> 	FE_HAS_SYNC	= 0x08,   /*  found sync bytes  */
> 	FE_HAS_LOCK	= 0x10,   /*  everything's working... */
> 	FE_TIMEDOUT	= 0x20,   /*  no lock within the last ~2 seconds */
> -	FE_REINIT	= 0x40    /*  frontend was reinitialized,  */
> +	FE_REINIT	= 0x40,   /*  frontend was reinitialized,  */
> +	FE_EXTENDED_TP  = 0x80    /*  providing new informations */
> } fe_status_t;			  /*  application is recommended to reset */

First, it's "new information". What shall be reset and what new kind of
information is provided through which interface?
 
> +struct dvb_dvbs2_parameters {
> +	__u32			symbol_rate;    /* symbol rate in Symbols per second */

It's symbols, not Symbols.

Best regards,
Andreas


_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


[prev in list] [next in list] [prev in thread] [next in thread]