On Sat, Dec 10, 2011 at 5:17 PM, Antti Palosaari <crope@xxxxxx> wrote: > Hello Manu, > That patch looks now much acceptable than the older for my eyes, since you > removed that .set_state() (change from .set_params() to .set_state()) I > criticized. Thanks! > :-) > > On 12/10/2011 06:44 AM, Manu Abraham wrote: >> >> static int cxd2820r_set_frontend(struct dvb_frontend *fe, > > [...] >> >> + switch (c->delivery_system) { >> + case SYS_DVBT: >> + ret = cxd2820r_init_t(fe); > > >> + ret = cxd2820r_set_frontend_t(fe, p); > > > > Anyhow, I don't now like idea you have put .init() calls to .set_frontend(). > Could you move .init() happen in .init() callback as it was earlier? This was there in the earlier patch as well. Maybe you have a new issue now ? ;-) ok. The argument what you make doesn't hold well, Why ? int cxd2820r_init_t(struct dvb_frontend *fe) { ret = cxd2820r_wr_reg(priv, 0x00085, 0x07); } int cxd2820r_init_c(struct dvb_frontend *fe) { ret = cxd2820r_wr_reg(priv, 0x00085, 0x07); } Now, you might like to point that, the Base I2C address location is different comparing DVB-T/DVBT2 to DVB-C So, If you have the init as in earlier with a common init, then you will likely init the wrong device at .init(), as init is called open(). So, this might result in an additional register write, which could be avoided altogether. One register access is not definitely something to brag about, but is definitely a small incremental difference. Other than that this register write doesn't do anything more than an ADC_START. So starting the ADC at init doesn't make sense. But does so when you want to select the right ADC. So definitely, this change is an improvement. Also, you can compare the time taken for the device to tune now. It is quite a lot faster compared to without this patch. So you or any other user should be happy. :-) I don't think that in any way, the init should be used at init as you say, which sounds pretty much incorrect. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html