ATH11K Archive on lore.kernel.org
 help / color / mirror / Atom feed
* State of multiple PCI(e) modules since 2022
@ 2024-10-27 14:11 Mihai Moldovan
  2024-10-29 16:39 ` Denis Kenzior
  2024-10-31 14:49 ` Jeff Johnson
  0 siblings, 2 replies; 6+ messages in thread
From: Mihai Moldovan @ 2024-10-27 14:11 UTC (permalink / raw)
  To: ath11k


[-- Attachment #1.1: Type: text/plain, Size: 3597 bytes --]

Hi


I'm quite sorry for also raising the issue on this list, but this is where it 
was last discussed.

For the past 15 years (or longer), I've been using two ath9k-based cards in a 
machine, without issues. Granted, they didn't need external firmware, and didn't 
use a complicated layered PCI/MHI/QRTR/QMI setup.

It's time to upgrade, so I put a QCA6390 (ath11k) and a WCN7850 (ath12k) card 
into a machine, assuming that it will probably work as well (especially since 
they are driven by different drivers, although ath12k started as a fork of 
ath11k) and quickly noticed that... it doesn't.

Wrote to the ath12k mailing list[0], got zero responses.
Created a bug report on the kernel bug tracker[1], because it's arguably a bug, 
switched to the ath.git kernel because that's what other developers work with. 
Got zero response, of course. Started debugging this myself and came to the 
conclusion that the qrtr stuff tries to bind to both cards with the same 
parameters and everything fails.

This isn't made easier by the fact that the documentation for the whole 
MHI/QRTR/QMI stack is very sparse (e.g., MHI, mostly documented for 
Android-based SoCs, but there's hardly any explanation of what MHI states are, 
how they are transitioned and what it all does) to non-existent (e.g., QRTR!) 
and especially how they work together on top of each other is arcane magic.

Meanwhile, a different user created another bug report[2] for essentially the 
same issue and even this mailing list had another request regarding this just a 
few days ago[3].


So far, so suspicious. Unfortunately, I only noticed yesterday that this issue 
has been known for almost 2 years now! Robert Mako even hacked together 
something[4] to work around it, but that only worked by accident (the 
BHI_ERRDBG2 register is actually supposed to be read-only and only some hardware 
uses it in the first place, so it can't be regarded as a generic fix).

There were talks[5] about pursuing Qualcomm to modify the firmware, so that a 
special register can be used to communicate an instance ID based on the PCI 
domain number and the bus number, which currently works for some firmware (and 
hence modules) and doesn't work for others, but aside from an RFC, this hasn't 
gained any traction. To be fair, this patch looks more like a generic solution 
to the issue, although it does require special firmware support. It's not even 
all that difficult to port it from other firmware, given that it only requires 
reading a host register and exposing the capability.

I'll actually try to pull this patch into my kernel and also port it to ath12k 
to see if it would work with my QCA6390 + WCN7850 combination, but even if it 
does, it's incredibly frustrating that this issue has been known for so long, 
users are actually looking for solutions to it and there seems to be no movement 
on it. Even more frustrating is that inquiries regarding this are essentially 
all but ignored.

Sorry for the rant, but I really hope that I can get some traction for this again.



Mihai

[0] https://lists.infradead.org/pipermail/ath12k/2024-October/003915.html
[1] https://bugzilla.kernel.org/show_bug.cgi?id=219380
[2] https://bugzilla.kernel.org/show_bug.cgi?id=218480
[3] https://lists.infradead.org/pipermail/ath11k/2024-October/006467.html
[4] 
https://patchwork.kernel.org/project/linux-wireless/patch/20221105194943.826847-2-robimarko@gmail.com/
[5] 
https://patchwork.kernel.org/project/linux-wireless/patch/20230111170033.32454-1-kvalo@kernel.org/

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: State of multiple PCI(e) modules since 2022
@ 2024-11-01 17:05 Mihai Moldovan
  0 siblings, 0 replies; 6+ messages in thread
From: Mihai Moldovan @ 2024-11-01 17:05 UTC (permalink / raw)
  To: ath11k


[-- Attachment #1.1: Type: text/plain, Size: 2348 bytes --]

* On 10/29/24 17:39, Denis Kenzior wrote:
> This RFC might be of interest then:
> https://lore.kernel.org/all/20241018181842.1368394-1-denkenz@gmail.com/

Wow, thank you!

I've tried applying Kalle's RFC to ath11k (and noticed that it already is part 
of ath12k), force-enabled it and noticed that neither QCA6390 nor WNC7850 honor 
the value written to the magic register. Since the initial firmware/bootloader 
can't be easily updated, that approach isn't going to work for all supported 
hardware.

My idea was to implement an optional proxy mode into qrtr, which identifies 
messages from specific cards based on their PCI domain and bus number (which 
should work at least for MHI, no idea about AHB), and uses one socket to 
communicate with the device directly, and another socket that can be used as a 
proxy socket which reads from the "normal" socket and modifies messages based on 
the PCI data to replace the instance ID (and vice versa from driver back to the 
card/MHI interface).


Your approach of using auxiliary data looks way easier, cleaner and generally 
better and more promising.

I've applied your patch set and tried to resolve most nits that came up - 
although I've neither understood what to change to use {WRITE,READ}_ONCCE or 
where to remove locking, because the code referred to (BIND_ENDPOINT) doesn't 
even use locking. Although I essentially tested V1.5, feel free to add a me as 
Tested-By. It's successfully doing what it says it does.


Obviously that won't work out of the box for ath11k, because qmi_helpers is 
still using the "old" API that doesn't use specific endpoint binding. I'm having 
a hard time finding a way to make use of this feature in ath11k, though. We're 
at the "wrong" side of the socket in ath11k/qmi_helpers, so can't access any 
qrtr internals there. struct mhi_device is accessible from both ends, so I could 
write the QRTR endpoint id into its driver data, read it in ath11k and pass it 
down to qmi_helpers to bind to the specific endpoint. That certainly doesn't 
sound elegant and it's hard to argue why QRTR data should be part of struct 
mhi_device, and there is some potential for breakage because it would only be 
available after the qrtr-mhi interface was initialized/probed, but that's the 
only common ground I see.



Mihai

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: State of multiple PCI(e) modules since 2022
@ 2024-11-01 17:17 Mihai Moldovan
  0 siblings, 0 replies; 6+ messages in thread
From: Mihai Moldovan @ 2024-11-01 17:17 UTC (permalink / raw)
  To: ath11k


[-- Attachment #1.1: Type: text/plain, Size: 558 bytes --]

* On 11/1/24 18:05, Mihai Moldovan wrote:
 > [...]

Sorry, my mail client crashed while composing the message and continuing the 
draft didn't automatically include the In-Reply-To or References headers. 
Unfortunately I only noticed that after receiving a copy of my message back from 
the list. I didn't intend to break the original thread. I'm aware that this is 
highly annoying.

I'll be more careful in the future and will manually supply an In-Reply-To 
header now that I'm aware of the issues with continuing a draft message.



Mihai

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-11-05  6:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-27 14:11 State of multiple PCI(e) modules since 2022 Mihai Moldovan
2024-10-29 16:39 ` Denis Kenzior
2024-10-31 14:49 ` Jeff Johnson
2024-11-05  6:52   ` Mihai Moldovan
  -- strict thread matches above, loose matches on Subject: below --
2024-11-01 17:05 Mihai Moldovan
2024-11-01 17:17 Mihai Moldovan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox