On Sat, May 06, 2006, Manu Abraham wrote:
> Johannes Stezenbach wrote:
> >On Fri, May 05, 2006, Manu Abraham wrote:
> >>+/**
> >>+ * Supported Delivery systems, some devices
> >>+ * are capable of supporting multiple delivery systems.
> >>+ *
> >>+ * FE_DELSYS_IGNORE, causes the delivery system
> >>+ * not to be queried.
> >>
> >
> >why is IGNORE useful?
As Trent explained it is used to query supported delivery systems
via GET_CAPS. So FE_DELSYS_IGNORE should be renamed to
FE_DELSYS_QUERY // use with FE_GET_CAPS to get bit set of delivery systems supported by hw
> >>+enum fe_matype {
> >>+ FE_MATYPE_IGNORE = (0 << 0),
> >>+ FE_MATYPE_TRANSPORT = (1 << 0),
> >>+ FE_MATYPE_GENERIC_PACKET = (1 << 1),
> >>+ FE_MATYPE_GENERIC_CONTINUOUS = (1 << 2),
> >>+ FE_MATYPE_RESERVED = (1 << 3),
> >>+ FE_MATYPE_AUTO = (1 << 31)
> >>+};
> >>
> >
> >enum fe_matype isn't used anywhere
> >
> >also, please always spell out non-obvious acronyms in a comment
> >(probably "mode adaption type")
> >
> >But EN 302 307 defines MATYPE as a two-byte field, this
> >enum fe_matype is just the 2bit TS/GS field of the
> >whole MATYPE.
> >
> >
>
> Form these 3 and
You wanted to say something? ;-)
> >>+
> >>+/**
> >>+ * Supported Modulations.
> >>+ * Some devices support multiple modulation types
> >>
> >
> >actually, most demods do
> >
> >
> >>+enum fe_modulations {
> >>
> >
> >I prefer the singular for enum type names.
> >
> >
>
> But that already exists, and did not want to tamper that.
That's actually another good reason to change the
name, people will confuse enum fe_modulation and
enum fe_modulations all the time.
Maybe fe_mod_types?
> >>+enum fe_modcod {
> >>+ FE_MODCOD_DUMMY_PLFRAME = 0,
> >>
> >
> >enum fe_modcod isn't used anywhere except the
> >decode_dvbs2_modcod() helper function. Either
> >use it in the API or move it to dvb_frontend.h.
> >
> >
> >
>
> Compliant drivers uses the modcod straight away. I was still working on
> the STB0899. I was of the thought would add it alongwith it.
But it isn't *used* anywhere in the API. Did you mean
to add a modcod field to dvbs2_params ?
If not, then why is fe_modcod in frontend.h?
> >>+enum fe_hierarchy_info {
> >>+enum fe_rolloff {
> >>+enum fe_interleaver {
> >>
> >
> >EN 300 468 V1.7.1 combines these in a single
> >hierarchy_information field. Is it useful to
> >split them into three enums?
> >
> >
>
> Well, i originally had it as one only, after the innumerous discussions
> it became like that.
OK
> >>+struct dvbs2_params {
> >>+ __u32 symbol_rate;
> >>+
> >>+ enum fe_modulations modulation;
> >>+ enum fe_fecrates fecrate;
> >>+ enum fe_stream streamtype;
> >>+ enum fe_fecrates coderate_HP;
> >>+ enum fe_fecrates coderate_LP;
> >>
> >
> >do we really have three fecs?
> >also, if you have hierachical modulation you need a way to
> >select the HP or LP stream
> >
>
> streamtype is used for that.
Ah, I missed that (was looking for "priority")...
Three fecs is still one too many, right?
> >I don't fully understand the DVB-S2 spec yet, but I think
> >the way to select the stream is by using an 8bit
> >input_stream_identifier.
>
> H.3 describes it. 8 bits isn;t needed, anyway boiling down to the demod,
> that doesn't happen, it is a simple knob to select between the streams.
>
> There aren't many S2 chipsets. ST has STB0899, Broadcom has 4500.
> Croadcom doesn't sell to PC card manufacturers, but only STB vendors.
> There is Conexant also as i heard, but a very buggy chip. Well the S2
> devices do have bugs. Even the datasheets have bugs.
You mean the dvbs2_delivery_system description from EN 300 468
is irrelevant for the current hardware? I.e. availbale
DVB-S2 tuners only support "normative broadcast mode"?
(I wouldn't be surprised).
It also explains why enum fe_matype isn't used. I vote
for removing it for now.
> >>+struct pad_params {
> >>+ __u8 pad[512];
> >>+};
> >>
> >
> >just scratch this struct and put the pad directly in the union
> >
> >
>
> It would make it look a bit consistent though. Everyone else really
> liked it, except for your comment and Trent's.
I prefer that if you look at the union you immediately
see that it is padded to N bytes.
> >>+struct dvb_frontend_cap {
> >>+ char name[128];
> >>+
> >>+ __u32 frequency_min;
> >>+ __u32 frequency_max;
> >>+ __u32 frequency_stepsize;
> >>+ __u32 frequency_tolerance;
> >>+ __u32 symbol_rate_min;
> >>+ __u32 symbol_rate_max;
> >>+ __u32 symbol_rate_tolerance;
> >>+
> >>+ enum fe_delsys delivery;
> >>
> >
> >is this a bitset?
OK Trent explained to me how it works.
How about not deprecating FE_GET_INFO, but just deprecate the "type"
and "caps" fields of struct dvb_frontend_info?
No need to query name and min/max stuff over and over...
> Delivery System, Yes. There can be multistandard devices
> >>+ enum fe_inversion inversion;
What does the inversion do? dvb-core emulates FE_INVERSION_AUTO
if the hw can't do it, so caps would always report full
capabilities here, right?
> static int stb0899_get_caps(struct dvb_frontend *fe,
> struct dvb_frontend_cap *caps)
> {
> switch (caps->delivery) {
> case FE_DELSYS_DVBS:
> stb0899_get_dvbs_caps(fe, caps);
> break;
> case FE_DELSYS_DSS:
> stb0899_get_dss_caps(fe, caps);
> break;
> case FE_DELSYS_DVBS2:
> stb0899_get_dvbs2_cap(fe, caps);
> }
>
> return 0;
> }
This misses the FE_DELSYS_IGNORE case...
> >BTW, is anyone working on a driver for a DVB-S2 tuner?
> >Or does anyone have a data sheet for such a device?
> >
> >
> Yes, quite a lot of people have it now (as i understand, other than some
> people around us), well that chips is something to make people real
> crazy. The documents are available under NDA only.
>
> Well, i have done ~ 3000 lines of code for the STB0899 + STB6100. I got
> approval from Twinhan to publish the driver under OSS.
>
> There are 4 cards now, (1) The Twinhan card (not the DST, but a brand
> new one) (2) KNC1 (SAA7146 based) (3) TechnoTrend (SAA7146 based) (4)
> Micronas (nGene) But none of these cards are out for purchase for the
> public as of now, some few sample cards were sold/given by all these
> vendors.
>
> Currently, myself is taking a go at it. Ralph is having a nGene. For
> testing we have a KNC1 and a Twinhan. We can test with the nGene too.
>
> As far as i heard some people are trying to cut and paste the Windows
> code for a minimal Linux DVB-S driver and trying to get an approval from
> the vendor (from the ST sample code), but the problem is that the
> original code for the DVB-S2 specific parts (It has 2 DSP's builtin, i2c
> is really different) is not from ST itself, forgetting about the vendor
> itself, but from a 3rd party and hence cannot be cut n' pasted.
>
> But we (myself, Christoph, Andrew and Ralph) are working on it (you
> might've seen the math functions, it all goes in there, there are more)
> (Other's did help help in fixing various Areas, people who can be
> mentioned are Marcel (mws) Sigmund and Julian providing various
> feedbacks and bug hunting etc) Most of the things are done, it needs
> some fixing here and there due to some bugs.
>
> It has a Silicon Tuner based on the STB6100, hence the earlier
> TUNER_RE_FACTORING by Andrew, which additionally took care of the PLL
> thing too.
>
> Regarding the Broadcom device, i think there is a driver (genpix) which
> has a dst style frontend. But as i looked at the code , i didn't see any
> S2 specifics.
Thanks for this info!
> Wow, didn't think that so much was there to be fixed ! Thanks for taking
> the time to hunt them down. Taking a deep breath now.
I was surprised that so many Ack'ed this in it's current state,
but then I guess you all have been discussing this in private
mail and irc and you were all happy with it already.
I'm sorry that my late commenting delays this further.
I'm still not sure about the DVB-S2 API. So I would prefer
if the whole change would not be merged into mainline
until there is at least one fully functional DVB-S2 driver
in the tree (i.e. keep it in a v4l-dvb-s2 repo for now).
If you think this is too much hassle, then at least
mark the DVB-S2 stuff as "EXPERIMENTAL / DO NOT USE / SUBJECT
TO CHANGE WITHOUT NOTICE" (in capitals). So that you can
break binary and source compatibilty for the DVB-S2 part
with clean conscience until it is proven stable.
Agreed?
Johannes
_______________________________________________
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
