Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752209AbdGFI1O (ORCPT ); Thu, 6 Jul 2017 04:27:14 -0400 Received: from mail-it0-f66.google.com ([209.85.214.66]:34801 "EHLO mail-it0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750998AbdGFI1K (ORCPT ); Thu, 6 Jul 2017 04:27:10 -0400 MIME-Version: 1.0 From: Geert Uytterhoeven Date: Thu, 6 Jul 2017 10:27:08 +0200 X-Google-Sender-Auth: gE2i6FdJ01VQelMFi1AO69Ftyb0 Message-ID: Subject: ath10k: ret used but uninitialized (was: Re: ath10k: add initial SDIO support) To: Erik Stromdahl , Kalle Valo Cc: Arnd Bergmann , "ath10k@lists.infradead.org" , linux-wireless , "netdev@vger.kernel.org" , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v668RPLE016869 Content-Length: 5796 Lines: 154 On Wed, Jul 5, 2017 at 9:52 PM, Linux Kernel Mailing List wrote: > Web: https://git.kernel.org/torvalds/c/d96db25d20256208ce47d71b9f673a1de4c6fd7e > Commit: d96db25d20256208ce47d71b9f673a1de4c6fd7e > Parent: f008d1537bf88396cf41a7c7a831e3acd1ee92a1 > Refname: refs/heads/master > Author: Erik Stromdahl > AuthorDate: Wed Apr 26 12:18:00 2017 +0300 > Committer: Kalle Valo > CommitDate: Thu May 4 15:55:55 2017 +0300 > > ath10k: add initial SDIO support > > Chipsets like QCA6584 have support for SDIO so add initial SDIO bus support to > ath10k. With this patch we have the low level HTC protocol working and it's > possible to boot the firmware, but it's still not possible to connect or > anything like. More changes are needed for full functionality. For that reason > we print during initialisation: > > WARNING: ath10k SDIO support is incomplete, don't expect anything to work! > > Signed-off-by: Erik Stromdahl > [kvalo@qca.qualcomm.com: refactoring, cleanup, commit log] > Signed-off-by: Kalle Valo > --- /dev/null > +++ b/drivers/net/wireless/ath/ath10k/sdio.c > +static int ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k *ar, > + u32 msg_lookahead, bool *done) > +{ > + struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar); > + u32 lookaheads[ATH10K_SDIO_MAX_RX_MSGS]; > + int n_lookaheads = 1; > + unsigned long timeout; > + int ret; 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. > + 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? > + 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@linux-m68k.org 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