* [bug report] mmc: core: Re-work HW reset for SDIO cards
@ 2019-12-13 18:50 Dan Carpenter
2019-12-17 7:54 ` Ulf Hansson
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2019-12-13 18:50 UTC (permalink / raw)
To: ulf.hansson; +Cc: linux-wireless, ath10k
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
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.
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
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] mmc: core: Re-work HW reset for SDIO cards
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
2019-12-18 5:27 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Ulf Hansson @ 2019-12-17 7:54 UTC (permalink / raw)
To: Dan Carpenter, Kalle Valo; +Cc: linux-wireless, ath10k
+ 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
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] mmc: core: Re-work HW reset for SDIO cards
2019-12-17 7:54 ` Ulf Hansson
@ 2019-12-18 5:27 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2019-12-18 5:27 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-wireless, ath10k, Kalle Valo
On Tue, Dec 17, 2019 at 08:54:47AM +0100, Ulf Hansson wrote:
> + 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?
I feel like checking for 1 would be more readable for humans as well.
Or we could just leave it as-is. I'm not likely to ever publish this
static checker warning.
regards,
dan carpenter
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-12-18 5:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2019-12-18 5:27 ` Dan Carpenter
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).