+ Kalle
On Fri, 13 Dec 2019 at 19:51, Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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
