Manu Abraham wrote:
> Oliver Endriss wrote:
> > Card drivers (budget-ci, budget-av)
> > -----------------------------------
> > I didn't check the details, but the extensions look ok.
> > You might consider whether parts of the stb0899/stb6100 stuff could be
> > factored out into a header file. See bsru6.h for an example.
> > It would reduce the file size, and tables etc could be reused.
>
> I did visit this, but there result looked thus, the device is not specific to a tuner
> as it is, and in each of the card configurations, the tables do have some changes
> It depends on the card manufacturer.
>
> Thinking thus, i thought of moving all the tables to a header where, something
> such as stb0899_config.h where all the card specific tables are thrown in. This
> doesn't reduce any of the compiled object size, but it does bring in the advantage
> that budget_ci.c, budget_av.c are not cluttered with large tables.
>
> What do you think of moving all the config's to a public config file ?
Fine. Imho we should focus on maintainability, not on object size.
Maintainer's time is the most valuable ressource. Having the configs in
a central file makes it easier to reuse the tables and to keep them
in-sync.
Otherwise someone would probably just copy/paste the stuff for a new
driver, as it was done in the ttpci driver family. There are tons of
config tables which should be cleaned up and (possibly) merged.
Due to lack of hardware and/or testers this is nearly impossible now
without risking to break a driver... :-(
> >
> >
> > + /* Legacy */
> > + if (fe->legacy) {
> > + if (fe->ops.set_frontend)
> > + fe->ops.set_frontend(fe, &fepriv->parameters);
> > + } else {
> > +// if ((dvbfe_sanity_check(fe) == 0)) {
> > + /* Superseding */
> > + if (fe->ops.set_params)
> > + fe->ops.set_params(fe, &fepriv->fe_params);
> > +// } else
> > +// return -EINVAL;
> > + }
> >
> > Sanity checks should be done in FE_SET_FRONTEND/DVBFE_SET_PARAMS ioctls.
> > (See HG master where I added this some time ago.)
> > Otherwise the application would not see an error status.
>
> Since you already have a check, i guess i can drop the dvbfe_sanity_check()
Yep. Feel free to add more checks to dvb_frontend_check_parameters().
Atm it only checks the limits of frequency and symbol rate.
> > /* don't actually do anything if we're in the LOSTLOCK state,
> > - * the frontend is set to FE_CAN_RECOVER, and the max_drift is 0 */
> > - if ((fepriv->state & FESTATE_LOSTLOCK) &&
> > - (fe->ops.info.caps & FE_CAN_RECOVER) && (fepriv->max_drift == 0)) {
> > - dvb_frontend_swzigzag_update_delay(fepriv, s & FE_HAS_LOCK);
> > - return;
> > + * the frontend is set to FE_CAN_RECOVER, and the max_drift is 0
> > + */
> > + /* Legacy */
> > + if (fe->legacy) {
> > + if ((fepriv->state & FESTATE_LOSTLOCK) && (fepriv->max_drift == 0)) {
> > + if (fe->ops.get_frontend_algo)
> > + if (fe->ops.get_frontend_algo(fe) == DVBFE_ALGO_RECOVERY)
> > + dvb_frontend_swzigzag_update_delay(fepriv, s & FE_HAS_LOCK);
> > +
> > + return 0;
> > + }
> > + } else {
> > + if (fepriv->state & FESTATE_LOSTLOCK) {
> > + if (fe->ops.get_frontend_algo) {
> > + if ((fe->ops.get_frontend_algo(fe) == DVBFE_ALGO_RECOVERY) &&
> > + (fepriv->max_drift == 0)) {
> > +
> > + dvb_frontend_swzigzag_update_delay(fepriv, s & DVBFE_HAS_LOCK);
> > + return 0;
> > + }
> > + }
> > + }
> > }
> >
> > The 'if (fe->legacy)' branch looks broken:
> > fe->ops.get_frontend_algo is most likely zero, so
> > dvb_frontend_swzigzag_update_delay will not be called for old drivers.
> >
>
> This one's fixed although i see no usage of FE_CAN_RECOVER which is used in the
> tree currently, rather than bogus usage
>
> > Finally, I did a quick test with this tree, and it worked. ;-)
> > - dvb-ttpci driver with DVB-S Rev 1.3 (ves1x93)
> > - budget driver with DVB-S Nova Rev 1.0 (stv0299)
> > - VDR (using old API)
>
> Marco pointed to another bug a wrong copy, which breaks backward compatibility
> for DVBFE_GET_PARAMS
>
> Any further issues that you see ?
No.
> Additionally i did work upon the Multiple TS stuff, haven't got it working yet.
> The concept works like this, unlike what we thought would be:
>
> Manu,
>
> The filtering is just like an IP address and subnet mask.
> ISI_ENTRY equivalent to IP address.
> IS_BIT_EN equivalent to subnet mask.
> The incoming Bbheaders are compared to ISI_ENTRY through mask IS_BIT_EN.
> Those Bbheaders that pass the test are allowed through to the TS. Those
> that do not are deleted.
Maybe someone with more knowledge about this stuff could comment and/or
review the code?
CU
Oliver
--
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
----------------------------------------------------------------
_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
