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

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

[Mihai Moldovan]

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@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@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 auxiliary data in the QRTR
socket and bound to for client sockets, which will automatically filter
messages 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 first iteration of this patch set[2] extended struct mhi_device with
a qrtr_endpoint_id field that was initialized to zero and populated by
the QRTR MHI driver as soon as it was loaded. Drivers could then query
this field through an mhi_device->mhi_cntrl->mhi_device chain (if they
also use the MHI bus, of course). This, however, was an incredibly ugly
hack because QRTR data should not be part of MHI device structures in
the first place, timing is critical (drivers querying the endpoint ID
must do so after the QRTR MHI module initialized, which is typically
only the case after QRTR socket was created) and it was not possible to
query or pre-assign an endpoint ID before creating a socket and directly
bind to it (which might lead to races such as seeing messages over the
socket that are not meant for the endpoint ID drivers are actually
interested in).

Since that was not elegant at all, and due to the other mentioned
issues, this iteration uses a different approach: endpoint IDs can now
be associated with (private) backend endpoint-specific data, which
allows us to identify which endpoint ID is being used with what backend,
and additionally new API is introduced so that other parts in the kernel
can either get an endpoint ID for given endpoint-specific data or even
register endpoint-specific with a new endpoint ID that is generated by
the QRTR driver. The QRTR system will try to use endpoint-specific data
if possible, but falls back to generating endpoint IDs without
endpoint-specific data (as in, NULL pointer) if that did not work.

Crucially, the endpoint-specific data pointer is used as an opaque void
pointer and at most compared with data stored in the endpoints XArray.

In the QRTR MHI backend, we use the MHI controller's structure pointer
as endpoint-specific data, and since the MHI controller is also the bus
master and responsible for the physical link, clients (drivers) can
pre-register an endpoint ID for their MHI controllers and directly tell
QMI to bind to the endpoint ID at socket creation time. 

The QRTR SMD backend uses its rpmsg_device pointer and the TUN backend
uses the inode pointer as their respective endpoint-specific data
pointers.

This approach is cleaner and works better, because it is not prone to
races (although it requires coordination between QRTR backends and
clients/drivers because both must use the same endpoint-specific data
for the scheme to work).

There are, however, also issues with this approach:
  - Any kernel part can generate an unlimited number of endpoint IDs
    with arbitrary pointers. The amount of endpoint IDs that can be
    tracked is limited, though, so there is potential for exhaustion of ID
    space.
  - Since previously endpoint IDs were only generated by the QRTR
    subsystem, there was no need to use any kind of life cycle management
    for the endpoint IDs: they were created at node creation time and
    also deleted at node deletion time. Since other subsystems can now
    create endpoint IDs, it would probably be good to have a way to
    reclaim created but unused endpoint IDs. No such implementation is
    provided here.
  - Multiple PCI/MHI devices will work, but no AHB + PCI/MHI interaction
    has been tested. Since PCI/MHI devices bind to their endpoint ID,
    these will likely work, but the AHB devices might still see messages
    for all endpoints and fail to work correctly. AHB devices seem to be
    using QMI and thus also QRTR, but without a specific QRTR backend
    driver (going through REMOTEPROC instead?), so this approach might
    not be viable for AHB devices.

I am much more comfortable with this patch set, even if it has some
rough edges and might not fix the situation for AHB devices.

[0] https://patch.msgid.link/20230111170033.32454-1-kvalo@kernel.org
[1] https://patch.msgid.link/20241018181842.1368394-1-denkenz@gmail.com
[2] https://msgid.link/cover.1730790058.git.ionic@ionic.de

Mihai Moldovan (13):
  net: qrtr: support registering endpoint-specific data
  net: qrtr: mhi: register mhi_controller as endpoint-specific data
  net: qrtr: smd: register rpmsg_device as endpoint-specific data
  net: qrtr: tun: register inode as endpoint-specific data
  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_init_service
  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_init_service

 drivers/net/wireless/ath/ath11k/ahb.c |   7 ++
 drivers/net/wireless/ath/ath11k/hif.h |   8 ++
 drivers/net/wireless/ath/ath11k/mhi.c |  19 ++++
 drivers/net/wireless/ath/ath11k/mhi.h |   1 +
 drivers/net/wireless/ath/ath11k/pci.c |   1 +
 drivers/net/wireless/ath/ath11k/qmi.c |   8 ++
 drivers/net/wireless/ath/ath12k/hif.h |   9 ++
 drivers/net/wireless/ath/ath12k/mhi.c |  19 ++++
 drivers/net/wireless/ath/ath12k/mhi.h |   2 +
 drivers/net/wireless/ath/ath12k/pci.c |   1 +
 drivers/net/wireless/ath/ath12k/qmi.c |   9 ++
 drivers/soc/qcom/qmi_interface.c      |  28 ++++++
 include/linux/soc/qcom/qmi.h          |   3 +
 include/net/qrtr.h                    |  11 +++
 net/qrtr/af_qrtr.c                    | 124 +++++++++++++++++++++++++-
 net/qrtr/mhi.c                        |   1 +
 net/qrtr/qrtr.h                       |   5 ++
 net/qrtr/smd.c                        |   1 +
 net/qrtr/tun.c                        |   1 +
 19 files changed, 255 insertions(+), 3 deletions(-)
 create mode 100644 include/net/qrtr.h

-- 
2.45.2

[RFC] [PATCH 01/13] net: qrtr: support registering endpoint-specific data

[Mihai Moldovan]

This adds the infrastructure for registering endpoint-specific data for
endpoint IDs.

Endpoint-specific data can be used to map an endpoint ID to a specific
endpoint backend.

Additional API is introduced as a common header in include/net to allow
other parts of the kernel to query endpoint IDs for endpoint-specific
data or make the QRTR subsystem assign a new endpoint ID for passed
endpoint-specific data. This will allow other systems to register
endpoint IDs before actual socket creation and usage, which in turn
allows proper binding to endpoint IDs from the start.

The endpoint registration function is changed to re-use endpoint IDs
that match the backend's endpoint-specific data if possible, assign a
new endpoint ID if the endpoint-specific data is not known yet, or
create a new endpoint ID without endpoint-specific data attached to it
if all else fails.

There is one gripe with this implementation: other kernel subsystems
can, theoretically, assign an unlimited number of new new endpoint IDs
and thus exhaust endpoint ID space. No API is provided to delete
endpoint IDs and we also do not track which endpoint IDs are in use and
which are not, which means that even the QRTR-internal code cannot
easily clean up unused endpoint IDs. The only exception to this are
endpoint IDs attached to QRTR nodes, which will be deleted when the QRTR
nodes themselves are deleted.

This is probably a potential memory leak that we can live with.

Fixing that is rather difficult. We would either have to add some form
of refcounting, wrap the endpoint-specific data pointer into yet another
structure together with a kref and use that to free unused endpoint IDs,
or periodically clean unused endpoint IDs up in a timer (executing, say,
every 10 minutes), essentially doing garbage collection.

Garbage collection is being frowned upon, especially in the kernel, but
in this case, it really would make the most sense. Clients might
allocate endpoint IDs that are never actually used (for instance because
the client uses a wrong endpoint-specific data pointer, which is not
used by a QRTR backend), and will need to hold a reference to this for
their entire life time, which essentially defeats the concept of
cleanup of unused endpoint IDs via reference counting.

Clients can create endpoint IDs quite some time before the QRTR
subsystem uses them, but there is no way to easily tell when this will
be. The idea is that minutes as orders of magnitude would probably be a
safe value for which to regard an endpoint ID as unused.

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
Depends-on: 25a7151cdc98 ("net: qrtr: ns: support multiple endpoints")
Link: https://patch.msgid.link/20241018181842.1368394-1-denkenz@gmail.com
---
 include/net/qrtr.h |  11 ++++
 net/qrtr/af_qrtr.c | 124 +++++++++++++++++++++++++++++++++++++++++++--
 net/qrtr/qrtr.h    |   5 ++
 3 files changed, 137 insertions(+), 3 deletions(-)
 create mode 100644 include/net/qrtr.h

diff --git a/include/net/qrtr.h b/include/net/qrtr.h
new file mode 100644
index 000000000000..799c84eb35ad
--- /dev/null
+++ b/include/net/qrtr.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __NET_QRTR_H
+#define __NET_QRTR_H
+
+#include <linux/types.h>
+
+int qrtr_endpoint_id_get(const void *data, u32 *id);
+int qrtr_endpoint_id_assign(void *data, u32 *id);
+int qrtr_endpoint_id_get_or_assign(void *data, u32 *id);
+
+#endif	/* __NET_QRTR_H */
diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
index cf8b5483ba2c..59227b3d49f4 100644
--- a/net/qrtr/af_qrtr.c
+++ b/net/qrtr/af_qrtr.c
@@ -11,6 +11,7 @@
 #include <linux/wait.h>
 
 #include <net/sock.h>
+#include <net/qrtr.h>
 
 #include "qrtr.h"
 
@@ -649,6 +650,114 @@ static struct sk_buff *qrtr_alloc_ctrl_packet(struct qrtr_ctrl_pkt **pkt,
 	return skb;
 }
 
+/**
+ * qrtr_endpoint_id_get() - get a registered endpoint for given data
+ * @data: endpoint-specific data to fetch ID for
+ * @id: pointer to store endpoint ID into
+ * Return: 0 on success, negative error code on failure
+ *
+ * The endpoint-specific data must not be NULL.
+ * The output parameter id must not be NULL.
+ * If no endpoint ID can be mapped to the endpoint-specific data, id will be
+ * set to 0.
+ */
+int qrtr_endpoint_id_get(const void *data, u32 *id)
+{
+	unsigned long idx = 0;
+	void *iter_data = NULL;
+
+	if (!id)
+		return -EINVAL;
+
+	if (!data)
+		return -EINVAL;
+
+	*id = 0;
+	rcu_read_lock();
+	xa_for_each_range(&qrtr_endpoints, idx, iter_data,
+			  QRTR_ENDPOINT_RANGE.min, QRTR_ENDPOINT_RANGE.max) {
+		if (iter_data == data) {
+			*id = idx;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(qrtr_endpoint_id_get);
+
+/**
+ * qrtr_endpoint_id_assign() - assigns a new endpoint ID for given data
+ * @data: endpoint-specific data to assign new ID for
+ * @id: pointer to store endpoint ID into
+ * Return: 0 on success, negative error code on failure
+ *
+ * The endpoint-specific data must not be NULL.
+ * The output parameter id must not be NULL.
+ * On error, id will be set to 0.
+ */
+int qrtr_endpoint_id_assign(void *data, u32 *id)
+{
+	int rc = 0;
+
+	if (!id)
+		return -EINVAL;
+
+	if (!data)
+		return -EINVAL;
+
+	rc = xa_alloc_cyclic(&qrtr_endpoints, id, data, QRTR_ENDPOINT_RANGE,
+			     &next_endpoint_id, GFP_KERNEL);
+	if (rc)
+		*id = 0;
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(qrtr_endpoint_id_assign);
+
+/**
+ * qrtr_endpoint_id_get_or_assign() - gets or assigns endpoint ID for data
+ * @data: endpoint-specific data to assign new ID for
+ * @id: pointer to store endpoint ID into
+ * Return: positive on success, negative error code on failure
+ *
+ * The endpoint-specific data must not be NULL.
+ *
+ * If the endpoint-specific data is already registered to an endpoint ID, this
+ * ID will be assigned to id. Otherwise, this function assigns a new
+ * endpoint ID and associates it with the given endpoint-specific data.
+ *
+ * The output parameter id must not be NULL. It will either be set to the
+ * fetched or newly assigned endpoint ID on success, or set to 0 on error.
+ */
+int qrtr_endpoint_id_get_or_assign(void *data, u32 *id)
+{
+	int rc = 0;
+
+	if (!data)
+		return -EINVAL;
+
+	if (!id)
+		return -EINVAL;
+
+	rc = qrtr_endpoint_id_get(data, id);
+
+	if (rc) {
+		*id = 0;
+		return rc;
+	}
+
+	if (!*id)
+		rc = qrtr_endpoint_id_assign(data, id);
+
+	if (rc)
+		*id = 0;
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(qrtr_endpoint_id_get_or_assign);
+
 /**
  * qrtr_endpoint_register() - register a new endpoint
  * @ep: endpoint to register
@@ -670,9 +779,18 @@ int qrtr_endpoint_register(struct qrtr_endpoint *ep, unsigned int nid)
 	if (!node)
 		return -ENOMEM;
 
-	rc = xa_alloc_cyclic(&qrtr_endpoints, &endpoint_id, NULL,
-			     QRTR_ENDPOINT_RANGE, &next_endpoint_id,
-			     GFP_KERNEL);
+	rc = qrtr_endpoint_id_get_or_assign(ep->endpoint_data, &endpoint_id);
+
+	/*
+	 * The previous function fails if ep->endpoint_data is NULL, so retry.
+	 *
+	 * We're going to assign an endpoint ID without endpoint-specific data
+	 * set in this case.
+	 */
+	if (rc)
+		rc = xa_alloc_cyclic(&qrtr_endpoints, &endpoint_id, NULL,
+				     QRTR_ENDPOINT_RANGE, &next_endpoint_id,
+				     GFP_KERNEL);
 
 	if (rc < 0)
 		goto free_node;
diff --git a/net/qrtr/qrtr.h b/net/qrtr/qrtr.h
index b4f50336ae75..3509168e8a40 100644
--- a/net/qrtr/qrtr.h
+++ b/net/qrtr/qrtr.h
@@ -12,13 +12,18 @@ struct sk_buff;
 /**
  * struct qrtr_endpoint - endpoint handle
  * @xmit: Callback for outgoing packets
+ * @endpoint_data: endpoint-specific data pointer, can be NULL
  *
  * The socket buffer passed to the xmit function becomes owned by the endpoint
  * driver.  As such, when the driver is done with the buffer, it should
  * call kfree_skb() on failure, or consume_skb() on success.
+ *
+ * If endpoint_data is NULL, endpoint IDs can not be directly mapped to a
+ * specific endpoint.
  */
 struct qrtr_endpoint {
 	int (*xmit)(struct qrtr_endpoint *ep, struct sk_buff *skb);
+	void *endpoint_data;
 	/* private: not for endpoint use */
 	struct qrtr_node *node;
 	u32 id;
-- 
2.45.2

[RFC] [PATCH 02/13] net: qrtr: mhi: register mhi_controller as endpoint-specific data

[Mihai Moldovan]

For the MHI backend, we will use the mhi_controller pointer as the
endpoint-specific data.

This means that we can only have one endpoint ID per MHI controller, but
since the MHI controller is the bus master in charge of the physical
link, this is probably okay.

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
---
 net/qrtr/mhi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
index 9a23c888e234..fc6869c3a7ec 100644
--- a/net/qrtr/mhi.c
+++ b/net/qrtr/mhi.c
@@ -95,6 +95,7 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
 	qdev->mhi_dev = mhi_dev;
 	qdev->dev = &mhi_dev->dev;
 	qdev->ep.xmit = qcom_mhi_qrtr_send;
+	qdev->ep.endpoint_data = mhi_dev->mhi_cntrl;
 
 	dev_set_drvdata(&mhi_dev->dev, qdev);
 	rc = qrtr_endpoint_register(&qdev->ep, QRTR_EP_NID_AUTO);
-- 
2.45.2

[RFC] [PATCH 03/13] net: qrtr: smd: register rpmsg_device as endpoint-specific data

[Mihai Moldovan]

For the SMD backend, we will use the rpmsg_device pointer as the endpoint-specific data.

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
---
 net/qrtr/smd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/qrtr/smd.c b/net/qrtr/smd.c
index c91bf030fbc7..cb3eeb6835ca 100644
--- a/net/qrtr/smd.c
+++ b/net/qrtr/smd.c
@@ -68,6 +68,7 @@ static int qcom_smd_qrtr_probe(struct rpmsg_device *rpdev)
 	qdev->channel = rpdev->ept;
 	qdev->dev = &rpdev->dev;
 	qdev->ep.xmit = qcom_smd_qrtr_send;
+	qdev->ep.endpoint_data = rpdev;
 
 	rc = qrtr_endpoint_register(&qdev->ep, QRTR_EP_NID_AUTO);
 	if (rc)
-- 
2.45.2

[RFC] [PATCH 04/13] net: qrtr: tun: register inode as endpoint-specific data

[Mihai Moldovan]

For the TUN backend, we will use the inode pointer as the endpoint-specific data.

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
---
 net/qrtr/tun.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/qrtr/tun.c b/net/qrtr/tun.c
index 304b41fea5ab..9dcfecd529f7 100644
--- a/net/qrtr/tun.c
+++ b/net/qrtr/tun.c
@@ -41,6 +41,7 @@ static int qrtr_tun_open(struct inode *inode, struct file *filp)
 	init_waitqueue_head(&tun->readq);
 
 	tun->ep.xmit = qrtr_tun_send;
+	tun->ep.endpoint_data = inode;
 
 	filp->private_data = tun;
 
-- 
2.45.2

[RFC] [PATCH 05/13] soc: qcom: qmi_helpers: add QRTR endpoint ID to qmi_handle

[Mihai Moldovan]

Adding this allows us to easily supply an endpoint ID to bind on later
on when creating the socket.

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
---
 include/linux/soc/qcom/qmi.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h
index 469e02d2aa0d..77743c855762 100644
--- a/include/linux/soc/qcom/qmi.h
+++ b/include/linux/soc/qcom/qmi.h
@@ -212,6 +212,7 @@ struct qmi_msg_handler {
  * @txns:	outstanding transactions
  * @txn_lock:	lock for modifications of @txns
  * @handlers:	list of handlers for incoming messages
+ * @endpoint_id: QRTR endpoint ID to bind on
  */
 struct qmi_handle {
 	struct socket *sock;
@@ -235,6 +236,8 @@ struct qmi_handle {
 	struct mutex txn_lock;
 
 	const struct qmi_msg_handler *handlers;
+
+	u32 endpoint_id;
 };
 
 int qmi_add_lookup(struct qmi_handle *qmi, unsigned int service,
-- 
2.45.2

[RFC] [PATCH 06/13] soc: qcom: qmi_helpers: optionally bind to QRTR endpoint ID in qmi_sock_create

[Mihai Moldovan]

For clients that already know the QRTR endpoint ID before actually
creating the QMI socket and set it in struct qmi_handle, optionally try
to bind to this QRTR endpoint ID when creating the socket.

This can fail, and qmi_sock_create will issue diagnostic messages, but
otherwise ignore the error.

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
---
 drivers/soc/qcom/qmi_interface.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/soc/qcom/qmi_interface.c b/drivers/soc/qcom/qmi_interface.c
index bb98b06e87f8..893e5a6accd9 100644
--- a/drivers/soc/qcom/qmi_interface.c
+++ b/drivers/soc/qcom/qmi_interface.c
@@ -586,6 +586,7 @@ static struct socket *qmi_sock_create(struct qmi_handle *qmi,
 				      struct sockaddr_qrtr *sq)
 {
 	struct socket *sock;
+	const struct proto_ops *ops = NULL;
 	int ret;
 
 	ret = sock_create_kern(&init_net, AF_QIPCRTR, SOCK_DGRAM,
@@ -593,6 +594,33 @@ static struct socket *qmi_sock_create(struct qmi_handle *qmi,
 	if (ret < 0)
 		return ERR_PTR(ret);
 
+	ops = READ_ONCE(sock->ops);
+
+	if (!ops) {
+		pr_warn("sock->ops not available for QMI socket, will not be "
+			"able to bind to endpoint ID.\n");
+		/* N.B.: this error value will not be passed out. */
+		ret = -ENXIO;
+	}
+
+	if (!ret && !ops->setsockopt) {
+		pr_warn("ops->setsockopt not available for QMI socket, will "
+			"not be able to bind to endpoint ID.\n");
+		/* N.B.: this error value will not be passed out. */
+		ret = -ENXIO;
+	}
+
+	/* Only bind to a specific endpoint if a valid one was provided. */
+	if (!ret && qmi->endpoint_id) {
+		ret = ops->setsockopt(sock, SOL_QRTR, QRTR_BIND_ENDPOINT,
+				      KERNEL_SOCKPTR(&qmi->endpoint_id),
+				      sizeof(qmi->endpoint_id));
+
+		if (ret < 0)
+			pr_warn("binding to QRTR endpoint ID requested, but "
+				"operation failed: %d\n", ret);
+	}
+
 	ret = kernel_getsockname(sock, (struct sockaddr *)sq);
 	if (ret < 0) {
 		sock_release(sock);
-- 
2.45.2

[RFC] [PATCH 07/13] wifi: ath11k: add QRTR endpoint ID hif feature

[Mihai Moldovan]

This will allow fetching the QRTR endpoint ID via hardware-specific
means.

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
---
 drivers/net/wireless/ath/ath11k/hif.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/hif.h b/drivers/net/wireless/ath/ath11k/hif.h
index 674ff772b181..22324fe59d48 100644
--- a/drivers/net/wireless/ath/ath11k/hif.h
+++ b/drivers/net/wireless/ath/ath11k/hif.h
@@ -31,6 +31,7 @@ struct ath11k_hif_ops {
 	void (*ce_irq_enable)(struct ath11k_base *ab);
 	void (*ce_irq_disable)(struct ath11k_base *ab);
 	void (*get_ce_msi_idx)(struct ath11k_base *ab, u32 ce_id, u32 *msi_idx);
+	int (*set_qrtr_endpoint_id)(struct ath11k_base *ab);
 };
 
 static inline void ath11k_hif_ce_irq_enable(struct ath11k_base *ab)
@@ -146,4 +147,11 @@ static inline void ath11k_get_ce_msi_idx(struct ath11k_base *ab, u32 ce_id,
 		*msi_data_idx = ce_id;
 }
 
+static inline int ath11k_set_qrtr_endpoint_id(struct ath11k_base *ab)
+{
+	if (!ab->hif.ops->set_qrtr_endpoint_id)
+		return -ENOSYS;
+	else
+		return ab->hif.ops->set_qrtr_endpoint_id(ab);
+}
 #endif /* _HIF_H_ */
-- 
2.45.2

[RFC] [PATCH 08/13] wifi: ath11k: stub QRTR endpoint ID fetching for AHB

[Mihai Moldovan]

QRTR endpoint ID fetching will currently not be available for AHB.

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
---
 drivers/net/wireless/ath/ath11k/ahb.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/ahb.c b/drivers/net/wireless/ath/ath11k/ahb.c
index 916402ad06b8..19c958b04b7a 100644
--- a/drivers/net/wireless/ath/ath11k/ahb.c
+++ b/drivers/net/wireless/ath/ath11k/ahb.c
@@ -773,6 +773,7 @@ static const struct ath11k_hif_ops ath11k_ahb_hif_ops_ipq8074 = {
 	.map_service_to_pipe = ath11k_ahb_map_service_to_pipe,
 	.power_down = ath11k_ahb_power_down,
 	.power_up = ath11k_ahb_power_up,
+	.set_qrtr_endpoint_id = ath11k_ahb_set_qrtr_endpoint_id,
 };
 
 static const struct ath11k_hif_ops ath11k_ahb_hif_ops_wcn6750 = {
@@ -792,6 +793,7 @@ static const struct ath11k_hif_ops ath11k_ahb_hif_ops_wcn6750 = {
 	.resume = ath11k_ahb_hif_resume,
 	.ce_irq_enable = ath11k_pci_enable_ce_irqs_except_wake_irq,
 	.ce_irq_disable = ath11k_pci_disable_ce_irqs_except_wake_irq,
+	.set_qrtr_endpoint_id = ath11k_ahb_set_qrtr_endpoint_id,
 };
 
 static int ath11k_core_get_rproc(struct ath11k_base *ab)
@@ -1312,6 +1314,11 @@ static void ath11k_ahb_shutdown(struct platform_device *pdev)
 	ath11k_ahb_free_resources(ab);
 }
 
+static ath11k_ahb_set_qrtr_endpoint_id(struct ath11k_base *ab)
+{
+	return -ENOSYS;
+}
+
 static struct platform_driver ath11k_ahb_driver = {
 	.driver         = {
 		.name   = "ath11k",
-- 
2.45.2

[RFC] [PATCH 09/13] wifi: ath11k: implement QRTR endpoint ID fetching for PCI

[Mihai Moldovan]

QRTR endpoint ID fetching for PCIe devices will use MHI.

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
---
 drivers/net/wireless/ath/ath11k/mhi.c | 19 +++++++++++++++++++
 drivers/net/wireless/ath/ath11k/mhi.h |  1 +
 drivers/net/wireless/ath/ath11k/pci.c |  1 +
 3 files changed, 21 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
index 6974a551883f..46ee049f176b 100644
--- a/drivers/net/wireless/ath/ath11k/mhi.c
+++ b/drivers/net/wireless/ath/ath11k/mhi.c
@@ -11,6 +11,8 @@
 #include <linux/of_address.h>
 #include <linux/ioport.h>
 
+#include <net/qrtr.h>
+
 #include "core.h"
 #include "debug.h"
 #include "mhi.h"
@@ -490,3 +492,20 @@ int ath11k_mhi_resume(struct ath11k_pci *ab_pci)
 
 	return 0;
 }
+
+int ath11k_mhi_set_qrtr_endpoint_id(struct ath11k_base *ab)
+{
+	struct ath11k_pci *ab_pci = ath11k_pci_priv(ab);
+	struct ath11k_qmi *qmi = &ab->qmi;
+	int ret;
+
+	/* Pass endpoint ID up for QMI usage. */
+	ret = qrtr_endpoint_id_get_or_assign(ab_pci->mhi_ctrl, &qmi->handle.endpoint_id);
+	ath11k_dbg(ab, ATH11K_DBG_PCI, "queried mhi_ctrl QRTR endpoint ID: %u\n", qmi->handle.endpoint_id);
+	if (ret) {
+		ath11k_warn(ab, "failed to query QRTR endpoint ID: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
diff --git a/drivers/net/wireless/ath/ath11k/mhi.h b/drivers/net/wireless/ath/ath11k/mhi.h
index a682aad52fc5..84465fb5d5da 100644
--- a/drivers/net/wireless/ath/ath11k/mhi.h
+++ b/drivers/net/wireless/ath/ath11k/mhi.h
@@ -27,4 +27,5 @@ void ath11k_mhi_clear_vector(struct ath11k_base *ab);
 int ath11k_mhi_suspend(struct ath11k_pci *ar_pci);
 int ath11k_mhi_resume(struct ath11k_pci *ar_pci);
 
+int ath11k_mhi_set_qrtr_endpoint_id(struct ath11k_base *ab);
 #endif
diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
index 18248b8e5b93..fd3664c8ff8b 100644
--- a/drivers/net/wireless/ath/ath11k/pci.c
+++ b/drivers/net/wireless/ath/ath11k/pci.c
@@ -744,6 +744,7 @@ static const struct ath11k_hif_ops ath11k_pci_hif_ops = {
 	.ce_irq_enable = ath11k_pci_hif_ce_irq_enable,
 	.ce_irq_disable = ath11k_pci_hif_ce_irq_disable,
 	.get_ce_msi_idx = ath11k_pcic_get_ce_msi_idx,
+	.set_qrtr_endpoint_id = ath11k_mhi_set_qrtr_endpoint_id,
 };
 
 static void ath11k_pci_read_hw_version(struct ath11k_base *ab, u32 *major, u32 *minor)
-- 
2.45.2

[RFC] [PATCH 10/13] wifi: ath11k: bind to QRTR endpoint ID in ath11k_qmi_init_service

[Mihai Moldovan]

If possible, fetch the QRTR endpoint ID in ath11k_qmi_init_service, just
before calling qmi_handle_init, and make it available in the qmi_handle.

qmi_helpers will then automatically bind to this endpoint for us.

This finally allows using multiple ath11k-based cards with the same QRTR
node/port combination to work simultanenous (and, for that matter, at
all).

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-05266-QCAHSTSWPLZ_V2_TO_X86-1
---
 drivers/net/wireless/ath/ath11k/qmi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 7a22483b35cd..371db19543c1 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -15,6 +15,7 @@
 #include <linux/ioport.h>
 #include <linux/firmware.h>
 #include <linux/of_irq.h>
+#include <net/sock.h>
 
 #define SLEEP_CLOCK_SELECT_INTERNAL_BIT	0x02
 #define HOST_CSTATE_BIT			0x04
@@ -3311,6 +3312,13 @@ int ath11k_qmi_init_service(struct ath11k_base *ab)
 	ab->qmi.ab = ab;
 
 	ab->qmi.target_mem_mode = ab->hw_params.fw_mem_mode;
+
+	ret = ath11k_set_qrtr_endpoint_id(ab);
+	if (ret)
+		ath11k_warn(ab, "failed to set QRTR endpoint ID: %d\n"
+				"continuing without, but only one device per "
+				"system will be supported\n", ret);
+
 	ret = qmi_handle_init(&ab->qmi.handle, ATH11K_QMI_RESP_LEN_MAX,
 			      &ath11k_qmi_ops, ath11k_qmi_msg_handlers);
 	if (ret < 0) {
-- 
2.45.2

[RFC] [PATCH 11/13] wifi: ath12k: add QRTR endpoint ID hif feature

[Mihai Moldovan]

This will allow fetching the QRTR endpoint ID via hardware-specific
means.

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
---
 drivers/net/wireless/ath/ath12k/hif.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/wireless/ath/ath12k/hif.h b/drivers/net/wireless/ath/ath12k/hif.h
index e8840fab6061..c00234ec64c1 100644
--- a/drivers/net/wireless/ath/ath12k/hif.h
+++ b/drivers/net/wireless/ath/ath12k/hif.h
@@ -32,6 +32,7 @@ struct ath12k_hif_ops {
 	void (*get_ce_msi_idx)(struct ath12k_base *ab, u32 ce_id, u32 *msi_idx);
 	int (*panic_handler)(struct ath12k_base *ab);
 	void (*coredump_download)(struct ath12k_base *ab);
+	int (*set_qrtr_endpoint_id)(struct ath12k_base *ab);
 };
 
 static inline int ath12k_hif_map_service_to_pipe(struct ath12k_base *ab, u16 service_id,
@@ -162,4 +163,12 @@ static inline void ath12k_hif_coredump_download(struct ath12k_base *ab)
 	if (ab->hif.ops->coredump_download)
 		ab->hif.ops->coredump_download(ab);
 }
+
+static inline int ath12k_hif_set_qrtr_endpoint_id(struct ath12k_base *ab)
+{
+	if (!ab->hif.ops->set_qrtr_endpoint_id)
+		return -ENOSYS;
+	else
+		return ab->hif.ops->set_qrtr_endpoint_id(ab);
+}
 #endif /* ATH12K_HIF_H */
-- 
2.45.2

[RFC] [PATCH 12/13] wifi: ath12k: implement QRTR endpoint ID fetching for PCI

[Mihai Moldovan]

QRTR endpoint ID fetching for PCIe devices will use MHI.

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
---
 drivers/net/wireless/ath/ath12k/mhi.c | 19 +++++++++++++++++++
 drivers/net/wireless/ath/ath12k/mhi.h |  2 ++
 drivers/net/wireless/ath/ath12k/pci.c |  1 +
 3 files changed, 22 insertions(+)

diff --git a/drivers/net/wireless/ath/ath12k/mhi.c b/drivers/net/wireless/ath/ath12k/mhi.c
index 2f6d14382ed7..e92ef20826bc 100644
--- a/drivers/net/wireless/ath/ath12k/mhi.c
+++ b/drivers/net/wireless/ath/ath12k/mhi.c
@@ -8,6 +8,8 @@
 #include <linux/pci.h>
 #include <linux/firmware.h>
 
+#include <net/qrtr.h>
+
 #include "core.h"
 #include "debug.h"
 #include "mhi.h"
@@ -654,3 +656,20 @@ void ath12k_mhi_coredump(struct mhi_controller *mhi_ctrl, bool in_panic)
 {
 	mhi_download_rddm_image(mhi_ctrl, in_panic);
 }
+
+int ath12k_mhi_set_qrtr_endpoint_id(struct ath12k_base *ab)
+{
+	struct ath12k_pci *ab_pci = ath12k_pci_priv(ab);
+	struct ath12k_qmi *qmi = &ab->qmi;
+	int ret;
+
+	/* Pass endpoint ID up for QMI usage. */
+	ret = qrtr_endpoint_id_get_or_assign(ab_pci->mhi_ctrl, &qmi->handle.endpoint_id);
+	ath12k_dbg(ab, ATH12K_DBG_PCI, "queried mhi_ctrl QRTR endpoint ID: %u\n", qmi->handle.endpoint_id);
+	if (ret) {
+		ath12k_warn(ab, "failed to query QRTR endpoint ID: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
diff --git a/drivers/net/wireless/ath/ath12k/mhi.h b/drivers/net/wireless/ath/ath12k/mhi.h
index 7358b8477536..c4e00c1747c8 100644
--- a/drivers/net/wireless/ath/ath12k/mhi.h
+++ b/drivers/net/wireless/ath/ath12k/mhi.h
@@ -44,4 +44,6 @@ void ath12k_mhi_clear_vector(struct ath12k_base *ab);
 void ath12k_mhi_suspend(struct ath12k_pci *ar_pci);
 void ath12k_mhi_resume(struct ath12k_pci *ar_pci);
 void ath12k_mhi_coredump(struct mhi_controller *mhi_ctrl, bool in_panic);
+
+int ath12k_mhi_set_qrtr_endpoint_id(struct ath12k_base *ab);
 #endif
diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
index cf907550e6a4..3d65c64d1b31 100644
--- a/drivers/net/wireless/ath/ath12k/pci.c
+++ b/drivers/net/wireless/ath/ath12k/pci.c
@@ -1514,6 +1514,7 @@ static const struct ath12k_hif_ops ath12k_pci_hif_ops = {
 #ifdef CONFIG_ATH12K_COREDUMP
 	.coredump_download = ath12k_pci_coredump_download,
 #endif
+	.set_qrtr_endpoint_id = ath12k_mhi_set_qrtr_endpoint_id,
 };
 
 static
-- 
2.45.2

[RFC] [PATCH 13/13] wifi: ath12k: bind to QRTR endpoint ID in ath12k_qmi_init_service

[Mihai Moldovan]

If possible, fetch the QRTR endpoint ID in ath12k_qmi_init_service, just
before calling qmi_handle_init, and make it available in the qmi_handle.

qmi_helpers will then automatically bind to this endpoint for us.

This finally allows using multiple ath12k-based cards with the same QRTR
node/port combination to work simultanenous (and, for that matter, at
all), including combinations of ath11k-based and ath12k-based cards.

Signed-off-by: Mihai Moldovan <ionic@ionic.de>
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
---
 drivers/net/wireless/ath/ath12k/qmi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index d2d9d03c7a28..b7bfcb184a6f 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -9,8 +9,10 @@
 #include "qmi.h"
 #include "core.h"
 #include "debug.h"
+#include "hif.h"
 #include <linux/of.h>
 #include <linux/firmware.h>
+#include <net/sock.h>
 
 #define SLEEP_CLOCK_SELECT_INTERNAL_BIT	0x02
 #define HOST_CSTATE_BIT			0x04
@@ -3371,6 +3373,13 @@ int ath12k_qmi_init_service(struct ath12k_base *ab)
 	ab->qmi.ab = ab;
 
 	ab->qmi.target_mem_mode = ATH12K_QMI_TARGET_MEM_MODE_DEFAULT;
+
+	ret = ath12k_hif_set_qrtr_endpoint_id(ab);
+	if (ret)
+		ath12k_warn(ab, "failed to set QRTR endpoint ID: %d\n"
+				"continuing without, but only one device per "
+				"system will be supported\n", ret);
+
 	ret = qmi_handle_init(&ab->qmi.handle, ATH12K_QMI_RESP_LEN_MAX,
 			      &ath12k_qmi_ops, ath12k_qmi_msg_handlers);
 	if (ret < 0) {
-- 
2.45.2

Re: [RFC] [PATCH 00/13] ath1{1,2}k: support multiple PCI devices in one system

[Mihai Moldovan]

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

Just some spelling fixes I'll stage for a v2, since mistakes naturally only 
become obvious after reading what was sent out...


* On 11/17/24 17:05, Mihai Moldovan wrote:
> [...]
> socket and bound to for client sockets, which will automatically filter
> messages from other endpoints and also make sure that messages clients
> send are routed to the correct endpoint.

[...] and also make sure that client messages are routed to the correct endpoint.


> [...]
> and additionally new API is introduced so that other parts in the kernel
> can either get an endpoint ID for given endpoint-specific data or even
> register endpoint-specific with a new endpoint ID that is generated by
> the QRTR driver. [...]

[...] or even attach endpoint-specific data to a new endpoint ID generated by 
the QRTR driver.



Mihai

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

Re: [RFC] [PATCH 00/13] ath1{1,2}k: support multiple PCI devices in one system

[Jeff Johnson]

On 11/17/2024 8:59 AM, Mihai Moldovan wrote:
> Just some spelling fixes I'll stage for a v2, since mistakes naturally only 
> become obvious after reading what was sent out...
> 
> 
> * On 11/17/24 17:05, Mihai Moldovan wrote:
>> [...]
>> socket and bound to for client sockets, which will automatically filter
>> messages from other endpoints and also make sure that messages clients
>> send are routed to the correct endpoint.
> 
> [...] and also make sure that client messages are routed to the correct endpoint.
> 
> 
>> [...]
>> and additionally new API is introduced so that other parts in the kernel
>> can either get an endpoint ID for given endpoint-specific data or even
>> register endpoint-specific with a new endpoint ID that is generated by
>> the QRTR driver. [...]
> 
> [...] or even attach endpoint-specific data to a new endpoint ID generated by 
> the QRTR driver.
> 
> 
> 
> Mihai

When you post v2, please use scripts/get_maintainer.pl to identify all of the
maintainers and mailing lists that should be notified of this RFC.

I suspect you won't get much feedback without doing so.

/jeff

Re: [RFC] [PATCH 00/13] ath1{1,2}k: support multiple PCI devices in one system

[Jeff Johnson]

On 11/18/2024 7:52 AM, Jeff Johnson wrote:
> On 11/17/2024 8:59 AM, Mihai Moldovan wrote:
>> Just some spelling fixes I'll stage for a v2, since mistakes naturally only 
>> become obvious after reading what was sent out...
>>
>>
>> * On 11/17/24 17:05, Mihai Moldovan wrote:
>>> [...]
>>> socket and bound to for client sockets, which will automatically filter
>>> messages from other endpoints and also make sure that messages clients
>>> send are routed to the correct endpoint.
>>
>> [...] and also make sure that client messages are routed to the correct endpoint.
>>
>>
>>> [...]
>>> and additionally new API is introduced so that other parts in the kernel
>>> can either get an endpoint ID for given endpoint-specific data or even
>>> register endpoint-specific with a new endpoint ID that is generated by
>>> the QRTR driver. [...]
>>
>> [...] or even attach endpoint-specific data to a new endpoint ID generated by 
>> the QRTR driver.
>>
>>
>>
>> Mihai
> 
> When you post v2, please use scripts/get_maintainer.pl to identify all of the
> maintainers and mailing lists that should be notified of this RFC.
> 
> I suspect you won't get much feedback without doing so.

The sooner you post a v2 and include all of the right people and lists, the
sooner the chances of getting good feedback. From internal WLAN team
discussion the concept is good, but the implementation is not ideal. Hopefully
pulling in the actual QRTR & MHI developers and maintainer will lead to a
cleaner and acceptable solution.

/jeff

Re: [RFC] [PATCH 00/13] ath1{1,2}k: support multiple PCI devices in one system

[Mihai Moldovan]

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

* On 11/22/24 17:40, Jeff Johnson wrote:
>> When you post v2, please use scripts/get_maintainer.pl to identify all of the
>> maintainers and mailing lists that should be notified of this RFC.
>>
>> I suspect you won't get much feedback without doing so.
> 
> The sooner you post a v2 and include all of the right people and lists, the
> sooner the chances of getting good feedback. From internal WLAN team
> discussion the concept is good, but the implementation is not ideal. Hopefully
> pulling in the actual QRTR & MHI developers and maintainer will lead to a
> cleaner and acceptable solution.

Thank you, Jeff.

My initial idea was to not "spam" other lists with this RFC, because it depends 
on/is based on yet another RFC (c.f., Denis) which has been reviewed once, but 
has a few rough edges and isn't merge-ready yet. Since ath11k and ath12k are the 
only users of the new feature, I figured it would be more appropriate to post it 
here only.


Unfortunately, I have been swamped with work this week, so wasn't able to follow 
up on this.

I'll rebase against the newest tag, use get_maintainer.pl, checkpatch.pl, fix 
warnings and resubmit with a fixed cover letter some time this weekend.



Mihai

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