[linux-dvb] RFC: merge dvb_tuner_ops & tuner-core second try

[Markus Rechberger]

On 2/27/07, Marcel Siegert <mws at linuxtv.org> wrote:
> On Tuesday 27 February 2007, Markus Rechberger wrote:
> > Hi guys,
> >
> > since I really want to get that xc3028 story done I propose to add
> > some changes to the dvb framework which would allow to support loading
> > tuner modules from v4l and dvb without having a real dependency of
> > each other.
> > The interface I aim at uses almost the same structure as dvb_tuner_ops.
> >
> > So to point it out more clearly:
> >
> > I'd like to replace or modify:
> > struct dvb_tuner_ops {
> >
> >         struct dvb_tuner_info info;
> >
> >         int (*release)(struct dvb_frontend *fe);
> >         int (*init)(struct dvb_frontend *fe);
> >         int (*sleep)(struct dvb_frontend *fe);
> >
> >         /** This is for simple PLLs - set all parameters in one go. */
> >         int (*set_params)(struct dvb_frontend *fe, struct
> > dvb_frontend_parameters *p);
> >
> >         /** This is support for demods like the mt352 - fills out the
> > supplied buffer with what to write. */
> >         int (*calc_regs)(struct dvb_frontend *fe, struct
> > dvb_frontend_parameters *p, u8 *buf, int buf_len);
> >
> >         int (*get_frequency)(struct dvb_frontend *fe, u32 *frequency);
> >         int (*get_bandwidth)(struct dvb_frontend *fe, u32 *bandwidth);
> >
> > #define TUNER_STATUS_LOCKED 1
> >         int (*get_status)(struct dvb_frontend *fe, u32 *status);
> >
> >         /** These are provided seperately from set_params in order to
> > facilitate silicon
> >          * tuners which require sophisticated tuning loops,
> > controlling each parameter seperately. */
> >         int (*set_frequency)(struct dvb_frontend *fe, u32 frequency);
> >         int (*set_bandwidth)(struct dvb_frontend *fe, u32 bandwidth);
> > };
> >
> > with:
> > struct v4l_dvb_tuner {
> >         /* wrapper */
> >         void *priv; /* some privat data for internal use */
> >         void *dev; /* v4l private data for tuner-core */
> >         struct dvb_frontend *fe; /* dvb_frontend, for dvb only
> > drivers, internal use */
> >
> >         int (*ioctl)(struct v4l_dvb_tuner *dev, int cmd, int arg);
> >         struct tuner_info info;
> >         int (*release)(struct v4l_dvb_tuner *dev);
> >         int (*init)(struct v4l_dvb_tuner *dev);
> >         int (*sleep)(struct v4l_dvb_tuner *dev);
> >
> >         /** This is for simple PLLs - set all parameters in one go. */
> >         int (*set_params)(struct v4l_dvb_tuner *dev, struct
> > tuner_parameters *p);
> >
> >         /** This is support for demods like the mt352 - fills out the
> > supplied buffer with what to write. */
> >         int (*calc_regs)(struct v4l_dvb_tuner *dev, struct
> > tuner_parameters *p, u8 *buf, int buf_len);
> >
> >         int (*get_frequency)(struct v4l_dvb_tuner *dev, u32 *frequency);
> >         int (*get_bandwidth)(struct v4l_dvb_tuner *dev, u32 *bandwidth);
> >
> > #define TUNER_STATUS_LOCKED 1
> >         int (*get_status)(struct v4l_dvb_tuner *dev, u32 *status);
> >
> >         /** These are provided seperately from set_params in order to
> > facilitate silicon
> >          * tuners which require sophisticated tuning loops,
> > controlling each parameter seperately. */
> >         int (*set_frequency)(struct v4l_dvb_tuner *dev, u32 frequency);
> >         int (*set_bandwidth)(struct v4l_dvb_tuner *dev, u32 bandwidth);
> >
> >         int  (*set_mode)(struct v4l_dvb_tuner *dev, struct
> > tuner_parameters *params);
> > };
> >
> > tuner_parameters is an extended version of dvb_frontend_params
> >
> > struct dvb_frontend_parameters {
> >         __u32 frequency;     /* (absolute) frequency in Hz for
> QAM/OFDM/ATSC */
> >                              /* intermediate frequency in kHz for QPSK */
> >         fe_spectral_inversion_t inversion;
> >         union {
> >                 struct dvb_qpsk_parameters qpsk;
> >                 struct dvb_qam_parameters  qam;
> >                 struct dvb_ofdm_parameters ofdm;
> >                 struct dvb_vsb_parameters vsb;
> >         } u;
> > };
> >
> > vs:
> >
> > struct tuner_parameters {
> >         __u32 frequency;     /* (absolute) frequency in Hz for
> QAM/OFDM/ATSC */
> >                              /* intermediate frequency in kHz for QPSK */
> >         enum v4l2_tuner_type    type;
> >         v4l2_std_id             std;
> >         fe_spectral_inversion_t inversion;
> >         union {
> >                 struct dvb_qpsk_parameters qpsk;
> >                 struct dvb_qam_parameters  qam;
> >                 struct dvb_ofdm_parameters ofdm;
> >                 struct dvb_vsb_parameters vsb;
> >         } u;
> > };
> >
> > for not breaking userspace we can use an internal converter for that
> > format just like:
> > #define V4L_OPS(i) ({ \
> >         struct tuner_parameters __o; \
> >         __o.frequency = i->frequency; \
> >         __o.inversion = i->inversion; \
> >         ....
> >         &__o; \
> > })
> >
> > this seems to be a good approach for hybrid devices
> > dvb only silicon tuners could still access dvb_frontend internally,
> > hybrid tuners have to avoid this since the structure since it wouldn't
> > be fully initialized.
> > So what do you guys think about that?
> >
> > I already have some code which does this and it works fine.
> > So I'm looking for some feedback/suggestions here
> >
> > thanks,
> > Markus
> >
> > _______________________________________________
> > linux-dvb mailing list
> > linux-dvb at linuxtv.org
> > http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
> >
> hi,
>
> as already told on irc (summary of one hour talk)
>
> i am against this patch as this sort of thing could be implemented in the
> same way
> like e.g. saa7134
>
> this implements your changes to be unneccessary.
>
> best regards
> marcel
>
>

Marcel you never had a look at the saa7134 so please only write about
things you really know about.

You mean lines like:
        /* setup tuner buffer */
        tuner_buf[0] = (tuner_frequency >> 8) & 0x7f;
        tuner_buf[1] = tuner_frequency & 0xff;
        tuner_buf[2] = 0xca;
        tuner_buf[3] = (cp << 5) | (filter << 3) | band;

        if (fe->ops.i2c_gate_ctrl)
                fe->ops.i2c_gate_ctrl(fe, 1);
        if (i2c_transfer(&dev->i2c_adap, &tuner_msg, 1) != 1) {
                printk("%s/dvb: could not write to tuner at addr:
0x%02x\n",dev->name, addr << 1);
                return -EIO;
        }
        msleep(1);
        return 0;
}

in saa7134-dvb.c .. then I say no thanks.
thanks,
Markus