Em 13-05-2011 04:08, Manoel PN escreveu:
> This patch implements mb86a20s_read_snr and adds mb86a20s_read_ber and 
> mb86a20s_read_ucblocks both without practical utility but that programs as 
> dvbsnoop need.

Same as the prev. patch: break into 75 columns max, and you don't need
to use "This patch".
> 
> Signed-off-by: Manoel Pinheiro <pinus...@hotmail.com>
> 
 
> diff --git a/drivers/media/dvb/frontends/mb86a20s.c 
> b/drivers/media/dvb/frontends/mb86a20s.c
> index 0f867a5..0de4abf 100644
> --- a/drivers/media/dvb/frontends/mb86a20s.c
> +++ b/drivers/media/dvb/frontends/mb86a20s.c
> @@ -411,6 +411,56 @@ err:
>       return rc;
>  }
>  
> +static int mb86a20s_read_snr(struct dvb_frontend *fe, u16 *snr)
> +{
> +     struct mb86a20s_state *state = fe->demodulator_priv;
> +     int i, cnr, val, val2;
> +
> +     for (i = 0; i < 30; i++) {
> +             if (mb86a20s_readreg(state, 0x0a) >= 2)
> +                     val = mb86a20s_readreg(state, 0x45); /* read cnr_flag */
> +             else
> +                     val = -1;
> +             if (val > 0 && ((val >> 6) & 1) != 0) {
> +                     val2 = mb86a20s_readreg(state, 0x46);
> +                     val = mb86a20s_readreg(state, 0x47);
> +                     if (val2 >=0 && val >= 0) {
> +                             cnr = (val2 << 0x08) | val;
> +                             if (cnr > 0x4cc0) cnr = 0x4cc0;
> +                             val = ((0x4cc0 - cnr) * 10000) / 0x4cc0;
> +                             val2 = (65535 * val) / 10000;
> +                             *snr = (u16)val2;
> +                             dprintk("snr=%i, cnr=%i, val=%i\n", val2, cnr, 
> val);
> +                             /* reset cnr_counter */
> +                             val = mb86a20s_readreg(state, 0x45);
> +                             if (val >= 0)
> +                             {
> +                                     mb86a20s_writereg(state, 0x45, val | 
> 0x10);
> +                                     msleep(5);
> +                                     mb86a20s_writereg(state, 0x45, val & 
> 0x6f); /* FIXME: or 0xef ? */
> +                             }
> +                             return 0;
> +                     }
> +             }
> +             msleep(30);
> +     }
> +     *snr = 0;
> +     dprintk("no signal!\n");
> +     return 0;
> +}



> +static int mb86a20s_read_ber(struct dvb_frontend *fe, u32 *ber)
> +{
> +     *ber = 0;
> +     return 0;
> +}
> +
> +static int mb86a20s_read_ucblocks(struct dvb_frontend *fe, u32 *ucblocks)
> +{
> +     *ucblocks = 0;
> +     return 0;
> +}

As you're providing just a stub function there, without an actual 
implementation,
the better is to add a FIXME note about that, explaining why you had to add such
function, like:

/*
 * FIXME: read_ucblocks callback is needed, as dvbsnoop uses it.
 * However, we currently don't know how to actually implement it.
 * So add a function that just returns 0 instead.
 */

This way, somebody else with enough information may later implement.

I don't like very much the idea of adding those two stub functions just to
play with dvbsnoop. If dvbsnoop should be working fine without this function,
a patch to dvbsnoop should be sending instead, to proper handle the error
messages generated when a function is not enabled.  If otherwise it doesn't
work properly, then you have no choice but to add a real code for it into the
Kernel driver.

> +
>  static int mb86a20s_read_signal_strength(struct dvb_frontend *fe, u16 
> *strength)
>  {
>       struct mb86a20s_state *state = fe->demodulator_priv;
> @@ -627,6 +677,9 @@ static struct dvb_frontend_ops mb86a20s_ops = {
>       .release = mb86a20s_release,
>  
>       .init = mb86a20s_initfe,
> +     .read_snr = mb86a20s_read_snr,
> +     .read_ber = mb86a20s_read_ber,
> +     .read_ucblocks = mb86a20s_read_ucblocks,
>       .set_frontend = mb86a20s_set_frontend,
>       .get_frontend = mb86a20s_get_frontend,
>       .read_status = mb86a20s_read_status,

--
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