[repeating comments I made in the gerrit review for Chrome OS : https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1585667 ] On Sat, Apr 27, 2019 at 7:17 PM Wen Gong <wgong@xxxxxxxxxxxxxx> wrote: > > For sdio 3.0 chip, the clock will drop from 200M Hz to 50M Hz after load > ath10k driver, it is because mmc_hw_reset will reset the sdio's power, > then mmc will consider it as sdio 2.0 and drop the clock. Wen, 5468e784c0600551ca03263f5255a375c05f88e7 commit message gives reasons for adding the mmc_hw_reset() call. The commit message for removing gives different reason for removal. Both are good but second one is incomplete. The commit message for removal should ALSO explain why adding this call wasn't necessary in the first place OR move the call to a different code path. > Remove mmc_hw_reset will avoid the drop of clock. This commit message makes it clear the original patch introduced a new problem. But the original patch fixed a different problem and that this proposed change seems likely to re-introduce and the commit message should explain why that isn't true (or how the original was fixed differently) cheers, grant > > Tested with QCA6174 SDIO with firmware > WLAN.RMH.4.4.1-00007-QCARMSWP-1. > > Signed-off-by: Wen Gong <wgong@xxxxxxxxxxxxxx> > --- > drivers/net/wireless/ath/ath10k/sdio.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c > index fae56c6..f1d2af8 100644 > --- a/drivers/net/wireless/ath/ath10k/sdio.c > +++ b/drivers/net/wireless/ath/ath10k/sdio.c > @@ -1433,10 +1433,6 @@ static void ath10k_sdio_hif_power_down(struct ath10k *ar) > return; > } > > - ret = mmc_hw_reset(ar_sdio->func->card->host); > - if (ret) > - ath10k_warn(ar, "unable to reset sdio: %d\n", ret); > - > sdio_release_host(ar_sdio->func); > > ar_sdio->is_disabled = true; > -- > 1.9.1 >
