With gcc 4.1.2:
drivers/net/wireless/ath/ath10k/sdio.c: In function
‘ath10k_sdio_mbox_rxmsg_pending_handler’:
drivers/net/wireless/ath/ath10k/sdio.c:676: warning: ‘ret’ may be used
uninitialized in this function
+
+ *done = true;
+
+ /* Copy the lookahead obtained from the HTC register table into our
+ * temp array as a start value.
+ */
+ lookaheads[0] = msg_lookahead;
+
+ timeout = jiffies + SDIO_MBOX_PROCESSING_TIMEOUT_HZ;
Although very unlikely due to the long timeout, if the code is preempted here,
and the loop below never entered, ret will indeed be uninitialized.
It's unclear to me what the proper initialization would be, though, so
that's why I didn't send a patch.
I think it would be best to use 0 as initial value of ret in this case.
This will make all other interrupts be processed in a normal way.
Kalle: Should I create a new patch (initializing ret with zero)?
+ while (time_before(jiffies, timeout)) {
+ /* Try to allocate as many HTC RX packets indicated by
+ * n_lookaheads.
+ */
+ ret = ath10k_sdio_mbox_rx_alloc(ar, lookaheads,
+ n_lookaheads);
+ if (ret)
+ break;
+
+ if (ar_sdio->n_rx_pkts >= 2)
+ /* A recv bundle was detected, force IRQ status
+ * re-check again.
+ */
+ *done = false;
+
+ ret = ath10k_sdio_mbox_rx_fetch(ar);
+
+ /* Process fetched packets. This will potentially update
+ * n_lookaheads depending on if the packets contain lookahead
+ * reports.
+ */
+ n_lookaheads = 0;
+ ret = ath10k_sdio_mbox_rx_process_packets(ar,
+ lookaheads,
+ &n_lookaheads);
+
+ if (!n_lookaheads || ret)
+ break;
+
+ /* For SYNCH processing, if we get here, we are running
+ * through the loop again due to updated lookaheads. Set
+ * flag that we should re-check IRQ status registers again
+ * before leaving IRQ processing, this can net better
+ * performance in high throughput situations.
+ */
+ *done = false;
+ }
+
+ if (ret && (ret != -ECANCELED))
+ ath10k_warn(ar, "failed to get pending recv messages: %d\n",
+ ret);
+
+ return ret;
+}
+static void ath10k_sdio_irq_handler(struct sdio_func *func)
+{
+ struct ath10k_sdio *ar_sdio = sdio_get_drvdata(func);
+ struct ath10k *ar = ar_sdio->ar;
+ unsigned long timeout;
+ bool done = false;
+ int ret;
drivers/net/wireless/ath/ath10k/sdio.c: In function ‘ath10k_sdio_irq_handler’:
drivers/net/wireless/ath/ath10k/sdio.c:1331: warning: ‘ret’ may be
used uninitialized in this function
+
+ /* Release the host during interrupts so we can pick it back up when
+ * we process commands.
+ */
+ sdio_release_host(ar_sdio->func);
+
+ timeout = jiffies + ATH10K_SDIO_HIF_COMMUNICATION_TIMEOUT_HZ;
Same here.
Should ret be preinitialized to 0, -ECANCELED, or something else?
ret = 0 or ret = -ECANCELED, will result in no warning message.
-ETIMEDOUT could be used perhaps.
Note that the function is a void function so the error will not get
propagated.
I am fine with ret = 0 in this case as well.
+ while (time_before(jiffies, timeout) && !done) {
+ ret = ath10k_sdio_mbox_proc_pending_irqs(ar, &done);
+ if (ret)
+ break;
+ }
+
+ sdio_claim_host(ar_sdio->func);
+
+ wake_up(&ar_sdio->irq_wq);
+
+ if (ret && ret != -ECANCELED)
+ ath10k_warn(ar, "failed to process pending SDIO interrupts: %d\n",
+ ret);
+}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
