On Miércoles, 27 de Julio de 2011 21:22:26 Antti Palosaari escribió:
> On 07/24/2011 12:45 AM, Jose Alberto Reguero wrote:
> > Read without write work as with write. Attached updated patch.
> > 
> > ttusb2-6.diff
> > 
> > -           read = i+1<  num&&  (msg[i+1].flags&  I2C_M_RD);
> > +           write_read = i+1<  num&&  (msg[i+1].flags&  I2C_M_RD);
> > +           read = msg[i].flags&  I2C_M_RD;
> 
> ttusb2 I2C-adapter seems to be fine for my eyes now. No more writing any
> random bytes in case of single read. Good!
> 
> > +   .dtv6_if_freq_khz = TDA10048_IF_4000,
> > +   .dtv7_if_freq_khz = TDA10048_IF_4500,
> > +   .dtv8_if_freq_khz = TDA10048_IF_5000,
> > +   .clk_freq_khz     = TDA10048_CLK_16000,
> > 
> > 
> > +static int ct3650_i2c_gate_ctrl(struct dvb_frontend* fe, int enable)
> 
> cosmetic issue rename to ttusb2_ct3650_i2c_gate_ctrl
> 
> >     { TDA10048_CLK_16000, TDA10048_IF_4000,  10, 3, 0 },
> > 
> > +   { TDA10048_CLK_16000, TDA10048_IF_4500,   5, 3, 0 },
> > +   { TDA10048_CLK_16000, TDA10048_IF_5000,   5, 3, 0 },
> > 
> > +   /* Set the  pll registers */
> > +   tda10048_writereg(state, TDA10048_CONF_PLL1, 0x0f);
> > +   tda10048_writereg(state, TDA10048_CONF_PLL2, (u8)(state-
>pll_mfactor));
> > +   tda10048_writereg(state, TDA10048_CONF_PLL3, tda10048_readreg(state,
> > TDA10048_CONF_PLL3) | ((u8)(state->pll_nfactor) | 0x40));
> 
> This if only issue can have effect to functionality and I want double
> check. I see few things.
> 
> 1) clock (and PLL) settings should be done generally only once at
> .init() and probably .sleep() in case of needed for sleep. Something
> like start clock in init, stop clock in sleep. It is usually very first
> thing to set before other. Now it is in wrong place - .set_frontend().
> 
> 2) Those clock settings seem somehow weird. As you set different PLL M
> divider for 6 MHz bandwidth than others. Have you looked those are
> really correct? I suspect there could be some other Xtal than 16MHz and
> thus those are wrong. Which Xtals there is inside device used? There is
> most likely 3 Xtals, one for each chip. It is metal box nearest to chip.

I left 6MHz like it was before in the driver. I try to do other way, allowing 
to put different PLL in config that the default ones of the driver and 
initialize it in init.

Jose Alberto

> 
> 
> Ran checkpatch.pl also to find out style issues before send patch.
> 
> I have send new version, hopefully final, of MFE. It changes array name
> from adap->mfe to adap-fe. You should also update that.
> 
> 
> regards
> Antti
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to