From: Kalle Valo <kvalo@codeaurora.org>
To: Brian Norris <briannorris@chromium.org>
Cc: Wen Gong <wgong@qti.qualcomm.com>,
"linux-wireless\@vger.kernel.org"
<linux-wireless@vger.kernel.org>,
"ath10k\@lists.infradead.org" <ath10k@lists.infradead.org>,
Wen Gong <wgong@codeaurora.org>
Subject: Re: [PATCH] ath10k: support PCIe enter L1 state
Date: Fri, 16 Nov 2018 09:00:27 +0200 [thread overview]
Message-ID: <87va4x8q2c.fsf@codeaurora.org> (raw)
In-Reply-To: <20181115184333.GA87504@google.com> (Brian Norris's message of "Thu, 15 Nov 2018 10:43:33 -0800")
Brian Norris <briannorris@chromium.org> writes:
> On Thu, Nov 15, 2018 at 06:38:25AM +0000, Wen Gong wrote:
>> >
>> > Is there some reason L1 was disabled in the first place? Was it known to be
>> > unreliable?
>>
>> Hi Brian,
>> It is a BUG for power, and it is not considered this BUG before.
>> So this change will fix the bug.
>
> I understand that the existing behavior is suboptimal for power, but on
> the other hand, code that goes out of its way to *clear* the L1 flag
> doesn't just pop up out of nowhere. Somebody clearly wrote that! If it
> just meant "we didn't verify L1 at first", then maybe it's fine to
> enable it unconditionally and see what happens, but if it meant "we
> tried L1 on some old chip XXXX and it caused problems", then it would be
> nice to know what those problems were.
>
> Or maybe that is hard to figure out, given there's no public git history
> tracking the original code, and we just need to try it out.
Yeah, it seems L1 was disabled already on the first ath10k commit:
5e3dd157d7e70 (Kalle Valo 2013-06-12 20:52:10 +0300 2404) pcie_config_flags &= ~PCIE_CONFIG_FLAG_ENABLE_L1;
I don't remember anymore why but my guess is that the proprietary driver
at the time didn't support it with QCA998X. Or maybe QCA988X doesn't
even support L1? Michal, do you remember?
Related to QCA988X supporting L1 state also the commit log is misleading
as it only talks QCA6174/QCA9377, and has been only tested on QCA6174,
but the actual change enables L1 on _all_ PCI and AHB devices. So this
patch needs a lot more testing so that we have confidence that no
existing setups break.
--
Kalle Valo
next prev parent reply other threads:[~2018-11-16 7:00 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-14 2:50 [PATCH] ath10k: support PCIe enter L1 state Wen Gong
2018-11-15 0:28 ` Brian Norris
2018-11-15 6:38 ` Wen Gong
2018-11-15 18:43 ` Brian Norris
2018-11-16 7:00 ` Kalle Valo [this message]
2018-11-16 7:56 ` Michał Kazior
2019-02-08 13:42 ` Kalle Valo
2019-02-08 17:05 ` Brian Norris
2019-03-08 9:42 ` Kalle Valo
2019-12-02 18:48 ` Brian Norris
2020-02-13 11:15 ` Kalle Valo
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=87va4x8q2c.fsf@codeaurora.org \
--to=kvalo@codeaurora.org \
--cc=ath10k@lists.infradead.org \
--cc=briannorris@chromium.org \
--cc=linux-wireless@vger.kernel.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).