On Wed, Sep 21, 2005 Oliver Endriss wrote:
> Imho the 'enhanced' code looks broken:
cvs annotate blames this to Andrew, maybe he can enlighten us?
Otherwise, if it is obviously broken we should just remove it.
Johannes
> if (state->config->enhanced_tuning) {
> /* check if we should do a finetune */
> int frequency_delta = p->frequency - state->tuner_frequency;
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> ...
> if ((frequency_delta > -minmax) && (frequency_delta < minmax) && (frequency_delta != 0) &&
> (state->fec_inner == p->u.qpsk.fec_inner) &&
> (state->symbol_rate == p->u.qpsk.symbol_rate) {
> /* fine-tuning */
> int Drot_freq = (frequency_delta << 16) / (state->config->mclk / 1000);
>
> // zap the derotator registers first
> stv0299_writeregI(state, 0x22, 0x00);
> stv0299_writeregI(state, 0x23, 0x00);
>
> // now set them as we want
> stv0299_writeregI(state, 0x22, Drot_freq >> 8);
> stv0299_writeregI(state, 0x23, Drot_freq);
> } else {
> ...
> state->config->pll_set(fe, state->i2c, p);
> ...
> }
> } else {
> ...
> state->config->pll_set(fe, state->i2c, p);
> ...
> }
>
> state->tuner_frequency = p->frequency;
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> ...
>
>
> For fine-tuning, this code finally sets state->tuner_frequency, too.
>
> The derotator can be used if (and only if)
> pll_frequency - delta < p->frequency < pll_frequency + delta
> AFAICS the real *pll* frequency is never checked. :-(
>
> If frequency_delta is small enough for subsequent calls, the pll will
> not be set, the derotator registers will be set incorrectly, and tuning
> will fail. Is this optimization really useful?
>
> Furthermore, there are two slightly different tuning sections:
> - 'enhanced':
> stv0299_writeregI(state, 0x05, 0xb5); /* enable i2c repeater on stv0299 */
> state->config->pll_set(fe, state->i2c, p);
> stv0299_writeregI(state, 0x05, 0x35); /* disable i2c repeater on stv0299 */
>
> stv0299_writeregI(state, 0x32, 0x80); <-- ???
> stv0299_writeregI(state, 0x22, 0x00);
> stv0299_writeregI(state, 0x23, 0x00);
> stv0299_writeregI(state, 0x32, 0x19); <-- ???
> stv0299_set_symbolrate (fe, p->u.qpsk.symbol_rate);
> stv0299_set_FEC (state, p->u.qpsk.fec_inner);
>
> - 'standard':
> stv0299_writeregI(state, 0x05, 0xb5); /* enable i2c repeater on stv0299 */
> state->config->pll_set(fe, state->i2c, p);
> stv0299_writeregI(state, 0x05, 0x35); /* disable i2c repeater on stv0299 */
>
> stv0299_set_FEC (state, p->u.qpsk.fec_inner);
> stv0299_set_symbolrate (fe, p->u.qpsk.symbol_rate);
> stv0299_writeregI(state, 0x22, 0x00);
> stv0299_writeregI(state, 0x23, 0x00);
> stv0299_readreg (state, 0x23); <-- why?
> stv0299_writeregI(state, 0x12, 0xb9); <-- should be set by xxx_inittab!
>
> The 'enhanced' version is used by the budget-ci driver for sub-system
> 13c2:100f. All other cards and drivers use the standard variant...
> Is there a good reason for the enhanced tuning code?
>
> Above I marked all lines which look strange to me. Maybe a stv0299
> expert could comment on this. Imho the standard code looks better.
>
> Btw. what is 'skip_reinit' in struct stv0299_config good for?
> It is not used anywhere...
