On Wed, Feb 3, 2010 at 3:36 PM, Stefan Ringel <stefan.rin...@arcor.de> wrote:
> signed-off-by: Stefan Ringel <stefan.rin...@arcor.de>
> --- a/drivers/media/common/tuners/tuner-xc2028.c
> +++ b/drivers/media/common/tuners/tuner-xc2028.c
> @@ -1114,7 +1122,11 @@ static int xc2028_set_params(struct dvb_frontend *fe,
>
>     /* All S-code tables need a 200kHz shift */
>     if (priv->ctrl.demod) {
> -        demod = priv->ctrl.demod + 200;
> +        if ((priv->ctrl.fname == "xc3028L-v36.fw") && (priv->ctrl.demod
> == XC3028_FE_ZARLINK456) && ((type & DTV78) | (type & DTV8)) ) {
> +            demod = priv->ctrl.demod;
> +        } else {
> +            demod = priv->ctrl.demod + 200;
> +        }
>         /*
>          * The DTV7 S-code table needs a 700 kHz shift.
>          * Thanks to Terry Wu <terrywu2...@gmail.com> for reporting this
> @@ -1123,8 +1135,8 @@ static int xc2028_set_params(struct dvb_frontend *fe,
>          * use this firmware after initialization, but a tune to a UHF
>          * channel should then cause DTV78 to be used.
>          */
> -        if (type & DTV7)
> -            demod += 500;
> +        if (type & DTV7)
> +            demod += 500;
>     }

Independent of the validity of this patch, you should not be
submitting patches that have a mix of whitespace changes and actual
changes.  In the above case (the if type & DTV7 part), it looks like
these shouldn't have been included at all since it makes no functional
change.

It sounds like a nit-pick, but the reality is that its inclusion had
me staring at it for 30 seconds trying to figure out whether there was
an *actual* difference there or if it was purely whitespace.

>
>     return generic_set_freq(fe, p->frequency,
> @@ -1240,6 +1252,10 @@ static const struct dvb_tuner_ops
> xc2028_dvb_tuner_ops = {
>     .get_rf_strength   = xc2028_signal,
>     .set_params        = xc2028_set_params,
>     .sleep             = xc2028_sleep,
> +#if 0
> +    int (*get_bandwidth)(struct dvb_frontend *fe, u32 *bandwidth);
> +    int (*get_status)(struct dvb_frontend *fe, u32 *status);
> +#endif
>  };

Likewise, you should not be including unrelated changes in patches -
the above "#if 0" section not only is never compiled in to the code
(presumably it is debug code), but it has nothing to do with the fix
this patch is claiming to address.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to