[DONOTMERGE] [NOTEVENRFC] [PATCH 00/11] ath1{1,2}k: support multiple PCI devices in one system

Mihai Moldovan ionic at ionic.de
Mon Nov 4 23:06:14 PST 2024


ath11k and ath12k suffer from a long-standing issue that is partly
caused by the QRTR implementation, which only supports one device per
node/port combination and partly caused by the fact that the
QMI instance ID of the devices are statically set to 1.

P Praneesh <quic_ppranees at quicinc.com> submitted a patch[0] that fixes
this issue generating a unique QMI instance ID based on the PCI bus data
for the device and passing that information to the QMI subsystem and the
device's firmware via a special PCI register.

However, it quickly turned out that this approach works for the hardware
he tested, but fails for other ath11k-based devices, including the
popular QCA6930, since its firmware just ignores the special register
being used.

Since we need QMI (and, for matter, QRTR) to work for the initial
firmware upload, this approach will not work generically.

Fortunately, Denis Kenzior <denkenz at gmail.com> cooked up a patch set for
QRTR[1] that introduces the concept of endpoint IDs, which are
dynamically allocated and can be used to distinguish different devices
even though they use the same node/port combination. Using this patch
set, endpoint IDs can be reported as part of auxillary data in the QRTR
socket and bound to for client sockets, which will automatically filter
mssages from other endpoints and also make sure that messages clients
send are routed to the correct endpoint.

This looked promising, and with that functionality, the only challenge
is to find out the correct endpoint IDs and bind to them in drivers to
finally support multiple devices in a generic way.

This patch set implements exactly that, and it WORKSFORME, but
unfortunately it turns out that "the only challenge" is very difficult
to overcome due to the socket-based architecture.

ath1{1,2}k and QRTR are at opposite sides of the socket, with QRTR
assigning endpoint IDs and ath1{1,2}k needing a way to fetch and operate
on them.

The endpoint reporting feature in QRTR is not helpful in this case,
because drivers do not generally know which endpoint belongs to the
device they currently handle (i.e., there is no central registry) and
even if we were to snoop on the socket and take the first endpoint ID we
are unaware of, this might not be the correct one, because it might be
in use by a different driver for instance, or correspond to a different
device.

The only way I found to communicate endpoint IDs between QRTR and the
drivers is to use MHI as the common layer utilized by both systems.
Since QRTR is using MHI as one underlying bus system, we can extend
struct mhi_device with a qrtr_endpoint_id field, initialize this to zero
(which is an invalid QRTR endpoint ID), have QRTR go through an
mhi_dev->mhi_cntrl->mhi_dev chain to select the MHI controller of the
device it is currently initializing for, save the newly assigned
endpoint ID to the MHI controller's mhi_device structure and do the same
for our device drivers ath1{1,2}k for fetching the endpoint ID QRTR
associated with the MHI controller.

This is not only incredibly ugly, because it essentially is a weird
pseudo-IPC implementation using the MHI bus device data space as a
communication platform, it comes short because of multiple issues:

  - The timing is critical. Since QRTR assigns endpoint IDs, clients can
    not query them before QRTR has been initialized for a specific
    device.
  - We would actually like to know the QRTR endpoint ID beforehand and
    bind to it BEFORE creating the QRTR/QMI socket, to not receive
    messages from other endpoints we are not interested in or send
    messages to wrong endpoints! But since there is no central registry
    for endpoint IDs, we cannot blindly assign just any ID from clients,
    and even if we could, the current architecture relies on QRTR doing
    it.
  - We should not use struct mhi_device to handle QRTR data. Obviously.
  - We are assuming that each device we are handling has a unique MHI
    controller associated with it, and also only one. This might not at
    all be true...

Personally, I am not comfortable with the patch set in its current form.
While it does fix the initial problem, it is not elegant in any way, and
I am aware of other issues, like not being able to unload the qrtr
module once it has been used (refcount only drops down to 2, even though
there is nothing still using it). I have not debugged this yet, and this
issue might have been introduced by Denis's changes, but I would not
find it hard to believe that my changes introduced a subtle refcount
increase that I am not aware of either.

Even with these issues, this patch set might be useful in the future and
it proves that multiple devices can work in one machine.

[0] https://patch.msgid.link/20230111170033.32454-1-kvalo@kernel.org
[1] https://patch.msgid.link/20241018181842.1368394-1-denkenz@gmail.com

Mihai Moldovan (11):
  bus: mhi: host: add QRTR endpoint ID to mhi_device
  net: qrtr: mhi: pass endpoint ID to MHI device on init
  soc: qcom: qmi_helpers: add QRTR endpoint ID to qmi_handle
  soc: qcom: qmi_helpers: optionally bind to QRTR endpoint ID in
    qmi_sock_create
  wifi: ath11k: add QRTR endpoint ID hif feature
  wifi: ath11k: stub QRTR endpoint ID fetching for AHB
  wifi: ath11k: implement QRTR endpoint ID fetching for PCI
  wifi: ath11k: bind to QRTR endpoint ID in ath11k_qmi_ops_new_server
  wifi: ath12k: add QRTR endpoint ID hif feature
  wifi: ath12k: implement QRTR endpoint ID fetching for PCI
  wifi: ath12k: bind to QRTR endpoint ID in ath12k_qmi_ops_new_server

 drivers/bus/mhi/host/init.c           |  1 +
 drivers/bus/mhi/host/main.c           | 64 +++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath11k/ahb.c |  7 +++
 drivers/net/wireless/ath/ath11k/hif.h |  8 ++++
 drivers/net/wireless/ath/ath11k/mhi.c | 17 +++++++
 drivers/net/wireless/ath/ath11k/mhi.h |  1 +
 drivers/net/wireless/ath/ath11k/pci.c |  1 +
 drivers/net/wireless/ath/ath11k/qmi.c | 50 +++++++++++++++++++++
 drivers/net/wireless/ath/ath11k/qmi.h |  1 +
 drivers/net/wireless/ath/ath12k/hif.h |  9 ++++
 drivers/net/wireless/ath/ath12k/mhi.c | 17 +++++++
 drivers/net/wireless/ath/ath12k/mhi.h |  2 +
 drivers/net/wireless/ath/ath12k/pci.c |  1 +
 drivers/net/wireless/ath/ath12k/qmi.c | 49 ++++++++++++++++++++
 drivers/net/wireless/ath/ath12k/qmi.h |  1 +
 drivers/soc/qcom/qmi_interface.c      | 28 ++++++++++++
 include/linux/mhi.h                   | 20 +++++++++
 include/linux/soc/qcom/qmi.h          |  3 ++
 net/qrtr/mhi.c                        |  4 ++
 19 files changed, 284 insertions(+)

-- 
2.45.2




More information about the ath11k mailing list