[PATCH v2 0/4] QRTR bus and Qualcomm Sensor Manager IIO drivers

[PATCH v2 0/4] QRTR bus and Qualcomm Sensor Manager IIO drivers

[Yassine Oudjana via B4 Relay]

Sensor Manager is a QMI service available on several Qualcomm SoCs which
exposes available sensors and allows for getting data from them. This
service is provided by either:

- SSC (Snapdragon Sensor Core): Also known as SLPI (Sensor Low Power
  Island). Has its own set of pins and peripherals to which sensors are
  connected. These peripherals are generally inaccessible from the AP,
  meaning sensors need to be operated exclusively through SSC. The only
  known SoCs in this category are MSM8996 and MSM8998 (and their
  derivatives).
- ADSP (Audio DSP): Shares pins and peripherals with the AP. At least on
  some devices, these pins could be configured as GPIOs which allows the AP
  to access sensors by bit-banging their interfaces. Some SoCs in this
  category are SDM630/660, MSM8953, MSM8974 and MSM8226.

Before Sensor Manager becomes accessible, another service known as Sensor
Registry needs to be provided by the AP. The remote processor that provides
Sensor Manager will then request data from it, and once that process is
done, will expose several services including Sensor Manager.

This series adds a kernel driver for the Sensor Manager service, exposing
sensors accessible through it as IIO devices. To facilitate probing of this
 driver, QRTR is turned into a bus, with services being exposed as devices.
Once the Sensor Manager service becomes available, the kernel attaches its
device to the driver added in this series. This allows for dynamic probing
of Sensor Manager without the need for static DT bindings, which would also
not be ideal because they would be describing software rather than
hardware. Sensor Manager is given as a working example of the QRTR bus.
Kernel drivers for other services may also be able to benefit from this
change.

As previously mentioned, a Sensor Registry server must run on the AP to
provide the remote processor (either SLPI or ADSP) with necessary data.
A userspace implementation of this server is made[1]. The server can be
supplied with the necessary data in the form of a plain-text configuration
file that can be pulled from the Android vendor partition (sample[2]), or
generated from a binary file that can be pulled from the persist partition.
A more recently developed kernel implementation of the Sensor Registry
server[3] can also be used. This last implementation only supports reading
data from the binary file pulled from persist. Sensor Registry remains out
of the scope of this patch series, as the Sensor Registry server and Sensor
Manager client (this series) are fully independent components.

Due to the total lack of documentation on Sensor Manager, this driver was
almost entirely the result of a process of capturing transactions between
SSC and the proprietary Android daemons with several methods and manually
decoding and interpreting them, sometimes by comparing with values acquired
from Android APIs. A blog post[4] describes part of this process more
detail. A little piece of downstream Android open-source code[5] was also
used as reference during later stages of development. All of this, as well
as a lack of time on my side for the last couple of years, meant that this
driver had to go through a slow and intermittent development process for
more than 3 years before reaching its current state.

Currently supported sensor types include accelerometers, gyroscopes,
magentometers, proximity and pressure sensors. Other types (namely
light and temperature sensors) are close to being implemented.

Some testing instructions may also be found here[6].

[1] https://gitlab.com/msm8996-mainline/sns-reg
[2] https://github.com/nian0114/android_vendor_xiaomi_scorpio/blob/mkn-mr1/proprietary/etc/sensors/sensor_def_qcomdev.conf
[3] https://github.com/sdm660-mainline/linux/pull/57
[4] https://emainline.gitlab.io/2022/04/08/Unlocking_SSC_P2.html
[5] https://android.googlesource.com/platform/system/chre/+/android-8.0.0_r2/platform/slpi
[6] https://gitlab.postmarketos.org/postmarketOS/pmaports/-/merge_requests/4118

Changes since v1:
- Split qdev renaming into separate patch
- Export new QRTR symbols with namespace
- Change struct initialization style
- Remove redundant NULL initialization of qdev->dev.driver
- Remove redundant devm_kfree
- Use variable in sizeof rather than type
- Change error return style in qcom_smd_qrtr_init
- Change order of operations in qcom_smd_qrtr_exit
- Use FIELD_PREP and GENMASK in QRTR_INSTANCE macro and add a CONST variant
- Remove per-sensor subdrivers and eliminate use of platform devices
- Put year range in copyright statements
- Use dev_err_probe for error messages in probe
- Remove unused include of linux/of.h
- Avoid casting away const in qcom_smgr_buffering_report_handler
- Use iio_push_to_buffers instead of iio_push_to_buffers_with_timestamp
- Preprocess proximity sensor data before pushing to buffer
- Add warning message for report with unknown ID received
- Change sentinel value style in array of struct initialization
- Refuse to set sampling frequency when buffer enabled
- Return -EINVAL inside default case in all applicable switch statements
- Move samp_freq_vals in qcom_smgr_iio_read_avail to priv and fix maximum
- Add devm_add_action_or_reset for releasing QMI handle and get rid of
  qcom_smgr_remove
- Add service versions and instance IDs found on some platforms to QRTR
  match table
- Fix null pointer dereference on registering unsupported sensor

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
Yassine Oudjana (4):
      net: qrtr: smd: Rename qdev to qsdev
      net: qrtr: Turn QRTR into a bus
      net: qrtr: Define macro to convert QMI version and instance to QRTR instance
      iio: Add Qualcomm Sensor Manager driver

 MAINTAINERS                                     |  13 +
 drivers/iio/accel/qcom_smgr_accel.c             | 138 ++++
 drivers/iio/common/Kconfig                      |   1 +
 drivers/iio/common/Makefile                     |   1 +
 drivers/iio/common/qcom_smgr/Kconfig            |  16 +
 drivers/iio/common/qcom_smgr/Makefile           |   8 +
 drivers/iio/common/qcom_smgr/qcom_smgr.c        | 840 ++++++++++++++++++++++++
 drivers/iio/common/qcom_smgr/qmi/Makefile       |   3 +
 drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.c | 713 ++++++++++++++++++++
 drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.h | 161 +++++
 drivers/soc/qcom/qmi_interface.c                |   5 +-
 include/linux/iio/common/qcom_smgr.h            |  80 +++
 include/linux/mod_devicetable.h                 |   9 +
 include/linux/soc/qcom/qrtr.h                   |  46 ++
 net/qrtr/af_qrtr.c                              |  23 +-
 net/qrtr/qrtr.h                                 |   3 +
 net/qrtr/smd.c                                  | 252 ++++++-
 scripts/mod/devicetable-offsets.c               |   4 +
 scripts/mod/file2alias.c                        |  10 +
 19 files changed, 2302 insertions(+), 24 deletions(-)
---
base-commit: 835244aba90de290b4b0b1fa92b6734f3ee7b3d9
change-id: 20250710-qcom-smgr-8db96d370b10

Best regards,
-- 
Yassine Oudjana <y.oudjana@protonmail.com>

[PATCH v2 1/4] net: qrtr: smd: Rename qdev to qsdev

[Yassine Oudjana via B4 Relay]

From: Yassine Oudjana <y.oudjana@protonmail.com>

In preparation to turn QRTR into a bus, rename replace all current
occurences of qdev in smd.c with qsdev in order to distinguish between the
to-be-added QRTR device which represents a QRTR service with the existing
QRTR SMD device which represents the endpoint through which services are
provided.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 net/qrtr/smd.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/net/qrtr/smd.c b/net/qrtr/smd.c
index c91bf030fbc7d973a90a1f88e90e66df42ab2bdd..940ee9349a9ce7438a01dd193c5c1d61c7b82ffd 100644
--- a/net/qrtr/smd.c
+++ b/net/qrtr/smd.c
@@ -20,15 +20,15 @@ struct qrtr_smd_dev {
 static int qcom_smd_qrtr_callback(struct rpmsg_device *rpdev,
 				  void *data, int len, void *priv, u32 addr)
 {
-	struct qrtr_smd_dev *qdev = dev_get_drvdata(&rpdev->dev);
+	struct qrtr_smd_dev *qsdev = dev_get_drvdata(&rpdev->dev);
 	int rc;
 
-	if (!qdev)
+	if (!qsdev)
 		return -EAGAIN;
 
-	rc = qrtr_endpoint_post(&qdev->ep, data, len);
+	rc = qrtr_endpoint_post(&qsdev->ep, data, len);
 	if (rc == -EINVAL) {
-		dev_err(qdev->dev, "invalid ipcrouter packet\n");
+		dev_err(qsdev->dev, "invalid ipcrouter packet\n");
 		/* return 0 to let smd drop the packet */
 		rc = 0;
 	}
@@ -39,14 +39,14 @@ static int qcom_smd_qrtr_callback(struct rpmsg_device *rpdev,
 /* from qrtr to smd */
 static int qcom_smd_qrtr_send(struct qrtr_endpoint *ep, struct sk_buff *skb)
 {
-	struct qrtr_smd_dev *qdev = container_of(ep, struct qrtr_smd_dev, ep);
+	struct qrtr_smd_dev *qsdev = container_of(ep, struct qrtr_smd_dev, ep);
 	int rc;
 
 	rc = skb_linearize(skb);
 	if (rc)
 		goto out;
 
-	rc = rpmsg_send(qdev->channel, skb->data, skb->len);
+	rc = rpmsg_send(qsdev->channel, skb->data, skb->len);
 
 out:
 	if (rc)
@@ -58,22 +58,24 @@ static int qcom_smd_qrtr_send(struct qrtr_endpoint *ep, struct sk_buff *skb)
 
 static int qcom_smd_qrtr_probe(struct rpmsg_device *rpdev)
 {
-	struct qrtr_smd_dev *qdev;
+	struct qrtr_smd_dev *qsdev;
 	int rc;
 
-	qdev = devm_kzalloc(&rpdev->dev, sizeof(*qdev), GFP_KERNEL);
-	if (!qdev)
+	qsdev = devm_kzalloc(&rpdev->dev, sizeof(*qsdev), GFP_KERNEL);
+	if (!qsdev)
 		return -ENOMEM;
 
-	qdev->channel = rpdev->ept;
-	qdev->dev = &rpdev->dev;
-	qdev->ep.xmit = qcom_smd_qrtr_send;
+	qsdev->channel = rpdev->ept;
+	qsdev->dev = &rpdev->dev;
+	qsdev->ep.xmit = qcom_smd_qrtr_send;
+	qsdev->ep.add_device = qcom_smd_qrtr_add_device;
+	qsdev->ep.del_device = qcom_smd_qrtr_del_device;
 
-	rc = qrtr_endpoint_register(&qdev->ep, QRTR_EP_NID_AUTO);
+	rc = qrtr_endpoint_register(&qsdev->ep, QRTR_EP_NID_AUTO);
 	if (rc)
 		return rc;
 
-	dev_set_drvdata(&rpdev->dev, qdev);
+	dev_set_drvdata(&rpdev->dev, qsdev);
 
 	dev_dbg(&rpdev->dev, "Qualcomm SMD QRTR driver probed\n");
 
@@ -82,9 +84,9 @@ static int qcom_smd_qrtr_probe(struct rpmsg_device *rpdev)
 
 static void qcom_smd_qrtr_remove(struct rpmsg_device *rpdev)
 {
-	struct qrtr_smd_dev *qdev = dev_get_drvdata(&rpdev->dev);
+	struct qrtr_smd_dev *qsdev = dev_get_drvdata(&rpdev->dev);
 
-	qrtr_endpoint_unregister(&qdev->ep);
+	qrtr_endpoint_unregister(&qsdev->ep);
 
 	dev_set_drvdata(&rpdev->dev, NULL);
 }

-- 
2.50.0

[PATCH v2 2/4] net: qrtr: Turn QRTR into a bus

[Yassine Oudjana via B4 Relay]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Implement a QRTR bus to allow for creating drivers for individual QRTR
services. With this in place, devices are dynamically registered for QRTR
services as they become available, and drivers for these devices are
matched using service and instance IDs.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 include/linux/mod_devicetable.h   |   9 ++
 include/linux/soc/qcom/qrtr.h     |  36 +++++++
 net/qrtr/af_qrtr.c                |  23 +++-
 net/qrtr/qrtr.h                   |   3 +
 net/qrtr/smd.c                    | 218 +++++++++++++++++++++++++++++++++++++-
 scripts/mod/devicetable-offsets.c |   4 +
 scripts/mod/file2alias.c          |  10 ++
 7 files changed, 297 insertions(+), 6 deletions(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 6077972e8b45de3d07881c0226459d815dd1f83d..23c6a1a4bca54f871c78765f8d5401cca5c1e6eb 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -549,6 +549,15 @@ struct spmi_device_id {
 	kernel_ulong_t driver_data;	/* Data private to the driver */
 };
 
+#define QRTR_NAME_SIZE	32
+#define QRTR_MODULE_PREFIX "qrtr:"
+
+struct qrtr_device_id {
+	__u16 service;
+	__u16 instance;
+	kernel_ulong_t driver_data;	/* Data private to the driver */
+};
+
 /* dmi */
 enum dmi_field {
 	DMI_NONE,
diff --git a/include/linux/soc/qcom/qrtr.h b/include/linux/soc/qcom/qrtr.h
new file mode 100644
index 0000000000000000000000000000000000000000..e9249d9422b8ca96baa43073cf07c4a75c163219
--- /dev/null
+++ b/include/linux/soc/qcom/qrtr.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __QCOM_QRTR_H__
+#define __QCOM_QRTR_H__
+
+#include <linux/mod_devicetable.h>
+
+struct qrtr_device {
+	struct device dev;
+	unsigned int node;
+	unsigned int port;
+	u16 service;
+	u16 instance;
+};
+
+#define to_qrtr_device(d) container_of(d, struct qrtr_device, dev)
+
+struct qrtr_driver {
+	int (*probe)(struct qrtr_device *qdev);
+	void (*remove)(struct qrtr_device *qdev);
+	const struct qrtr_device_id *id_table;
+	struct device_driver driver;
+};
+
+#define to_qrtr_driver(d) container_of(d, struct qrtr_driver, driver)
+
+#define qrtr_driver_register(drv) __qrtr_driver_register(drv, THIS_MODULE)
+
+int __qrtr_driver_register(struct qrtr_driver *drv, struct module *owner);
+void qrtr_driver_unregister(struct qrtr_driver *drv);
+
+#define module_qrtr_driver(__qrtr_driver) \
+	module_driver(__qrtr_driver, qrtr_driver_register, \
+			qrtr_driver_unregister)
+
+#endif /* __QCOM_QRTR_H__ */
diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
index 00c51cf693f3d054f1771de5246498bf013775d0..e11682fd796083a9866527824faf428affba19bc 100644
--- a/net/qrtr/af_qrtr.c
+++ b/net/qrtr/af_qrtr.c
@@ -435,6 +435,7 @@ static void qrtr_node_assign(struct qrtr_node *node, unsigned int nid)
 int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
 {
 	struct qrtr_node *node = ep->node;
+	const struct qrtr_ctrl_pkt *pkt;
 	const struct qrtr_hdr_v1 *v1;
 	const struct qrtr_hdr_v2 *v2;
 	struct qrtr_sock *ipc;
@@ -443,6 +444,7 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
 	size_t size;
 	unsigned int ver;
 	size_t hdrlen;
+	int ret = 0;
 
 	if (len == 0 || len & 3)
 		return -EINVAL;
@@ -516,12 +518,24 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
 
 	qrtr_node_assign(node, cb->src_node);
 
+	pkt = data + hdrlen;
+
 	if (cb->type == QRTR_TYPE_NEW_SERVER) {
 		/* Remote node endpoint can bridge other distant nodes */
-		const struct qrtr_ctrl_pkt *pkt;
-
-		pkt = data + hdrlen;
 		qrtr_node_assign(node, le32_to_cpu(pkt->server.node));
+
+		/* Create a QRTR device */
+		ret = ep->add_device(ep, le32_to_cpu(pkt->server.node),
+					       le32_to_cpu(pkt->server.port),
+					       le32_to_cpu(pkt->server.service),
+					       le32_to_cpu(pkt->server.instance));
+		if (ret)
+			goto err;
+	} else if (cb->type == QRTR_TYPE_DEL_SERVER) {
+		/* Remove QRTR device corresponding to service */
+		ret = ep->del_device(ep, le32_to_cpu(pkt->server.port));
+		if (ret)
+			goto err;
 	}
 
 	if (cb->type == QRTR_TYPE_RESUME_TX) {
@@ -543,8 +557,7 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
 
 err:
 	kfree_skb(skb);
-	return -EINVAL;
-
+	return ret ? ret : -EINVAL;
 }
 EXPORT_SYMBOL_GPL(qrtr_endpoint_post);
 
diff --git a/net/qrtr/qrtr.h b/net/qrtr/qrtr.h
index 3f2d28696062a56201f8774ee50fd1c3daa50708..6590483367304b97239de939d4f0206aac52c527 100644
--- a/net/qrtr/qrtr.h
+++ b/net/qrtr/qrtr.h
@@ -19,6 +19,9 @@ struct sk_buff;
  */
 struct qrtr_endpoint {
 	int (*xmit)(struct qrtr_endpoint *ep, struct sk_buff *skb);
+	int (*add_device)(struct qrtr_endpoint *parent, unsigned int node, unsigned int port,
+			  u16 service, u16 instance);
+	int (*del_device)(struct qrtr_endpoint *parent, unsigned int port);
 	/* private: not for endpoint use */
 	struct qrtr_node *node;
 };
diff --git a/net/qrtr/smd.c b/net/qrtr/smd.c
index 940ee9349a9ce7438a01dd193c5c1d61c7b82ffd..40fd32fa0890799a88b947b2b7bc00c2249dea0d 100644
--- a/net/qrtr/smd.c
+++ b/net/qrtr/smd.c
@@ -7,6 +7,7 @@
 #include <linux/module.h>
 #include <linux/skbuff.h>
 #include <linux/rpmsg.h>
+#include <linux/soc/qcom/qrtr.h>
 
 #include "qrtr.h"
 
@@ -16,6 +17,197 @@ struct qrtr_smd_dev {
 	struct device *dev;
 };
 
+struct qrtr_new_server {
+	struct qrtr_smd_dev *parent;
+	unsigned int node;
+	unsigned int port;
+	u16 service;
+	u16 instance;
+
+	struct work_struct work;
+};
+
+struct qrtr_del_server {
+	struct qrtr_smd_dev *parent;
+	unsigned int port;
+
+	struct work_struct work;
+};
+
+static int qcom_smd_qrtr_device_match(struct device *dev, const struct device_driver *drv)
+{
+	struct qrtr_device *qdev = to_qrtr_device(dev);
+	struct qrtr_driver *qdrv = to_qrtr_driver(drv);
+	const struct qrtr_device_id *id = qdrv->id_table;
+
+	if (!id)
+		return 0;
+
+	while (id->service != 0) {
+		if (id->service == qdev->service && id->instance == qdev->instance)
+			return 1;
+		id++;
+	}
+
+	return 0;
+}
+
+static int qcom_smd_qrtr_uevent(const struct device *dev, struct kobj_uevent_env *env)
+{
+	const struct qrtr_device *qdev = to_qrtr_device(dev);
+
+	return add_uevent_var(env, "MODALIAS=%s%x:%x", QRTR_MODULE_PREFIX, qdev->service,
+			      qdev->instance);
+}
+
+static int qcom_smd_qrtr_device_probe(struct device *dev)
+{
+	struct qrtr_device *qdev = to_qrtr_device(dev);
+	struct qrtr_driver *qdrv = to_qrtr_driver(dev->driver);
+
+	return qdrv->probe(qdev);
+}
+
+static void qcom_smd_qrtr_device_remove(struct device *dev)
+{
+	struct qrtr_device *qdev = to_qrtr_device(dev);
+	struct qrtr_driver *qdrv = to_qrtr_driver(dev->driver);
+
+	if (qdrv->remove)
+		qdrv->remove(qdev);
+}
+
+const struct bus_type qrtr_bus = {
+	.name		= "qrtr",
+	.match		= qcom_smd_qrtr_device_match,
+	.uevent		= qcom_smd_qrtr_uevent,
+	.probe		= qcom_smd_qrtr_device_probe,
+	.remove		= qcom_smd_qrtr_device_remove,
+};
+EXPORT_SYMBOL_NS_GPL(qrtr_bus, "QRTR");
+
+int __qrtr_driver_register(struct qrtr_driver *drv, struct module *owner)
+{
+	drv->driver.bus = &qrtr_bus;
+	drv->driver.owner = owner;
+
+	return driver_register(&drv->driver);
+}
+EXPORT_SYMBOL_NS_GPL(__qrtr_driver_register, "QRTR");
+
+void qrtr_driver_unregister(struct qrtr_driver *drv)
+{
+	driver_unregister(&drv->driver);
+}
+EXPORT_SYMBOL_NS_GPL(qrtr_driver_unregister, "QRTR");
+
+static void qcom_smd_qrtr_dev_release(struct device *dev)
+{
+	struct qrtr_device *qdev = to_qrtr_device(dev);
+
+	kfree(qdev);
+}
+
+static int qcom_smd_qrtr_match_device_by_port(struct device *dev, const void *data)
+{
+	struct qrtr_device *qdev = to_qrtr_device(dev);
+	unsigned const int *port = data;
+
+	return qdev->port == *port;
+}
+
+static void qcom_smd_qrtr_add_device_worker(struct work_struct *work)
+{
+	struct qrtr_new_server *new_server = container_of(work, struct qrtr_new_server, work);
+	struct qrtr_smd_dev *qsdev = new_server->parent;
+	struct qrtr_device *qdev;
+	int ret;
+
+	qdev = kzalloc(sizeof(*qdev), GFP_KERNEL);
+	if (!qdev)
+		return;
+
+	*qdev = (struct qrtr_device) {
+		.node = new_server->node,
+		.port = new_server->port,
+		.service = new_server->service,
+		.instance = new_server->instance
+	};
+
+	devm_kfree(qsdev->dev, new_server);
+
+	dev_set_name(&qdev->dev, "%d-%d", qdev->node, qdev->port);
+
+	qdev->dev.bus = &qrtr_bus;
+	qdev->dev.parent = qsdev->dev;
+	qdev->dev.release = qcom_smd_qrtr_dev_release;
+
+	ret = device_register(&qdev->dev);
+	if (ret) {
+		dev_err(qsdev->dev, "Failed to register QRTR device: %pe\n", ERR_PTR(ret));
+		put_device(&qdev->dev);
+	}
+}
+
+static void qcom_smd_qrtr_del_device_worker(struct work_struct *work)
+{
+	struct qrtr_del_server *del_server = container_of(work, struct qrtr_del_server, work);
+	struct qrtr_smd_dev *qsdev = del_server->parent;
+	struct device *dev = device_find_child(qsdev->dev, &del_server->port,
+					       qcom_smd_qrtr_match_device_by_port);
+
+	device_unregister(dev);
+}
+
+static int qcom_smd_qrtr_add_device(struct qrtr_endpoint *parent, unsigned int node,
+				    unsigned int port, u16 service, u16 instance)
+{
+	struct qrtr_smd_dev *qsdev = container_of(parent, struct qrtr_smd_dev, ep);
+	struct qrtr_new_server *new_server;
+
+	new_server = devm_kzalloc(qsdev->dev, sizeof(*new_server), GFP_KERNEL);
+	if (!new_server)
+		return -ENOMEM;
+
+	*new_server = (struct qrtr_new_server) {
+		.parent = qsdev,
+		.node = node,
+		.port = port,
+		.service = service,
+		.instance = instance
+	};
+
+	INIT_WORK(&new_server->work, qcom_smd_qrtr_add_device_worker);
+	schedule_work(&new_server->work);
+
+	return 0;
+}
+
+static int qcom_smd_qrtr_del_device(struct qrtr_endpoint *parent, unsigned int port)
+{
+	struct qrtr_smd_dev *qsdev = container_of(parent, struct qrtr_smd_dev, ep);
+	struct qrtr_del_server *del_server;
+
+	del_server = devm_kzalloc(qsdev->dev, sizeof(*del_server), GFP_KERNEL);
+	if (!del_server)
+		return -ENOMEM;
+
+	del_server->parent = qsdev;
+	del_server->port = port;
+
+	INIT_WORK(&del_server->work, qcom_smd_qrtr_del_device_worker);
+	schedule_work(&del_server->work);
+
+	return 0;
+}
+
+static int qcom_smd_qrtr_device_unregister(struct device *dev, void *data)
+{
+	device_unregister(dev);
+
+	return 0;
+}
+
 /* from smd to qrtr */
 static int qcom_smd_qrtr_callback(struct rpmsg_device *rpdev,
 				  void *data, int len, void *priv, u32 addr)
@@ -86,6 +278,8 @@ static void qcom_smd_qrtr_remove(struct rpmsg_device *rpdev)
 {
 	struct qrtr_smd_dev *qsdev = dev_get_drvdata(&rpdev->dev);
 
+	device_for_each_child(qsdev->dev, NULL, qcom_smd_qrtr_device_unregister);
+
 	qrtr_endpoint_unregister(&qsdev->ep);
 
 	dev_set_drvdata(&rpdev->dev, NULL);
@@ -106,7 +300,29 @@ static struct rpmsg_driver qcom_smd_qrtr_driver = {
 	},
 };
 
-module_rpmsg_driver(qcom_smd_qrtr_driver);
+static int __init qcom_smd_qrtr_init(void)
+{
+	int ret;
+
+	ret = bus_register(&qrtr_bus);
+	if (ret)
+		return ret;
+
+	ret = register_rpmsg_driver(&qcom_smd_qrtr_driver);
+	if (ret)
+		bus_unregister(&qrtr_bus);
+
+	return ret;
+}
+
+static void __exit qcom_smd_qrtr_exit(void)
+{
+	unregister_rpmsg_driver(&qcom_smd_qrtr_driver);
+	bus_unregister(&qrtr_bus);
+}
+
+subsys_initcall(qcom_smd_qrtr_init);
+module_exit(qcom_smd_qrtr_exit);
 
 MODULE_ALIAS("rpmsg:IPCRTR");
 MODULE_DESCRIPTION("Qualcomm IPC-Router SMD interface driver");
diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index d3d00e85edf73553ba3d9b5f9fccf1ff61c99026..0a90323c35d330f13a151948467d72b927f474f3 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -280,5 +280,9 @@ int main(void)
 	DEVID(coreboot_device_id);
 	DEVID_FIELD(coreboot_device_id, tag);
 
+	DEVID(qrtr_device_id);
+	DEVID_FIELD(qrtr_device_id, service);
+	DEVID_FIELD(qrtr_device_id, instance);
+
 	return 0;
 }
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 00586119a25b7fd399eeeef3760a26467ffbb50c..fef69db4d7023305f03fd8bf85ac60c47ae7d0ca 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1370,6 +1370,15 @@ static void do_coreboot_entry(struct module *mod, void *symval)
 	module_alias_printf(mod, false, "coreboot:t%08X", tag);
 }
 
+/* Looks like: qrtr:N:N */
+static void do_qrtr_entry(struct module *mod, void *symval)
+{
+	DEF_FIELD(symval, qrtr_device_id, service);
+	DEF_FIELD(symval, qrtr_device_id, instance);
+
+	module_alias_printf(mod, false, "qrtr:%x:%x", service, instance);
+}
+
 /* Does namelen bytes of name exactly match the symbol? */
 static bool sym_is(const char *name, unsigned namelen, const char *symbol)
 {
@@ -1466,6 +1475,7 @@ static const struct devtable devtable[] = {
 	{"usb", SIZE_usb_device_id, do_usb_entry_multi},
 	{"pnp", SIZE_pnp_device_id, do_pnp_device_entry},
 	{"pnp_card", SIZE_pnp_card_device_id, do_pnp_card_entry},
+	{"qrtr", SIZE_qrtr_device_id, do_qrtr_entry},
 };
 
 /* Create MODULE_ALIAS() statements.

-- 
2.50.0

[PATCH v2 3/4] net: qrtr: Define macro to convert QMI version and instance to QRTR instance

[Yassine Oudjana via B4 Relay]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Move QRTR instance conversion from qmi_interface into a new macro in order
to reuse it in QRTR device ID tables.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 drivers/soc/qcom/qmi_interface.c |  5 +++--
 include/linux/soc/qcom/qrtr.h    | 10 ++++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/qmi_interface.c b/drivers/soc/qcom/qmi_interface.c
index 6500f863aae5ca218892d13233d4c9bf4b63a0f4..bd97b006757fc20778e4bb512945d85f4cd31b24 100644
--- a/drivers/soc/qcom/qmi_interface.c
+++ b/drivers/soc/qcom/qmi_interface.c
@@ -14,6 +14,7 @@
 #include <linux/workqueue.h>
 #include <trace/events/sock.h>
 #include <linux/soc/qcom/qmi.h>
+#include <linux/soc/qcom/qrtr.h>
 
 static struct socket *qmi_sock_create(struct qmi_handle *qmi,
 				      struct sockaddr_qrtr *sq);
@@ -173,7 +174,7 @@ static void qmi_send_new_lookup(struct qmi_handle *qmi, struct qmi_service *svc)
 	memset(&pkt, 0, sizeof(pkt));
 	pkt.cmd = cpu_to_le32(QRTR_TYPE_NEW_LOOKUP);
 	pkt.server.service = cpu_to_le32(svc->service);
-	pkt.server.instance = cpu_to_le32(svc->version | svc->instance << 8);
+	pkt.server.instance = cpu_to_le32(QRTR_INSTANCE(svc->version, svc->instance));
 
 	sq.sq_family = qmi->sq.sq_family;
 	sq.sq_node = qmi->sq.sq_node;
@@ -236,7 +237,7 @@ static void qmi_send_new_server(struct qmi_handle *qmi, struct qmi_service *svc)
 	memset(&pkt, 0, sizeof(pkt));
 	pkt.cmd = cpu_to_le32(QRTR_TYPE_NEW_SERVER);
 	pkt.server.service = cpu_to_le32(svc->service);
-	pkt.server.instance = cpu_to_le32(svc->version | svc->instance << 8);
+	pkt.server.instance = cpu_to_le32(QRTR_INSTANCE(svc->version, svc->instance));
 	pkt.server.node = cpu_to_le32(qmi->sq.sq_node);
 	pkt.server.port = cpu_to_le32(qmi->sq.sq_port);
 
diff --git a/include/linux/soc/qcom/qrtr.h b/include/linux/soc/qcom/qrtr.h
index e9249d9422b8ca96baa43073cf07c4a75c163219..e2aca520fdbe22bc855004143dc8baa7a3f67517 100644
--- a/include/linux/soc/qcom/qrtr.h
+++ b/include/linux/soc/qcom/qrtr.h
@@ -3,6 +3,8 @@
 #ifndef __QCOM_QRTR_H__
 #define __QCOM_QRTR_H__
 
+#include <linux/bitfield.h>
+#include <linux/bits.h>
 #include <linux/mod_devicetable.h>
 
 struct qrtr_device {
@@ -15,6 +17,14 @@ struct qrtr_device {
 
 #define to_qrtr_device(d) container_of(d, struct qrtr_device, dev)
 
+#define QRTR_INSTANCE(qmi_version, qmi_instance) \
+	(FIELD_PREP(GENMASK(7, 0), qmi_version) | \
+	 FIELD_PREP(GENMASK(15, 8), qmi_instance))
+
+#define QRTR_INSTANCE_CONST(qmi_version, qmi_instance) \
+	(FIELD_PREP_CONST(GENMASK(7, 0), qmi_version) | \
+	 FIELD_PREP_CONST(GENMASK(15, 8), qmi_instance))
+
 struct qrtr_driver {
 	int (*probe)(struct qrtr_device *qdev);
 	void (*remove)(struct qrtr_device *qdev);

-- 
2.50.0

[PATCH v2 4/4] iio: Add Qualcomm Sensor Manager driver

[Yassine Oudjana via B4 Relay]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Add a driver for sensors exposed by the Qualcomm Sensor Manager service,
which is provided by SLPI or ADSP on Qualcomm SoCs. Supported sensors
include accelerometers, gyroscopes, pressure sensors, proximity sensors
and magnetometers.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 MAINTAINERS                                     |  13 +
 drivers/iio/accel/qcom_smgr_accel.c             | 138 ++++
 drivers/iio/common/Kconfig                      |   1 +
 drivers/iio/common/Makefile                     |   1 +
 drivers/iio/common/qcom_smgr/Kconfig            |  16 +
 drivers/iio/common/qcom_smgr/Makefile           |   8 +
 drivers/iio/common/qcom_smgr/qcom_smgr.c        | 840 ++++++++++++++++++++++++
 drivers/iio/common/qcom_smgr/qmi/Makefile       |   3 +
 drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.c | 713 ++++++++++++++++++++
 drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.h | 161 +++++
 include/linux/iio/common/qcom_smgr.h            |  80 +++
 11 files changed, 1974 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b5a472a544cfe2ad87691209c34d7bafe058ba42..0fb91c9bce431fc899776ff10b728ecdc957f51a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20702,6 +20702,19 @@ F:	Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
 F:	drivers/net/ethernet/qualcomm/rmnet/
 F:	include/linux/if_rmnet.h
 
+QUALCOMM SENSOR MANAGER IIO DRIVER
+M:	Yassine Oudjana <y.oudjana@protonmail.com>
+L:	linux-iio@vger.kernel.org
+L:	linux-arm-msm@vger.kernel.org
+S:	Maintained
+F:	drivers/iio/common/qcom_smgr/Kconfig
+F:	drivers/iio/common/qcom_smgr/Makefile
+F:	drivers/iio/common/qcom_smgr/qcom_smgr.c
+F:	drivers/iio/common/qcom_smgr/qmi/Makefile
+F:	drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.c
+F:	drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.h
+F:	include/linux/iio/common/qcom_smgr.h
+
 QUALCOMM TRUST ZONE MEMORY ALLOCATOR
 M:	Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
 L:	linux-arm-msm@vger.kernel.org
diff --git a/drivers/iio/accel/qcom_smgr_accel.c b/drivers/iio/accel/qcom_smgr_accel.c
new file mode 100644
index 0000000000000000000000000000000000000000..ce854312d1d9386300785f1965d5886c16995806
--- /dev/null
+++ b/drivers/iio/accel/qcom_smgr_accel.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Qualcomm Sensor Manager accelerometer driver
+ *
+ * Copyright (c) 2022, Yassine Oudjana <y.oudjana@protonmail.com>
+ */
+
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/common/qcom_smgr.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/kfifo_buf.h>
+
+static const struct iio_chan_spec qcom_smgr_accel_iio_channels[] = {
+	{
+		.type = IIO_ACCEL,
+		.modified = true,
+		.channel2 = IIO_MOD_X,
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = qcom_smgr_iio_ext_info
+	},
+	{
+		.type = IIO_ACCEL,
+		.modified = true,
+		.channel2 = IIO_MOD_Y,
+		.scan_index = 1,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = qcom_smgr_iio_ext_info
+	},
+	{
+		.type = IIO_ACCEL,
+		.modified = true,
+		.channel2 = IIO_MOD_Z,
+		.scan_index = 2,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = qcom_smgr_iio_ext_info
+	},
+	{
+		.type = IIO_TIMESTAMP,
+		.channel = -1,
+		.scan_index = 3,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 64,
+			.endianness = IIO_LE,
+		},
+	},
+};
+
+static int qcom_smgr_accel_probe(struct platform_device *pdev)
+{
+	struct iio_dev *iio_dev;
+	struct qcom_smgr_iio_priv *priv;
+	int ret;
+
+	iio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
+	if (!iio_dev)
+		return -ENOMEM;
+
+	priv = iio_priv(iio_dev);
+	priv->sensor = *(struct qcom_smgr_sensor **)pdev->dev.platform_data;
+	priv->sensor->iio_dev = iio_dev;
+
+	iio_dev->name = "qcom-smgr-accel";
+	iio_dev->info = &qcom_smgr_iio_info;
+	iio_dev->channels = qcom_smgr_accel_iio_channels;
+	iio_dev->num_channels = ARRAY_SIZE(qcom_smgr_accel_iio_channels);
+
+	ret = devm_iio_kfifo_buffer_setup(&pdev->dev, iio_dev,
+					  &qcom_smgr_buffer_ops);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to setup buffer: %pe\n",
+			ERR_PTR(ret));
+		return ret;
+	}
+
+	ret = devm_iio_device_register(&pdev->dev, iio_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register IIO device: %pe\n",
+			ERR_PTR(ret));
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, priv->sensor);
+
+	return 0;
+}
+
+static void qcom_smgr_accel_remove(struct platform_device *pdev)
+{
+	struct qcom_smgr_sensor *sensor = platform_get_drvdata(pdev);
+
+	sensor->iio_dev = NULL;
+}
+
+static const struct platform_device_id qcom_smgr_accel_ids[] = {
+	{ .name = "qcom-smgr-accel" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, qcom_smgr_accel_ids);
+
+static struct platform_driver qcom_smgr_accel_driver = {
+	.probe = qcom_smgr_accel_probe,
+	.remove = qcom_smgr_accel_remove,
+	.driver	= {
+		.name = "qcom_smgr_accel",
+	},
+	.id_table = qcom_smgr_accel_ids,
+};
+module_platform_driver(qcom_smgr_accel_driver);
+
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_DESCRIPTION("Qualcomm Sensor Manager accelerometer driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
index 1ccb5ccf3706609fc57a4ade7c6a4354fa41c6bd..0ad8b39720874ca64a5560384e8cb6ae78fac1cf 100644
--- a/drivers/iio/common/Kconfig
+++ b/drivers/iio/common/Kconfig
@@ -8,5 +8,6 @@ source "drivers/iio/common/hid-sensors/Kconfig"
 source "drivers/iio/common/inv_sensors/Kconfig"
 source "drivers/iio/common/ms_sensors/Kconfig"
 source "drivers/iio/common/scmi_sensors/Kconfig"
+source "drivers/iio/common/qcom_smgr/Kconfig"
 source "drivers/iio/common/ssp_sensors/Kconfig"
 source "drivers/iio/common/st_sensors/Kconfig"
diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
index d3e952239a62191decd0bf3e84c7b59cc46bcfb3..f3f18484c91b9215f9302985fcc7e0aa401a7a4e 100644
--- a/drivers/iio/common/Makefile
+++ b/drivers/iio/common/Makefile
@@ -13,5 +13,6 @@ obj-y += hid-sensors/
 obj-y += inv_sensors/
 obj-y += ms_sensors/
 obj-y += scmi_sensors/
+obj-y += qcom_smgr/
 obj-y += ssp_sensors/
 obj-y += st_sensors/
diff --git a/drivers/iio/common/qcom_smgr/Kconfig b/drivers/iio/common/qcom_smgr/Kconfig
new file mode 100644
index 0000000000000000000000000000000000000000..30baee9b0eff2bf90a6047f14c4dfb9d70d28792
--- /dev/null
+++ b/drivers/iio/common/qcom_smgr/Kconfig
@@ -0,0 +1,16 @@
+#
+# Qualcomm Sensor Manager IIO
+#
+
+config IIO_QCOM_SMGR
+	tristate "Qualcomm SSC Sensor Manager"
+	depends on ARCH_QCOM
+	depends on QRTR_SMD
+	select QCOM_QMI_HELPERS
+	select IIO_BUFFER
+	help
+	  Say yes here to build support for the Sensor Manager (SMGR)
+	  service provided by the Qualcomm Snapdragon Sensor Core (SSC).
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called smgr.
diff --git a/drivers/iio/common/qcom_smgr/Makefile b/drivers/iio/common/qcom_smgr/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..84554cedd2e5f2f13c04df884e1b51d3b9d8a9cf
--- /dev/null
+++ b/drivers/iio/common/qcom_smgr/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for Qualcomm Sensor Manager driver
+#
+
+obj-y += qmi/
+
+obj-$(CONFIG_IIO_QCOM_SMGR)	+= qcom_smgr.o
diff --git a/drivers/iio/common/qcom_smgr/qcom_smgr.c b/drivers/iio/common/qcom_smgr/qcom_smgr.c
new file mode 100644
index 0000000000000000000000000000000000000000..79d1160f7a5c32f1a9e0a20f29e304e5cb18be8f
--- /dev/null
+++ b/drivers/iio/common/qcom_smgr/qcom_smgr.c
@@ -0,0 +1,840 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Qualcomm Sensor Manager driver
+ *
+ * Copyright (c) 2021-2025, Yassine Oudjana <y.oudjana@protonmail.com>
+ */
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/common/qcom_smgr.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/kfifo_buf.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc/qcom_rproc.h>
+#include <linux/soc/qcom/qmi.h>
+#include <linux/soc/qcom/qrtr.h>
+#include <linux/types.h>
+#include <net/sock.h>
+
+#include "qmi/qmi_sns_smgr.h"
+
+MODULE_IMPORT_NS("QRTR");
+
+#define SMGR_TICKS_PER_SECOND		32768
+#define SMGR_REPORT_RATE_HZ		(SMGR_TICKS_PER_SECOND * 2)
+#define SMGR_VALUE_DECIMAL_OFFSET	16
+
+static int qcom_smgr_request_all_sensor_info(struct qcom_smgr *smgr,
+					     struct qcom_smgr_sensor **sensors)
+{
+	struct sns_smgr_all_sensor_info_resp resp = {};
+	struct qmi_txn txn;
+	u8 i;
+	int ret;
+
+	dev_dbg(smgr->dev, "Getting available sensors\n");
+
+	ret = qmi_txn_init(&smgr->sns_smgr_hdl, &txn,
+			   sns_smgr_all_sensor_info_resp_ei, &resp);
+	if (ret < 0) {
+		dev_err(smgr->dev, "Failed to initialize QMI TXN: %d\n", ret);
+		return ret;
+	}
+
+	ret = qmi_send_request(&smgr->sns_smgr_hdl, &smgr->sns_smgr_info, &txn,
+			       SNS_SMGR_ALL_SENSOR_INFO_MSG_ID,
+			       SNS_SMGR_ALL_SENSOR_INFO_REQ_MAX_LEN, NULL,
+			       NULL);
+	if (ret) {
+		dev_err(smgr->dev,
+			"Failed to send available sensors request: %d\n", ret);
+		qmi_txn_cancel(&txn);
+		return ret;
+	}
+
+	ret = qmi_txn_wait(&txn, 5 * HZ);
+	if (ret < 0)
+		return ret;
+
+	/* Check the response */
+	if (resp.result) {
+		dev_err(smgr->dev, "Available sensors request failed: 0x%x\n",
+			resp.result);
+		return -EREMOTEIO;
+	}
+
+	*sensors = devm_kzalloc(smgr->dev,
+				sizeof(struct qcom_smgr_sensor) * resp.item_len,
+				GFP_KERNEL);
+
+	for (i = 0; i < resp.item_len; ++i) {
+		(*sensors)[i].id = resp.items[i].id;
+		(*sensors)[i].type =
+			sns_smgr_sensor_type_from_str(resp.items[i].type);
+	}
+
+	return resp.item_len;
+}
+
+static int qcom_smgr_request_single_sensor_info(struct qcom_smgr *smgr,
+						struct qcom_smgr_sensor *sensor)
+{
+	struct sns_smgr_single_sensor_info_req req = {
+		.sensor_id = sensor->id,
+	};
+	struct sns_smgr_single_sensor_info_resp resp = {};
+	struct qmi_txn txn;
+	u8 i;
+	int ret;
+
+	dev_vdbg(smgr->dev, "Getting single sensor info for ID 0x%02x\n",
+		 sensor->id);
+
+	ret = qmi_txn_init(&smgr->sns_smgr_hdl, &txn,
+			   sns_smgr_single_sensor_info_resp_ei, &resp);
+	if (ret < 0) {
+		dev_err(smgr->dev, "Failed to initialize QMI transaction: %d\n",
+			ret);
+		return ret;
+	}
+
+	ret = qmi_send_request(&smgr->sns_smgr_hdl, &smgr->sns_smgr_info, &txn,
+			       SNS_SMGR_SINGLE_SENSOR_INFO_MSG_ID,
+			       SNS_SMGR_SINGLE_SENSOR_INFO_REQ_MAX_LEN,
+			       sns_smgr_single_sensor_info_req_ei, &req);
+	if (ret < 0) {
+		dev_err(smgr->dev, "Failed to send sensor data request: %d\n",
+			ret);
+		qmi_txn_cancel(&txn);
+		return ret;
+	}
+
+	ret = qmi_txn_wait(&txn, 5 * HZ);
+	if (ret < 0)
+		return ret;
+
+	/* Check the response */
+	if (resp.result) {
+		dev_err(smgr->dev, "Single sensor info request failed: 0x%x\n",
+			resp.result);
+		return -EREMOTEIO;
+	}
+
+	sensor->data_type_count = resp.data_type_len;
+	sensor->data_types =
+		devm_kzalloc(smgr->dev,
+			     sizeof(struct qcom_smgr_data_type_item) *
+				     sensor->data_type_count,
+			     GFP_KERNEL);
+	if (!sensor->data_types)
+		return -ENOMEM;
+
+	for (i = 0; i < sensor->data_type_count; ++i) {
+		sensor->data_types[i].name = devm_kstrdup_const(
+			smgr->dev, resp.data_types[i].name, GFP_KERNEL);
+		sensor->data_types[i].vendor = devm_kstrdup_const(
+			smgr->dev, resp.data_types[i].vendor, GFP_KERNEL);
+
+		sensor->data_types[i].range = resp.data_types[i].range;
+
+		sensor->data_types[i].native_sample_rate_count =
+			resp.native_sample_rates[i].rate_count;
+		if (sensor->data_types[i].native_sample_rate_count) {
+			sensor->data_types[i]
+				.native_sample_rates = devm_kmemdup_array(
+				smgr->dev, resp.native_sample_rates[i].rates,
+				sensor->data_types[i].native_sample_rate_count,
+				sizeof(u16), GFP_KERNEL);
+			if (!sensor->data_types[i].native_sample_rates)
+				return -ENOMEM;
+		}
+
+		sensor->data_types[i].max_sample_rate =
+			resp.data_types[i].max_sample_rate_hz;
+	}
+
+	return 0;
+}
+
+static int qcom_smgr_request_buffering(struct qcom_smgr *smgr,
+				       struct qcom_smgr_sensor *sensor,
+				       bool enable)
+{
+	struct sns_smgr_buffering_req req = {
+		/*
+		 * Reuse sensor ID as a report ID to avoid having to keep track
+		 * of a separate set of IDs
+		 */
+		.report_id = sensor->id,
+		.notify_suspend_valid = false
+	};
+	struct sns_smgr_buffering_resp resp = {};
+	struct qmi_txn txn;
+	u16 sample_rate = 0;
+	int ret;
+
+	if (enable) {
+		req.action = SNS_SMGR_BUFFERING_ACTION_ADD;
+
+		/*
+		 * Report rate and sample rate can be configured separately.
+		 * The former is the rate at which buffering report indications
+		 * are sent, while the latter is the actual sample rate of the
+		 * sensor. If report rate is set lower than sample rate,
+		 * multiple samples can be bundled and sent in one report.
+		 * A report cannot have 0 samples, therefore report rate cannot
+		 * be higher than sample rate.
+		 *
+		 * Fow now we default to the maximum sample rate and set the
+		 * report rate such that every report contains only 1 sample.
+		 * This gives us the lowest latency.
+		 */
+		if (sensor->data_types[0].native_sample_rates)
+			sample_rate = sensor->data_types[0].native_sample_rates
+					[sensor->data_types[0]
+						 .native_sample_rate_count - 1];
+
+		/*
+		 * SMGR may support a lower maximum sample rate than natively
+		 * supported by the sensor.
+		 */
+		if (sample_rate == 0 ||
+		    sample_rate > sensor->data_types[0].max_sample_rate)
+			sample_rate = sensor->data_types[0].max_sample_rate;
+
+		req.report_rate = sample_rate * SMGR_REPORT_RATE_HZ;
+
+		req.item_len = 1;
+		req.items[0].sensor_id = sensor->id;
+		req.items[0].data_type = SNS_SMGR_DATA_TYPE_PRIMARY;
+
+		req.items[0].sampling_rate = sample_rate;
+
+		/*
+		 * Unknown fields set to values frequently seen in dumps and
+		 * known to be working (although many different random values
+		 * appear to not cause any trouble).
+		 */
+		req.items[0].val1 = 3;
+		req.items[0].val2 = 1;
+	} else
+		req.action = SNS_SMGR_BUFFERING_ACTION_DELETE;
+
+	ret = qmi_txn_init(&smgr->sns_smgr_hdl, &txn,
+			   sns_smgr_buffering_resp_ei, &resp);
+	if (ret < 0) {
+		dev_err(smgr->dev, "Failed to initialize QMI TXN: %d\n", ret);
+		return ret;
+	}
+
+	ret = qmi_send_request(&smgr->sns_smgr_hdl, &smgr->sns_smgr_info, &txn,
+			       SNS_SMGR_BUFFERING_MSG_ID,
+			       SNS_SMGR_BUFFERING_REQ_MAX_LEN,
+			       sns_smgr_buffering_req_ei, &req);
+	if (ret < 0) {
+		dev_err(smgr->dev, "Failed to send buffering request: %d\n",
+			ret);
+		qmi_txn_cancel(&txn);
+		return ret;
+	}
+
+	ret = qmi_txn_wait(&txn, 5 * HZ);
+	if (ret < 0)
+		return ret;
+
+	/* Check the response */
+	if (resp.result) {
+		dev_err(smgr->dev, "Buffering request failed: 0x%x\n",
+			resp.result);
+		return -EREMOTEIO;
+	}
+
+	/* Keep track of requested sample rate */
+	sensor->data_types[0].cur_sample_rate = sample_rate;
+
+	return 0;
+}
+
+static void qcom_smgr_buffering_report_handler(struct qmi_handle *hdl,
+					       struct sockaddr_qrtr *sq,
+					       struct qmi_txn *txn,
+					       const void *data)
+{
+	struct qcom_smgr *smgr =
+		container_of(hdl, struct qcom_smgr, sns_smgr_hdl);
+	const struct sns_smgr_buffering_report_ind *ind = data;
+	struct qcom_smgr_sensor *sensor;
+	struct qcom_smgr_iio_data iio_data;
+	int temp;
+	u8 i, j;
+
+	for (i = 0; i < smgr->sensor_count; ++i) {
+		sensor = &smgr->sensors[i];
+
+		/* Find sensor matching report */
+		if (sensor->id == ind->report_id)
+			break;
+	}
+
+	if (i == smgr->sensor_count) {
+		dev_warn_ratelimited(smgr->dev,
+			"Received buffering report with unknown ID: %02x",
+			ind->report_id);
+		return;
+	}
+
+	/*
+	 * Construct data to be passed to IIO. Since we are matching report rate
+	 * with sample rate, we only get a single sample in every report.
+	 */
+	for (j = 0; j < ARRAY_SIZE(ind->samples[0].values); ++j)
+		iio_data.values[j] = ind->samples[0].values[j];
+
+	/*
+	 * SMGR reports sensor data in 32-bit fixed-point values, with 16 bits
+	 * holding the integer part and the other 16 bits holding the numerator
+	 * of a fraction with the denominator 2**16.
+	 *
+	 * Proximity sensor values are reported differently from other sensors.
+	 * The value reported is a boolean (0 or 1, still in the same fixed-point
+	 * format) where 1 means the sensor is activated, i.e. something is
+	 * within its range. Use the reported range to pass an actual distance
+	 * value to IIO. We pass the sensor range when nothing is within range
+	 * (sensor maxed out) and 0 when something is within range (assume
+	 * sensor is covered).
+	 */
+	if (sensor->type == SNS_SMGR_SENSOR_TYPE_PROX_LIGHT) {
+		temp = le32_to_cpu(iio_data.values[0]);
+		temp >>= SMGR_VALUE_DECIMAL_OFFSET;
+		temp = ~temp & 1;
+		temp *= sensor->data_types[0].range;
+		iio_data.values[0] = cpu_to_le32(temp);
+	}
+
+	iio_push_to_buffers(sensor->iio_dev, &iio_data);
+}
+
+static const struct qmi_msg_handler qcom_smgr_msg_handlers[] = {
+	{
+		.type = QMI_INDICATION,
+		.msg_id = SNS_SMGR_BUFFERING_REPORT_MSG_ID,
+		.ei = sns_smgr_buffering_report_ind_ei,
+		.decoded_size = sizeof(struct sns_smgr_buffering_report_ind),
+		.fn = qcom_smgr_buffering_report_handler,
+	},
+	{ }
+};
+
+static int qcom_smgr_sensor_postenable(struct iio_dev *iio_dev)
+{
+	struct qcom_smgr *smgr = dev_get_drvdata(iio_dev->dev.parent);
+	struct qcom_smgr_iio_priv *priv = iio_priv(iio_dev);
+	struct qcom_smgr_sensor *sensor = priv->sensor;
+
+	return qcom_smgr_request_buffering(smgr, sensor, true);
+}
+
+static int qcom_smgr_sensor_predisable(struct iio_dev *iio_dev)
+{
+	struct qcom_smgr *smgr = dev_get_drvdata(iio_dev->dev.parent);
+	struct qcom_smgr_iio_priv *priv = iio_priv(iio_dev);
+	struct qcom_smgr_sensor *sensor = priv->sensor;
+
+	dev_info(smgr->dev, "disable buffering %02x\n", sensor->id);
+	return qcom_smgr_request_buffering(smgr, sensor, false);
+}
+
+static const struct iio_buffer_setup_ops qcom_smgr_buffer_ops = {
+	.postenable = &qcom_smgr_sensor_postenable,
+	.predisable = &qcom_smgr_sensor_predisable
+};
+
+/* SMGR reports values for 3-axis sensors in north-east-down coordinates */
+static const struct iio_mount_matrix qcom_smgr_iio_mount_matrix = {
+	.rotation = {
+		"0", "-1", "0",
+		"-1", "0", "0",
+		"0", "0", "1"
+	}
+};
+
+static const struct iio_mount_matrix *
+qcom_smgr_iio_get_mount_matrix(const struct iio_dev *iio_dev,
+			       const struct iio_chan_spec *chan)
+{
+	return &qcom_smgr_iio_mount_matrix;
+}
+
+static const struct iio_chan_spec_ext_info qcom_smgr_iio_ext_info[] = {
+	IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, qcom_smgr_iio_get_mount_matrix),
+	{ }
+};
+
+static int qcom_smgr_iio_read_raw(struct iio_dev *iio_dev,
+				  struct iio_chan_spec const *chan, int *val,
+				  int *val2, long mask)
+{
+	struct qcom_smgr_iio_priv *priv = iio_priv(iio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = priv->sensor->data_types[0].cur_sample_rate;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = 1;
+		*val2 = 1 << SMGR_VALUE_DECIMAL_OFFSET;
+		return IIO_VAL_FRACTIONAL;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int qcom_smgr_iio_write_raw(struct iio_dev *iio_dev,
+				   struct iio_chan_spec const *chan, int val,
+				   int val2, long mask)
+{
+	struct qcom_smgr_iio_priv *priv = iio_priv(iio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (!iio_device_claim_direct(iio_dev))
+			return -EBUSY;
+
+		priv->sensor->data_types[0].cur_sample_rate = val;
+
+		iio_device_release_direct(iio_dev);
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int qcom_smgr_iio_read_avail(struct iio_dev *iio_dev,
+				    struct iio_chan_spec const *chan,
+				    const int **vals, int *type, int *length,
+				    long mask)
+{
+	struct qcom_smgr_iio_priv *priv = iio_priv(iio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*type = IIO_VAL_INT;
+		*vals = priv->samp_freq_vals;
+		*length = ARRAY_SIZE(priv->samp_freq_vals);
+		return IIO_AVAIL_RANGE;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info qcom_smgr_iio_info = {
+	.read_raw = qcom_smgr_iio_read_raw,
+	.write_raw = qcom_smgr_iio_write_raw,
+	.read_avail = qcom_smgr_iio_read_avail,
+};
+
+static const struct iio_chan_spec qcom_smgr_accel_iio_channels[] = {
+	{
+		.type = IIO_ACCEL,
+		.modified = true,
+		.channel2 = IIO_MOD_X,
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = qcom_smgr_iio_ext_info
+	},
+	{
+		.type = IIO_ACCEL,
+		.modified = true,
+		.channel2 = IIO_MOD_Y,
+		.scan_index = 1,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = qcom_smgr_iio_ext_info
+	},
+	{
+		.type = IIO_ACCEL,
+		.modified = true,
+		.channel2 = IIO_MOD_Z,
+		.scan_index = 2,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = qcom_smgr_iio_ext_info
+	},
+	{
+		.type = IIO_TIMESTAMP,
+		.channel = -1,
+		.scan_index = 3,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 64,
+			.endianness = IIO_LE,
+		},
+	}
+};
+
+static const struct iio_chan_spec qcom_smgr_gyro_iio_channels[] = {
+	{
+		.type = IIO_ANGL_VEL,
+		.modified = true,
+		.channel2 = IIO_MOD_X,
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = qcom_smgr_iio_ext_info
+	},
+	{
+		.type = IIO_ANGL_VEL,
+		.modified = true,
+		.channel2 = IIO_MOD_Y,
+		.scan_index = 1,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = qcom_smgr_iio_ext_info
+	},
+	{
+		.type = IIO_ANGL_VEL,
+		.modified = true,
+		.channel2 = IIO_MOD_Z,
+		.scan_index = 2,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = qcom_smgr_iio_ext_info
+	},
+	{
+		.type = IIO_TIMESTAMP,
+		.channel = -1,
+		.scan_index = 3,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 64,
+			.endianness = IIO_LE,
+		},
+	}
+};
+
+static const struct iio_chan_spec qcom_smgr_mag_iio_channels[] = {
+	{
+		.type = IIO_MAGN,
+		.modified = true,
+		.channel2 = IIO_MOD_X,
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = qcom_smgr_iio_ext_info
+	},
+	{
+		.type = IIO_MAGN,
+		.modified = true,
+		.channel2 = IIO_MOD_Y,
+		.scan_index = 1,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = qcom_smgr_iio_ext_info
+	},
+	{
+		.type = IIO_MAGN,
+		.modified = true,
+		.channel2 = IIO_MOD_Z,
+		.scan_index = 2,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.ext_info = qcom_smgr_iio_ext_info
+	},
+	{
+		.type = IIO_TIMESTAMP,
+		.channel = -1,
+		.scan_index = 3,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 64,
+			.endianness = IIO_LE,
+		},
+	}
+};
+
+static const struct iio_chan_spec qcom_smgr_prox_iio_channels[] = {
+	{
+		.type = IIO_PROXIMITY,
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) |
+				      BIT(IIO_CHAN_INFO_SAMP_FREQ)
+	},
+	{
+		.type = IIO_TIMESTAMP,
+		.channel = -1,
+		.scan_index = 3,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 64,
+			.endianness = IIO_LE,
+		},
+	},
+};
+
+static const struct iio_chan_spec qcom_smgr_pressure_iio_channels[] = {
+	{
+		.type = IIO_PRESSURE,
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_LE,
+		},
+		.info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) |
+				      BIT(IIO_CHAN_INFO_SAMP_FREQ)
+	},
+	{
+		.type = IIO_TIMESTAMP,
+		.channel = -1,
+		.scan_index = 3,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 64,
+			.endianness = IIO_LE,
+		},
+	}
+};
+
+static const char *const qcom_smgr_sensor_type_device_names[] = {
+	[SNS_SMGR_SENSOR_TYPE_ACCEL] = "qcom-smgr-accel",
+	[SNS_SMGR_SENSOR_TYPE_GYRO] = "qcom-smgr-gyro",
+	[SNS_SMGR_SENSOR_TYPE_MAG] = "qcom-smgr-mag",
+	[SNS_SMGR_SENSOR_TYPE_PROX_LIGHT] = "qcom-smgr-prox-light",
+	[SNS_SMGR_SENSOR_TYPE_PRESSURE] = "qcom-smgr-pressure",
+	[SNS_SMGR_SENSOR_TYPE_HALL_EFFECT] = "qcom-smgr-hall-effect"
+};
+
+static const struct iio_chan_spec *const qcom_smgr_sensor_type_iio_channels[] = {
+	[SNS_SMGR_SENSOR_TYPE_ACCEL] = qcom_smgr_accel_iio_channels,
+	[SNS_SMGR_SENSOR_TYPE_GYRO] = qcom_smgr_gyro_iio_channels,
+	[SNS_SMGR_SENSOR_TYPE_MAG] = qcom_smgr_mag_iio_channels,
+	[SNS_SMGR_SENSOR_TYPE_PROX_LIGHT] = qcom_smgr_prox_iio_channels,
+	[SNS_SMGR_SENSOR_TYPE_PRESSURE] = qcom_smgr_pressure_iio_channels,
+	[SNS_SMGR_SENSOR_TYPE_HALL_EFFECT] = NULL /* Unsupported */
+};
+
+static const size_t qcom_smgr_sensor_type_num_channels[] = {
+	[SNS_SMGR_SENSOR_TYPE_ACCEL] = ARRAY_SIZE(qcom_smgr_accel_iio_channels),
+	[SNS_SMGR_SENSOR_TYPE_GYRO] = ARRAY_SIZE(qcom_smgr_gyro_iio_channels),
+	[SNS_SMGR_SENSOR_TYPE_MAG] = ARRAY_SIZE(qcom_smgr_mag_iio_channels),
+	[SNS_SMGR_SENSOR_TYPE_PROX_LIGHT] = ARRAY_SIZE(qcom_smgr_prox_iio_channels),
+	[SNS_SMGR_SENSOR_TYPE_PRESSURE] = ARRAY_SIZE(qcom_smgr_pressure_iio_channels)
+};
+
+static int qcom_smgr_register_sensor(struct qcom_smgr *smgr,
+				     struct qcom_smgr_sensor *sensor)
+{
+	struct iio_dev *iio_dev;
+	struct qcom_smgr_iio_priv *priv;
+	int ret;
+
+	iio_dev = devm_iio_device_alloc(smgr->dev, sizeof(*priv));
+	if (!iio_dev)
+		return -ENOMEM;
+
+	priv = iio_priv(iio_dev);
+	priv->sensor = sensor;
+
+	priv->samp_freq_vals[0] = 1;
+	priv->samp_freq_vals[1] = 1;
+	priv->samp_freq_vals[2] = priv->sensor->data_types[0].max_sample_rate;
+
+	iio_dev->name = qcom_smgr_sensor_type_device_names[sensor->type];
+	iio_dev->info = &qcom_smgr_iio_info;
+	iio_dev->channels = qcom_smgr_sensor_type_iio_channels[sensor->type];
+	iio_dev->num_channels = qcom_smgr_sensor_type_num_channels[sensor->type];
+
+	sensor->iio_dev = iio_dev;
+
+	ret = devm_iio_kfifo_buffer_setup(smgr->dev, iio_dev,
+					  &qcom_smgr_buffer_ops);
+	if (ret) {
+		dev_err(smgr->dev, "Failed to setup buffer: %pe\n",
+			ERR_PTR(ret));
+		return ret;
+	}
+
+	ret = devm_iio_device_register(smgr->dev, iio_dev);
+	if (ret) {
+		dev_err(smgr->dev, "Failed to register IIO device: %pe\n",
+			ERR_PTR(ret));
+		return ret;
+	}
+
+	return 0;
+}
+
+static int qcom_smgr_probe(struct qrtr_device *qdev)
+{
+	struct qcom_smgr *smgr;
+	int i, j;
+	int ret;
+
+	smgr = devm_kzalloc(&qdev->dev, sizeof(*smgr), GFP_KERNEL);
+	if (!smgr)
+		return -ENOMEM;
+
+	smgr->dev = &qdev->dev;
+
+	smgr->sns_smgr_info.sq_family = AF_QIPCRTR;
+	smgr->sns_smgr_info.sq_node = qdev->node;
+	smgr->sns_smgr_info.sq_port = qdev->port;
+
+	dev_set_drvdata(&qdev->dev, smgr);
+
+	ret = qmi_handle_init(&smgr->sns_smgr_hdl,
+			      SNS_SMGR_SINGLE_SENSOR_INFO_RESP_MAX_LEN, NULL,
+			      qcom_smgr_msg_handlers);
+	if (ret < 0)
+		return dev_err_probe(smgr->dev, ret,
+			"Failed to initialize sensor manager handle\n");
+
+	ret = devm_add_action_or_reset(smgr->dev,
+				       (void(*)(void *))qmi_handle_release,
+				       &smgr->sns_smgr_hdl);
+	if (ret)
+		return ret;
+
+	ret = qcom_smgr_request_all_sensor_info(smgr, &smgr->sensors);
+	if (ret < 0)
+		return dev_err_probe(smgr->dev, ret,
+			"Failed to get available sensors\n");
+
+	smgr->sensor_count = ret;
+
+	/* Get primary and secondary sensors from each sensor ID */
+	for (i = 0; i < smgr->sensor_count; i++) {
+		ret = qcom_smgr_request_single_sensor_info(smgr,
+							   &smgr->sensors[i]);
+		if (ret < 0)
+			return dev_err_probe(smgr->dev, ret,
+				"Failed to get sensors from ID 0x%02x\n",
+				smgr->sensors[i].id);
+
+		for (j = 0; j < smgr->sensors[i].data_type_count; j++) {
+			/* Default to maximum sample rate */
+			smgr->sensors[i].data_types->cur_sample_rate =
+				smgr->sensors[i].data_types->max_sample_rate;
+
+			dev_dbg(smgr->dev, "0x%02x,%d: %s %s\n",
+				smgr->sensors[i].id, j,
+				smgr->sensors[i].data_types[j].vendor,
+				smgr->sensors[i].data_types[j].name);
+		}
+
+		/* Skip if sensor type is not supported */
+		if (smgr->sensors[i]->type == SNS_SMGR_SENSOR_TYPE_UNKNOWN ||
+		    !qcom_smgr_sensor_type_iio_channels[smgr->sensors[i]->type])
+			continue;
+
+		ret = qcom_smgr_register_sensor(smgr, &smgr->sensors[i]);
+		if (ret)
+			return dev_err_probe(smgr->dev, ret,
+				"Failed to register sensor 0x%02x\n",
+				smgr->sensors[i].id);
+	}
+
+	return 0;
+}
+
+static const struct qrtr_device_id qcom_smgr_qrtr_match[] = {
+	{
+		.service = SNS_SMGR_QMI_SVC_ID,
+		/* Found on MSM8953 */
+		.instance = QRTR_INSTANCE_CONST(0, 1)
+	},
+	{
+		.service = SNS_SMGR_QMI_SVC_ID,
+		/* Found on MSM8974 and MSM8226 */
+		.instance = QRTR_INSTANCE_CONST(1, 0)
+	},
+	{
+		.service = SNS_SMGR_QMI_SVC_ID,
+		/* Found on MSM8996 and SDM660 */
+		.instance = QRTR_INSTANCE_CONST(1, 50)
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(qrtr, qcom_smgr_qrtr_match);
+
+static struct qrtr_driver qcom_smgr_driver = {
+	.probe = qcom_smgr_probe,
+	.id_table = qcom_smgr_qrtr_match,
+	.driver	= {
+		.name = "qcom_smgr",
+	},
+};
+module_qrtr_driver(qcom_smgr_driver);
+
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_DESCRIPTION("Qualcomm Sensor Manager driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/iio/common/qcom_smgr/qmi/Makefile b/drivers/iio/common/qcom_smgr/qmi/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..e69208db495f999e962a651340405ee3632bf463
--- /dev/null
+++ b/drivers/iio/common/qcom_smgr/qmi/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_IIO_QCOM_SMGR)	+= qmi_sns_smgr.o
diff --git a/drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.c b/drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.c
new file mode 100644
index 0000000000000000000000000000000000000000..1b58e266280d0b26a8d62f1ba64edc5ac3136b1e
--- /dev/null
+++ b/drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.c
@@ -0,0 +1,713 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * QMI element info arrays and helper functions for Qualcomm Sensor Manager
+ *
+ * Copyright (c) 2021-2025, Yassine Oudjana <y.oudjana@protonmail.com>
+ */
+
+#include <linux/iio/common/qcom_smgr.h>
+#include <linux/module.h>
+#include <linux/soc/qcom/qmi.h>
+#include <linux/types.h>
+
+#include "qmi_sns_smgr.h"
+
+static const struct qmi_elem_info sns_smgr_all_sensor_info_ei[] = {
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_all_sensor_info, id),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_all_sensor_info, id),
+	},
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size =
+			sizeof_field(struct sns_smgr_all_sensor_info, type_len),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_all_sensor_info, type_len),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = SNS_SMGR_SENSOR_TYPE_MAX_LEN,
+		.elem_size = sizeof(char),
+		.array_type = VAR_LEN_ARRAY,
+		.offset = offsetof(struct sns_smgr_all_sensor_info, type),
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+
+const struct qmi_elem_info sns_smgr_all_sensor_info_resp_ei[] = {
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_all_sensor_info_resp,
+					  result),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x02,
+		.offset =
+			offsetof(struct sns_smgr_all_sensor_info_resp, result),
+	},
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_all_sensor_info_resp,
+					  item_len),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x03,
+		.offset = offsetof(struct sns_smgr_all_sensor_info_resp,
+				   item_len),
+	},
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = SNS_SMGR_ALL_SENSOR_INFO_MAX_LEN,
+		.elem_size = sizeof(struct sns_smgr_all_sensor_info),
+		.array_type = VAR_LEN_ARRAY,
+		.tlv_type = 0x03,
+		.offset = offsetof(struct sns_smgr_all_sensor_info_resp, items),
+		.ei_array = sns_smgr_all_sensor_info_ei,
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+EXPORT_SYMBOL_GPL(sns_smgr_all_sensor_info_resp_ei);
+
+const struct qmi_elem_info sns_smgr_single_sensor_info_req_ei[] = {
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_req, sensor_id),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x01,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_req,
+				   sensor_id),
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+EXPORT_SYMBOL_GPL(sns_smgr_single_sensor_info_req_ei);
+
+static const struct qmi_elem_info sns_smgr_single_sensor_info_data_type_ei[] = {
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_data_type,
+			sensor_id),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_data_type,
+				   sensor_id),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_data_type,
+			data_type),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_data_type,
+				   data_type),
+	},
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_data_type, name_len),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_data_type,
+				   name_len),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 0xff,
+		.elem_size = sizeof(char),
+		.array_type = VAR_LEN_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_data_type,
+				   name),
+	},
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_data_type,
+			vendor_len),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_data_type,
+				   vendor_len),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 0xff,
+		.elem_size = sizeof(char),
+		.array_type = VAR_LEN_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_data_type,
+				   vendor),
+	},
+	{
+		.data_type = QMI_UNSIGNED_4_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_data_type, val1),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_data_type,
+				   val1),
+	},
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_data_type,
+			max_sample_rate_hz),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_data_type,
+				   max_sample_rate_hz),
+	},
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_data_type, val2),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_data_type,
+				   val2),
+	},
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_data_type,
+			current_ua),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_data_type,
+				   current_ua),
+	},
+	{
+		.data_type = QMI_UNSIGNED_4_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_data_type, range),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_data_type,
+				   range),
+	},
+	{
+		.data_type = QMI_UNSIGNED_4_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_data_type,
+			resolution),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_data_type,
+				   resolution),
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+
+static const struct qmi_elem_info sns_smgr_single_sensor_info_native_sample_rates_ei[] = {
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_native_sample_rates, rate_count),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_native_sample_rates,
+				   rate_count),
+	},
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = SNS_SMGR_NATIVE_SAMPLE_RATES_MAX_LEN,
+		.elem_size = sizeof(u16),
+		.array_type = VAR_LEN_ARRAY,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_native_sample_rates,
+				   rates),
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+
+const struct qmi_elem_info sns_smgr_single_sensor_info_resp_ei[] = {
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_resp, result),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x02,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_resp,
+				   result),
+	},
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_resp, data_type_len),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x03,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_resp,
+				   data_type_len),
+	},
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = SNS_SMGR_DATA_TYPE_COUNT,
+		.elem_size =
+			sizeof(struct sns_smgr_single_sensor_info_data_type),
+		.array_type = VAR_LEN_ARRAY,
+		.tlv_type = 0x03,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_resp,
+				   data_types),
+		.ei_array = sns_smgr_single_sensor_info_data_type_ei,
+	},
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_resp, data1_len),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x10,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_resp,
+				   data1_len),
+	},
+	{
+		.data_type = QMI_UNSIGNED_4_BYTE,
+		.elem_len = SNS_SMGR_DATA_TYPE_COUNT,
+		.elem_size = sizeof(u32),
+		.array_type = VAR_LEN_ARRAY,
+		.tlv_type = 0x10,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_resp,
+				   data1),
+	},
+	{
+		.data_type = QMI_UNSIGNED_4_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_resp, data2),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x11,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_resp,
+				   data2),
+	},
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_resp, data3_len),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x12,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_resp,
+				   data3_len),
+	},
+	{
+		.data_type = QMI_UNSIGNED_8_BYTE,
+		.elem_len = SNS_SMGR_DATA_TYPE_COUNT,
+		.elem_size = sizeof(u64),
+		.array_type = VAR_LEN_ARRAY,
+		.tlv_type = 0x12,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_resp,
+				   data3),
+	},
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_resp,
+			native_sample_rates_len),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x13,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_resp,
+				   native_sample_rates_len),
+	},
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = SNS_SMGR_DATA_TYPE_COUNT,
+		.elem_size = sizeof(
+			struct sns_smgr_single_sensor_info_native_sample_rates),
+		.array_type = VAR_LEN_ARRAY,
+		.tlv_type = 0x13,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_resp,
+				   native_sample_rates),
+		.ei_array = sns_smgr_single_sensor_info_native_sample_rates_ei,
+	},
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_single_sensor_info_resp, data5_len),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x14,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_resp,
+				   data5_len),
+	},
+	{
+		.data_type = QMI_UNSIGNED_4_BYTE,
+		.elem_len = SNS_SMGR_DATA_TYPE_COUNT,
+		.elem_size = sizeof(u32),
+		.array_type = VAR_LEN_ARRAY,
+		.tlv_type = 0x14,
+		.offset = offsetof(struct sns_smgr_single_sensor_info_resp,
+				   data5),
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+EXPORT_SYMBOL_GPL(sns_smgr_single_sensor_info_resp_ei);
+
+static const struct qmi_elem_info sns_smgr_buffering_req_item_ei[] = {
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_buffering_req_item,
+					  sensor_id),
+		.array_type = NO_ARRAY,
+		.offset =
+			offsetof(struct sns_smgr_buffering_req_item, sensor_id),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_buffering_req_item,
+					  data_type),
+		.array_type = NO_ARRAY,
+		.offset =
+			offsetof(struct sns_smgr_buffering_req_item, data_type),
+	},
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_buffering_req_item,
+					  val1),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_buffering_req_item,
+				   val1),
+	},
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_buffering_req_item,
+					  sampling_rate),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_buffering_req_item,
+				   sampling_rate),
+	},
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_buffering_req_item,
+					  val2),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_buffering_req_item,
+				   val2),
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+
+static const struct qmi_elem_info sns_smgr_buffering_req_notify_suspend_ei[] = {
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_buffering_req_notify_suspend,
+			proc_type),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_buffering_req_notify_suspend,
+				   proc_type),
+	},
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_buffering_req_notify_suspend,
+			send_indications_during_suspend),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_buffering_req_notify_suspend,
+				   send_indications_during_suspend),
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+
+const struct qmi_elem_info sns_smgr_buffering_req_ei[] = {
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size =
+			sizeof_field(struct sns_smgr_buffering_req, report_id),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x01,
+		.offset = offsetof(struct sns_smgr_buffering_req, report_id),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size =
+			sizeof_field(struct sns_smgr_buffering_req, action),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x02,
+		.offset = offsetof(struct sns_smgr_buffering_req, action),
+	},
+	{
+		.data_type = QMI_UNSIGNED_4_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_buffering_req,
+					  report_rate),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x03,
+		.offset = offsetof(struct sns_smgr_buffering_req, report_rate),
+	},
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size =
+			sizeof_field(struct sns_smgr_buffering_req, item_len),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x04,
+		.offset = offsetof(struct sns_smgr_buffering_req, item_len),
+	},
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = SNS_SMGR_DATA_TYPE_COUNT,
+		.elem_size = sizeof(struct sns_smgr_buffering_req_item),
+		.array_type = VAR_LEN_ARRAY,
+		.tlv_type = 0x04,
+		.offset = offsetof(struct sns_smgr_buffering_req, items),
+		.ei_array = sns_smgr_buffering_req_item_ei,
+	},
+	{
+		.data_type = QMI_OPT_FLAG,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_buffering_req,
+					  notify_suspend_valid),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x10,
+		.offset = offsetof(struct sns_smgr_buffering_req,
+				   notify_suspend_valid),
+	},
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_buffering_req,
+					  notify_suspend),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x10,
+		.offset =
+			offsetof(struct sns_smgr_buffering_req, notify_suspend),
+		.ei_array = sns_smgr_buffering_req_notify_suspend_ei,
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+EXPORT_SYMBOL_GPL(sns_smgr_buffering_req_ei);
+
+const struct qmi_elem_info sns_smgr_buffering_resp_ei[] = {
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = 1,
+		.elem_size =
+			sizeof_field(struct sns_smgr_buffering_resp, result),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x02,
+		.offset = offsetof(struct sns_smgr_buffering_resp, result),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size =
+			sizeof_field(struct sns_smgr_buffering_resp, report_id),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x10,
+		.offset = offsetof(struct sns_smgr_buffering_resp, report_id),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size =
+			sizeof_field(struct sns_smgr_buffering_resp, ack_nak),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x11,
+		.offset = offsetof(struct sns_smgr_buffering_resp, ack_nak),
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+EXPORT_SYMBOL_GPL(sns_smgr_buffering_resp_ei);
+
+static const struct qmi_elem_info sns_smgr_buffering_report_metadata_ei[] = {
+	{
+		.data_type = QMI_UNSIGNED_4_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_buffering_report_metadata, val1),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_buffering_report_metadata,
+				   val1),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size =
+			sizeof_field(struct sns_smgr_buffering_report_metadata,
+				     sample_count),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_buffering_report_metadata,
+				   sample_count),
+	},
+	{
+		.data_type = QMI_UNSIGNED_4_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_buffering_report_metadata, timestamp),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_buffering_report_metadata,
+				   timestamp),
+	},
+	{
+		.data_type = QMI_UNSIGNED_4_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_buffering_report_metadata, val2),
+		.array_type = NO_ARRAY,
+		.offset = offsetof(struct sns_smgr_buffering_report_metadata,
+				   val2),
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+
+static const struct qmi_elem_info sns_smgr_buffering_report_sample_ei[] = {
+	{
+		.data_type = QMI_UNSIGNED_4_BYTE,
+		.elem_len = 3,
+		.elem_size = sizeof(u32),
+		.array_type = STATIC_ARRAY,
+		.offset = offsetof(struct sns_smgr_buffering_report_sample,
+				   values),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_buffering_report_sample, val1),
+		.array_type = NO_ARRAY,
+		.offset =
+			offsetof(struct sns_smgr_buffering_report_sample, val1),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_buffering_report_sample, val2),
+		.array_type = NO_ARRAY,
+		.offset =
+			offsetof(struct sns_smgr_buffering_report_sample, val2),
+	},
+	{
+		.data_type = QMI_UNSIGNED_2_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(
+			struct sns_smgr_buffering_report_sample, val3),
+		.array_type = NO_ARRAY,
+		.offset =
+			offsetof(struct sns_smgr_buffering_report_sample, val3),
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+
+const struct qmi_elem_info sns_smgr_buffering_report_ind_ei[] = {
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_buffering_report_ind,
+					  report_id),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x01,
+		.offset = offsetof(struct sns_smgr_buffering_report_ind,
+				   report_id),
+	},
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_buffering_report_ind,
+					  metadata),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x02,
+		.offset = offsetof(struct sns_smgr_buffering_report_ind,
+				   metadata),
+		.ei_array = sns_smgr_buffering_report_metadata_ei,
+	},
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_buffering_report_ind,
+					  samples_len),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x03,
+		.offset = offsetof(struct sns_smgr_buffering_report_ind,
+				   samples_len),
+	},
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = SNS_SMGR_SAMPLES_MAX_LEN,
+		.elem_size = sizeof(struct sns_smgr_buffering_report_sample),
+		.array_type = VAR_LEN_ARRAY,
+		.tlv_type = 0x03,
+		.offset =
+			offsetof(struct sns_smgr_buffering_report_ind, samples),
+		.ei_array = sns_smgr_buffering_report_sample_ei,
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof_field(struct sns_smgr_buffering_report_ind,
+					  val2),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x10,
+		.offset = offsetof(struct sns_smgr_buffering_report_ind, val2),
+	},
+	{
+		.data_type = QMI_EOTI,
+	},
+};
+EXPORT_SYMBOL_GPL(sns_smgr_buffering_report_ind_ei);
+
+static const char *smgr_sensor_type_names[SNS_SMGR_SENSOR_TYPE_COUNT] = {
+	[SNS_SMGR_SENSOR_TYPE_ACCEL] = "ACCEL",
+	[SNS_SMGR_SENSOR_TYPE_GYRO] = "GYRO",
+	[SNS_SMGR_SENSOR_TYPE_MAG] = "MAG",
+	[SNS_SMGR_SENSOR_TYPE_PROX_LIGHT] = "PROX_LIGHT",
+	[SNS_SMGR_SENSOR_TYPE_PRESSURE] = "PRESSURE",
+	[SNS_SMGR_SENSOR_TYPE_HALL_EFFECT] = "HALL_EFFECT"
+};
+
+enum qcom_smgr_sensor_type sns_smgr_sensor_type_from_str(const char *str)
+{
+	enum qcom_smgr_sensor_type i;
+
+	for (i = SNS_SMGR_SENSOR_TYPE_UNKNOWN + 1;
+	     i < SNS_SMGR_SENSOR_TYPE_COUNT; i++)
+		if (!strcmp(str, smgr_sensor_type_names[i]))
+			return i;
+
+	return SNS_SMGR_SENSOR_TYPE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(sns_smgr_sensor_type_from_str);
+
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_DESCRIPTION("Qualcomm Sensor Manager QMI service helpers");
+MODULE_LICENSE("GPL");
diff --git a/drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.h b/drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.h
new file mode 100644
index 0000000000000000000000000000000000000000..5d0d622b9381bd053f13d5e6a01ee9fbd0d59470
--- /dev/null
+++ b/drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.h
@@ -0,0 +1,161 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __QMI_SNS_SMGR_H__
+#define __QMI_SNS_SMGR_H__
+
+#include <linux/iio/common/qcom_smgr.h>
+#include <linux/soc/qcom/qmi.h>
+#include <linux/types.h>
+
+/*
+ * The structures of QMI messages used by the service were determined
+ * purely by watching transactions between proprietary Android userspace
+ * components and SSC. along with comparing values reported by Android APIs
+ * to values received in response messages. Due to that, the purpose or
+ * meaning of many fields remains unknown. Such fields are named "val*",
+ * "data*" or similar. Furthermore, the true maximum sizes of some messages
+ * with unknown array fields may be different than defined here.
+ */
+
+#define SNS_SMGR_QMI_SVC_ID			0x0100
+
+#define SNS_SMGR_ALL_SENSOR_INFO_MSG_ID		0x05
+#define SNS_SMGR_SINGLE_SENSOR_INFO_MSG_ID	0x06
+#define SNS_SMGR_BUFFERING_MSG_ID		0x21
+#define SNS_SMGR_BUFFERING_REPORT_MSG_ID	0x22
+
+#define SNS_SMGR_ALL_SENSOR_INFO_REQ_MAX_LEN		0x0
+#define SNS_SMGR_ALL_SENSOR_INFO_RESP_MAX_LEN		0x3e
+#define SNS_SMGR_SINGLE_SENSOR_INFO_REQ_MAX_LEN		0x4
+#define SNS_SMGR_SINGLE_SENSOR_INFO_RESP_MAX_LEN	0x110
+#define SNS_SMGR_BUFFERING_REQ_MAX_LEN			0x30
+#define SNS_SMGR_BUFFERING_RESP_MAX_LEN			0x1e
+
+/* TODO: find actual maximums */
+#define SNS_SMGR_ALL_SENSOR_INFO_MAX_LEN	0x10
+#define SNS_SMGR_SENSOR_TYPE_MAX_LEN		0x10
+#define SNS_SMGR_NATIVE_SAMPLE_RATES_MAX_LEN	0x20
+#define SNS_SMGR_SAMPLES_MAX_LEN		0x100
+
+enum sns_smgr_buffering_action {
+	SNS_SMGR_BUFFERING_ACTION_ADD	 = 1,
+	SNS_SMGR_BUFFERING_ACTION_DELETE = 2,
+};
+
+struct sns_smgr_all_sensor_info {
+	u8 id;
+	u8 type_len;
+	char type[SNS_SMGR_SENSOR_TYPE_MAX_LEN];
+};
+
+struct sns_smgr_all_sensor_info_resp {
+	u16 result;
+	u8 item_len;
+	struct sns_smgr_all_sensor_info items[SNS_SMGR_ALL_SENSOR_INFO_MAX_LEN];
+};
+
+struct sns_smgr_single_sensor_info_req {
+	u8 sensor_id;
+};
+
+struct sns_smgr_single_sensor_info_data_type {
+	u8 sensor_id;
+	u8 data_type;
+	u8 name_len;
+	char name[0xff];
+	u8 vendor_len;
+	char vendor[0xff];
+	/*
+	 * Might be separate u8 or u16 fields, but taken as a u32 it is seen
+	 * to hold the value 1 for all sensors in dumps.
+	 */
+	u32 val1;
+	u16 max_sample_rate_hz;
+	u16 val2;
+	u16 current_ua;
+	u32 range;
+	u32 resolution;
+};
+
+struct sns_smgr_single_sensor_info_native_sample_rates {
+	u8 rate_count;
+	u16 rates[SNS_SMGR_NATIVE_SAMPLE_RATES_MAX_LEN];
+};
+
+struct sns_smgr_single_sensor_info_resp {
+	u16 result;
+	u8 data_type_len;
+	struct sns_smgr_single_sensor_info_data_type data_types[SNS_SMGR_DATA_TYPE_COUNT];
+	u8 data1_len;
+	u32 data1[SNS_SMGR_DATA_TYPE_COUNT];
+	u32 data2;
+	u8 data3_len;
+	u64 data3[SNS_SMGR_DATA_TYPE_COUNT];
+	u8 native_sample_rates_len;
+	struct sns_smgr_single_sensor_info_native_sample_rates
+		native_sample_rates[SNS_SMGR_DATA_TYPE_COUNT];
+	u8 data5_len;
+	u32 data5[SNS_SMGR_DATA_TYPE_COUNT];
+};
+
+struct sns_smgr_buffering_req_item {
+	u8 sensor_id;
+	u8 data_type;
+	u16 val1;
+	u16 sampling_rate;
+	u16 val2;
+};
+
+struct sns_smgr_buffering_req_notify_suspend {
+	u16 proc_type;
+	u16 send_indications_during_suspend;
+};
+
+struct sns_smgr_buffering_req {
+	u8 report_id;
+	u8 action;
+	u32 report_rate;
+	u8 item_len;
+	struct sns_smgr_buffering_req_item items[SNS_SMGR_DATA_TYPE_COUNT];
+	u8 notify_suspend_valid;
+	struct sns_smgr_buffering_req_notify_suspend notify_suspend;
+};
+
+struct sns_smgr_buffering_resp {
+	u16 result;
+	u8 report_id;
+	u8 ack_nak;
+};
+
+struct sns_smgr_buffering_report_metadata {
+	u32 val1;
+	u8 sample_count;
+	u32 timestamp;
+	u32 val2;
+};
+
+struct sns_smgr_buffering_report_sample {
+	u32 values[3];
+	u8 val1;
+	u8 val2;
+	u16 val3;
+};
+
+struct sns_smgr_buffering_report_ind {
+	u8 report_id;
+	struct sns_smgr_buffering_report_metadata metadata;
+	u8 samples_len;
+	struct sns_smgr_buffering_report_sample samples[SNS_SMGR_SAMPLES_MAX_LEN];
+	u8 val2;
+};
+
+extern const struct qmi_elem_info sns_smgr_all_sensor_info_resp_ei[];
+extern const struct qmi_elem_info sns_smgr_single_sensor_info_req_ei[];
+extern const struct qmi_elem_info sns_smgr_single_sensor_info_resp_ei[];
+extern const struct qmi_elem_info sns_smgr_buffering_req_ei[];
+extern const struct qmi_elem_info sns_smgr_buffering_resp_ei[];
+extern const struct qmi_elem_info sns_smgr_buffering_report_ind_ei[];
+
+extern enum qcom_smgr_sensor_type sns_smgr_sensor_type_from_str(const char *str);
+
+#endif /* __SSC_SNS_SMGR_H__ */
diff --git a/include/linux/iio/common/qcom_smgr.h b/include/linux/iio/common/qcom_smgr.h
new file mode 100644
index 0000000000000000000000000000000000000000..fdd3de12bb0a48f1fb9e51cd0463c9a9b9ed500f
--- /dev/null
+++ b/include/linux/iio/common/qcom_smgr.h
@@ -0,0 +1,80 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __QCOM_SMGR_H__
+#define __QCOM_SMGR_H__
+
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+#include <linux/soc/qcom/qmi.h>
+#include <linux/types.h>
+
+enum qcom_smgr_sensor_type {
+	SNS_SMGR_SENSOR_TYPE_UNKNOWN,
+	SNS_SMGR_SENSOR_TYPE_ACCEL,
+	SNS_SMGR_SENSOR_TYPE_GYRO,
+	SNS_SMGR_SENSOR_TYPE_MAG,
+	SNS_SMGR_SENSOR_TYPE_PROX_LIGHT,
+	SNS_SMGR_SENSOR_TYPE_PRESSURE,
+	SNS_SMGR_SENSOR_TYPE_HALL_EFFECT,
+
+	SNS_SMGR_SENSOR_TYPE_COUNT
+};
+
+enum qcom_smgr_data_type {
+	SNS_SMGR_DATA_TYPE_PRIMARY,
+	SNS_SMGR_DATA_TYPE_SECONDARY,
+
+	SNS_SMGR_DATA_TYPE_COUNT
+};
+
+struct qcom_smgr_data_type_item {
+	const char *name;
+	const char *vendor;
+
+	u32 range;
+
+	size_t native_sample_rate_count;
+	u16 *native_sample_rates;
+
+	u16 max_sample_rate;
+	u16 cur_sample_rate;
+};
+
+struct qcom_smgr_sensor {
+	u8 id;
+	enum qcom_smgr_sensor_type type;
+
+	u8 data_type_count;
+	/*
+	 * Only SNS_SMGR_DATA_TYPE_PRIMARY is used at the moment, but we store
+	 * SNS_SMGR_DATA_TYPE_SECONDARY when available as well for future use.
+	 */
+	struct qcom_smgr_data_type_item *data_types;
+
+	struct iio_dev *iio_dev;
+};
+
+struct qcom_smgr_iio_priv {
+	struct qcom_smgr_sensor *sensor;
+
+	int samp_freq_vals[3];
+};
+
+
+struct qcom_smgr_iio_data {
+	u32 values[3];
+	u64 timestamp;
+};
+
+struct qcom_smgr {
+	struct device *dev;
+
+	struct qmi_handle sns_smgr_hdl;
+	struct sockaddr_qrtr sns_smgr_info;
+	struct work_struct sns_smgr_work;
+
+	u8 sensor_count;
+	struct qcom_smgr_sensor *sensors;
+};
+
+#endif /* __QCOM_SMGR_H__ */

-- 
2.50.0

Re: [PATCH v2 2/4] net: qrtr: Turn QRTR into a bus

[Andy Shevchenko]

On Thu, Jul 10, 2025 at 11:06 AM Yassine Oudjana via B4 Relay
<devnull+y.oudjana.protonmail.com@kernel.org> wrote:
>
> Implement a QRTR bus to allow for creating drivers for individual QRTR
> services. With this in place, devices are dynamically registered for QRTR
> services as they become available, and drivers for these devices are
> matched using service and instance IDs.

...

> +struct qrtr_device_id {
> +       __u16 service;
> +       __u16 instance;
> +       kernel_ulong_t driver_data;     /* Data private to the driver */

Can we not repeat mistakes from the past and use const void * from day 1 please?

> +};
> +
>  /* dmi */

Wouldn't it be better to keep sections ordered alphabetically so 'q'
will go at least after 'd'?

...

> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __QCOM_QRTR_H__
> +#define __QCOM_QRTR_H__
> +
> +#include <linux/mod_devicetable.h>

Not enough. Please, follow IWYU principle and include / forward
declare all this header uses.

> +struct qrtr_device {
> +       struct device dev;
> +       unsigned int node;
> +       unsigned int port;
> +       u16 service;
> +       u16 instance;
> +};
> +
> +#define to_qrtr_device(d) container_of(d, struct qrtr_device, dev)
> +
> +struct qrtr_driver {
> +       int (*probe)(struct qrtr_device *qdev);
> +       void (*remove)(struct qrtr_device *qdev);
> +       const struct qrtr_device_id *id_table;
> +       struct device_driver driver;
> +};
> +
> +#define to_qrtr_driver(d) container_of(d, struct qrtr_driver, driver)
> +
> +#define qrtr_driver_register(drv) __qrtr_driver_register(drv, THIS_MODULE)
> +
> +int __qrtr_driver_register(struct qrtr_driver *drv, struct module *owner);
> +void qrtr_driver_unregister(struct qrtr_driver *drv);
> +
> +#define module_qrtr_driver(__qrtr_driver) \
> +       module_driver(__qrtr_driver, qrtr_driver_register, \
> +                       qrtr_driver_unregister)
> +
> +#endif /* __QCOM_QRTR_H__ */

...

> +       int ret = 0;

What is this assignment for? (The below is left for the context)

>         if (cb->type == QRTR_TYPE_NEW_SERVER) {
>                 /* Remote node endpoint can bridge other distant nodes */
>                 qrtr_node_assign(node, le32_to_cpu(pkt->server.node));
> +
> +               /* Create a QRTR device */
> +               ret = ep->add_device(ep, le32_to_cpu(pkt->server.node),
> +                                              le32_to_cpu(pkt->server.port),
> +                                              le32_to_cpu(pkt->server.service),
> +                                              le32_to_cpu(pkt->server.instance));
> +               if (ret)
> +                       goto err;
> +       } else if (cb->type == QRTR_TYPE_DEL_SERVER) {
> +               /* Remove QRTR device corresponding to service */
> +               ret = ep->del_device(ep, le32_to_cpu(pkt->server.port));
> +               if (ret)
> +                       goto err;
>         }

...

> +       return ret ? ret : -EINVAL;

It's also possible

       return ret ?: -EINVAL;

>  }

...

> +++ b/net/qrtr/smd.c
> @@ -7,6 +7,7 @@
>  #include <linux/module.h>
>  #include <linux/skbuff.h>
>  #include <linux/rpmsg.h>
> +#include <linux/soc/qcom/qrtr.h>

Can we keep this more ordered?

Also include export.h when the code uses one of the EXPORT_*() macros.

...

> +static int qcom_smd_qrtr_device_match(struct device *dev, const struct device_driver *drv)
> +{
> +       struct qrtr_device *qdev = to_qrtr_device(dev);
> +       struct qrtr_driver *qdrv = to_qrtr_driver(drv);
> +       const struct qrtr_device_id *id = qdrv->id_table;
> +
> +       if (!id)
> +               return 0;
> +
> +       while (id->service != 0) {

' != 0' is redundant

> +               if (id->service == qdev->service && id->instance == qdev->instance)
> +                       return 1;
> +               id++;
> +       }
> +
> +       return 0;
> +}

...

> +static int qcom_smd_qrtr_match_device_by_port(struct device *dev, const void *data)
> +{
> +       struct qrtr_device *qdev = to_qrtr_device(dev);
> +       unsigned const int *port = data;

Why not

       unsigned int port = *((const unsigned int *)data);

> +       return qdev->port == *port;
> +}

...

> +static void qcom_smd_qrtr_add_device_worker(struct work_struct *work)
> +{
> +       struct qrtr_new_server *new_server = container_of(work, struct qrtr_new_server, work);
> +       struct qrtr_smd_dev *qsdev = new_server->parent;
> +       struct qrtr_device *qdev;
> +       int ret;
> +
> +       qdev = kzalloc(sizeof(*qdev), GFP_KERNEL);
> +       if (!qdev)
> +               return;
> +
> +       *qdev = (struct qrtr_device) {
> +               .node = new_server->node,
> +               .port = new_server->port,
> +               .service = new_server->service,
> +               .instance = new_server->instance

Leave trailing comma.

> +       };

> +       devm_kfree(qsdev->dev, new_server);

?!?! No, just no. Please, fix the object lifetimes and use proper
allocators (not managed).

> +       dev_set_name(&qdev->dev, "%d-%d", qdev->node, qdev->port);

No error check?

> +       qdev->dev.bus = &qrtr_bus;
> +       qdev->dev.parent = qsdev->dev;
> +       qdev->dev.release = qcom_smd_qrtr_dev_release;
> +
> +       ret = device_register(&qdev->dev);
> +       if (ret) {
> +               dev_err(qsdev->dev, "Failed to register QRTR device: %pe\n", ERR_PTR(ret));
> +               put_device(&qdev->dev);
> +       }
> +}

...

> +static int qcom_smd_qrtr_add_device(struct qrtr_endpoint *parent, unsigned int node,
> +                                   unsigned int port, u16 service, u16 instance)
> +{
> +       struct qrtr_smd_dev *qsdev = container_of(parent, struct qrtr_smd_dev, ep);
> +       struct qrtr_new_server *new_server;
> +
> +       new_server = devm_kzalloc(qsdev->dev, sizeof(*new_server), GFP_KERNEL);

Why is the managed API in use?!

> +       if (!new_server)
> +               return -ENOMEM;
> +
> +       *new_server = (struct qrtr_new_server) {
> +               .parent = qsdev,
> +               .node = node,
> +               .port = port,
> +               .service = service,
> +               .instance = instance

Leave trailing comma.

> +       };
> +
> +       INIT_WORK(&new_server->work, qcom_smd_qrtr_add_device_worker);
> +       schedule_work(&new_server->work);
> +
> +       return 0;
> +}
> +
> +static int qcom_smd_qrtr_del_device(struct qrtr_endpoint *parent, unsigned int port)
> +{
> +       struct qrtr_smd_dev *qsdev = container_of(parent, struct qrtr_smd_dev, ep);
> +       struct qrtr_del_server *del_server;
> +
> +       del_server = devm_kzalloc(qsdev->dev, sizeof(*del_server), GFP_KERNEL);

Ditto.

> +       if (!del_server)
> +               return -ENOMEM;
> +
> +       del_server->parent = qsdev;
> +       del_server->port = port;
> +
> +       INIT_WORK(&del_server->work, qcom_smd_qrtr_del_device_worker);
> +       schedule_work(&del_server->work);
> +
> +       return 0;
> +}

...

> +static int qcom_smd_qrtr_device_unregister(struct device *dev, void *data)
> +{
> +       device_unregister(dev);
> +
> +       return 0;

Why? Can't this function be void?

> +}

...

>  {
>         struct qrtr_smd_dev *qsdev = dev_get_drvdata(&rpdev->dev);
>
> +       device_for_each_child(qsdev->dev, NULL, qcom_smd_qrtr_device_unregister);

Perhaps _reversed() ?

>         qrtr_endpoint_unregister(&qsdev->ep);
>
>         dev_set_drvdata(&rpdev->dev, NULL);

>  };


> +static void __exit qcom_smd_qrtr_exit(void)
> +{
> +       unregister_rpmsg_driver(&qcom_smd_qrtr_driver);
> +       bus_unregister(&qrtr_bus);
> +}
> +
> +subsys_initcall(qcom_smd_qrtr_init);
> +module_exit(qcom_smd_qrtr_exit);

Move these two closer to the mentioned callbacks.

--
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 4/4] iio: Add Qualcomm Sensor Manager driver

[Andy Shevchenko]

On Thu, Jul 10, 2025 at 11:06 AM Yassine Oudjana via B4 Relay
<devnull+y.oudjana.protonmail.com@kernel.org> wrote:
>
> Add a driver for sensors exposed by the Qualcomm Sensor Manager service,
> which is provided by SLPI or ADSP on Qualcomm SoCs. Supported sensors
> include accelerometers, gyroscopes, pressure sensors, proximity sensors
> and magnetometers.

First of all it's almost 2kLoCs, it's on the edge of unreviewable
code. Please, try to make 3+ patches out of this one.
Second, take your time and check what your code is using from the
kernel internal libraries and APIs and follow IWYU principle when
including headers.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/4] QRTR bus and Qualcomm Sensor Manager IIO drivers

[Simon Horman]

On Thu, Jul 10, 2025 at 09:06:26AM +0100, Yassine Oudjana via B4 Relay wrote:
> Sensor Manager is a QMI service available on several Qualcomm SoCs which
> exposes available sensors and allows for getting data from them. This
> service is provided by either:
> 
> - SSC (Snapdragon Sensor Core): Also known as SLPI (Sensor Low Power
>   Island). Has its own set of pins and peripherals to which sensors are
>   connected. These peripherals are generally inaccessible from the AP,
>   meaning sensors need to be operated exclusively through SSC. The only
>   known SoCs in this category are MSM8996 and MSM8998 (and their
>   derivatives).
> - ADSP (Audio DSP): Shares pins and peripherals with the AP. At least on
>   some devices, these pins could be configured as GPIOs which allows the AP
>   to access sensors by bit-banging their interfaces. Some SoCs in this
>   category are SDM630/660, MSM8953, MSM8974 and MSM8226.
> 
> Before Sensor Manager becomes accessible, another service known as Sensor
> Registry needs to be provided by the AP. The remote processor that provides
> Sensor Manager will then request data from it, and once that process is
> done, will expose several services including Sensor Manager.
> 
> This series adds a kernel driver for the Sensor Manager service, exposing
> sensors accessible through it as IIO devices. To facilitate probing of this
>  driver, QRTR is turned into a bus, with services being exposed as devices.
> Once the Sensor Manager service becomes available, the kernel attaches its
> device to the driver added in this series. This allows for dynamic probing
> of Sensor Manager without the need for static DT bindings, which would also
> not be ideal because they would be describing software rather than
> hardware. Sensor Manager is given as a working example of the QRTR bus.
> Kernel drivers for other services may also be able to benefit from this
> change.

...

Hi Yassine,

This series both adds an IIO driver and updates Networking code.

I'd suggest splitting the series so that the Networking updates can be
targeted at net-next, while the IIO driver is targeted at a different tree.

Also, I note that this series does not compile against current net-next.
This seems like it should be addressed, at least for the Networking
changes.

-- 
pw-bot: changes-requested

Re: [PATCH v2 4/4] iio: Add Qualcomm Sensor Manager driver

[Jonathan Cameron]

On Thu, 10 Jul 2025 09:06:30 +0100
Yassine Oudjana via B4 Relay <devnull+y.oudjana.protonmail.com@kernel.org> wrote:

> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Add a driver for sensors exposed by the Qualcomm Sensor Manager service,
> which is provided by SLPI or ADSP on Qualcomm SoCs. Supported sensors
> include accelerometers, gyroscopes, pressure sensors, proximity sensors
> and magnetometers.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

As Andy commented - this is big.  Break it up for v3.

So far I haven't understood why a separate accelerometer driver was necessary.
Some comments in the patch description would perhaps help me understand that.

> diff --git a/drivers/iio/accel/qcom_smgr_accel.c b/drivers/iio/accel/qcom_smgr_accel.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..ce854312d1d9386300785f1965d5886c16995806
> --- /dev/null
> +++ b/drivers/iio/accel/qcom_smgr_accel.c
> @@ -0,0 +1,138 @@


> +static void qcom_smgr_accel_remove(struct platform_device *pdev)
> +{
> +	struct qcom_smgr_sensor *sensor = platform_get_drvdata(pdev);
> +
> +	sensor->iio_dev = NULL;

Add a comment for why this is needed. I can't immediately spot anything
explicitly checking it so it doesn't seem to be about safe handling
of device removal or similar.


> +}
> +
> +static const struct platform_device_id qcom_smgr_accel_ids[] = {
> +	{ .name = "qcom-smgr-accel" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, qcom_smgr_accel_ids);
> +
> +static struct platform_driver qcom_smgr_accel_driver = {
> +	.probe = qcom_smgr_accel_probe,
> +	.remove = qcom_smgr_accel_remove,
> +	.driver	= {
> +		.name = "qcom_smgr_accel",
> +	},
> +	.id_table = qcom_smgr_accel_ids,
> +};
> +module_platform_driver(qcom_smgr_accel_driver);
> +
> +MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
> +MODULE_DESCRIPTION("Qualcomm Sensor Manager accelerometer driver");
> +MODULE_LICENSE("GPL");

I'm struggling to understand what the relationship between this driver
the main sensor driver is.

> diff --git a/drivers/iio/common/qcom_smgr/qcom_smgr.c b/drivers/iio/common/qcom_smgr/qcom_smgr.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..79d1160f7a5c32f1a9e0a20f29e304e5cb18be8f
> --- /dev/null
> +++ b/drivers/iio/common/qcom_smgr/qcom_smgr.c
> @@ -0,0 +1,840 @@

> +static void qcom_smgr_buffering_report_handler(struct qmi_handle *hdl,
> +					       struct sockaddr_qrtr *sq,
> +					       struct qmi_txn *txn,
> +					       const void *data)
> +{
> +	struct qcom_smgr *smgr =
> +		container_of(hdl, struct qcom_smgr, sns_smgr_hdl);
> +	const struct sns_smgr_buffering_report_ind *ind = data;
> +	struct qcom_smgr_sensor *sensor;
> +	struct qcom_smgr_iio_data iio_data;
> +	int temp;
> +	u8 i, j;
> +
> +	for (i = 0; i < smgr->sensor_count; ++i) {
> +		sensor = &smgr->sensors[i];
> +
> +		/* Find sensor matching report */
> +		if (sensor->id == ind->report_id)
> +			break;
> +	}
> +
> +	if (i == smgr->sensor_count) {
> +		dev_warn_ratelimited(smgr->dev,
> +			"Received buffering report with unknown ID: %02x",
> +			ind->report_id);
> +		return;
> +	}
> +
> +	/*
> +	 * Construct data to be passed to IIO. Since we are matching report rate
> +	 * with sample rate, we only get a single sample in every report.
> +	 */
> +	for (j = 0; j < ARRAY_SIZE(ind->samples[0].values); ++j)
> +		iio_data.values[j] = ind->samples[0].values[j];
> +
> +	/*
> +	 * SMGR reports sensor data in 32-bit fixed-point values, with 16 bits
> +	 * holding the integer part and the other 16 bits holding the numerator
> +	 * of a fraction with the denominator 2**16.
> +	 *
> +	 * Proximity sensor values are reported differently from other sensors.
> +	 * The value reported is a boolean (0 or 1, still in the same fixed-point
> +	 * format) where 1 means the sensor is activated, i.e. something is
> +	 * within its range. Use the reported range to pass an actual distance
> +	 * value to IIO. We pass the sensor range when nothing is within range
> +	 * (sensor maxed out) and 0 when something is within range (assume
> +	 * sensor is covered).
> +	 */
> +	if (sensor->type == SNS_SMGR_SENSOR_TYPE_PROX_LIGHT) {
> +		temp = le32_to_cpu(iio_data.values[0]);
> +		temp >>= SMGR_VALUE_DECIMAL_OFFSET;
> +		temp = ~temp & 1;
> +		temp *= sensor->data_types[0].range;
> +		iio_data.values[0] = cpu_to_le32(temp);
> +	}
> +
> +	iio_push_to_buffers(sensor->iio_dev, &iio_data);
You have a structure with space for timestamps but don't provide one which
is odd.  Either don't make space, or provide it.

> +}

> +
> +static int qcom_smgr_sensor_predisable(struct iio_dev *iio_dev)
> +{
> +	struct qcom_smgr *smgr = dev_get_drvdata(iio_dev->dev.parent);
> +	struct qcom_smgr_iio_priv *priv = iio_priv(iio_dev);
> +	struct qcom_smgr_sensor *sensor = priv->sensor;
> +
> +	dev_info(smgr->dev, "disable buffering %02x\n", sensor->id);

Too nosy. dev_dbg()

> +	return qcom_smgr_request_buffering(smgr, sensor, false);
> +}

> +static int qcom_smgr_iio_read_raw(struct iio_dev *iio_dev,
> +				  struct iio_chan_spec const *chan, int *val,
> +				  int *val2, long mask)
> +{
> +	struct qcom_smgr_iio_priv *priv = iio_priv(iio_dev);
> +
> +	switch (mask) {

No sysfs access at all to data is unusual but not completely unheard of.

> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = priv->sensor->data_types[0].cur_sample_rate;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 1;
> +		*val2 = 1 << SMGR_VALUE_DECIMAL_OFFSET;
> +		return IIO_VAL_FRACTIONAL;
> +	default:
> +		return -EINVAL;
> +	}

> +
> +static const struct iio_chan_spec qcom_smgr_pressure_iio_channels[] = {
> +	{
> +		.type = IIO_PRESSURE,
> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 32,
> +			.storagebits = 32,
> +			.endianness = IIO_LE,
> +		},
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) |
> +				      BIT(IIO_CHAN_INFO_SAMP_FREQ)
> +	},
> +	{
> +		.type = IIO_TIMESTAMP,
> +		.channel = -1,
> +		.scan_index = 3,

Why 3?

> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 32,

If it's realbits 32 and no shift, why not store it in a 32 bit value?
I assume this is a hardware provided timestamp rather than typical software
filled in one?  Anyhow, I'm not immediately spotting it being used yet
so for now perhaps best to drop the channel descriptions.

> +			.storagebits = 64,
> +			.endianness = IIO_LE,
> +		},
> +	}
> +};

> +static int qcom_smgr_register_sensor(struct qcom_smgr *smgr,
> +				     struct qcom_smgr_sensor *sensor)
> +{
> +	struct iio_dev *iio_dev;
> +	struct qcom_smgr_iio_priv *priv;
> +	int ret;

> +	sensor->iio_dev = iio_dev;
> +
> +	ret = devm_iio_kfifo_buffer_setup(smgr->dev, iio_dev,
> +					  &qcom_smgr_buffer_ops);
> +	if (ret) {
> +		dev_err(smgr->dev, "Failed to setup buffer: %pe\n",

Use return dev_err_probe() for all errors that occur in code that only
runs at probe() time.

> +			ERR_PTR(ret));
> +		return ret;
> +	}
> +
> +	ret = devm_iio_device_register(smgr->dev, iio_dev);
> +	if (ret) {
> +		dev_err(smgr->dev, "Failed to register IIO device: %pe\n",
> +			ERR_PTR(ret));
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int qcom_smgr_probe(struct qrtr_device *qdev)
> +{
> +	struct qcom_smgr *smgr;
> +	int i, j;
> +	int ret;
> +
> +	smgr = devm_kzalloc(&qdev->dev, sizeof(*smgr), GFP_KERNEL);
> +	if (!smgr)
> +		return -ENOMEM;
> +
> +	smgr->dev = &qdev->dev;
> +
> +	smgr->sns_smgr_info.sq_family = AF_QIPCRTR;
> +	smgr->sns_smgr_info.sq_node = qdev->node;
> +	smgr->sns_smgr_info.sq_port = qdev->port;
> +
> +	dev_set_drvdata(&qdev->dev, smgr);
This code is a bit random on whether it uses qdev->dev, or smgr->dev

I'd be tempted to just introce
	struct device *dev = &qdev->dev; and use that pretty much everywhere.

> +
> +	ret = qmi_handle_init(&smgr->sns_smgr_hdl,
> +			      SNS_SMGR_SINGLE_SENSOR_INFO_RESP_MAX_LEN, NULL,
> +			      qcom_smgr_msg_handlers);
> +	if (ret < 0)
> +		return dev_err_probe(smgr->dev, ret,
> +			"Failed to initialize sensor manager handle\n");
> +
> +	ret = devm_add_action_or_reset(smgr->dev,
> +				       (void(*)(void *))qmi_handle_release,

I'd much prefer a local wrapper to casting types of functions.

> +				       &smgr->sns_smgr_hdl);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_smgr_request_all_sensor_info(smgr, &smgr->sensors);
> +	if (ret < 0)
> +		return dev_err_probe(smgr->dev, ret,
> +			"Failed to get available sensors\n");
> +
> +	smgr->sensor_count = ret;
> +
> +	/* Get primary and secondary sensors from each sensor ID */
> +	for (i = 0; i < smgr->sensor_count; i++) {
> +		ret = qcom_smgr_request_single_sensor_info(smgr,
> +							   &smgr->sensors[i]);
> +		if (ret < 0)
> +			return dev_err_probe(smgr->dev, ret,
> +				"Failed to get sensors from ID 0x%02x\n",
> +				smgr->sensors[i].id);
> +
> +		for (j = 0; j < smgr->sensors[i].data_type_count; j++) {
> +			/* Default to maximum sample rate */
> +			smgr->sensors[i].data_types->cur_sample_rate =
> +				smgr->sensors[i].data_types->max_sample_rate;
> +
> +			dev_dbg(smgr->dev, "0x%02x,%d: %s %s\n",
> +				smgr->sensors[i].id, j,
> +				smgr->sensors[i].data_types[j].vendor,
> +				smgr->sensors[i].data_types[j].name);
> +		}
> +
> +		/* Skip if sensor type is not supported */
> +		if (smgr->sensors[i]->type == SNS_SMGR_SENSOR_TYPE_UNKNOWN ||
> +		    !qcom_smgr_sensor_type_iio_channels[smgr->sensors[i]->type])
> +			continue;
> +
> +		ret = qcom_smgr_register_sensor(smgr, &smgr->sensors[i]);
> +		if (ret)
> +			return dev_err_probe(smgr->dev, ret,
> +				"Failed to register sensor 0x%02x\n",
> +				smgr->sensors[i].id);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct qrtr_device_id qcom_smgr_qrtr_match[] = {
> +	{
> +		.service = SNS_SMGR_QMI_SVC_ID,
> +		/* Found on MSM8953 */
> +		.instance = QRTR_INSTANCE_CONST(0, 1)
> +	},
> +	{
> +		.service = SNS_SMGR_QMI_SVC_ID,
> +		/* Found on MSM8974 and MSM8226 */
> +		.instance = QRTR_INSTANCE_CONST(1, 0)
> +	},
> +	{
> +		.service = SNS_SMGR_QMI_SVC_ID,
> +		/* Found on MSM8996 and SDM660 */
> +		.instance = QRTR_INSTANCE_CONST(1, 50)
> +	},
> +	{ },

No comma on a terminating entry like this.

> +};
> +MODULE_DEVICE_TABLE(qrtr, qcom_smgr_qrtr_match);


> +const struct qmi_elem_info sns_smgr_buffering_report_ind_ei[] = {
> +	{
> +		.data_type = QMI_UNSIGNED_1_BYTE,
> +		.elem_len = 1,
> +		.elem_size = sizeof_field(struct sns_smgr_buffering_report_ind,
> +					  report_id),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x01,
> +		.offset = offsetof(struct sns_smgr_buffering_report_ind,
> +				   report_id),
> +	},
> +	{
> +		.data_type = QMI_STRUCT,
> +		.elem_len = 1,
> +		.elem_size = sizeof_field(struct sns_smgr_buffering_report_ind,
> +					  metadata),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x02,
> +		.offset = offsetof(struct sns_smgr_buffering_report_ind,
> +				   metadata),
> +		.ei_array = sns_smgr_buffering_report_metadata_ei,
> +	},
> +	{
> +		.data_type = QMI_DATA_LEN,
> +		.elem_len = 1,
> +		.elem_size = sizeof_field(struct sns_smgr_buffering_report_ind,
> +					  samples_len),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x03,
> +		.offset = offsetof(struct sns_smgr_buffering_report_ind,
> +				   samples_len),
> +	},
> +	{
> +		.data_type = QMI_STRUCT,
> +		.elem_len = SNS_SMGR_SAMPLES_MAX_LEN,
> +		.elem_size = sizeof(struct sns_smgr_buffering_report_sample),
> +		.array_type = VAR_LEN_ARRAY,
> +		.tlv_type = 0x03,
> +		.offset =
> +			offsetof(struct sns_smgr_buffering_report_ind, samples),

I'm fine with slightly over 80 chars to avoid readability issues like this.


> diff --git a/include/linux/iio/common/qcom_smgr.h b/include/linux/iio/common/qcom_smgr.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..fdd3de12bb0a48f1fb9e51cd0463c9a9b9ed500f
> --- /dev/null
> +++ b/include/linux/iio/common/qcom_smgr.h
> @@ -0,0 +1,80 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __QCOM_SMGR_H__
> +#define __QCOM_SMGR_H__
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/types.h>
> +#include <linux/soc/qcom/qmi.h>
> +#include <linux/types.h>

> +
> +struct qcom_smgr_sensor {
> +	u8 id;
> +	enum qcom_smgr_sensor_type type;
> +
> +	u8 data_type_count;
> +	/*
> +	 * Only SNS_SMGR_DATA_TYPE_PRIMARY is used at the moment, but we store
> +	 * SNS_SMGR_DATA_TYPE_SECONDARY when available as well for future use.
> +	 */
> +	struct qcom_smgr_data_type_item *data_types;
> +
> +	struct iio_dev *iio_dev;
> +};
> +
> +struct qcom_smgr_iio_priv {
> +	struct qcom_smgr_sensor *sensor;
> +
> +	int samp_freq_vals[3];
> +};
> +
> +
> +struct qcom_smgr_iio_data {
> +	u32 values[3];
> +	u64 timestamp;

Timestamps in kernel tend to be s64. Also for IIO buffers aligned_s64 required.
Whilst this will probably never be used on an architecture that doesn't naturally
align 8 byte variables, we should still code for it.

Given this tends to be used locally maybe just define it where you need it.


> +};

> 

Re: [PATCH v2 2/4] net: qrtr: Turn QRTR into a bus

[Yassine Oudjana]

On Thursday, July 10th, 2025 at 9:53 AM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Thu, Jul 10, 2025 at 11:06 AM Yassine Oudjana via B4 Relay
> devnull+y.oudjana.protonmail.com@kernel.org wrote:
> 
> > Implement a QRTR bus to allow for creating drivers for individual QRTR
> > services. With this in place, devices are dynamically registered for QRTR
> > services as they become available, and drivers for these devices are
> > matched using service and instance IDs.
> 
> 
> ...
> 
> > +struct qrtr_device_id {
> > + __u16 service;
> > + __u16 instance;
> > + kernel_ulong_t driver_data; /* Data private to the driver */
> 
> 
> Can we not repeat mistakes from the past and use const void * from day 1 please?

I just looked at what most other *_device_id structs had and did the same. I guess
they were left like that for legacy reasons? Might be good to add a comment next to
the kernel_ulong_t definition on why not to use it for future contributors.

> 
> > +};
> > +
> > /* dmi */
> 
> 
> Wouldn't it be better to keep sections ordered alphabetically so 'q'
> will go at least after 'd'?

It didn't look ordered so I didn't pay much attention to ordering but
sure, will reorder.

> 
> ...
> 
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef QCOM_QRTR_H
> > +#define QCOM_QRTR_H
> > +
> > +#include <linux/mod_devicetable.h>
> 
> 
> Not enough. Please, follow IWYU principle and include / forward
> declare all this header uses.

So you're saying forward declare struct qrtr_device_id instead of including
mod_devicetable.h?

> 
> > +struct qrtr_device {
> > + struct device dev;
> > + unsigned int node;
> > + unsigned int port;
> > + u16 service;
> > + u16 instance;
> > +};
> > +
> > +#define to_qrtr_device(d) container_of(d, struct qrtr_device, dev)
> > +
> > +struct qrtr_driver {
> > + int (*probe)(struct qrtr_device *qdev);
> > + void (*remove)(struct qrtr_device *qdev);
> > + const struct qrtr_device_id *id_table;
> > + struct device_driver driver;
> > +};
> > +
> > +#define to_qrtr_driver(d) container_of(d, struct qrtr_driver, driver)
> > +
> > +#define qrtr_driver_register(drv) __qrtr_driver_register(drv, THIS_MODULE)
> > +
> > +int __qrtr_driver_register(struct qrtr_driver *drv, struct module *owner);
> > +void qrtr_driver_unregister(struct qrtr_driver drv);
> > +
> > +#define module_qrtr_driver(__qrtr_driver) \
> > + module_driver(__qrtr_driver, qrtr_driver_register, \
> > + qrtr_driver_unregister)
> > +
> > +#endif / QCOM_QRTR_H */
> 
> 
> ...
> 
> > + int ret = 0;
> 
> 
> What is this assignment for? (The below is left for the context)

Yup looks unnecessary, will remove.

> 
> > if (cb->type == QRTR_TYPE_NEW_SERVER) {
> > /* Remote node endpoint can bridge other distant nodes /
> > qrtr_node_assign(node, le32_to_cpu(pkt->server.node));
> > +
> > + / Create a QRTR device /
> > + ret = ep->add_device(ep, le32_to_cpu(pkt->server.node),
> > + le32_to_cpu(pkt->server.port),
> > + le32_to_cpu(pkt->server.service),
> > + le32_to_cpu(pkt->server.instance));
> > + if (ret)
> > + goto err;
> > + } else if (cb->type == QRTR_TYPE_DEL_SERVER) {
> > + / Remove QRTR device corresponding to service */
> > + ret = ep->del_device(ep, le32_to_cpu(pkt->server.port));
> > + if (ret)
> > + goto err;
> > }
> 
> 
> ...
> 
> > + return ret ? ret : -EINVAL;
> 
> 
> It's also possible
> 
> return ret ?: -EINVAL;

Ack

> 
> > }
> 
> 
> ...
> 
> > +++ b/net/qrtr/smd.c
> > @@ -7,6 +7,7 @@
> > #include <linux/module.h>
> > #include <linux/skbuff.h>
> > #include <linux/rpmsg.h>
> > +#include <linux/soc/qcom/qrtr.h>
> 
> 
> Can we keep this more ordered?
> 
> Also include export.h when the code uses one of the EXPORT_*() macros.

Sure thing

> 
> ...
> 
> > +static int qcom_smd_qrtr_device_match(struct device *dev, const struct device_driver *drv)
> > +{
> > + struct qrtr_device *qdev = to_qrtr_device(dev);
> > + struct qrtr_driver *qdrv = to_qrtr_driver(drv);
> > + const struct qrtr_device_id *id = qdrv->id_table;
> > +
> > + if (!id)
> > + return 0;
> > +
> > + while (id->service != 0) {
> 
> 
> ' != 0' is redundant

Ack

> 
> > + if (id->service == qdev->service && id->instance == qdev->instance)
> > + return 1;
> > + id++;
> > + }
> > +
> > + return 0;
> > +}
> 
> 
> ...
> 
> > +static int qcom_smd_qrtr_match_device_by_port(struct device *dev, const void *data)
> > +{
> > + struct qrtr_device *qdev = to_qrtr_device(dev);
> > + unsigned const int *port = data;
> 
> 
> Why not
> 
> unsigned int port = *((const unsigned int *)data);

What does this achieve? Isn't it fine to implicitly cast void *?

> 
> > + return qdev->port == *port;
> > +}
> 
> 
> ...
> 
> > +static void qcom_smd_qrtr_add_device_worker(struct work_struct *work)
> > +{
> > + struct qrtr_new_server *new_server = container_of(work, struct qrtr_new_server, work);
> > + struct qrtr_smd_dev *qsdev = new_server->parent;
> > + struct qrtr_device *qdev;
> > + int ret;
> > +
> > + qdev = kzalloc(sizeof(*qdev), GFP_KERNEL);
> > + if (!qdev)
> > + return;
> > +
> > + *qdev = (struct qrtr_device) {
> > + .node = new_server->node,
> > + .port = new_server->port,
> > + .service = new_server->service,
> > + .instance = new_server->instance
> 
> 
> Leave trailing comma.

Ok

> 
> > + };
> 
> > + devm_kfree(qsdev->dev, new_server);
> 
> 
> ?!?! No, just no. Please, fix the object lifetimes and use proper
> allocators (not managed).

Missed this redundant managed kfree. See below about use of managed API

> 
> > + dev_set_name(&qdev->dev, "%d-%d", qdev->node, qdev->port);
> 
> 
> No error check?

Oops. Will add.

> 
> > + qdev->dev.bus = &qrtr_bus;
> > + qdev->dev.parent = qsdev->dev;
> > + qdev->dev.release = qcom_smd_qrtr_dev_release;
> > +
> > + ret = device_register(&qdev->dev);
> > + if (ret) {
> > + dev_err(qsdev->dev, "Failed to register QRTR device: %pe\n", ERR_PTR(ret));
> > + put_device(&qdev->dev);
> > + }
> > +}
> 
> 
> ...
> 
> > +static int qcom_smd_qrtr_add_device(struct qrtr_endpoint *parent, unsigned int node,
> > + unsigned int port, u16 service, u16 instance)
> > +{
> > + struct qrtr_smd_dev *qsdev = container_of(parent, struct qrtr_smd_dev, ep);
> > + struct qrtr_new_server *new_server;
> > +
> > + new_server = devm_kzalloc(qsdev->dev, sizeof(*new_server), GFP_KERNEL);
> 
> 
> Why is the managed API in use?!

When should I use or not use the managed API? I thought I was supposed to
use it whenever possible.

> 
> > + if (!new_server)
> > + return -ENOMEM;
> > +
> > + *new_server = (struct qrtr_new_server) {
> > + .parent = qsdev,
> > + .node = node,
> > + .port = port,
> > + .service = service,
> > + .instance = instance
> 
> 
> Leave trailing comma.

Sure

> 
> > + };
> > +
> > + INIT_WORK(&new_server->work, qcom_smd_qrtr_add_device_worker);
> > + schedule_work(&new_server->work);
> > +
> > + return 0;
> > +}
> > +
> > +static int qcom_smd_qrtr_del_device(struct qrtr_endpoint *parent, unsigned int port)
> > +{
> > + struct qrtr_smd_dev *qsdev = container_of(parent, struct qrtr_smd_dev, ep);
> > + struct qrtr_del_server *del_server;
> > +
> > + del_server = devm_kzalloc(qsdev->dev, sizeof(*del_server), GFP_KERNEL);
> 
> 
> Ditto.
> 
> > + if (!del_server)
> > + return -ENOMEM;
> > +
> > + del_server->parent = qsdev;
> > + del_server->port = port;
> > +
> > + INIT_WORK(&del_server->work, qcom_smd_qrtr_del_device_worker);
> > + schedule_work(&del_server->work);
> > +
> > + return 0;
> > +}
> 
> 
> ...
> 
> > +static int qcom_smd_qrtr_device_unregister(struct device *dev, void *data)
> > +{
> > + device_unregister(dev);
> > +
> > + return 0;
> 
> 
> Why? Can't this function be void?

Did it this way after seeing device_iter_t having int return type.

> 
> > +}
> 
> 
> ...
> 
> > {
> > struct qrtr_smd_dev *qsdev = dev_get_drvdata(&rpdev->dev);
> > 
> > + device_for_each_child(qsdev->dev, NULL, qcom_smd_qrtr_device_unregister);
> 
> 
> Perhaps _reversed() ?

What difference does the order make?

> 
> > qrtr_endpoint_unregister(&qsdev->ep);
> > 
> > dev_set_drvdata(&rpdev->dev, NULL);
> 
> > };
> 
> 
> 
> > +static void __exit qcom_smd_qrtr_exit(void)
> > +{
> > + unregister_rpmsg_driver(&qcom_smd_qrtr_driver);
> > + bus_unregister(&qrtr_bus);
> > +}
> > +
> > +subsys_initcall(qcom_smd_qrtr_init);
> > +module_exit(qcom_smd_qrtr_exit);
> 
> 
> Move these two closer to the mentioned callbacks.

Ack

Re: [PATCH v2 0/4] QRTR bus and Qualcomm Sensor Manager IIO drivers

[Yassine Oudjana]

On Thursday, July 10th, 2025 at 12:22 PM, Simon Horman <horms@kernel.org> wrote:

> On Thu, Jul 10, 2025 at 09:06:26AM +0100, Yassine Oudjana via B4 Relay wrote:
> 
> > Sensor Manager is a QMI service available on several Qualcomm SoCs which
> > exposes available sensors and allows for getting data from them. This
> > service is provided by either:
> > 
> > - SSC (Snapdragon Sensor Core): Also known as SLPI (Sensor Low Power
> > Island). Has its own set of pins and peripherals to which sensors are
> > connected. These peripherals are generally inaccessible from the AP,
> > meaning sensors need to be operated exclusively through SSC. The only
> > known SoCs in this category are MSM8996 and MSM8998 (and their
> > derivatives).
> > - ADSP (Audio DSP): Shares pins and peripherals with the AP. At least on
> > some devices, these pins could be configured as GPIOs which allows the AP
> > to access sensors by bit-banging their interfaces. Some SoCs in this
> > category are SDM630/660, MSM8953, MSM8974 and MSM8226.
> > 
> > Before Sensor Manager becomes accessible, another service known as Sensor
> > Registry needs to be provided by the AP. The remote processor that provides
> > Sensor Manager will then request data from it, and once that process is
> > done, will expose several services including Sensor Manager.
> > 
> > This series adds a kernel driver for the Sensor Manager service, exposing
> > sensors accessible through it as IIO devices. To facilitate probing of this
> > driver, QRTR is turned into a bus, with services being exposed as devices.
> > Once the Sensor Manager service becomes available, the kernel attaches its
> > device to the driver added in this series. This allows for dynamic probing
> > of Sensor Manager without the need for static DT bindings, which would also
> > not be ideal because they would be describing software rather than
> > hardware. Sensor Manager is given as a working example of the QRTR bus.
> > Kernel drivers for other services may also be able to benefit from this
> > change.
> 
> 
> ...
> 
> Hi Yassine,
> 
> This series both adds an IIO driver and updates Networking code.
> 
> I'd suggest splitting the series so that the Networking updates can be
> targeted at net-next, while the IIO driver is targeted at a different tree.
> 
> Also, I note that this series does not compile against current net-next.
> This seems like it should be addressed, at least for the Networking
> changes.

I targeted linux-next. By including the IIO driver my idea was to show
an example of using the QRTR bus, but if it has to target different trees
then sure, I'll split it.

Re: [PATCH v2 4/4] iio: Add Qualcomm Sensor Manager driver

[Yassine Oudjana]

On Thursday, July 10th, 2025 at 9:58 AM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Thu, Jul 10, 2025 at 11:06 AM Yassine Oudjana via B4 Relay
> devnull+y.oudjana.protonmail.com@kernel.org wrote:
> 
> > Add a driver for sensors exposed by the Qualcomm Sensor Manager service,
> > which is provided by SLPI or ADSP on Qualcomm SoCs. Supported sensors
> > include accelerometers, gyroscopes, pressure sensors, proximity sensors
> > and magnetometers.
> 
> 
> First of all it's almost 2kLoCs, it's on the edge of unreviewable
> code. Please, try to make 3+ patches out of this one.
> Second, take your time and check what your code is using from the
> kernel internal libraries and APIs and follow IWYU principle when
> including headers.

I can cleanly split it into 2 patches by putting the QMI components
in a separate patch. Not sure about 3+ patches but will try my best.
Will review includes.

Re: [PATCH v2 4/4] iio: Add Qualcomm Sensor Manager driver

[Yassine Oudjana]

On Sunday, July 13th, 2025 at 4:40 PM, Jonathan Cameron <jic23@kernel.org> wrote:

> On Thu, 10 Jul 2025 09:06:30 +0100
> Yassine Oudjana via B4 Relay devnull+y.oudjana.protonmail.com@kernel.org wrote:
> 
> > From: Yassine Oudjana y.oudjana@protonmail.com
> > 
> > Add a driver for sensors exposed by the Qualcomm Sensor Manager service,
> > which is provided by SLPI or ADSP on Qualcomm SoCs. Supported sensors
> > include accelerometers, gyroscopes, pressure sensors, proximity sensors
> > and magnetometers.
> > 
> > Signed-off-by: Yassine Oudjana y.oudjana@protonmail.com
> 
> 
> As Andy commented - this is big. Break it up for v3.

Ok

> 
> So far I haven't understood why a separate accelerometer driver was necessary.
> Some comments in the patch description would perhaps help me understand that.

That was not supposed to be here still. My bad.

> 
> > diff --git a/drivers/iio/accel/qcom_smgr_accel.c b/drivers/iio/accel/qcom_smgr_accel.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..ce854312d1d9386300785f1965d5886c16995806
> > --- /dev/null
> > +++ b/drivers/iio/accel/qcom_smgr_accel.c
> > @@ -0,0 +1,138 @@
> 
> 
> 
> > +static void qcom_smgr_accel_remove(struct platform_device *pdev)
> > +{
> > + struct qcom_smgr_sensor *sensor = platform_get_drvdata(pdev);
> > +
> > + sensor->iio_dev = NULL;
> 
> 
> Add a comment for why this is needed. I can't immediately spot anything
> explicitly checking it so it doesn't seem to be about safe handling
> of device removal or similar.
> 
> > +}
> > +
> > +static const struct platform_device_id qcom_smgr_accel_ids[] = {
> > + { .name = "qcom-smgr-accel" },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(platform, qcom_smgr_accel_ids);
> > +
> > +static struct platform_driver qcom_smgr_accel_driver = {
> > + .probe = qcom_smgr_accel_probe,
> > + .remove = qcom_smgr_accel_remove,
> > + .driver = {
> > + .name = "qcom_smgr_accel",
> > + },
> > + .id_table = qcom_smgr_accel_ids,
> > +};
> > +module_platform_driver(qcom_smgr_accel_driver);
> > +
> > +MODULE_AUTHOR("Yassine Oudjana y.oudjana@protonmail.com");
> > +MODULE_DESCRIPTION("Qualcomm Sensor Manager accelerometer driver");
> > +MODULE_LICENSE("GPL");
> 
> 
> I'm struggling to understand what the relationship between this driver
> the main sensor driver is.
> 
> > diff --git a/drivers/iio/common/qcom_smgr/qcom_smgr.c b/drivers/iio/common/qcom_smgr/qcom_smgr.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..79d1160f7a5c32f1a9e0a20f29e304e5cb18be8f
> > --- /dev/null
> > +++ b/drivers/iio/common/qcom_smgr/qcom_smgr.c
> > @@ -0,0 +1,840 @@
> 
> > +static void qcom_smgr_buffering_report_handler(struct qmi_handle *hdl,
> > + struct sockaddr_qrtr *sq,
> > + struct qmi_txn *txn,
> > + const void *data)
> > +{
> > + struct qcom_smgr *smgr =
> > + container_of(hdl, struct qcom_smgr, sns_smgr_hdl);
> > + const struct sns_smgr_buffering_report_ind *ind = data;
> > + struct qcom_smgr_sensor sensor;
> > + struct qcom_smgr_iio_data iio_data;
> > + int temp;
> > + u8 i, j;
> > +
> > + for (i = 0; i < smgr->sensor_count; ++i) {
> > + sensor = &smgr->sensors[i];
> > +
> > + / Find sensor matching report /
> > + if (sensor->id == ind->report_id)
> > + break;
> > + }
> > +
> > + if (i == smgr->sensor_count) {
> > + dev_warn_ratelimited(smgr->dev,
> > + "Received buffering report with unknown ID: %02x",
> > + ind->report_id);
> > + return;
> > + }
> > +
> > + /
> > + * Construct data to be passed to IIO. Since we are matching report rate
> > + * with sample rate, we only get a single sample in every report.
> > + /
> > + for (j = 0; j < ARRAY_SIZE(ind->samples[0].values); ++j)
> > + iio_data.values[j] = ind->samples[0].values[j];
> > +
> > + /
> > + * SMGR reports sensor data in 32-bit fixed-point values, with 16 bits
> > + * holding the integer part and the other 16 bits holding the numerator
> > + * of a fraction with the denominator 2**16.
> > + *
> > + * Proximity sensor values are reported differently from other sensors.
> > + * The value reported is a boolean (0 or 1, still in the same fixed-point
> > + * format) where 1 means the sensor is activated, i.e. something is
> > + * within its range. Use the reported range to pass an actual distance
> > + * value to IIO. We pass the sensor range when nothing is within range
> > + * (sensor maxed out) and 0 when something is within range (assume
> > + * sensor is covered).
> > + */
> > + if (sensor->type == SNS_SMGR_SENSOR_TYPE_PROX_LIGHT) {
> > + temp = le32_to_cpu(iio_data.values[0]);
> > + temp >>= SMGR_VALUE_DECIMAL_OFFSET;
> > + temp = ~temp & 1;
> > + temp *= sensor->data_types[0].range;
> > + iio_data.values[0] = cpu_to_le32(temp);
> > + }
> > +
> > + iio_push_to_buffers(sensor->iio_dev, &iio_data);
> 
> You have a structure with space for timestamps but don't provide one which
> is odd. Either don't make space, or provide it.

I forgot to copy the timestamp into iio_data. Will fix.

> 
> > +}
> 
> > +
> > +static int qcom_smgr_sensor_predisable(struct iio_dev *iio_dev)
> > +{
> > + struct qcom_smgr *smgr = dev_get_drvdata(iio_dev->dev.parent);
> > + struct qcom_smgr_iio_priv *priv = iio_priv(iio_dev);
> > + struct qcom_smgr_sensor *sensor = priv->sensor;
> > +
> > + dev_info(smgr->dev, "disable buffering %02x\n", sensor->id);
> 
> 
> Too nosy. dev_dbg()

I added this while debugging and missed it while cleaning up to remove it. Will remove
entirely.

> 
> > + return qcom_smgr_request_buffering(smgr, sensor, false);
> > +}
> 
> > +static int qcom_smgr_iio_read_raw(struct iio_dev *iio_dev,
> > + struct iio_chan_spec const *chan, int *val,
> > + int *val2, long mask)
> > +{
> > + struct qcom_smgr_iio_priv *priv = iio_priv(iio_dev);
> > +
> > + switch (mask) {
> 
> 
> No sysfs access at all to data is unusual but not completely unheard of.

There is no (known) method to request a single reading from the QMI
service. The only known way to get sensor data is to send a buffering
request to initiate sending data, then the remoteproc sends QMI
indications at a regular interval carrying sensor data which I am
pushing to the IIO buffers. The only way to implement direct sysfs
access would be to store the last received value somewhere then pass
it to sysfs when requested. This will also require enabling buffering
if disabled at the time of reading, then waiting until new data is
received. I didn't like this solution so I skipped direct sysfs access
altogether. Buffer access is enough for the current use case with
iio-sensor-proxy in userspace.

> 
> > + case IIO_CHAN_INFO_SAMP_FREQ:
> > + *val = priv->sensor->data_types[0].cur_sample_rate;
> > + return IIO_VAL_INT;
> > + case IIO_CHAN_INFO_SCALE:
> > + *val = 1;
> > + *val2 = 1 << SMGR_VALUE_DECIMAL_OFFSET;
> > + return IIO_VAL_FRACTIONAL;
> > + default:
> > + return -EINVAL;
> > + }
> 
> > +
> > +static const struct iio_chan_spec qcom_smgr_pressure_iio_channels[] = {
> > + {
> > + .type = IIO_PRESSURE,
> > + .scan_index = 0,
> > + .scan_type = {
> > + .sign = 'u',
> > + .realbits = 32,
> > + .storagebits = 32,
> > + .endianness = IIO_LE,
> > + },
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) |
> > + BIT(IIO_CHAN_INFO_SAMP_FREQ)
> > + },
> > + {
> > + .type = IIO_TIMESTAMP,
> > + .channel = -1,
> > + .scan_index = 3,
> 
> 
> Why 3?

Because the same struct is used for this and 3-axis sensors, so we should
skip the unused values.

> 
> > + .scan_type = {
> > + .sign = 'u',
> > + .realbits = 32,
> 
> 
> If it's realbits 32 and no shift, why not store it in a 32 bit value?
> I assume this is a hardware provided timestamp rather than typical software
> filled in one? Anyhow, I'm not immediately spotting it being used yet
> so for now perhaps best to drop the channel descriptions.

The hardware (or firmware rather) passes an unsigned 32-bit timestamp
value in a 64-bit QMI field. I was previously passing it as-is to IIO
but now since I introduced a new struct I can make it 32-bit storagebits.

But below you said s64 for timestamp so which is it going to be?

> 
> > + .storagebits = 64,
> > + .endianness = IIO_LE,
> > + },
> > + }
> > +};
> 
> > +static int qcom_smgr_register_sensor(struct qcom_smgr *smgr,
> > + struct qcom_smgr_sensor *sensor)
> > +{
> > + struct iio_dev *iio_dev;
> > + struct qcom_smgr_iio_priv *priv;
> > + int ret;
> 
> > + sensor->iio_dev = iio_dev;
> > +
> > + ret = devm_iio_kfifo_buffer_setup(smgr->dev, iio_dev,
> > + &qcom_smgr_buffer_ops);
> > + if (ret) {
> > + dev_err(smgr->dev, "Failed to setup buffer: %pe\n",
> 
> 
> Use return dev_err_probe() for all errors that occur in code that only
> runs at probe() time.

Ack

> 
> > + ERR_PTR(ret));
> > + return ret;
> > + }
> > +
> > + ret = devm_iio_device_register(smgr->dev, iio_dev);
> > + if (ret) {
> > + dev_err(smgr->dev, "Failed to register IIO device: %pe\n",
> > + ERR_PTR(ret));
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int qcom_smgr_probe(struct qrtr_device *qdev)
> > +{
> > + struct qcom_smgr *smgr;
> > + int i, j;
> > + int ret;
> > +
> > + smgr = devm_kzalloc(&qdev->dev, sizeof(*smgr), GFP_KERNEL);
> > + if (!smgr)
> > + return -ENOMEM;
> > +
> > + smgr->dev = &qdev->dev;
> > +
> > + smgr->sns_smgr_info.sq_family = AF_QIPCRTR;
> > + smgr->sns_smgr_info.sq_node = qdev->node;
> > + smgr->sns_smgr_info.sq_port = qdev->port;
> > +
> > + dev_set_drvdata(&qdev->dev, smgr);
> 
> This code is a bit random on whether it uses qdev->dev, or smgr->dev
> 
> 
> I'd be tempted to just introce
> struct device *dev = &qdev->dev; and use that pretty much everywhere.

I think smgr->dev is good, as is used everywhere else in the driver.
Will change this one.

> 
> > +
> > + ret = qmi_handle_init(&smgr->sns_smgr_hdl,
> > + SNS_SMGR_SINGLE_SENSOR_INFO_RESP_MAX_LEN, NULL,
> > + qcom_smgr_msg_handlers);
> > + if (ret < 0)
> > + return dev_err_probe(smgr->dev, ret,
> > + "Failed to initialize sensor manager handle\n");
> > +
> > + ret = devm_add_action_or_reset(smgr->dev,
> > + (void(*)(void *))qmi_handle_release,
> 
> 
> I'd much prefer a local wrapper to casting types of functions.

A function containing a single function call felt unnecessary to
me but I will add a wrapper if you prefer it that way.

> 
> > + &smgr->sns_smgr_hdl);
> > + if (ret)
> > + return ret;
> > +
> > + ret = qcom_smgr_request_all_sensor_info(smgr, &smgr->sensors);
> > + if (ret < 0)
> > + return dev_err_probe(smgr->dev, ret,
> > + "Failed to get available sensors\n");
> > +
> > + smgr->sensor_count = ret;
> > +
> > + /* Get primary and secondary sensors from each sensor ID /
> > + for (i = 0; i < smgr->sensor_count; i++) {
> > + ret = qcom_smgr_request_single_sensor_info(smgr,
> > + &smgr->sensors[i]);
> > + if (ret < 0)
> > + return dev_err_probe(smgr->dev, ret,
> > + "Failed to get sensors from ID 0x%02x\n",
> > + smgr->sensors[i].id);
> > +
> > + for (j = 0; j < smgr->sensors[i].data_type_count; j++) {
> > + / Default to maximum sample rate /
> > + smgr->sensors[i].data_types->cur_sample_rate =
> > + smgr->sensors[i].data_types->max_sample_rate;
> > +
> > + dev_dbg(smgr->dev, "0x%02x,%d: %s %s\n",
> > + smgr->sensors[i].id, j,
> > + smgr->sensors[i].data_types[j].vendor,
> > + smgr->sensors[i].data_types[j].name);
> > + }
> > +
> > + / Skip if sensor type is not supported /
> > + if (smgr->sensors[i]->type == SNS_SMGR_SENSOR_TYPE_UNKNOWN ||
> > + !qcom_smgr_sensor_type_iio_channels[smgr->sensors[i]->type])
> > + continue;
> > +
> > + ret = qcom_smgr_register_sensor(smgr, &smgr->sensors[i]);
> > + if (ret)
> > + return dev_err_probe(smgr->dev, ret,
> > + "Failed to register sensor 0x%02x\n",
> > + smgr->sensors[i].id);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct qrtr_device_id qcom_smgr_qrtr_match[] = {
> > + {
> > + .service = SNS_SMGR_QMI_SVC_ID,
> > + / Found on MSM8953 /
> > + .instance = QRTR_INSTANCE_CONST(0, 1)
> > + },
> > + {
> > + .service = SNS_SMGR_QMI_SVC_ID,
> > + / Found on MSM8974 and MSM8226 /
> > + .instance = QRTR_INSTANCE_CONST(1, 0)
> > + },
> > + {
> > + .service = SNS_SMGR_QMI_SVC_ID,
> > + / Found on MSM8996 and SDM660 */
> > + .instance = QRTR_INSTANCE_CONST(1, 50)
> > + },
> > + { },
> 
> 
> No comma on a terminating entry like this.

Ok. Gotta keep track of all the conventions used in different subsystems.

> 
> > +};
> > +MODULE_DEVICE_TABLE(qrtr, qcom_smgr_qrtr_match);
> 
> 
> 
> > +const struct qmi_elem_info sns_smgr_buffering_report_ind_ei[] = {
> > + {
> > + .data_type = QMI_UNSIGNED_1_BYTE,
> > + .elem_len = 1,
> > + .elem_size = sizeof_field(struct sns_smgr_buffering_report_ind,
> > + report_id),
> > + .array_type = NO_ARRAY,
> > + .tlv_type = 0x01,
> > + .offset = offsetof(struct sns_smgr_buffering_report_ind,
> > + report_id),
> > + },
> > + {
> > + .data_type = QMI_STRUCT,
> > + .elem_len = 1,
> > + .elem_size = sizeof_field(struct sns_smgr_buffering_report_ind,
> > + metadata),
> > + .array_type = NO_ARRAY,
> > + .tlv_type = 0x02,
> > + .offset = offsetof(struct sns_smgr_buffering_report_ind,
> > + metadata),
> > + .ei_array = sns_smgr_buffering_report_metadata_ei,
> > + },
> > + {
> > + .data_type = QMI_DATA_LEN,
> > + .elem_len = 1,
> > + .elem_size = sizeof_field(struct sns_smgr_buffering_report_ind,
> > + samples_len),
> > + .array_type = NO_ARRAY,
> > + .tlv_type = 0x03,
> > + .offset = offsetof(struct sns_smgr_buffering_report_ind,
> > + samples_len),
> > + },
> > + {
> > + .data_type = QMI_STRUCT,
> > + .elem_len = SNS_SMGR_SAMPLES_MAX_LEN,
> > + .elem_size = sizeof(struct sns_smgr_buffering_report_sample),
> > + .array_type = VAR_LEN_ARRAY,
> > + .tlv_type = 0x03,
> > + .offset =
> > + offsetof(struct sns_smgr_buffering_report_ind, samples),
> 
> 
> I'm fine with slightly over 80 chars to avoid readability issues like this.

Ok

> 
> > diff --git a/include/linux/iio/common/qcom_smgr.h b/include/linux/iio/common/qcom_smgr.h
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..fdd3de12bb0a48f1fb9e51cd0463c9a9b9ed500f
> > --- /dev/null
> > +++ b/include/linux/iio/common/qcom_smgr.h
> > @@ -0,0 +1,80 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef QCOM_SMGR_H
> > +#define QCOM_SMGR_H
> > +
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/types.h>
> > +#include <linux/soc/qcom/qmi.h>
> > +#include <linux/types.h>
> 
> > +
> > +struct qcom_smgr_sensor {
> > + u8 id;
> > + enum qcom_smgr_sensor_type type;
> > +
> > + u8 data_type_count;
> > + /*
> > + * Only SNS_SMGR_DATA_TYPE_PRIMARY is used at the moment, but we store
> > + * SNS_SMGR_DATA_TYPE_SECONDARY when available as well for future use.
> > + */
> > + struct qcom_smgr_data_type_item *data_types;
> > +
> > + struct iio_dev *iio_dev;
> > +};
> > +
> > +struct qcom_smgr_iio_priv {
> > + struct qcom_smgr_sensor *sensor;
> > +
> > + int samp_freq_vals[3];
> > +};
> > +
> > +
> > +struct qcom_smgr_iio_data {
> > + u32 values[3];
> > + u64 timestamp;
> 
> 
> Timestamps in kernel tend to be s64. Also for IIO buffers aligned_s64 required.
> Whilst this will probably never be used on an architecture that doesn't naturally
> align 8 byte variables, we should still code for it.
> 
> Given this tends to be used locally maybe just define it where you need it.

Ack

Re: [PATCH v2 4/4] iio: Add Qualcomm Sensor Manager driver

[Jonathan Cameron]

> > > +static int qcom_smgr_iio_read_raw(struct iio_dev *iio_dev,
> > > + struct iio_chan_spec const *chan, int *val,
> > > + int *val2, long mask)
> > > +{
> > > + struct qcom_smgr_iio_priv *priv = iio_priv(iio_dev);
> > > +
> > > + switch (mask) {  
> > 
> > 
> > No sysfs access at all to data is unusual but not completely unheard of.  
> 
> There is no (known) method to request a single reading from the QMI
> service. The only known way to get sensor data is to send a buffering
> request to initiate sending data, then the remoteproc sends QMI
> indications at a regular interval carrying sensor data which I am
> pushing to the IIO buffers. The only way to implement direct sysfs
> access would be to store the last received value somewhere then pass
> it to sysfs when requested. This will also require enabling buffering
> if disabled at the time of reading, then waiting until new data is
> received. I didn't like this solution so I skipped direct sysfs access
> altogether. Buffer access is enough for the current use case with
> iio-sensor-proxy in userspace.

This is absolutely fine.   I have mulled in the past implementing core
code to deal with cases where we are in buffered mode but want to still
provide sysfs access.  That applies for cases like ADCs where a couple
of channels are used for a touchscreen but where there is a hardware
restriction on accessing other channels on a oneshot basis whilst streaming
data on the others.  Maybe one day we'll have that support and it will
also help here, but it's not a high priority thing.

> > > +static const struct iio_chan_spec qcom_smgr_pressure_iio_channels[] = {
> > > + {
> > > + .type = IIO_PRESSURE,
> > > + .scan_index = 0,
> > > + .scan_type = {
> > > + .sign = 'u',
> > > + .realbits = 32,
> > > + .storagebits = 32,
> > > + .endianness = IIO_LE,
> > > + },
> > > + .info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) |
> > > + BIT(IIO_CHAN_INFO_SAMP_FREQ)
> > > + },
> > > + {
> > > + .type = IIO_TIMESTAMP,
> > > + .channel = -1,
> > > + .scan_index = 3,  
> > 
> > 
> > Why 3?  
> 
> Because the same struct is used for this and 3-axis sensors, so we should
> skip the unused values.

I'm not sure how that is related to this value.  These are effectively monotonic
but shouldn't be used to index anything driver side.  So there is nothing
wrong with the value 3, it's just a bit odd.

> 
> >   
> > > + .scan_type = {
> > > + .sign = 'u',
> > > + .realbits = 32,  
> > 
> > 
> > If it's realbits 32 and no shift, why not store it in a 32 bit value?
> > I assume this is a hardware provided timestamp rather than typical software
> > filled in one? Anyhow, I'm not immediately spotting it being used yet
> > so for now perhaps best to drop the channel descriptions.  
> 
> The hardware (or firmware rather) passes an unsigned 32-bit timestamp
> value in a 64-bit QMI field. I was previously passing it as-is to IIO
> but now since I introduced a new struct I can make it 32-bit storagebits.
> 
> But below you said s64 for timestamp so which is it going to be?

I wasn't sure if it was a software or hardware timestamp. Given it's coming
from the QMI thing it's 'hardware' so 32 bit is correct here.

> 
> > > + {
> > > + .service = SNS_SMGR_QMI_SVC_ID,
> > > + / Found on MSM8996 and SDM660 */
> > > + .instance = QRTR_INSTANCE_CONST(1, 50)
> > > + },
> > > + { },  
> > 
> > 
> > No comma on a terminating entry like this.  
> 
> Ok. Gotta keep track of all the conventions used in different subsystems.
I'm curious - have you ever had anyone request the comma?

I know some don't care, but it seems like an odd thing to insist on.

> 
> >   
> > > +};
> > > +MODULE_DEVICE_TABLE(qrtr, qcom_smgr_qrtr_match);  

Jonathan

Re: [PATCH v2 0/4] QRTR bus and Qualcomm Sensor Manager IIO drivers

[Casey Connolly]

Hi Yassine,

On 10/07/2025 10:06, Yassine Oudjana via B4 Relay wrote:
> Sensor Manager is a QMI service available on several Qualcomm SoCs which
> exposes available sensors and allows for getting data from them. This
> service is provided by either:
> 
> - SSC (Snapdragon Sensor Core): Also known as SLPI (Sensor Low Power
>   Island). Has its own set of pins and peripherals to which sensors are
>   connected. These peripherals are generally inaccessible from the AP,
>   meaning sensors need to be operated exclusively through SSC. The only
>   known SoCs in this category are MSM8996 and MSM8998 (and their
>   derivatives).
> - ADSP (Audio DSP): Shares pins and peripherals with the AP. At least on
>   some devices, these pins could be configured as GPIOs which allows the AP
>   to access sensors by bit-banging their interfaces. Some SoCs in this
>   category are SDM630/660, MSM8953, MSM8974 and MSM8226.
> 
> Before Sensor Manager becomes accessible, another service known as Sensor
> Registry needs to be provided by the AP. The remote processor that provides
> Sensor Manager will then request data from it, and once that process is
> done, will expose several services including Sensor Manager.

arguably a bit of a nit pick, but it might be worth clarifying that
newer SoCs starting with sdm845 also work in much the same way, except
the actual data is packed into protobuf messages which are sent over
QMI, rather than using QMI itself for the sensor data (and hence aren't
supported by this series).

That said, this is really awesome :D

Kind regards,>
> This series adds a kernel driver for the Sensor Manager service, exposing
> sensors accessible through it as IIO devices. To facilitate probing of this
>  driver, QRTR is turned into a bus, with services being exposed as devices.
> Once the Sensor Manager service becomes available, the kernel attaches its
> device to the driver added in this series. This allows for dynamic probing
> of Sensor Manager without the need for static DT bindings, which would also
> not be ideal because they would be describing software rather than
> hardware. Sensor Manager is given as a working example of the QRTR bus.
> Kernel drivers for other services may also be able to benefit from this
> change.
> 
> As previously mentioned, a Sensor Registry server must run on the AP to
> provide the remote processor (either SLPI or ADSP) with necessary data.
> A userspace implementation of this server is made[1]. The server can be
> supplied with the necessary data in the form of a plain-text configuration
> file that can be pulled from the Android vendor partition (sample[2]), or
> generated from a binary file that can be pulled from the persist partition.
> A more recently developed kernel implementation of the Sensor Registry
> server[3] can also be used. This last implementation only supports reading
> data from the binary file pulled from persist. Sensor Registry remains out
> of the scope of this patch series, as the Sensor Registry server and Sensor
> Manager client (this series) are fully independent components.
> 
> Due to the total lack of documentation on Sensor Manager, this driver was
> almost entirely the result of a process of capturing transactions between
> SSC and the proprietary Android daemons with several methods and manually
> decoding and interpreting them, sometimes by comparing with values acquired
> from Android APIs. A blog post[4] describes part of this process more
> detail. A little piece of downstream Android open-source code[5] was also
> used as reference during later stages of development. All of this, as well
> as a lack of time on my side for the last couple of years, meant that this
> driver had to go through a slow and intermittent development process for
> more than 3 years before reaching its current state.
> 
> Currently supported sensor types include accelerometers, gyroscopes,
> magentometers, proximity and pressure sensors. Other types (namely
> light and temperature sensors) are close to being implemented.
> 
> Some testing instructions may also be found here[6].
> 
> [1] https://gitlab.com/msm8996-mainline/sns-reg
> [2] https://github.com/nian0114/android_vendor_xiaomi_scorpio/blob/mkn-mr1/proprietary/etc/sensors/sensor_def_qcomdev.conf
> [3] https://github.com/sdm660-mainline/linux/pull/57
> [4] https://emainline.gitlab.io/2022/04/08/Unlocking_SSC_P2.html
> [5] https://android.googlesource.com/platform/system/chre/+/android-8.0.0_r2/platform/slpi
> [6] https://gitlab.postmarketos.org/postmarketOS/pmaports/-/merge_requests/4118
> 
> Changes since v1:
> - Split qdev renaming into separate patch
> - Export new QRTR symbols with namespace
> - Change struct initialization style
> - Remove redundant NULL initialization of qdev->dev.driver
> - Remove redundant devm_kfree
> - Use variable in sizeof rather than type
> - Change error return style in qcom_smd_qrtr_init
> - Change order of operations in qcom_smd_qrtr_exit
> - Use FIELD_PREP and GENMASK in QRTR_INSTANCE macro and add a CONST variant
> - Remove per-sensor subdrivers and eliminate use of platform devices
> - Put year range in copyright statements
> - Use dev_err_probe for error messages in probe
> - Remove unused include of linux/of.h
> - Avoid casting away const in qcom_smgr_buffering_report_handler
> - Use iio_push_to_buffers instead of iio_push_to_buffers_with_timestamp
> - Preprocess proximity sensor data before pushing to buffer
> - Add warning message for report with unknown ID received
> - Change sentinel value style in array of struct initialization
> - Refuse to set sampling frequency when buffer enabled
> - Return -EINVAL inside default case in all applicable switch statements
> - Move samp_freq_vals in qcom_smgr_iio_read_avail to priv and fix maximum
> - Add devm_add_action_or_reset for releasing QMI handle and get rid of
>   qcom_smgr_remove
> - Add service versions and instance IDs found on some platforms to QRTR
>   match table
> - Fix null pointer dereference on registering unsupported sensor
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
> Yassine Oudjana (4):
>       net: qrtr: smd: Rename qdev to qsdev
>       net: qrtr: Turn QRTR into a bus
>       net: qrtr: Define macro to convert QMI version and instance to QRTR instance
>       iio: Add Qualcomm Sensor Manager driver
> 
>  MAINTAINERS                                     |  13 +
>  drivers/iio/accel/qcom_smgr_accel.c             | 138 ++++
>  drivers/iio/common/Kconfig                      |   1 +
>  drivers/iio/common/Makefile                     |   1 +
>  drivers/iio/common/qcom_smgr/Kconfig            |  16 +
>  drivers/iio/common/qcom_smgr/Makefile           |   8 +
>  drivers/iio/common/qcom_smgr/qcom_smgr.c        | 840 ++++++++++++++++++++++++
>  drivers/iio/common/qcom_smgr/qmi/Makefile       |   3 +
>  drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.c | 713 ++++++++++++++++++++
>  drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.h | 161 +++++
>  drivers/soc/qcom/qmi_interface.c                |   5 +-
>  include/linux/iio/common/qcom_smgr.h            |  80 +++
>  include/linux/mod_devicetable.h                 |   9 +
>  include/linux/soc/qcom/qrtr.h                   |  46 ++
>  net/qrtr/af_qrtr.c                              |  23 +-
>  net/qrtr/qrtr.h                                 |   3 +
>  net/qrtr/smd.c                                  | 252 ++++++-
>  scripts/mod/devicetable-offsets.c               |   4 +
>  scripts/mod/file2alias.c                        |  10 +
>  19 files changed, 2302 insertions(+), 24 deletions(-)
> ---
> base-commit: 835244aba90de290b4b0b1fa92b6734f3ee7b3d9
> change-id: 20250710-qcom-smgr-8db96d370b10
> 
> Best regards,

-- 
// Casey (she/her)

Re: [PATCH v2 4/4] iio: Add Qualcomm Sensor Manager driver

[Casey Connolly]

Hi Yassine,

On 10/07/2025 10:06, Yassine Oudjana via B4 Relay wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Add a driver for sensors exposed by the Qualcomm Sensor Manager service,
> which is provided by SLPI or ADSP on Qualcomm SoCs. Supported sensors
> include accelerometers, gyroscopes, pressure sensors, proximity sensors
> and magnetometers.
> 
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
>  MAINTAINERS                                     |  13 +
>  drivers/iio/accel/qcom_smgr_accel.c             | 138 ++++
>  drivers/iio/common/Kconfig                      |   1 +
>  drivers/iio/common/Makefile                     |   1 +
>  drivers/iio/common/qcom_smgr/Kconfig            |  16 +
>  drivers/iio/common/qcom_smgr/Makefile           |   8 +
>  drivers/iio/common/qcom_smgr/qcom_smgr.c        | 840 ++++++++++++++++++++++++
>  drivers/iio/common/qcom_smgr/qmi/Makefile       |   3 +
>  drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.c | 713 ++++++++++++++++++++
>  drivers/iio/common/qcom_smgr/qmi/qmi_sns_smgr.h | 161 +++++
>  include/linux/iio/common/qcom_smgr.h            |  80 +++
>  11 files changed, 1974 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b5a472a544cfe2ad87691209c34d7bafe058ba42..0fb91c9bce431fc899776ff10b728ecdc957f51a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20702,6 +20702,19 @@ F:	Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
>  F:	drivers/net/ethernet/qualcomm/rmnet/
>  F:	include/linux/if_rmnet.h
>  
> +QUALCOMM SENSOR MANAGER IIO DRIVER
> +M:	Yassine Oudjana <y.oudjana@protonmail.com>
> +L:	linux-iio@vger.kernel.org
> +L:	linux-arm-msm@vger.kernel.org
> +S:	Maintained

Missing drivers/iio/accel/qcom_smgr_accel.c here

Kind regards,

-- 
// Casey (she/her)