* [PATCH] [media] dvb-frontends/mxl5xx: fix lock check order @ 2017-08-20 10:45 Daniel Scheller 2017-08-27 12:18 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 3+ messages in thread From: Daniel Scheller @ 2017-08-20 10:45 UTC (permalink / raw) To: linux-media, mchehab, mchehab; +Cc: jasmin, rjkm From: Daniel Scheller <d.scheller@gmx.net> Signed-off-by: Daniel Scheller <d.scheller@gmx.net> --- When the mxl5xx driver together with the ddbridge glue gets merged ([1]), this one should go in aswell - this fix is part of the dddvb-0.9.31 release. drivers/media/dvb-frontends/mxl5xx.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/dvb-frontends/mxl5xx.c b/drivers/media/dvb-frontends/mxl5xx.c index 676c96c216c3..d9136d67f5d4 100644 --- a/drivers/media/dvb-frontends/mxl5xx.c +++ b/drivers/media/dvb-frontends/mxl5xx.c @@ -638,13 +638,14 @@ static int tune(struct dvb_frontend *fe, bool re_tune, state->tune_time = jiffies; return 0; } - if (*status & FE_HAS_LOCK) - return 0; r = read_status(fe, status); if (r) return r; + if (*status & FE_HAS_LOCK) + return 0; + return 0; } -- 2.13.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] [media] dvb-frontends/mxl5xx: fix lock check order 2017-08-20 10:45 [PATCH] [media] dvb-frontends/mxl5xx: fix lock check order Daniel Scheller @ 2017-08-27 12:18 ` Mauro Carvalho Chehab 2017-08-27 12:50 ` Daniel Scheller 0 siblings, 1 reply; 3+ messages in thread From: Mauro Carvalho Chehab @ 2017-08-27 12:18 UTC (permalink / raw) To: Daniel Scheller; +Cc: linux-media, mchehab, jasmin, rjkm Em Sun, 20 Aug 2017 12:45:45 +0200 Daniel Scheller <d.scheller.oss@gmail.com> escreveu: > From: Daniel Scheller <d.scheller@gmx.net> > Always add a description at the patch. > Signed-off-by: Daniel Scheller <d.scheller@gmx.net> > --- > When the mxl5xx driver together with the ddbridge glue gets merged ([1]), > this one should go in aswell - this fix is part of the dddvb-0.9.31 > release. > > drivers/media/dvb-frontends/mxl5xx.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/dvb-frontends/mxl5xx.c b/drivers/media/dvb-frontends/mxl5xx.c > index 676c96c216c3..d9136d67f5d4 100644 > --- a/drivers/media/dvb-frontends/mxl5xx.c > +++ b/drivers/media/dvb-frontends/mxl5xx.c > @@ -638,13 +638,14 @@ static int tune(struct dvb_frontend *fe, bool re_tune, > state->tune_time = jiffies; > return 0; > } > - if (*status & FE_HAS_LOCK) > - return 0; > > r = read_status(fe, status); > if (r) > return r; > > + if (*status & FE_HAS_LOCK) > + return 0; > + > return 0; That's stupid! it will return 0 on all situations, no matter if FE_HAS_LOCK or not. So, no need for the if. Anyway, IMHO, either the original code is right and it needs to use a previously cached value (with sounds weird to me, although it is possible), or the logic is utterly broken, and we should, instead, apply the enclosed patch. > if (r) > return r; > } > Thanks, Mauro [PATCH RFC] media: mxl5xx: fix tuning logic The tuning logic is broken with regards to status report: it relies on a previously-cached value that may not be valid if retuned. Change the logic to always read the status. Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> diff --git a/drivers/media/dvb-frontends/mxl5xx.c b/drivers/media/dvb-frontends/mxl5xx.c index 676c96c216c3..4b449a6943c5 100644 --- a/drivers/media/dvb-frontends/mxl5xx.c +++ b/drivers/media/dvb-frontends/mxl5xx.c @@ -636,16 +636,9 @@ static int tune(struct dvb_frontend *fe, bool re_tune, if (r) return r; state->tune_time = jiffies; - return 0; } - if (*status & FE_HAS_LOCK) - return 0; - r = read_status(fe, status); - if (r) - return r; - - return 0; + return read_status(fe, status); } static enum fe_code_rate conv_fec(enum MXL_HYDRA_FEC_E fec) ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] [media] dvb-frontends/mxl5xx: fix lock check order 2017-08-27 12:18 ` Mauro Carvalho Chehab @ 2017-08-27 12:50 ` Daniel Scheller 0 siblings, 0 replies; 3+ messages in thread From: Daniel Scheller @ 2017-08-27 12:50 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, mchehab, jasmin, rjkm Am Sun, 27 Aug 2017 09:18:07 -0300 schrieb Mauro Carvalho Chehab <mchehab@s-opensource.com>: Thanks for looking at this. > Em Sun, 20 Aug 2017 12:45:45 +0200 > Daniel Scheller <d.scheller.oss@gmail.com> escreveu: > > > From: Daniel Scheller <d.scheller@gmx.net> > > > > Always add a description at the patch. Sorry. Will remember to do so even for these rather self-explanatory changes. > > Signed-off-by: Daniel Scheller <d.scheller@gmx.net> > > --- > > When the mxl5xx driver together with the ddbridge glue gets merged > > ([1]), this one should go in aswell - this fix is part of the > > dddvb-0.9.31 release. > > > > drivers/media/dvb-frontends/mxl5xx.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/dvb-frontends/mxl5xx.c > > b/drivers/media/dvb-frontends/mxl5xx.c index > > 676c96c216c3..d9136d67f5d4 100644 --- > > a/drivers/media/dvb-frontends/mxl5xx.c +++ > > b/drivers/media/dvb-frontends/mxl5xx.c @@ -638,13 +638,14 @@ static > > int tune(struct dvb_frontend *fe, bool re_tune, state->tune_time = > > jiffies; return 0; > > } > > - if (*status & FE_HAS_LOCK) > > - return 0; > > > > r = read_status(fe, status); > > if (r) > > return r; > > > > + if (*status & FE_HAS_LOCK) > > + return 0; > > + > > return 0; > > That's stupid! it will return 0 on all situations, no matter if > FE_HAS_LOCK or not. So, no need for the if. > > Anyway, IMHO, either the original code is right and it needs to > use a previously cached value (with sounds weird to me, although > it is possible), or the logic is utterly broken, and we should, > instead, apply the enclosed patch. Well, in fact, this change was straight picked from upstream ([1]), but I honestly didn't take into account that during cleanup the #if-0/#endif was removed (which will in turn result in the same behaviour though - return 0 in all cases; some leftovers?). [1] https://github.com/DigitalDevices/dddvb/commit/a44dbc889b7030a100599d2bd2c93c976c011a03 > > > if (r) > > return r; > > > } > > > > Thanks, > Mauro > > [PATCH RFC] media: mxl5xx: fix tuning logic > > The tuning logic is broken with regards to status report: > it relies on a previously-cached value that may not be valid > if retuned. > > Change the logic to always read the status. > > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > > > diff --git a/drivers/media/dvb-frontends/mxl5xx.c > b/drivers/media/dvb-frontends/mxl5xx.c index > 676c96c216c3..4b449a6943c5 100644 --- > a/drivers/media/dvb-frontends/mxl5xx.c +++ > b/drivers/media/dvb-frontends/mxl5xx.c @@ -636,16 +636,9 @@ static > int tune(struct dvb_frontend *fe, bool re_tune, if (r) > return r; > state->tune_time = jiffies; > - return 0; > } > - if (*status & FE_HAS_LOCK) > - return 0; > > - r = read_status(fe, status); > - if (r) > - return r; > - > - return 0; > + return read_status(fe, status); > } > > static enum fe_code_rate conv_fec(enum MXL_HYDRA_FEC_E fec) > Indeed that makes more sense in this context (compared to dddvb upstream), albeit on re_tune=1 read_status() will now also be called, which I guess is totally fine. Let's do it this way. So: Acked-by: Daniel Scheller <d.scheller@gmx.net> Thanks & best regards, Daniel Scheller -- https://github.com/herrnst ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-08-27 12:50 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-20 10:45 [PATCH] [media] dvb-frontends/mxl5xx: fix lock check order Daniel Scheller 2017-08-27 12:18 ` Mauro Carvalho Chehab 2017-08-27 12:50 ` Daniel Scheller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.