Hello everybody,
While looking into Coverity ID 1227035 I ran into the following piece
of code at drivers/media/dvb-frontends/af9013.c:595:
595/* program CFOE coefficients */
596if (c->bandwidth_hz != state->bandwidth_hz) {
597for (i = 0; i < ARRAY_SIZE(coeff_lut); i++) {
598if (coeff_lut[i].clock == state->config.clock &&
599coeff_lut[i].bandwidth_hz ==
c->bandwidth_hz) {
600break;
601}
602}
603
604/* Return an error if can't find bandwidth or the
right clock */
605if (i == ARRAY_SIZE(coeff_lut))
606return -EINVAL;
608ret = af9013_wr_regs(state, 0xae00, coeff_lut[i].val,
609sizeof(coeff_lut[i].val));
610}
611
612/* program frequency control */
613if (c->bandwidth_hz != state->bandwidth_hz || state->first_tune) {
614/* get used IF frequency */
615if (fe->ops.tuner_ops.get_if_frequency)
616fe->ops.tuner_ops.get_if_frequency(fe,
_frequency);
617else
618if_frequency = state->config.if_frequency;
619
620dev_dbg(>i2c->dev, "%s: if_frequency=%d\n",
621__func__, if_frequency);
622
623sampling_freq = if_frequency;
624
625while (sampling_freq > (state->config.clock / 2))
626sampling_freq -= state->config.clock;
627
628if (sampling_freq < 0) {
629sampling_freq *= -1;
630spec_inv = state->config.spec_inv;
631} else {
632spec_inv = !state->config.spec_inv;
633}
634
635freq_cw = af9013_div(state, sampling_freq,
state->config.clock,
63623);
637
638if (spec_inv)
639freq_cw = 0x80 - freq_cw;
640
641buf[0] = (freq_cw >> 0) & 0xff;
642buf[1] = (freq_cw >> 8) & 0xff;
643buf[2] = (freq_cw >> 16) & 0x7f;
644
645freq_cw = 0x80 - freq_cw;
646
647buf[3] = (freq_cw >> 0) & 0xff;
648buf[4] = (freq_cw >> 8) & 0xff;
649buf[5] = (freq_cw >> 16) & 0x7f;
650
651ret = af9013_wr_regs(state, 0xd140, buf, 3);
652if (ret)
653goto err;
654
655ret = af9013_wr_regs(state, 0x9be7, buf, 6);
656if (ret)
657goto err;
658}
659
660/* clear TPS lock flag */
661ret = af9013_wr_reg_bits(state, 0xd330, 3, 1, 1);
662if (ret)
663goto err;
664
665/* clear MPEG2 lock flag */
666ret = af9013_wr_reg_bits(state, 0xd507, 6, 1, 0);
667if (ret)
668goto err;
669
670/* empty channel function */
671ret = af9013_wr_reg_bits(state, 0x9bfe, 0, 1, 0);
672if (ret)
673goto err;
674
675/* empty DVB-T channel function */
676ret = af9013_wr_reg_bits(state, 0x9bc2, 0, 1, 0);
677if (ret)
678goto err;
679
The issue here is that the value stored in variable _ret_ at line 608,
is not being evaluated as it happens at line 662, 667, 672 and 677.
Then after looking into function af9013_wr_regs(), I noticed that this
function always returns zero, no matter what, as you can see below:
121static int af9013_wr_regs(struct af9013_state *priv, u16 reg, const
u8 *val,
122int len)
123{
124int ret, i;
125u8 mbox = (0 << 7)|(0 << 6)|(1 << 1)|(1 << 0);
126
127if ((priv->config.ts_mode == AF9013_TS_USB) &&
128((reg & 0xff00) != 0xff00) && ((reg & 0xff00) != 0xae00)) {
129mbox |= ((len - 1) << 2);
130ret = af9013_wr_regs_i2c(priv, mbox, reg, val, len);
131} else {
132for (i = 0; i < len; i++) {
133ret = af9013_wr_regs_i2c(priv, mbox, reg+i,
val+i, 1);
134if (ret)
135goto err;
136}
137}
138
139err:
140return 0;
141}
So I am wondering if such function is really intended to ignore the
return value of af9013_wr_regs_i2c()?
If that is the case maybe it should be refactored as follows:
--- a/drivers/media/dvb-frontends/af9013.c
+++ b/drivers/media/dvb-frontends/af9013.c
@@ -118,26 +118,21 @@ static int af9013_rd_regs_i2c(struct
af9013_state *priv, u8 mbox, u16 reg,
}
/* write multiple registers */
-static int af9013_wr_regs(struct af9013_state *priv, u16 reg, const u8 *val,
+static void af9013_wr_regs(struct af9013_state *priv, u16 reg, const u8 *val,
int len)
{
-
