[linux-dvb] PATCH: Generic FE_DISHNETWORK_SEND_LEGACY_CMD support

[Johannes Stezenbach]

On Thu, Sep 22, 2005 NooneImportant wrote:
> I had been requested to add support for the legacy dish-network switches for
> other frontends besides the stv0299. This patch does this using set_voltage
> commands in the case that fe->ops->dishnetwork_send_legacy_command isn't
> defined (as I've said before, at least the stv0299 based budget cards
> respond too slowly to a set_voltage for thegeneric method to work). this
> will likely not work with all frontends, but hopefully will work with
> enough.
> 
> I also moved the timing functions into dvb_frontend.c so that we don't need
> duplicate copies for each frontend.

If you add dishnetwork_send_legacy_command code to dvb_frontend,
shouldn't stv0299 just use it, too? Otherwise it *is* a duplication
of code, isn't it?

Also, please add your Signed-off-by: if you want the patch to be
applied.

There are a few nits:

> Index: linux/drivers/media/dvb/dvb-core/dvb_frontend.c
> ===================================================================
> RCS file: /cvs/linuxtv/dvb-kernel/linux/drivers/media/dvb/dvb-core/dvb_frontend.c,v
> retrieving revision 1.112
> diff -u -r1.112 dvb_frontend.c
> --- linux/drivers/media/dvb/dvb-core/dvb_frontend.c	1 Jul 2005 23:20:19 -0000	1.112
> +++ linux/drivers/media/dvb/dvb-core/dvb_frontend.c	22 Sep 2005 17:15:13 -0000
> @@ -577,6 +577,36 @@
>  				fepriv->thread_pid);
>  }
>  
> +inline s32 dvb_frontend_calc_usec_delay (struct timeval lasttime, struct timeval curtime)
> +{
> +	return ((curtime.tv_usec < lasttime.tv_usec) ?
> +		1000000 - lasttime.tv_usec + curtime.tv_usec :
> +		curtime.tv_usec - lasttime.tv_usec);
> +}

shouldn't this be called something like timeval_usec_diff()? It isn't
dvb_frontend specific. BTW, it doesn't look like you need usec
precision, so it would be better to use msec units.

Also: No space between function name and opening parenthesis (repeats
all over).

> +void dvb_frontend_sleep_until (struct timeval *waketime, u32 add_usec)
> +{
> +	struct timeval lasttime;
> +	s32 delta, newdelta;
> +
> +	waketime->tv_usec += add_usec;
> +	if (waketime->tv_usec >= 1000000) {
> +		waketime->tv_usec -= 1000000;
> +		waketime->tv_sec++;
> +	}

You might as well add timeval_usec_add().

> +
> +	do_gettimeofday (&lasttime);
> +	delta = dvb_frontend_calc_usec_delay (lasttime, *waketime);
> +	if (delta > 2500) {
> +		msleep ((delta - 1500) / 1000);
> +		do_gettimeofday (&lasttime);
> +		newdelta = dvb_frontend_calc_usec_delay (lasttime, *waketime);
> +		delta = (newdelta > delta) ? 0 : newdelta;
> +	}
> +	if (delta > 0)
> +		udelay (delta);
> +}

dvb_frontend_sleep_until is so idosyncratic, it needs at least a
comment. And of course it will fail to meet its deadline when system
load is high enough, or with HZ == 100. But I guess there's nothing
one ca do about it.

>  static int dvb_frontend_start(struct dvb_frontend *fe)
>  {
>  	int ret;
> @@ -728,6 +758,41 @@
>  			err = fe->ops->dishnetwork_send_legacy_command(fe, (unsigned int) parg);
>  			fepriv->state = FESTATE_DISEQC;
>  			fepriv->status = 0;
> +		} else if (fe->ops->set_voltage) {

This also needs a comment, and if possible a reference to the
dishnetwork spec.

> +			for (i=0; i<9; i++) {

Please add space between operators.

Otherwise patch looks OK.

Johannes