From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from ec2-52-27-115-49.us-west-2.compute.amazonaws.com ([52.27.115.49]:47406 "EHLO s-opensource.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752610AbcGYT2r (ORCPT ); Mon, 25 Jul 2016 15:28:47 -0400 Date: Mon, 25 Jul 2016 16:28:41 -0300 From: Mauro Carvalho Chehab To: Michael Ira Krufky Cc: Abylay Ospan , linux-media Subject: Re: [PATCH] [media] lgdt3306a: remove 20*50 msec unnecessary timeout Message-ID: <20160725162841.6e11fd2b@recife.lan> In-Reply-To: References: <1469471939-25393-1-git-send-email-aospan@netup.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: Hi Michael, Em Mon, 25 Jul 2016 14:55:51 -0400 Michael Ira Krufky escreveu: > On Mon, Jul 25, 2016 at 2:38 PM, Abylay Ospan 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 > > --- > > 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