Am 21.02.2014 09:50, schrieb Dan Carpenter:
> This code is needlessly complicated and checkpatch.pl complains that we
> go over the 80 characters per line limit.
>
> If we flip the "if (!lock) {" test to "if (lock) return;" then we can
> remove an indent level from the rest of the function.
>
> We can add two returns in the "if (state->srate >= 10000000) {"
> condition and move the else statement back an additional indent level.
>
> There is another "if (!lock) {" check which can be removed since we have
> already checked "lock" and know it is zero at this point. This second
> check on "lock" is also a problem because it sets off a static checker
> warning. I have reviewed this code for some time to see if something
> else was intended, but have concluded that it was simply an oversight
> and should be removed. Removing this duplicative check gains us an
> third indent level.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> v2: add the returns in the "if (state->srate >= 10000000) {" condition.
>
> diff --git a/drivers/media/dvb-frontends/stv090x.c b/drivers/media/dvb-frontends/stv090x.c
> index 23e872f84742..93f4979ea6e9 100644
> --- a/drivers/media/dvb-frontends/stv090x.c
> +++ b/drivers/media/dvb-frontends/stv090x.c
> @@ -2146,7 +2146,7 @@ static int stv090x_get_coldlock(struct stv090x_state *state, s32 timeout_dmd)
>
> u32 reg;
> s32 car_step, steps, cur_step, dir, freq, timeout_lock;
> - int lock = 0;
> + int lock;
>
> if (state->srate >= 10000000)
> timeout_lock = timeout_dmd / 3;
> @@ -2154,98 +2154,96 @@ static int stv090x_get_coldlock(struct stv090x_state *state, s32 timeout_dmd)
> timeout_lock = timeout_dmd / 2;
>
> lock = stv090x_get_dmdlock(state, timeout_lock); /* cold start wait */
> - if (!lock) {
> - if (state->srate >= 10000000) {
> - if (stv090x_chk_tmg(state)) {
> - if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x1f) < 0)
> - goto err;
> - if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x15) < 0)
> - goto err;
> - lock = stv090x_get_dmdlock(state, timeout_dmd);
> - } else {
> - lock = 0;
> - }
> - } else {
> - if (state->srate <= 4000000)
> - car_step = 1000;
> - else if (state->srate <= 7000000)
> - car_step = 2000;
> - else if (state->srate <= 10000000)
> - car_step = 3000;
> - else
> - car_step = 5000;
> -
> - steps = (state->search_range / 1000) / car_step;
> - steps /= 2;
> - steps = 2 * (steps + 1);
> - if (steps < 0)
> - steps = 2;
> - else if (steps > 12)
> - steps = 12;
> -
> - cur_step = 1;
> - dir = 1;
> -
> - if (!lock) {
> - freq = state->frequency;
> - state->tuner_bw = stv090x_car_width(state->srate, state->rolloff) + state->srate;
> - while ((cur_step <= steps) && (!lock)) {
> - if (dir > 0)
> - freq += cur_step * car_step;
> - else
> - freq -= cur_step * car_step;
> -
> - /* Setup tuner */
> - if (stv090x_i2c_gate_ctrl(state, 1) < 0)
> - goto err;
> + if (lock)
> + return lock;
>
> - if (state->config->tuner_set_frequency) {
> - if (state->config->tuner_set_frequency(fe, freq) < 0)
> - goto err_gateoff;
> - }
> + if (state->srate >= 10000000) {
> + if (stv090x_chk_tmg(state)) {
> + if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x1f) < 0)
> + goto err;
> + if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x15) < 0)
> + goto err;
> + return stv090x_get_dmdlock(state, timeout_dmd);
> + }
> + return 0;
> + }
>
> - if (state->config->tuner_set_bandwidth) {
> - if (state->config->tuner_set_bandwidth(fe, state->tuner_bw) < 0)
> - goto err_gateoff;
> - }
> + if (state->srate <= 4000000)
> + car_step = 1000;
> + else if (state->srate <= 7000000)
> + car_step = 2000;
> + else if (state->srate <= 10000000)
> + car_step = 3000;
> + else
> + car_step = 5000;
hi dan, just a hint
when you encounter large numbers like this: please format it in a way
that it is easy to to see if a number is missing like:
car_step = 5000;
if (state->srate <= 4000000) car_step = 1000;
else if (state->srate <= 7000000) car_step = 2000;
else (state->srate <= 10000000) car_step = 3000;
an other hint from the department of silly steps: do not add 0 in front
or you will searching for the bug a long time :)
just my 2 cents,
re,
wh
>
> - if (stv090x_i2c_gate_ctrl(state, 0) < 0)
> - goto err;
> + steps = (state->search_range / 1000) / car_step;
> + steps /= 2;
> + steps = 2 * (steps + 1);
> + if (steps < 0)
> + steps = 2;
> + else if (steps > 12)
> + steps = 12;
>
> - msleep(50);
> + cur_step = 1;
> + dir = 1;
>
> - if (stv090x_i2c_gate_ctrl(state, 1) < 0)
> - goto err;
> + freq = state->frequency;
> + state->tuner_bw = stv090x_car_width(state->srate, state->rolloff) + state->srate;
> + while ((cur_step <= steps) && (!lock)) {
> + if (dir > 0)
> + freq += cur_step * car_step;
> + else
> + freq -= cur_step * car_step;
>
> - if (state->config->tuner_get_status) {
> - if (state->config->tuner_get_status(fe, ®) < 0)
> - goto err_gateoff;
> - }
> + /* Setup tuner */
> + if (stv090x_i2c_gate_ctrl(state, 1) < 0)
> + goto err;
>
> - if (reg)
> - dprintk(FE_DEBUG, 1, "Tuner phase locked");
> - else
> - dprintk(FE_DEBUG, 1, "Tuner unlocked");
> + if (state->config->tuner_set_frequency) {
> + if (state->config->tuner_set_frequency(fe, freq) < 0)
> + goto err_gateoff;
> + }
>
> - if (stv090x_i2c_gate_ctrl(state, 0) < 0)
> - goto err;
> + if (state->config->tuner_set_bandwidth) {
> + if (state->config->tuner_set_bandwidth(fe, state->tuner_bw) < 0)
> + goto err_gateoff;
> + }
>
> - STV090x_WRITE_DEMOD(state, DMDISTATE, 0x1c);
> - if (STV090x_WRITE_DEMOD(state, CFRINIT1, 0x00) < 0)
> - goto err;
> - if (STV090x_WRITE_DEMOD(state, CFRINIT0, 0x00) < 0)
> - goto err;
> - if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x1f) < 0)
> - goto err;
> - if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x15) < 0)
> - goto err;
> - lock = stv090x_get_dmdlock(state, (timeout_dmd / 3));
> + if (stv090x_i2c_gate_ctrl(state, 0) < 0)
> + goto err;
>
> - dir *= -1;
> - cur_step++;
> - }
> - }
> + msleep(50);
> +
> + if (stv090x_i2c_gate_ctrl(state, 1) < 0)
> + goto err;
> +
> + if (state->config->tuner_get_status) {
> + if (state->config->tuner_get_status(fe, ®) < 0)
> + goto err_gateoff;
> }
> +
> + if (reg)
> + dprintk(FE_DEBUG, 1, "Tuner phase locked");
> + else
> + dprintk(FE_DEBUG, 1, "Tuner unlocked");
> +
> + if (stv090x_i2c_gate_ctrl(state, 0) < 0)
> + goto err;
> +
> + STV090x_WRITE_DEMOD(state, DMDISTATE, 0x1c);
> + if (STV090x_WRITE_DEMOD(state, CFRINIT1, 0x00) < 0)
> + goto err;
> + if (STV090x_WRITE_DEMOD(state, CFRINIT0, 0x00) < 0)
> + goto err;
> + if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x1f) < 0)
> + goto err;
> + if (STV090x_WRITE_DEMOD(state, DMDISTATE, 0x15) < 0)
> + goto err;
> + lock = stv090x_get_dmdlock(state, (timeout_dmd / 3));
> +
> + dir *= -1;
> + cur_step++;
> }
>
> return lock;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
