Hi,
now we had bad weather, and I had some time to review the code. ;-)
General note
------------
Obviously, the multiproto tree has not been updated from master for a
very long time. When merging care must be taken that no regressions flow
back to the main development tree.
For now I ignored all differences, except for:
- frontend.h
- dvb_frontend.[ch]
- budget-ci.c
- budget-av.c
API extensions (frontend.h)
---------------------------
looks fine
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.
dvb-core: dvb_frontend.c
------------------------
fe->legacy is turned on/off by various ioctls:
- FE_SET_FRONTEND, FE_GET_FRONTEND -> 1
- DVBFE_SET_PARAMS -> 0/1
- DVBFE_GET_PARAMS, DVBFE_GET_DELSYS, DVBFE_GET_INFO -> 0
fe->legacy controls how the frontend thread works.
Since the frontend device can be accessed by multiple readers,
fe->legacy might be toggled in funny ways.
This might confuse the fe thread or dvb_frontend_add_event. ;-(
Imho ioctls should not set fe->legacy. All parameter conversions should
be done within the ioctl. For example:
- new driver: FE_SET_FRONTEND converts parms to new driver API
- old driver: DVBFE_SET_PARAMS converts parms to old driver API
Then the fe thread can simply use the old driver interface
(fe->legacy==1) or the new one (fe->legacy==0).
The error msg should display the offending parameter:
Instead of
+ printk("%s: Unsupported FEC\n", __func__);
you might use
+ printk(KERN_ERR "%s: Unsupported FEC %x\n", __func__, new_fec);
(same way for other conversion routines)
+ /* 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.
/* 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.
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)
CU
Oliver
--
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
----------------------------------------------------------------
_______________________________________________
linux-dvb mailing list
[email protected]
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
