| Message ID | 28425caf-2736-96ae-00a7-3fb273b1f9d5@users.sourceforge.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 26.09.2017 13:27, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Tue, 26 Sep 2017 11:01:44 +0200 > > * Add a jump target so that a bit of exception handling can be better > reused at the end of this function. > > This issue was detected by using the Coccinelle software. > > * The script "checkpatch.pl" pointed information out like the following. > > ERROR: do not use assignment in if condition > > Thus fix an affected source code place. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > drivers/media/dvb-frontends/tda8261.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/dvb-frontends/tda8261.c b/drivers/media/dvb-frontends/tda8261.c > index 4eb294f330bc..5a8a9b6b8107 100644 > --- a/drivers/media/dvb-frontends/tda8261.c > +++ b/drivers/media/dvb-frontends/tda8261.c > @@ -129,18 +129,18 @@ static int tda8261_set_params(struct dvb_frontend *fe) > > /* Set params */ > err = tda8261_write(state, buf); > - if (err < 0) { > - pr_err("%s: I/O Error\n", __func__); > - return err; > - } > + err = tda8261_get_status(fe, &status); > + if (err < 0) > + goto report_failure; > + Is this change really correct? Doesn't it query the status once more often than before? > /* sleep for some time */ > pr_debug("%s: Waiting to Phase LOCK\n", __func__); > msleep(20); > /* check status */ > - if ((err = tda8261_get_status(fe, &status)) < 0) { > - pr_err("%s: I/O Error\n", __func__); > - return err; > - } > + err = tda8261_get_status(fe, &status); > + if (err < 0) > + goto report_failure; > + > if (status == 1) { > pr_debug("%s: Tuner Phase locked: status=%d\n", __func__, > status); > @@ -150,6 +150,10 @@ static int tda8261_set_params(struct dvb_frontend *fe) > } > > return 0; > + > +report_failure: > + pr_err("%s: I/O Error\n", __func__); > + return err; > } > > static void tda8261_release(struct dvb_frontend *fe) >
>> @@ -129,18 +129,18 @@ static int tda8261_set_params(struct dvb_frontend *fe) >> >> /* Set params */ >> err = tda8261_write(state, buf); >> - if (err < 0) { >> - pr_err("%s: I/O Error\n", __func__); >> - return err; >> - } >> + err = tda8261_get_status(fe, &status); >> + if (err < 0) >> + goto report_failure; >> + > > Is this change really correct? Doesn't it query the status once more > often than before? Thanks for your inquiry. Unfortunately, I made a copy mistake at this source code place. When should I send a corrected suggestion for this update step in the patch series? Regards, Markus
diff --git a/drivers/media/dvb-frontends/tda8261.c b/drivers/media/dvb-frontends/tda8261.c index 4eb294f330bc..5a8a9b6b8107 100644 --- a/drivers/media/dvb-frontends/tda8261.c +++ b/drivers/media/dvb-frontends/tda8261.c @@ -129,18 +129,18 @@ static int tda8261_set_params(struct dvb_frontend *fe) /* Set params */ err = tda8261_write(state, buf); - if (err < 0) { - pr_err("%s: I/O Error\n", __func__); - return err; - } + err = tda8261_get_status(fe, &status); + if (err < 0) + goto report_failure; + /* sleep for some time */ pr_debug("%s: Waiting to Phase LOCK\n", __func__); msleep(20); /* check status */ - if ((err = tda8261_get_status(fe, &status)) < 0) { - pr_err("%s: I/O Error\n", __func__); - return err; - } + err = tda8261_get_status(fe, &status); + if (err < 0) + goto report_failure; + if (status == 1) { pr_debug("%s: Tuner Phase locked: status=%d\n", __func__, status); @@ -150,6 +150,10 @@ static int tda8261_set_params(struct dvb_frontend *fe) } return 0; + +report_failure: + pr_err("%s: I/O Error\n", __func__); + return err; } static void tda8261_release(struct dvb_frontend *fe)