Am Sun, 27 Aug 2017 09:18:07 -0300 schrieb Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>: Thanks for looking at this. > Em Sun, 20 Aug 2017 12:45:45 +0200 > Daniel Scheller <d.scheller.oss@xxxxxxxxx> escreveu: > > > From: Daniel Scheller <d.scheller@xxxxxxx> > > > > 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@xxxxxxx> > > --- > > 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@xxxxxxxxxxxxxxxx> > > > 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@xxxxxxx> Thanks & best regards, Daniel Scheller -- https://github.com/herrnst
