Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux