[linux-dvb] [PATCH] add DVB-S2 support to frontend.h

[Marcel Siegert]

On Tuesday 28 February 2006 19:21, Alan Nisota wrote:
> On 2/27/06, Marcel Siegertwrote:
> > -struct dvb_qam_parameters {
> > +struct dvb_dvbc_parameters {
> >         __u32           symbol_rate; /* symbol rate in Symbols per second */
> >         fe_code_rate_t  fec_inner;   /* forward error correction (see above) */
> >         fe_modulation_t modulation;  /* modulation type (see above) */
> ...
> > +struct dvb_dvbs2_parameters {
> > +       __u32                   symbol_rate;    /* symbol rate in Symbols per second */
> > +       fe_code_rate_t          fec_inner;      /* forward error correction (see above) */
> > +       fe_rolloff_factor_t     rolloff_factor; /* rolloff factor needed for dvb-s2 */
> > +       fe_modulation_t         modulation; /* modulation type (see above) */
> > +};
> 
> Just a tiny nit.  can we keep the order of variables the same where
> possible between dvb types?  It should never matter, but it just makes
> more sense to me.  So the struct would look like this instead:
we can of course, but as you always will call the struct members by its names
it is just a kind of beautiness thing. i'll change that for having a similiar looking but extended 
structure.

> > +struct dvb_dvbs2_parameters {
> > +       __u32                   symbol_rate;    /* symbol rate in Symbols per second */
> > +       fe_code_rate_t          fec_inner;      /* forward error correction (see above) */
> > +       fe_modulation_t         modulation; /* modulation type (see above) */
> > +       fe_rolloff_factor_t     rolloff_factor; /* rolloff factor needed for dvb-s2 */
> > +};
> 
> Also, why did you define FE_CAN_DVB_S2?  Isn't
> 'FE_HAS_EXTENDED_CAM_VALUES' good enough to be able to determine this?
the FE_HAS_EXTENDED_CAN_VALUES just reports about capabilities(mod/fec/misc things) NOT the frontend type

> And as for the comment that goes with the FE_SET_STANDARD define, do
> you expect the frontend to change its caps based on how set-standard
> is defined?  In general,  I can see how this would be needed in some
> cases, but for dvb-s/s2 cards, it is likely unnecessary as one is a
> superset of the other.
imho, no it is not. of course there is a kind of dvb-s2 is a superset of dvb-s
but is dvb-t a superset of dvb-c on a combined frontend? i don't think so.
and yes, if a FE_SET_STANDARD is called, e.g. DVB-S on a DVB-S frontend
it should still report, that it is a DVB-S2 frontendend but should only reflect
DVB-S capabilities.
> 
> Otherwise it looks ok to me.  I can update the genpix patch to work
> with this API easily enough.  Are you planning to implement
> FE_GET_EXTENDED_INFO and FE_SET_STANDARD once this gets approved?
i will at the additional ioctl handling in dvb-core - adjusting the frontend drivers is just 
needed for those who support, imho. all other will return -ENOTSUPPORTED or something
similar.

> Thanks
you're welcome

marcel