On Mon, Jul 25, 2016 at 3:28 PM, Mauro Carvalho Chehab
<mche...@osg.samsung.com> wrote:
> Hi Michael,
>
> Em Mon, 25 Jul 2016 14:55:51 -0400
> Michael Ira Krufky <mkru...@linuxtv.org> escreveu:
>
>> On Mon, Jul 25, 2016 at 2:38 PM, Abylay Ospan <aos...@netup.ru> wrote:
>> > inside lgdt3306a_search we reading demod status 20 times with 50 msec
>> > sleep after each read.
>> > This gives us more than 1 sec of delay. Removing this delay should not
>> > affect demod functionality.
>> >
>> > Signed-off-by: Abylay Ospan <aos...@netup.ru>
>> > ---
>> > drivers/media/dvb-frontends/lgdt3306a.c | 16 ++++------------
>> > 1 file changed, 4 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/drivers/media/dvb-frontends/lgdt3306a.c
>> > b/drivers/media/dvb-frontends/lgdt3306a.c
>> > index 179c26e..dad7ad3 100644
>> > --- a/drivers/media/dvb-frontends/lgdt3306a.c
>> > +++ b/drivers/media/dvb-frontends/lgdt3306a.c
>> > @@ -1737,24 +1737,16 @@ static int lgdt3306a_get_tune_settings(struct
>> > dvb_frontend *fe,
>> > static int lgdt3306a_search(struct dvb_frontend *fe)
>> > {
>> > enum fe_status status = 0;
>> > - int i, ret;
>> > + int ret;
>> >
>> > /* set frontend */
>> > ret = lgdt3306a_set_parameters(fe);
>> > if (ret)
>> > goto error;
>> >
>> > - /* wait frontend lock */
>> > - for (i = 20; i > 0; i--) {
>> > - dbg_info(": loop=%d\n", i);
>> > - msleep(50);
>> > - ret = lgdt3306a_read_status(fe, &status);
>> > - if (ret)
>> > - goto error;
>> > -
>> > - if (status & FE_HAS_LOCK)
>> > - break;
>> > - }
>
> Could you please explain why lgdt3306a needs the above ugly hack?
>
>
>> > + ret = lgdt3306a_read_status(fe, &status);
>> > + if (ret)
>> > + goto error;
>
>
>> >
>> > /* check if we have a valid signal */
>> > if (status & FE_HAS_LOCK)
>>
>> Your patch removes a loop that was purposefully written here to handle
>> conditions that are not ideal. Are you sure this change is best for
>> all users?
>>
>> I would disagree with merging this patch.
>>
>> Best regards,
>>
>> Michael Ira Krufky
Mauro,
I cannot speak for the LG DT3306a part itself, but based on my past
experience I can say the following:
To my understanding, the hardware might not report a lock on the first
read_status request, so the driver author chose to include a loop to
retry a few times before giving up.
In real life scenarios, there are marginal signals that may take a
longer time to lock onto, but once locked, the demod will deliver a
reliable stream.
Most applications will only issue a single tune request when trying to
tune to a given program. The application does not retry the tune
request if the driver reports no lock.
Applying this patch will have the potential to cause userspace to
appear broken. Some users will not be able to receive some weaker
channels anymore, and they will have no way to diagnose the problem
from within their application.
-Mike
--
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
