Re: [PATCH] [media] lgdt3306a: remove 20*50 msec unnecessary timeout

[Mauro Carvalho Chehab <mchehab@osg.samsung.com>]

Hi Michael,

Em Mon, 25 Jul 2016 14:55:51 -0400
Michael Ira Krufky <mkrufky@linuxtv.org> escreveu:

> On Mon, Jul 25, 2016 at 2:38 PM, Abylay Ospan <aospan@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 <aospan@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


-- 
Thanks,
Mauro