From: Ulf Hansson <ulf.hansson@linaro.org>
To: Dan Carpenter <dan.carpenter@oracle.com>,
Kalle Valo <kvalo@codeaurora.org>
Cc: ath10k@lists.infradead.org,
linux-wireless <linux-wireless@vger.kernel.org>
Subject: Re: [bug report] mmc: core: Re-work HW reset for SDIO cards
Date: Tue, 17 Dec 2019 08:54:47 +0100 [thread overview]
Message-ID: <CAPDyKFoKvhGDuQ+C0Sp2N3==CXiMWfpcrT28+dQ2+Nq_UPaXYQ@mail.gmail.com> (raw)
In-Reply-To: <20191213185050.m6iku7defq44syrl@kili.mountain>
+ Kalle
On Fri, 13 Dec 2019 at 19:51, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Hello Ulf Hansson,
>
> The patch 2ac55d5e5ec9: "mmc: core: Re-work HW reset for SDIO cards"
> from Oct 17, 2019, leads to the following static checker warning:
>
> drivers/net/wireless/ath/ath10k/sdio.c:1521 ath10k_sdio_hif_power_down()
> warn: 'ret' can be either negative or positive
Thanks for reporting!
>
> drivers/net/wireless/ath/ath10k/sdio.c
> 1495 static void ath10k_sdio_hif_power_down(struct ath10k *ar)
> 1496 {
> 1497 struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
> 1498 int ret;
> 1499
> 1500 if (ar_sdio->is_disabled)
> 1501 return;
> 1502
> 1503 ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio power off\n");
> 1504
> 1505 /* Disable the card */
> 1506 sdio_claim_host(ar_sdio->func);
> 1507
> 1508 ret = sdio_disable_func(ar_sdio->func);
> 1509 if (ret) {
> 1510 ath10k_warn(ar, "unable to disable sdio function: %d\n", ret);
> 1511 sdio_release_host(ar_sdio->func);
> 1512 return;
> 1513 }
> 1514
> 1515 ret = mmc_hw_reset(ar_sdio->func->card->host);
> 1516 if (ret)
>
> It used to be that mmc_hw_reset() return negative error codes or zero
> but now it returns 1 on certain success paths.
Correct.
I was actually looking into this while changing the behaviour of
mmc_hw_reset(). However I decided to leave this as is.
The main reason is, that mmc_hw_reset() is not going to power down the
card. It's hard power cycle, so I am kind of surprised that is being
used at all in this path. This in combination of expecting the value
from mmc_hw_reset() to never be 1 here, seemed like a good idea to
preserve the logging of the warning message.
To silent the static checker tool from warning, we could check
explicitly for "1". Is that something you want me to do?
>
> 1517 ath10k_warn(ar, "unable to reset sdio: %d\n", ret);
> 1518
> 1519 sdio_release_host(ar_sdio->func);
> 1520
> 1521 ar_sdio->is_disabled = true;
> 1522 }
>
>
> regards,
> dan carpenter
Kind regards
Uffe
next prev parent reply other threads:[~2019-12-17 7:55 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-13 18:50 [bug report] mmc: core: Re-work HW reset for SDIO cards Dan Carpenter
2019-12-17 7:54 ` Ulf Hansson [this message]
2019-12-18 5:27 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPDyKFoKvhGDuQ+C0Sp2N3==CXiMWfpcrT28+dQ2+Nq_UPaXYQ@mail.gmail.com' \
--to=ulf.hansson@linaro.org \
--cc=ath10k@lists.infradead.org \
--cc=dan.carpenter@oracle.com \
--cc=kvalo@codeaurora.org \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).