From: Kalle Valo <kvalo@codeaurora.org>
To: Wen Gong <wgong@qti.qualcomm.com>
Cc: Grant Grundler <grundler@google.com>,
Wen Gong <wgong@codeaurora.org>,
"linux-wireless\@vger.kernel.org"
<linux-wireless@vger.kernel.org>,
"ath10k\@lists.infradead.org" <ath10k@lists.infradead.org>,
Ulf Hansson <ulf.hansson@linaro.org>
Subject: Re: [PATCH] ath10k: remove mmc_hw_reset while hif power down
Date: Tue, 07 May 2019 12:34:59 +0300 [thread overview]
Message-ID: <87pnour4jg.fsf@codeaurora.org> (raw)
In-Reply-To: <36950ff25c0747629e60ccb68819e93a@aptaiexm02f.ap.qualcomm.com> (Wen Gong's message of "Tue, 7 May 2019 05:05:21 +0000")
+ Ulf to give comments from SDIO point of view
Wen Gong <wgong@qti.qualcomm.com> writes:
>> -----Original Message-----
>> From: ath10k <ath10k-bounces@lists.infradead.org> On Behalf Of Grant
>> Grundler
>> Sent: Saturday, May 4, 2019 2:01 AM
>> To: Wen Gong <wgong@codeaurora.org>
>> Cc: linux-wireless@vger.kernel.org; ath10k@lists.infradead.org
>> Subject: [EXT] Re: [PATCH] ath10k: remove mmc_hw_reset while hif power
>> down
>>
>> [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@codeaurora.org> 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)
>
> The mmc_hw_reset's effect depends on the hardware layout/configure
> software's behavior, recently it will effect the clock of sdio for the
> platform I used. And it will still work well without mmc_hw_reset for
> the platform I Used currently. If sdio cannot work on other platform,
> I think it can add flag in ath10k_hw_params_list for the platform to
> call the mmc_hw_reset depends on the flag.
I don't see how you can use ath10k_hw_params_list to separate SDIO
controller functionality, I assume that's the real reason for difference
of functionality? Maybe this is a bug on the SDIO controller?
Ulf, what do you think? Any suggestions? Full discussion here:
https://patchwork.kernel.org/patch/10920563/
--
Kalle Valo
next prev parent reply other threads:[~2019-05-07 9:35 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-28 2:17 [PATCH] ath10k: remove mmc_hw_reset while hif power down Wen Gong
2019-05-03 18:01 ` Grant Grundler
2019-05-07 5:05 ` Wen Gong
2019-05-07 9:34 ` Kalle Valo [this message]
2019-05-28 12:45 ` Ulf Hansson
2019-06-18 10:39 ` Nicolas Boichat
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=87pnour4jg.fsf@codeaurora.org \
--to=kvalo@codeaurora.org \
--cc=ath10k@lists.infradead.org \
--cc=grundler@google.com \
--cc=linux-wireless@vger.kernel.org \
--cc=ulf.hansson@linaro.org \
--cc=wgong@codeaurora.org \
--cc=wgong@qti.qualcomm.com \
/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).