Michał Kazior <kazikcz@xxxxxxxxx> writes: > On Fri, 16 Nov 2018 at 08:00, Kalle Valo <kvalo@xxxxxxxxxxxxxx> wrote: >> >> Brian Norris <briannorris@xxxxxxxxxxxx> 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? > > Proprietary driver has it ifdef-ed to enable/disable. > > Older driver enabled it only for some station-only target/product so > by default QCA988X would build with L1 flag masked out. It made sense > to be conservative and change as little as possible to avoid random > bugs and breakage - so the logic was inherited minus the build-time > ifdef. > > However 10.4 driver seems to enable it unconditionally. I'm not sure > if this depends on target firmware in any way or if some other host > driver or bus settings need to be pre-set before L1 can be expected to > work reliably. > > I guess there's no way other than testing it out. No replies from anyone (including Wen) for 3 months about testing this patch on anything else than QCA6174. So I'll drop this now, please resubmit once test coverage is better. -- Kalle Valo
