Re: PATCH: Query DVB frontend capabilities

[Andreas Oberritter <obi@linuxtv.org>]

On 11.11.2011 18:14, Mauro Carvalho Chehab wrote:
> Em 11-11-2011 13:06, Andreas Oberritter escreveu:
>> On 11.11.2011 15:43, Mauro Carvalho Chehab wrote:
>>> IMHO, the better is to set all parameters via stb0899_get_property(). We should
>>> work on deprecate the old way, as, by having all frontends implementing the
>>> get/set property ops, we can remove part of the legacy code inside the DVB core.
>>
>> I'm not sure what "legacy code" you're referring to. If you're referring
>> to the big switch in dtv_property_process_get(), which presets output
>> values based on previously set tuning parameters, then no, please don't
>> deprecate it. It doesn't improve driver code if you move this switch
>> down into every driver.
> 
> What I mean is that drivers should get rid of implementing get_frontend() and 
> set_frontend(), restricting the usage of struct dvb_frontend_parameters for DVBv3
> calls from userspace.

This would generate quite some work without much benefit.

> In other words, it should be part of the core logic to get all the parameters
> passed from userspace and passing them via one single call to something similar
> to set_property.

That's exactly what we have now with the set_frontend, tune, search and
track callbacks.

> In other words, ideally, the implementation for DTV set should be
> like:
> 
> static int dtv_property_process_set(struct dvb_frontend *fe,
> 				    struct dtv_property *tvp,
> 				    struct file *file)
> {
> 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> 
> 	switch(tvp->cmd) {
> 	case DTV_CLEAR:
> 		dvb_frontend_clear_cache(fe);
> 		break;
> 	case DTV_FREQUENCY:
> 		c->frequency = tvp->u.data;
> 		break;
> 	case DTV_MODULATION:
> 		c->modulation = tvp->u.data;
> 		break;
> 	case DTV_BANDWIDTH_HZ:
> 		c->bandwidth_hz = tvp->u.data;
> 		break;
> ...
> 	case DTV_TUNE:
> 		/* interpret the cache of data */
> 		if (fe->ops.new_set_frontend) {
> 			r = fe->ops.new_set_frontend(fe);
> 			if (r < 0)
> 				return r;
> 		}

set_frontend is called by the frontend thread, multiple times with
alternating parameters if necessary. Depending on the tuning algorithm,
drivers may implement tune or search and track instead. You cannot just
call a "new_set_frontend" driver function from here and expect it to
work as before.

> 		break;
> 
> E. g. instead of using the struct dvb_frontend_parameters, the drivers would
> use struct dtv_frontend_properties (already stored at the frontend
> struct as fe->dtv_property_cache).

Drivers are already free to ignore dvb_frontend_parameters and use the
properties stored in dtv_property_cache today.

> Btw, with such change, it would actually make sense the original proposal
> from Manu of having a separate callback for supported delivery systems.

Why? How does setting parameters relate to querying capabilies?

Regards,
Andreas