* [PATCH 0/3] QRTR bus and Qualcomm Sensor Manager IIO drivers
@ 2025-04-06 14:07 Yassine Oudjana
2025-04-06 14:07 ` [PATCH 1/3] net: qrtr: Turn QRTR into a bus Yassine Oudjana
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: Yassine Oudjana @ 2025-04-06 14:07 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen, Bjorn Andersson,
Konrad Dybcio, Manivannan Sadhasivam, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Sverdlin, Sean Nyekjaer, Javier Carrasco,
Matti Vaittinen, Antoniu Miclaus, Ramona Gradinariu,
Yo-Jung (Leo) Lin, Andy Shevchenko, Neil Armstrong,
Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar
Cc: Yassine Oudjana, Yassine Oudjana, linux-kernel, linux-iio,
linux-arm-msm, netdev, linux-kbuild
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 kernel drivers for the Sensor Manager service, exposing
sensors accessible through it as IIO devices. To facilitate probing of the
Sensor Manager core 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
Yassine Oudjana (3):
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 drivers
MAINTAINERS | 18 +
drivers/iio/accel/Kconfig | 10 +
drivers/iio/accel/Makefile | 2 +
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 | 589 ++++++++++++++++
drivers/iio/common/qcom_smgr/qmi/Makefile | 3 +
drivers/iio/common/qcom_smgr/qmi/sns_smgr.c | 711 ++++++++++++++++++++
drivers/iio/common/qcom_smgr/qmi/sns_smgr.h | 163 +++++
drivers/iio/gyro/Kconfig | 10 +
drivers/iio/gyro/Makefile | 2 +
drivers/iio/gyro/qcom_smgr_gyro.c | 138 ++++
drivers/iio/magnetometer/Kconfig | 9 +
drivers/iio/magnetometer/Makefile | 2 +
drivers/iio/magnetometer/qcom_smgr_mag.c | 138 ++++
drivers/iio/pressure/Kconfig | 10 +
drivers/iio/pressure/Makefile | 1 +
drivers/iio/pressure/qcom_smgr_pressure.c | 106 +++
drivers/iio/proximity/Kconfig | 10 +
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/qcom_smgr_prox.c | 106 +++
drivers/soc/qcom/qmi_interface.c | 5 +-
include/linux/iio/common/qcom_smgr.h | 64 ++
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 | 250 ++++++-
scripts/mod/devicetable-offsets.c | 4 +
scripts/mod/file2alias.c | 10 +
33 files changed, 2573 insertions(+), 24 deletions(-)
create mode 100644 drivers/iio/accel/qcom_smgr_accel.c
create mode 100644 drivers/iio/common/qcom_smgr/Kconfig
create mode 100644 drivers/iio/common/qcom_smgr/Makefile
create mode 100644 drivers/iio/common/qcom_smgr/qcom_smgr.c
create mode 100644 drivers/iio/common/qcom_smgr/qmi/Makefile
create mode 100644 drivers/iio/common/qcom_smgr/qmi/sns_smgr.c
create mode 100644 drivers/iio/common/qcom_smgr/qmi/sns_smgr.h
create mode 100644 drivers/iio/gyro/qcom_smgr_gyro.c
create mode 100644 drivers/iio/magnetometer/qcom_smgr_mag.c
create mode 100644 drivers/iio/pressure/qcom_smgr_pressure.c
create mode 100644 drivers/iio/proximity/qcom_smgr_prox.c
create mode 100644 include/linux/iio/common/qcom_smgr.h
create mode 100644 include/linux/soc/qcom/qrtr.h
--
2.49.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/3] net: qrtr: Turn QRTR into a bus
2025-04-06 14:07 [PATCH 0/3] QRTR bus and Qualcomm Sensor Manager IIO drivers Yassine Oudjana
@ 2025-04-06 14:07 ` Yassine Oudjana
2025-04-06 16:01 ` Jonathan Cameron
2025-04-06 14:07 ` [PATCH 2/3] net: qrtr: Define macro to convert QMI version and instance to QRTR instance Yassine Oudjana
` (2 subsequent siblings)
3 siblings, 1 reply; 21+ messages in thread
From: Yassine Oudjana @ 2025-04-06 14:07 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen, Bjorn Andersson,
Konrad Dybcio, Manivannan Sadhasivam, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Sverdlin, Sean Nyekjaer, Javier Carrasco,
Matti Vaittinen, Antoniu Miclaus, Ramona Gradinariu,
Yo-Jung (Leo) Lin, Andy Shevchenko, Neil Armstrong,
Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar
Cc: Yassine Oudjana, Yassine Oudjana, linux-kernel, linux-iio,
linux-arm-msm, netdev, linux-kbuild
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.
In smd.c, replace all current occurences of qdev with qsdev in order to
distinguish between the newly 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>
---
include/linux/mod_devicetable.h | 9 ++
include/linux/soc/qcom/qrtr.h | 34 ++++
net/qrtr/af_qrtr.c | 23 ++-
net/qrtr/qrtr.h | 3 +
net/qrtr/smd.c | 250 ++++++++++++++++++++++++++++--
scripts/mod/devicetable-offsets.c | 4 +
scripts/mod/file2alias.c | 10 ++
7 files changed, 311 insertions(+), 22 deletions(-)
create mode 100644 include/linux/soc/qcom/qrtr.h
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index bd7e60c0b72f..e2204344e7c4 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 000000000000..4d7f25c64c56
--- /dev/null
+++ b/include/linux/soc/qcom/qrtr.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __QCOM_QRTR_H__
+#define __QCOM_QRTR_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 00c51cf693f3..e11682fd7960 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 3f2d28696062..659048336730 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 c91bf030fbc7..fd5ad6a8d1c3 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,19 +17,210 @@ 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_GPL(qrtr_bus);
+
+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_GPL(__qrtr_driver_register);
+
+void qrtr_driver_unregister(struct qrtr_driver *drv)
+{
+ driver_unregister(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(qrtr_driver_unregister);
+
+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 int port = *(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->node = new_server->node;
+ qdev->port = new_server->port;
+ qdev->service = new_server->service;
+ qdev->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;
+ qdev->dev.driver = NULL;
+
+ 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);
+
+ devm_kfree(qsdev->dev, del_server);
+
+ if (dev)
+ 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(struct qrtr_new_server), GFP_KERNEL);
+ if (!new_server)
+ return -ENOMEM;
+
+ new_server->parent = qsdev;
+ new_server->node = node;
+ new_server->port = port;
+ new_server->service = service;
+ new_server->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(struct qrtr_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)
{
- 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 +231,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 +250,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 +276,11 @@ 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);
+
+ device_for_each_child(qsdev->dev, NULL, qcom_smd_qrtr_device_unregister);
- qrtr_endpoint_unregister(&qdev->ep);
+ qrtr_endpoint_unregister(&qsdev->ep);
dev_set_drvdata(&rpdev->dev, NULL);
}
@@ -104,7 +300,27 @@ 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)
+ ret = register_rpmsg_driver(&qcom_smd_qrtr_driver);
+ else
+ bus_unregister(&qrtr_bus);
+
+ return ret;
+}
+
+static void __exit qcom_smd_qrtr_exit(void)
+{
+ bus_unregister(&qrtr_bus);
+ unregister_rpmsg_driver(&qcom_smd_qrtr_driver);
+}
+
+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 d3d00e85edf7..0a90323c35d3 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 00586119a25b..fef69db4d702 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.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/3] net: qrtr: Define macro to convert QMI version and instance to QRTR instance
2025-04-06 14:07 [PATCH 0/3] QRTR bus and Qualcomm Sensor Manager IIO drivers Yassine Oudjana
2025-04-06 14:07 ` [PATCH 1/3] net: qrtr: Turn QRTR into a bus Yassine Oudjana
@ 2025-04-06 14:07 ` Yassine Oudjana
2025-04-09 14:54 ` Konrad Dybcio
2025-04-06 14:08 ` [PATCH 3/3] iio: Add Qualcomm Sensor Manager drivers Yassine Oudjana
2025-04-08 10:27 ` [PATCH 0/3] QRTR bus and Qualcomm Sensor Manager IIO drivers Luca Weiss
3 siblings, 1 reply; 21+ messages in thread
From: Yassine Oudjana @ 2025-04-06 14:07 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen, Bjorn Andersson,
Konrad Dybcio, Manivannan Sadhasivam, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Sverdlin, Sean Nyekjaer, Javier Carrasco,
Matti Vaittinen, Antoniu Miclaus, Ramona Gradinariu,
Yo-Jung (Leo) Lin, Andy Shevchenko, Neil Armstrong,
Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar
Cc: Yassine Oudjana, Yassine Oudjana, linux-kernel, linux-iio,
linux-arm-msm, netdev, linux-kbuild
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 | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/qcom/qmi_interface.c b/drivers/soc/qcom/qmi_interface.c
index bc6d6379d8b1..cb57b7e1f252 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 4d7f25c64c56..10c89a35cbb9 100644
--- a/include/linux/soc/qcom/qrtr.h
+++ b/include/linux/soc/qcom/qrtr.h
@@ -13,6 +13,8 @@ struct qrtr_device {
#define to_qrtr_device(d) container_of(d, struct qrtr_device, dev)
+#define QRTR_INSTANCE(qmi_version, qmi_instance) (qmi_version | qmi_instance << 8)
+
struct qrtr_driver {
int (*probe)(struct qrtr_device *qdev);
void (*remove)(struct qrtr_device *qdev);
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/3] iio: Add Qualcomm Sensor Manager drivers
2025-04-06 14:07 [PATCH 0/3] QRTR bus and Qualcomm Sensor Manager IIO drivers Yassine Oudjana
2025-04-06 14:07 ` [PATCH 1/3] net: qrtr: Turn QRTR into a bus Yassine Oudjana
2025-04-06 14:07 ` [PATCH 2/3] net: qrtr: Define macro to convert QMI version and instance to QRTR instance Yassine Oudjana
@ 2025-04-06 14:08 ` Yassine Oudjana
2025-04-06 16:29 ` Jonathan Cameron
2025-06-18 19:19 ` Luca Weiss
2025-04-08 10:27 ` [PATCH 0/3] QRTR bus and Qualcomm Sensor Manager IIO drivers Luca Weiss
3 siblings, 2 replies; 21+ messages in thread
From: Yassine Oudjana @ 2025-04-06 14:08 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen, Bjorn Andersson,
Konrad Dybcio, Manivannan Sadhasivam, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Sverdlin, Sean Nyekjaer, Javier Carrasco,
Matti Vaittinen, Antoniu Miclaus, Ramona Gradinariu,
Yo-Jung (Leo) Lin, Andy Shevchenko, Neil Armstrong,
Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar
Cc: Yassine Oudjana, Yassine Oudjana, linux-kernel, linux-iio,
linux-arm-msm, netdev, linux-kbuild
Add drivers 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 | 18 +
drivers/iio/accel/Kconfig | 10 +
drivers/iio/accel/Makefile | 2 +
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 | 589 ++++++++++++++++
drivers/iio/common/qcom_smgr/qmi/Makefile | 3 +
drivers/iio/common/qcom_smgr/qmi/sns_smgr.c | 711 ++++++++++++++++++++
drivers/iio/common/qcom_smgr/qmi/sns_smgr.h | 163 +++++
drivers/iio/gyro/Kconfig | 10 +
drivers/iio/gyro/Makefile | 2 +
drivers/iio/gyro/qcom_smgr_gyro.c | 138 ++++
drivers/iio/magnetometer/Kconfig | 9 +
drivers/iio/magnetometer/Makefile | 2 +
drivers/iio/magnetometer/qcom_smgr_mag.c | 138 ++++
drivers/iio/pressure/Kconfig | 10 +
drivers/iio/pressure/Makefile | 1 +
drivers/iio/pressure/qcom_smgr_pressure.c | 106 +++
drivers/iio/proximity/Kconfig | 10 +
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/qcom_smgr_prox.c | 106 +++
include/linux/iio/common/qcom_smgr.h | 64 ++
25 files changed, 2257 insertions(+)
create mode 100644 drivers/iio/accel/qcom_smgr_accel.c
create mode 100644 drivers/iio/common/qcom_smgr/Kconfig
create mode 100644 drivers/iio/common/qcom_smgr/Makefile
create mode 100644 drivers/iio/common/qcom_smgr/qcom_smgr.c
create mode 100644 drivers/iio/common/qcom_smgr/qmi/Makefile
create mode 100644 drivers/iio/common/qcom_smgr/qmi/sns_smgr.c
create mode 100644 drivers/iio/common/qcom_smgr/qmi/sns_smgr.h
create mode 100644 drivers/iio/gyro/qcom_smgr_gyro.c
create mode 100644 drivers/iio/magnetometer/qcom_smgr_mag.c
create mode 100644 drivers/iio/pressure/qcom_smgr_pressure.c
create mode 100644 drivers/iio/proximity/qcom_smgr_prox.c
create mode 100644 include/linux/iio/common/qcom_smgr.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 78467ad7a8fe..d0183c09cc3f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20004,6 +20004,24 @@ 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/accel/qcom_smgr_accel.c
+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/sns_smgr.c
+F: drivers/iio/common/qcom_smgr/qmi/sns_smgr.h
+F: drivers/iio/gyro/qcom_smgr_gyro.c
+F: drivers/iio/magnetometer/qcom_smgr_mag.c
+F: drivers/iio/pressure/qcom_smgr_pressure.c
+F: drivers/iio/proximity/qcom_smgr_prox.c
+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/Kconfig b/drivers/iio/accel/Kconfig
index 8c3f7cf55d5f..d3253b856771 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -407,6 +407,16 @@ config IIO_CROS_EC_ACCEL_LEGACY
Sensor data is retrieved through IO memory.
Newer devices should use IIO_CROS_EC_SENSORS.
+config IIO_QCOM_SMGR_ACCEL
+ tristate "Qualcomm SSC Sensor Manager Accelerometer Sensor"
+ depends on IIO_QCOM_SMGR
+ select IIO_BUFFER
+ select IIO_KFIFO_BUF
+ help
+ Say yes here to get support for accelerometers connected to
+ a Qualcomm Snapdragon Sensor Core and accessed through its
+ Sensor Manager service.
+
config IIO_ST_ACCEL_3AXIS
tristate "STMicroelectronics accelerometers 3-Axis Driver"
depends on (I2C || SPI_MASTER) && SYSFS
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index ca8569e25aba..5f7fcd88a502 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -78,6 +78,8 @@ obj-$(CONFIG_STK8BA50) += stk8ba50.o
obj-$(CONFIG_IIO_CROS_EC_ACCEL_LEGACY) += cros_ec_accel_legacy.o
+obj-$(CONFIG_IIO_QCOM_SMGR_ACCEL) += qcom_smgr_accel.o
+
obj-$(CONFIG_IIO_SSP_SENSORS_COMMONS) += ssp_accel_sensor.o
obj-$(CONFIG_IIO_ST_ACCEL_3AXIS) += st_accel.o
diff --git a/drivers/iio/accel/qcom_smgr_accel.c b/drivers/iio/accel/qcom_smgr_accel.c
new file mode 100644
index 000000000000..ce854312d1d9
--- /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 1ccb5ccf3706..0ad8b3972087 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 d3e952239a62..f3f18484c91b 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 000000000000..da1965185001
--- /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 QCOM_RPROC_COMMON
+ select QCOM_QMI_HELPERS
+ select IIO_BUFFER
+ help
+ Say yes here to build core 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 000000000000..84554cedd2e5
--- /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 000000000000..8d46be11d5b6
--- /dev/null
+++ b/drivers/iio/common/qcom_smgr/qcom_smgr.c
@@ -0,0 +1,589 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Qualcomm Sensor Manager core driver
+ *
+ * Copyright (c) 2021, 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/math64.h>
+#include <linux/module.h>
+#include <linux/of.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/sns_smgr.h"
+
+#define SMGR_TICKS_PER_SECOND 32768
+#define SMGR_REPORT_RATE_HZ (SMGR_TICKS_PER_SECOND * 2)
+#define SMGR_VALUE_DIV 65536
+
+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;
+};
+
+static const char *const qcom_smgr_sensor_type_platform_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 void qcom_smgr_unregister_sensor(void *data)
+{
+ struct platform_device *pdev = data;
+
+ platform_device_unregister(pdev);
+}
+
+static int qcom_smgr_register_sensor(struct qcom_smgr *smgr,
+ struct qcom_smgr_sensor *sensor)
+{
+ struct platform_device *pdev;
+ const char *name = qcom_smgr_sensor_type_platform_names[sensor->type];
+
+ pdev = platform_device_register_data(smgr->dev, name, sensor->id,
+ &sensor, sizeof(sensor));
+ if (IS_ERR(pdev)) {
+ dev_err(smgr->dev, "Failed to register %s: %pe\n", name, pdev);
+ return PTR_ERR(pdev);
+ }
+
+ return devm_add_action_or_reset(smgr->dev, qcom_smgr_unregister_sensor,
+ pdev);
+}
+
+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);
+ struct sns_smgr_buffering_report_ind *ind =
+ (struct sns_smgr_buffering_report_ind *)data;
+ struct qcom_smgr_sensor *sensor;
+ u8 i;
+
+ for (i = 0; i < smgr->sensor_count; ++i) {
+ sensor = &smgr->sensors[i];
+
+ /* Find sensor matching report */
+ if (sensor->id != ind->report_id)
+ continue;
+
+ if (!sensor->iio_dev)
+ /* Corresponding driver was unloaded. Ignore remaining reports. */
+ return;
+
+ /*
+ * Since we are matching report rate with sample rate, we only
+ * get a single sample in every report.
+ */
+ iio_push_to_buffers_with_timestamp(sensor->iio_dev,
+ ind->samples[0].values,
+ ind->metadata.timestamp);
+
+ break;
+ }
+}
+
+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->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_postdisable(struct iio_dev *iio_dev)
+{
+ struct qcom_smgr *smgr = dev_get_drvdata(iio_dev->dev.parent->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, false);
+}
+
+const struct iio_buffer_setup_ops qcom_smgr_buffer_ops = {
+ .postenable = &qcom_smgr_sensor_postenable,
+ .postdisable = &qcom_smgr_sensor_postdisable
+};
+EXPORT_SYMBOL_GPL(qcom_smgr_buffer_ops);
+
+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:
+ switch (chan->type) {
+ case IIO_PROXIMITY:
+ /*
+ * Proximity value is reported as (SMGR_VALUE_DIV - x)/SMGR_VALUE_DIV of
+ * the sensor range. As with sensor values, range is also reported as range
+ * in meters * SMGR_VALUE_DIV. Proximity in meters can be calculated as
+ * such:
+ *
+ * proximity = -value * range / SMGR_VALUE_DIV**2
+ *
+ * Since our denominator (val2) is an int, we cannot fit SMGR_VALUE_DIV**2.
+ * Without losing too much accuracy, we can instead divide by 2 in the
+ * numerator and denominator, and move the -1 coefficient to the
+ * denominator. This way we can exactly fit within the lower bound of int.
+ */
+ *val = priv->sensor->data_types[0].range / 2;
+ *val2 = -SMGR_VALUE_DIV / 2 * SMGR_VALUE_DIV;
+ return IIO_VAL_FRACTIONAL;
+ default:
+ /*
+ * Sensor values are generally reported as 1/SMGR_VALUE_DIVths of the
+ * corresponding unit.
+ */
+ *val = 1;
+ *val2 = SMGR_VALUE_DIV;
+ return IIO_VAL_FRACTIONAL;
+ }
+ case IIO_CHAN_INFO_OFFSET:
+ /*
+ * Proximity values are inverted and start from the upper bound as explained above.
+ * No other channel types have an offset.
+ */
+ *val = priv->sensor->data_types[0].range;
+ *val2 = SMGR_VALUE_DIV;
+ return IIO_VAL_FRACTIONAL;
+ }
+
+ 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:
+ priv->sensor->data_types[0].cur_sample_rate = val;
+
+ /*
+ * Send new SMGR buffering request with updated rates
+ * if buffer is enabled
+ */
+ if (iio_buffer_enabled(iio_dev))
+ return iio_dev->setup_ops->postenable(iio_dev);
+
+ return 0;
+ }
+
+ 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);
+ const int samp_freq_vals[3] = {
+ 1, 1, priv->sensor->data_types[0].cur_sample_rate
+ };
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ *type = IIO_VAL_INT;
+ *vals = samp_freq_vals;
+ *length = ARRAY_SIZE(samp_freq_vals);
+ return IIO_AVAIL_RANGE;
+ }
+
+ return -EINVAL;
+}
+
+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,
+};
+EXPORT_SYMBOL_GPL(qcom_smgr_iio_info);
+
+/* 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;
+}
+
+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),
+ {}
+};
+EXPORT_SYMBOL_GPL(qcom_smgr_iio_ext_info);
+
+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) {
+ dev_err(smgr->dev,
+ "Failed to initialize sensor manager handle: %d\n",
+ ret);
+ return ret;
+ }
+
+ ret = qcom_smgr_request_all_sensor_info(smgr, &smgr->sensors);
+ if (ret < 0) {
+ dev_err(smgr->dev, "Failed to get available sensors: %pe\n",
+ ERR_PTR(ret));
+ return ret;
+ }
+ 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) {
+ dev_err(smgr->dev,
+ "Failed to get sensors from ID 0x%02x: %pe\n",
+ smgr->sensors[i].id, ERR_PTR(ret));
+ return ret;
+ }
+
+ 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);
+ }
+
+ qcom_smgr_register_sensor(smgr, &smgr->sensors[i]);
+ }
+
+ return 0;
+}
+
+static void qcom_smgr_remove(struct qrtr_device *qdev)
+{
+ struct qcom_smgr *smgr = dev_get_drvdata(&qdev->dev);
+
+ qmi_handle_release(&smgr->sns_smgr_hdl);
+}
+
+static const struct qrtr_device_id qcom_smgr_qrtr_match[] = {
+ {
+ .service = SNS_SMGR_QMI_SVC_ID,
+ .instance = QRTR_INSTANCE(SNS_SMGR_QMI_SVC_V1,
+ SNS_SMGR_QMI_INS_ID)
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(qrtr, qcom_smgr_qrtr_match);
+
+static struct qrtr_driver qcom_smgr_driver = {
+ .probe = qcom_smgr_probe,
+ .remove = qcom_smgr_remove,
+ .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 000000000000..e5722f0f8f68
--- /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) += sns_smgr.o
diff --git a/drivers/iio/common/qcom_smgr/qmi/sns_smgr.c b/drivers/iio/common/qcom_smgr/qmi/sns_smgr.c
new file mode 100644
index 000000000000..624e3074a051
--- /dev/null
+++ b/drivers/iio/common/qcom_smgr/qmi/sns_smgr.c
@@ -0,0 +1,711 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * QMI element info arrays and helper functions for Qualcomm Sensor Manager
+ *
+ * Copyright (c) 2021, 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 "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_LICENSE("GPL");
diff --git a/drivers/iio/common/qcom_smgr/qmi/sns_smgr.h b/drivers/iio/common/qcom_smgr/qmi/sns_smgr.h
new file mode 100644
index 000000000000..a741dfd87452
--- /dev/null
+++ b/drivers/iio/common/qcom_smgr/qmi/sns_smgr.h
@@ -0,0 +1,163 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __SSC_SNS_SMGR_H__
+#define __SSC_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_QMI_SVC_V1 1
+#define SNS_SMGR_QMI_INS_ID 50
+
+#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/drivers/iio/gyro/Kconfig b/drivers/iio/gyro/Kconfig
index 3e193ee0fb61..b9ee2c3b5bc5 100644
--- a/drivers/iio/gyro/Kconfig
+++ b/drivers/iio/gyro/Kconfig
@@ -147,6 +147,16 @@ config IIO_ST_GYRO_3AXIS
Also need to enable at least one of I2C and SPI interface drivers
below.
+config IIO_QCOM_SMGR_GYRO
+ tristate "Qualcomm SSC Sensor Manager Gyroscope Sensor"
+ depends on IIO_QCOM_SMGR
+ select IIO_BUFFER
+ select IIO_KFIFO_BUF
+ help
+ Say yes here to get support for gyroscopes connected to
+ a Qualcomm Snapdragon Sensor Core and accessed through its
+ Sensor Manager service.
+
config IIO_ST_GYRO_I2C_3AXIS
tristate "STMicroelectronics gyroscopes 3-Axis I2C Interface"
depends on I2C && IIO_ST_GYRO_3AXIS
diff --git a/drivers/iio/gyro/Makefile b/drivers/iio/gyro/Makefile
index 0319b397dc3f..63ff40acdedc 100644
--- a/drivers/iio/gyro/Makefile
+++ b/drivers/iio/gyro/Makefile
@@ -28,6 +28,8 @@ itg3200-y := itg3200_core.o
itg3200-$(CONFIG_IIO_BUFFER) += itg3200_buffer.o
obj-$(CONFIG_ITG3200) += itg3200.o
+obj-$(CONFIG_IIO_QCOM_SMGR_ACCEL) += qcom_smgr_gyro.o
+
obj-$(CONFIG_IIO_SSP_SENSORS_COMMONS) += ssp_gyro_sensor.o
obj-$(CONFIG_IIO_ST_GYRO_3AXIS) += st_gyro.o
diff --git a/drivers/iio/gyro/qcom_smgr_gyro.c b/drivers/iio/gyro/qcom_smgr_gyro.c
new file mode 100644
index 000000000000..4cb55c6fe9e8
--- /dev/null
+++ b/drivers/iio/gyro/qcom_smgr_gyro.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Qualcomm Sensor Manager gyroscope 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_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 int qcom_smgr_gyro_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-gyro";
+ iio_dev->info = &qcom_smgr_iio_info;
+ iio_dev->channels = qcom_smgr_gyro_iio_channels;
+ iio_dev->num_channels = ARRAY_SIZE(qcom_smgr_gyro_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_gyro_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_gyro_ids[] = {
+ { .name = "qcom-smgr-gyro" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, qcom_smgr_gyro_ids);
+
+static struct platform_driver qcom_smgr_gyro_driver = {
+ .probe = qcom_smgr_gyro_probe,
+ .remove = qcom_smgr_gyro_remove,
+ .driver = {
+ .name = "qcom_smgr_gyro",
+ },
+ .id_table = qcom_smgr_gyro_ids,
+};
+module_platform_driver(qcom_smgr_gyro_driver);
+
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_DESCRIPTION("Qualcomm Sensor Manager gyroscope driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
index 3debf1320ad1..917c123f08bb 100644
--- a/drivers/iio/magnetometer/Kconfig
+++ b/drivers/iio/magnetometer/Kconfig
@@ -139,6 +139,15 @@ config MMC35240
To compile this driver as a module, choose M here: the module
will be called mmc35240.
+config IIO_QCOM_SMGR_MAG
+ tristate "Qualcomm SSC Sensor Manager Magnetometer"
+ depends on IIO_QCOM_SMGR
+ select IIO_BUFFER
+ select IIO_KFIFO_BUF
+ help
+ Say yes here to get support for magnetometers connected to a Qualcomm
+ Snapdragon Sensor Core and accessed through its Sensor Manager service.
+
config IIO_ST_MAGN_3AXIS
tristate "STMicroelectronics magnetometers 3-Axis Driver"
depends on (I2C || SPI_MASTER) && SYSFS
diff --git a/drivers/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
index 9297723a97d8..7576c226239b 100644
--- a/drivers/iio/magnetometer/Makefile
+++ b/drivers/iio/magnetometer/Makefile
@@ -16,6 +16,8 @@ obj-$(CONFIG_MAG3110) += mag3110.o
obj-$(CONFIG_HID_SENSOR_MAGNETOMETER_3D) += hid-sensor-magn-3d.o
obj-$(CONFIG_MMC35240) += mmc35240.o
+obj-$(CONFIG_IIO_QCOM_SMGR_MAG) += qcom_smgr_mag.o
+
obj-$(CONFIG_IIO_ST_MAGN_3AXIS) += st_magn.o
st_magn-y := st_magn_core.o
st_magn-$(CONFIG_IIO_BUFFER) += st_magn_buffer.o
diff --git a/drivers/iio/magnetometer/qcom_smgr_mag.c b/drivers/iio/magnetometer/qcom_smgr_mag.c
new file mode 100644
index 000000000000..dc197db509bc
--- /dev/null
+++ b/drivers/iio/magnetometer/qcom_smgr_mag.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Qualcomm Sensor Manager magnetometer 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_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 int qcom_smgr_mag_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-mag";
+ iio_dev->info = &qcom_smgr_iio_info;
+ iio_dev->channels = qcom_smgr_mag_iio_channels;
+ iio_dev->num_channels = ARRAY_SIZE(qcom_smgr_mag_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_mag_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_mag_ids[] = {
+ { .name = "qcom-smgr-mag" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, qcom_smgr_mag_ids);
+
+static struct platform_driver qcom_smgr_mag_driver = {
+ .probe = qcom_smgr_mag_probe,
+ .remove = qcom_smgr_mag_remove,
+ .driver = {
+ .name = "qcom_smgr_mag",
+ },
+ .id_table = qcom_smgr_mag_ids,
+};
+module_platform_driver(qcom_smgr_mag_driver);
+
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_DESCRIPTION("Qualcomm Sensor Manager magnetometer driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
index d2cb8c871f6a..ca1053472969 100644
--- a/drivers/iio/pressure/Kconfig
+++ b/drivers/iio/pressure/Kconfig
@@ -67,6 +67,16 @@ config IIO_CROS_EC_BARO
To compile this driver as a module, choose M here: the module
will be called cros_ec_baro.
+config IIO_QCOM_SMGR_PRESSURE
+ tristate "Qualcomm SSC Sensor Manager Pressure Sensor"
+ depends on IIO_QCOM_SMGR
+ select IIO_BUFFER
+ select IIO_KFIFO_BUF
+ help
+ Say yes here to get support for pressure sensors connected to
+ a Qualcomm Snapdragon Sensor Core and accessed through its Sensor
+ Manager service.
+
config DLHL60D
tristate "All Sensors DLHL60D and DLHL60G low voltage digital pressure sensors"
depends on I2C
diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile
index 6482288e07ee..0e4a1fbef7b8 100644
--- a/drivers/iio/pressure/Makefile
+++ b/drivers/iio/pressure/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_BMP280_SPI) += bmp280-spi.o
obj-$(CONFIG_DLHL60D) += dlhl60d.o
obj-$(CONFIG_DPS310) += dps310.o
obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o
+obj-$(CONFIG_IIO_QCOM_SMGR_PROX) += qcom_smgr_pressure.o
obj-$(CONFIG_HID_SENSOR_PRESS) += hid-sensor-press.o
obj-$(CONFIG_HP03) += hp03.o
obj-$(CONFIG_HSC030PA) += hsc030pa.o
diff --git a/drivers/iio/pressure/qcom_smgr_pressure.c b/drivers/iio/pressure/qcom_smgr_pressure.c
new file mode 100644
index 000000000000..66e165ad5c9a
--- /dev/null
+++ b/drivers/iio/pressure/qcom_smgr_pressure.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Qualcomm Sensor Manager pressure sensor 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_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_OFFSET) |
+ 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 int qcom_smgr_pressure_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-pressure";
+ iio_dev->info = &qcom_smgr_iio_info;
+ iio_dev->channels = qcom_smgr_pressure_iio_channels;
+ iio_dev->num_channels = ARRAY_SIZE(qcom_smgr_pressure_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_pressure_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_pressure_ids[] = {
+ { .name = "qcom-smgr-pressure" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, qcom_smgr_pressure_ids);
+
+static struct platform_driver qcom_smgr_pressure_driver = {
+ .probe = qcom_smgr_pressure_probe,
+ .remove = qcom_smgr_pressure_remove,
+ .driver = {
+ .name = "qcom_smgr_pressure",
+ },
+ .id_table = qcom_smgr_pressure_ids,
+};
+module_platform_driver(qcom_smgr_pressure_driver);
+
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_DESCRIPTION("Qualcomm Sensor Manager pressure sensor driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index a562a78b7d0d..486514e54cc1 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -46,6 +46,16 @@ config HX9023S
To compile this driver as a module, choose M here: the
module will be called hx9023s.
+config IIO_QCOM_SMGR_PROX
+ tristate "Qualcomm SSC Sensor Manager Proximity Sensor"
+ depends on IIO_QCOM_SMGR
+ select IIO_BUFFER
+ select IIO_KFIFO_BUF
+ help
+ Say yes here to get support for proximity sensors connected to
+ a Qualcomm Snapdragon Sensor Core and accessed through its Sensor
+ Manager service.
+
config IRSD200
tristate "Murata IRS-D200 PIR sensor"
select IIO_BUFFER
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index c5e76995764a..c3dab31e1fc1 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -7,6 +7,7 @@
obj-$(CONFIG_AS3935) += as3935.o
obj-$(CONFIG_CROS_EC_MKBP_PROXIMITY) += cros_ec_mkbp_proximity.o
obj-$(CONFIG_HX9023S) += hx9023s.o
+obj-$(CONFIG_IIO_QCOM_SMGR_PROX) += qcom_smgr_prox.o
obj-$(CONFIG_IRSD200) += irsd200.o
obj-$(CONFIG_ISL29501) += isl29501.o
obj-$(CONFIG_LIDAR_LITE_V2) += pulsedlight-lidar-lite-v2.o
diff --git a/drivers/iio/proximity/qcom_smgr_prox.c b/drivers/iio/proximity/qcom_smgr_prox.c
new file mode 100644
index 000000000000..2900fb7b7a20
--- /dev/null
+++ b/drivers/iio/proximity/qcom_smgr_prox.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Qualcomm Sensor Manager proximity sensor 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_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_OFFSET) |
+ 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 int qcom_smgr_prox_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-prox";
+ iio_dev->info = &qcom_smgr_iio_info;
+ iio_dev->channels = qcom_smgr_prox_iio_channels;
+ iio_dev->num_channels = ARRAY_SIZE(qcom_smgr_prox_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_prox_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_prox_ids[] = {
+ { .name = "qcom-smgr-prox-light" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, qcom_smgr_prox_ids);
+
+static struct platform_driver qcom_smgr_prox_driver = {
+ .probe = qcom_smgr_prox_probe,
+ .remove = qcom_smgr_prox_remove,
+ .driver = {
+ .name = "qcom_smgr_prox",
+ },
+ .id_table = qcom_smgr_prox_ids,
+};
+module_platform_driver(qcom_smgr_prox_driver);
+
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_DESCRIPTION("Qualcomm Sensor Manager proximity sensor driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/iio/common/qcom_smgr.h b/include/linux/iio/common/qcom_smgr.h
new file mode 100644
index 000000000000..756cb83e26ec
--- /dev/null
+++ b/include/linux/iio/common/qcom_smgr.h
@@ -0,0 +1,64 @@
+/* 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/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;
+};
+
+extern const struct iio_buffer_setup_ops qcom_smgr_buffer_ops;
+extern const struct iio_info qcom_smgr_iio_info;
+extern const struct iio_chan_spec_ext_info qcom_smgr_iio_ext_info[];
+
+#endif /* __QCOM_SMGR_H__ */
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] net: qrtr: Turn QRTR into a bus
2025-04-06 14:07 ` [PATCH 1/3] net: qrtr: Turn QRTR into a bus Yassine Oudjana
@ 2025-04-06 16:01 ` Jonathan Cameron
2025-04-10 12:10 ` Yassine Oudjana
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Jonathan Cameron @ 2025-04-06 16:01 UTC (permalink / raw)
To: Yassine Oudjana
Cc: Lars-Peter Clausen, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Sverdlin,
Sean Nyekjaer, Javier Carrasco, Matti Vaittinen, Antoniu Miclaus,
Ramona Gradinariu, Yo-Jung (Leo) Lin, Andy Shevchenko,
Neil Armstrong, Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
On Sun, 06 Apr 2025 14:07:43 +0000
Yassine Oudjana <y.oudjana@protonmail.com> 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.
>
> In smd.c, replace all current occurences of qdev with qsdev in order to
> distinguish between the newly 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>
Hi Yassine
Just took a quick look through.
It might make more sense to do this with an auxiliary_bus rather
than defining a new bus.
I'd also split out the renames as a precursor patch.
Various other comments inline.
Jonathan
> diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
> index 00c51cf693f3..e11682fd7960 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;
How do we get here with non error value given we couldn't before?
> }
> EXPORT_SYMBOL_GPL(qrtr_endpoint_post);
>
> diff --git a/net/qrtr/smd.c b/net/qrtr/smd.c
> index c91bf030fbc7..fd5ad6a8d1c3 100644
> --- a/net/qrtr/smd.c
> +++ b/net/qrtr/smd.c
> @@ -7,6 +7,7 @@
> +
> +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);
> +}
> +void qrtr_driver_unregister(struct qrtr_driver *drv)
> +{
> + driver_unregister(&drv->driver);
> +}
> +EXPORT_SYMBOL_GPL(qrtr_driver_unregister);
Given this is a 'new thing' maybe namespace it from the start?
EXPORT_SYMBOL_NS_GPL();
> +
> +static int qcom_smd_qrtr_match_device_by_port(struct device *dev, const void *data)
> +{
> + struct qrtr_device *qdev = to_qrtr_device(dev);
> + unsigned int port = *(unsigned int *)data;
unsinged int *port = data;
return qdev->port == *port;
> +
> + 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;
> +
Maybe
*qdev = (struct qrtr_device *) {
};
and free new_server after all of these are filled in.
> + qdev->node = new_server->node;
> + qdev->port = new_server->port;
> + qdev->service = new_server->service;
> + qdev->instance = new_server->instance;
> +
> + devm_kfree(qsdev->dev, new_server);
As below.
> +
> + 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;
> + qdev->dev.driver = NULL;
it's kzalloc'd so no need to set this.
> +
> + 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);
> +
> + devm_kfree(qsdev->dev, del_server);
If we are always going to free what was alocated in qcom_smd_qrtr_del_device()
why use devm at all?
> +
> + if (dev)
> + device_unregister(dev);
If this doesn't match anything I'm guessing it's a bug? So maybe an error message?
> +}
> +
> +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(struct qrtr_new_server), GFP_KERNEL);
As below. sizeof(*new_server)
> + if (!new_server)
> + return -ENOMEM;
> +
*new_server = (struct qtr_new_server) {
.parent = qsdev,
.ndoe = node,
...
};
perhaps a tiny bit easier to read?
> + new_server->parent = qsdev;
> + new_server->node = node;
> + new_server->port = port;
> + new_server->service = service;
> + new_server->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(struct qrtr_del_server), GFP_KERNEL);
sizeof(*del_server)
preferred as then no one has to check types match.
> + 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);
One option that may simplify this is to do the device_unregister() handling
a devm_action_or_reset() handler that is using the parent device as it's dev
but unregistering the children. That way the unregister is called in the
reverse order of setup and you only register a handler for those devices
registered (rather walking children). I did this in the CXL pmu driver
for instance.
> +
> + return 0;
> +}
> +
> @@ -82,9 +276,11 @@ 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);
May be worth doing the rename in a precursor patch to simplify a little what is
in this one.
> +
> + device_for_each_child(qsdev->dev, NULL, qcom_smd_qrtr_device_unregister);
>
> - qrtr_endpoint_unregister(&qdev->ep);
> + qrtr_endpoint_unregister(&qsdev->ep);
>
> dev_set_drvdata(&rpdev->dev, NULL);
> }
> @@ -104,7 +300,27 @@ 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)
> + ret = register_rpmsg_driver(&qcom_smd_qrtr_driver);
This style tends to extend badly. Go with more conventional errors
out of line style.
if (ret)
return ret;
ret = register_rpmsg_driver(&qcom_smd_qrtr_driver);
if (ret) {
bus_unregister(&qtr_bus);
return ret;
}
return 0;
> + else
> + bus_unregister(&qrtr_bus);
> +
> + return ret;
> +}
> +
> +static void __exit qcom_smd_qrtr_exit(void)
> +{
> + bus_unregister(&qrtr_bus);
Order should be the reverse of what happened in probe so swap these round.
> + unregister_rpmsg_driver(&qcom_smd_qrtr_driver);
> +}
> +
> +subsys_initcall(qcom_smd_qrtr_init);
> +module_exit(qcom_smd_qrtr_exit);
>
> MODULE_ALIAS("rpmsg:IPCRTR");
> MODULE_DESCRIPTION("Qualcomm IPC-Router SMD interface driver");
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] iio: Add Qualcomm Sensor Manager drivers
2025-04-06 14:08 ` [PATCH 3/3] iio: Add Qualcomm Sensor Manager drivers Yassine Oudjana
@ 2025-04-06 16:29 ` Jonathan Cameron
2025-04-10 12:31 ` Yassine Oudjana
2025-06-18 19:19 ` Luca Weiss
1 sibling, 1 reply; 21+ messages in thread
From: Jonathan Cameron @ 2025-04-06 16:29 UTC (permalink / raw)
To: Yassine Oudjana
Cc: Lars-Peter Clausen, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Sverdlin,
Sean Nyekjaer, Javier Carrasco, Matti Vaittinen, Antoniu Miclaus,
Ramona Gradinariu, Yo-Jung (Leo) Lin, Andy Shevchenko,
Neil Armstrong, Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
On Sun, 06 Apr 2025 14:08:03 +0000
Yassine Oudjana <y.oudjana@protonmail.com> wrote:
> Add drivers 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 | 18 +
> drivers/iio/accel/Kconfig | 10 +
> drivers/iio/accel/Makefile | 2 +
> 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 | 589 ++++++++++++++++
> drivers/iio/common/qcom_smgr/qmi/Makefile | 3 +
> drivers/iio/common/qcom_smgr/qmi/sns_smgr.c | 711 ++++++++++++++++++++
> drivers/iio/common/qcom_smgr/qmi/sns_smgr.h | 163 +++++
> drivers/iio/gyro/Kconfig | 10 +
> drivers/iio/gyro/Makefile | 2 +
> drivers/iio/gyro/qcom_smgr_gyro.c | 138 ++++
> drivers/iio/magnetometer/Kconfig | 9 +
> drivers/iio/magnetometer/Makefile | 2 +
> drivers/iio/magnetometer/qcom_smgr_mag.c | 138 ++++
> drivers/iio/pressure/Kconfig | 10 +
> drivers/iio/pressure/Makefile | 1 +
> drivers/iio/pressure/qcom_smgr_pressure.c | 106 +++
> drivers/iio/proximity/Kconfig | 10 +
> drivers/iio/proximity/Makefile | 1 +
> drivers/iio/proximity/qcom_smgr_prox.c | 106 +++
> include/linux/iio/common/qcom_smgr.h | 64 ++
> 25 files changed, 2257 insertions(+)
Split this up. Common library code first, then
individual drivers making use of it.
> create mode 100644 drivers/iio/accel/qcom_smgr_accel.c
> create mode 100644 drivers/iio/common/qcom_smgr/Kconfig
> create mode 100644 drivers/iio/common/qcom_smgr/Makefile
> create mode 100644 drivers/iio/common/qcom_smgr/qcom_smgr.c
> create mode 100644 drivers/iio/common/qcom_smgr/qmi/Makefile
> create mode 100644 drivers/iio/common/qcom_smgr/qmi/sns_smgr.c
> create mode 100644 drivers/iio/common/qcom_smgr/qmi/sns_smgr.h
> create mode 100644 drivers/iio/gyro/qcom_smgr_gyro.c
> create mode 100644 drivers/iio/magnetometer/qcom_smgr_mag.c
> create mode 100644 drivers/iio/pressure/qcom_smgr_pressure.c
> create mode 100644 drivers/iio/proximity/qcom_smgr_prox.c
> create mode 100644 include/linux/iio/common/qcom_smgr.h
> diff --git a/drivers/iio/accel/qcom_smgr_accel.c b/drivers/iio/accel/qcom_smgr_accel.c
> new file mode 100644
> index 000000000000..ce854312d1d9
> --- /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>
Given ongoing work, a range on that date to go up to this year
probably makes sense!
> + */
> +
> +#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));
For all error message in probe() use
return dev_err_probe(&pdev->dev, ERR_PTR(ret), "Failed to setup buffer\n")
etc.
> + 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)
I'm surprised to see a platform device here - will read on but I
doubt that is the way to go. Maybe an auxbus or similar or
just squashing this all down to be registered directly by
the parent driver.
> +{
> + 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/qcom_smgr/qcom_smgr.c b/drivers/iio/common/qcom_smgr/qcom_smgr.c
> new file mode 100644
> index 000000000000..8d46be11d5b6
> --- /dev/null
> +++ b/drivers/iio/common/qcom_smgr/qcom_smgr.c
> @@ -0,0 +1,589 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Qualcomm Sensor Manager core driver
> + *
> + * Copyright (c) 2021, Yassine Oudjana <y.oudjana@protonmail.com>
As above, I'd add a date range to reflect that this is ongoing.
> + */
> +
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/common/qcom_smgr.h>
> +#include <linux/iio/iio.h>
Be consistent with ordering. Above you have iio as a separate block.
Either option is fine, but not a mixture.
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
Unless there are very strong reasons for of specific code please
use property.h and the generic firmware accessors.
> +#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>
> +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);
> + struct sns_smgr_buffering_report_ind *ind =
> + (struct sns_smgr_buffering_report_ind *)data;
Casting away a const isn't a good sign. Why do you need to do that?
const struct sns_smg_buffer_repor_ind *ind = data;
should be fine I think.
> + struct qcom_smgr_sensor *sensor;
> + u8 i;
> +
> + for (i = 0; i < smgr->sensor_count; ++i) {
> + sensor = &smgr->sensors[i];
> +
> + /* Find sensor matching report */
> + if (sensor->id != ind->report_id)
> + continue;
> +
> + if (!sensor->iio_dev)
> + /* Corresponding driver was unloaded. Ignore remaining reports. */
> + return;
> +
> + /*
> + * Since we are matching report rate with sample rate, we only
> + * get a single sample in every report.
> + */
> + iio_push_to_buffers_with_timestamp(sensor->iio_dev,
> + ind->samples[0].values,
> + ind->metadata.timestamp);
You are using a 64 bit timestamp writer that doesn't know about the endianness of
that timestamp. I'd not do this. Just write the timestamp in like any normal
channel and call iio_push_to_buffers().
> +
> + break;
return;
> + }
> +}
> +
> +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,
> + },
> + {}
{ }
given it's in IIO and I get to pick silly formatting rules.
More seriously I wanted this consistent so picked a choice mostly at random.
> +};
> +
> +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:
> + priv->sensor->data_types[0].cur_sample_rate = val;
> +
> + /*
> + * Send new SMGR buffering request with updated rates
> + * if buffer is enabled
> + */
> + if (iio_buffer_enabled(iio_dev))
> + return iio_dev->setup_ops->postenable(iio_dev);
Generally we'd just refuse to set the sampling frequency.
This is racy as nothing holds the buffer enabled.
So I'd do
if (!iio_device_claim_direct(iio_dev);
return -EBUSY;
priv->sensor->data_types[0].cur_sample_rate = val;
iio_device_release_diect(iio_dev);
Change sampling frequency when doing buffered capture is really confusing
anyway for userspace software as it has no way to know when the
change occurred so just don't bother supporting that.
> +
> + return 0;
> + }
> +
> + return -EINVAL;
Put this in a default in the switch. That makes it clear we don't
expect to see anything else.
Same for other similar cases above.
> +}
> +
> +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);
> + const int samp_freq_vals[3] = {
> + 1, 1, priv->sensor->data_types[0].cur_sample_rate
> + };
Lifetime of this needs to last beyond the end of this call as some users
or read_avail hang on to it. Embed the storage in your priv
structure rather than local in this function. I'm also a little confused
how the maximum comes from something called cur_sample_rate.
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *type = IIO_VAL_INT;
> + *vals = samp_freq_vals;
> + *length = ARRAY_SIZE(samp_freq_vals);
> + return IIO_AVAIL_RANGE;
> + }
> +
> + return -EINVAL;
> +}
...
> +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),
> + {}
trivial but I'm trying to standardize on
{ }
for this in IIO.
> +};
> +EXPORT_SYMBOL_GPL(qcom_smgr_iio_ext_info);
> +
> +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);
On error this handle doesn't seem to be released.
> + if (ret < 0) {
> + dev_err(smgr->dev,
> + "Failed to initialize sensor manager handle: %d\n",
> + ret);
> + return ret;
return dev_err_probe()
Same in all other similar cases that are only called from probe.
> + }
> +
> + ret = qcom_smgr_request_all_sensor_info(smgr, &smgr->sensors);
> + if (ret < 0) {
> + dev_err(smgr->dev, "Failed to get available sensors: %pe\n",
> + ERR_PTR(ret));
> + return ret;
> + }
> + 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) {
> + dev_err(smgr->dev,
> + "Failed to get sensors from ID 0x%02x: %pe\n",
> + smgr->sensors[i].id, ERR_PTR(ret));
> + return ret;
> + }
> +
> + 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);
> + }
> +
> + qcom_smgr_register_sensor(smgr, &smgr->sensors[i]);
Above I suggest that maybe you should just skip the platform devices and register
directly with IIO as you find the sensors. So have the struct iio_dev->device
parent directly off this one.
> + }
> +
> + return 0;
> +}
> +
> +static void qcom_smgr_remove(struct qrtr_device *qdev)
> +{
> + struct qcom_smgr *smgr = dev_get_drvdata(&qdev->dev);
> +
> + qmi_handle_release(&smgr->sns_smgr_hdl);
If that is all you have, use a devm_add_action_or_reset() to
register a handler and drop this remove entirely.
> +}
> +
> +static const struct qrtr_device_id qcom_smgr_qrtr_match[] = {
> + {
> + .service = SNS_SMGR_QMI_SVC_ID,
> + .instance = QRTR_INSTANCE(SNS_SMGR_QMI_SVC_V1,
> + SNS_SMGR_QMI_INS_ID)
> + },
> + {},
{ }
for IIO terminating entries like this.
> +};
> +MODULE_DEVICE_TABLE(qrtr, qcom_smgr_qrtr_match);
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] QRTR bus and Qualcomm Sensor Manager IIO drivers
2025-04-06 14:07 [PATCH 0/3] QRTR bus and Qualcomm Sensor Manager IIO drivers Yassine Oudjana
` (2 preceding siblings ...)
2025-04-06 14:08 ` [PATCH 3/3] iio: Add Qualcomm Sensor Manager drivers Yassine Oudjana
@ 2025-04-08 10:27 ` Luca Weiss
3 siblings, 0 replies; 21+ messages in thread
From: Luca Weiss @ 2025-04-08 10:27 UTC (permalink / raw)
To: Yassine Oudjana, Jonathan Cameron, Lars-Peter Clausen,
Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Sverdlin, Sean Nyekjaer, Javier Carrasco,
Matti Vaittinen, Antoniu Miclaus, Ramona Gradinariu,
Yo-Jung (Leo) Lin, Andy Shevchenko, Neil Armstrong,
Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar
Cc: Yassine Oudjana, linux-kernel, linux-iio, linux-arm-msm, netdev,
linux-kbuild
Hi Yassine,
On Sun Apr 6, 2025 at 4:07 PM CEST, Yassine Oudjana 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 kernel drivers for the Sensor Manager service, exposing
> sensors accessible through it as IIO devices. To facilitate probing of the
> Sensor Manager core 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].
It's awesome to see this work being sent! I remember trying this quite a
while ago, so I definitely need to pick this up again and try it out!
I can try on msm8226, msm8974 and msm8953 so lots of platforms which
will gain sensor support thanks to you!
Regards
Luca
>
> [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
>
> Yassine Oudjana (3):
> 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 drivers
>
> MAINTAINERS | 18 +
> drivers/iio/accel/Kconfig | 10 +
> drivers/iio/accel/Makefile | 2 +
> 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 | 589 ++++++++++++++++
> drivers/iio/common/qcom_smgr/qmi/Makefile | 3 +
> drivers/iio/common/qcom_smgr/qmi/sns_smgr.c | 711 ++++++++++++++++++++
> drivers/iio/common/qcom_smgr/qmi/sns_smgr.h | 163 +++++
> drivers/iio/gyro/Kconfig | 10 +
> drivers/iio/gyro/Makefile | 2 +
> drivers/iio/gyro/qcom_smgr_gyro.c | 138 ++++
> drivers/iio/magnetometer/Kconfig | 9 +
> drivers/iio/magnetometer/Makefile | 2 +
> drivers/iio/magnetometer/qcom_smgr_mag.c | 138 ++++
> drivers/iio/pressure/Kconfig | 10 +
> drivers/iio/pressure/Makefile | 1 +
> drivers/iio/pressure/qcom_smgr_pressure.c | 106 +++
> drivers/iio/proximity/Kconfig | 10 +
> drivers/iio/proximity/Makefile | 1 +
> drivers/iio/proximity/qcom_smgr_prox.c | 106 +++
> drivers/soc/qcom/qmi_interface.c | 5 +-
> include/linux/iio/common/qcom_smgr.h | 64 ++
> 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 | 250 ++++++-
> scripts/mod/devicetable-offsets.c | 4 +
> scripts/mod/file2alias.c | 10 +
> 33 files changed, 2573 insertions(+), 24 deletions(-)
> create mode 100644 drivers/iio/accel/qcom_smgr_accel.c
> create mode 100644 drivers/iio/common/qcom_smgr/Kconfig
> create mode 100644 drivers/iio/common/qcom_smgr/Makefile
> create mode 100644 drivers/iio/common/qcom_smgr/qcom_smgr.c
> create mode 100644 drivers/iio/common/qcom_smgr/qmi/Makefile
> create mode 100644 drivers/iio/common/qcom_smgr/qmi/sns_smgr.c
> create mode 100644 drivers/iio/common/qcom_smgr/qmi/sns_smgr.h
> create mode 100644 drivers/iio/gyro/qcom_smgr_gyro.c
> create mode 100644 drivers/iio/magnetometer/qcom_smgr_mag.c
> create mode 100644 drivers/iio/pressure/qcom_smgr_pressure.c
> create mode 100644 drivers/iio/proximity/qcom_smgr_prox.c
> create mode 100644 include/linux/iio/common/qcom_smgr.h
> create mode 100644 include/linux/soc/qcom/qrtr.h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] net: qrtr: Define macro to convert QMI version and instance to QRTR instance
2025-04-06 14:07 ` [PATCH 2/3] net: qrtr: Define macro to convert QMI version and instance to QRTR instance Yassine Oudjana
@ 2025-04-09 14:54 ` Konrad Dybcio
2025-07-05 18:29 ` Yassine Oudjana
0 siblings, 1 reply; 21+ messages in thread
From: Konrad Dybcio @ 2025-04-09 14:54 UTC (permalink / raw)
To: Yassine Oudjana, Jonathan Cameron, Lars-Peter Clausen,
Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Sverdlin, Sean Nyekjaer, Javier Carrasco,
Matti Vaittinen, Antoniu Miclaus, Ramona Gradinariu,
Yo-Jung (Leo) Lin, Andy Shevchenko, Neil Armstrong,
Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar
Cc: Yassine Oudjana, linux-kernel, linux-iio, linux-arm-msm, netdev,
linux-kbuild
On 4/6/25 4:07 PM, Yassine Oudjana wrote:
> 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 | 2 ++
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/qcom/qmi_interface.c b/drivers/soc/qcom/qmi_interface.c
> index bc6d6379d8b1..cb57b7e1f252 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 4d7f25c64c56..10c89a35cbb9 100644
> --- a/include/linux/soc/qcom/qrtr.h
> +++ b/include/linux/soc/qcom/qrtr.h
> @@ -13,6 +13,8 @@ struct qrtr_device {
>
> #define to_qrtr_device(d) container_of(d, struct qrtr_device, dev)
>
> +#define QRTR_INSTANCE(qmi_version, qmi_instance) (qmi_version | qmi_instance << 8)
Please use FIELD_PREP + GENMASK to avoid potential overflows
Konrad
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] net: qrtr: Turn QRTR into a bus
2025-04-06 16:01 ` Jonathan Cameron
@ 2025-04-10 12:10 ` Yassine Oudjana
2025-04-12 10:58 ` Jonathan Cameron
2025-04-10 12:44 ` Yassine Oudjana
2025-06-25 22:20 ` Yassine Oudjana
2 siblings, 1 reply; 21+ messages in thread
From: Yassine Oudjana @ 2025-04-10 12:10 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Sverdlin,
Sean Nyekjaer, Javier Carrasco, Matti Vaittinen, Antoniu Miclaus,
Ramona Gradinariu, Yo-Jung (Leo) Lin, Andy Shevchenko,
Neil Armstrong, Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
On 06/04/2025 7:01 pm, Jonathan Cameron wrote:
> On Sun, 06 Apr 2025 14:07:43 +0000
> Yassine Oudjana <y.oudjana@protonmail.com> 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.
>>
>> In smd.c, replace all current occurences of qdev with qsdev in order to
>> distinguish between the newly 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>
> Hi Yassine
>
> Just took a quick look through.
>
> It might make more sense to do this with an auxiliary_bus rather
> than defining a new bus.
>
> I'd also split out the renames as a precursor patch.
>
> Various other comments inline.
>
> Jonathan
>
>> diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
>> index 00c51cf693f3..e11682fd7960 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;
> How do we get here with non error value given we couldn't before?
We don't, but we may have errors in ret other than -EINVAL returned by
the newly added add_device and del_device which we should propagate.
>
>
>> }
>> EXPORT_SYMBOL_GPL(qrtr_endpoint_post);
>>
>
>> diff --git a/net/qrtr/smd.c b/net/qrtr/smd.c
>> index c91bf030fbc7..fd5ad6a8d1c3 100644
>> --- a/net/qrtr/smd.c
>> +++ b/net/qrtr/smd.c
>> @@ -7,6 +7,7 @@
>
>> +
>> +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);
>> +}
>
>
>> +void qrtr_driver_unregister(struct qrtr_driver *drv)
>> +{
>> + driver_unregister(&drv->driver);
>> +}
>> +EXPORT_SYMBOL_GPL(qrtr_driver_unregister);
>
> Given this is a 'new thing' maybe namespace it from the start?
> EXPORT_SYMBOL_NS_GPL();
Ack.
>
>
>> +
>> +static int qcom_smd_qrtr_match_device_by_port(struct device *dev, const void *data)
>> +{
>> + struct qrtr_device *qdev = to_qrtr_device(dev);
>> + unsigned int port = *(unsigned int *)data;
> unsinged int *port = data;
>
> return qdev->port == *port;
>
Ack.
>> +
>> + 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;
>> +
> Maybe
> *qdev = (struct qrtr_device *) {
> };
(struct qrtr_device)
...
> and free new_server after all of these are filled in.
>
>> + qdev->node = new_server->node;
>> + qdev->port = new_server->port;
>> + qdev->service = new_server->service;
>> + qdev->instance = new_server->instance;
>> +
>> + devm_kfree(qsdev->dev, new_server);
>
> As below.
Ack.
>
>> +
>> + 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;
>> + qdev->dev.driver = NULL;
>
> it's kzalloc'd so no need to set this.
Ack.
>
>> +
>> + 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);
>> +
>> + devm_kfree(qsdev->dev, del_server);
> If we are always going to free what was alocated in qcom_smd_qrtr_del_device()
> why use devm at all?
True, I guess this one is redundant.
>> +
>> + if (dev)
>> + device_unregister(dev);
> If this doesn't match anything I'm guessing it's a bug? So maybe an error message?
I don't quite remember the reason I put this check in the first place
but right now it seems to me like it should always be true so I'll just
remove it and see what happens.
>
>> +}
>> +
>> +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(struct qrtr_new_server), GFP_KERNEL);
>
> As below. sizeof(*new_server)
>
>> + if (!new_server)
>> + return -ENOMEM;
>> +
> *new_server = (struct qtr_new_server) {
> .parent = qsdev,
> .ndoe = node,
> ...
> };
>
> perhaps a tiny bit easier to read?
Ack.
>
>> + new_server->parent = qsdev;
>> + new_server->node = node;
>> + new_server->port = port;
>> + new_server->service = service;
>> + new_server->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(struct qrtr_del_server), GFP_KERNEL);
>
> sizeof(*del_server)
> preferred as then no one has to check types match.
Ack.
>
>> + 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);
>
> One option that may simplify this is to do the device_unregister() handling
> a devm_action_or_reset() handler that is using the parent device as it's dev
> but unregistering the children. That way the unregister is called in the
> reverse order of setup and you only register a handler for those devices
> registered (rather walking children). I did this in the CXL pmu driver
> for instance.
Will look into it.
>
>> +
>> + return 0;
>> +}
>> +
>
>> @@ -82,9 +276,11 @@ 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);
>
> May be worth doing the rename in a precursor patch to simplify a little what is
> in this one.
Sure.
>
>> +
>> + device_for_each_child(qsdev->dev, NULL, qcom_smd_qrtr_device_unregister);
>>
>> - qrtr_endpoint_unregister(&qdev->ep);
>> + qrtr_endpoint_unregister(&qsdev->ep);
>>
>> dev_set_drvdata(&rpdev->dev, NULL);
>> }
>> @@ -104,7 +300,27 @@ 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)
>> + ret = register_rpmsg_driver(&qcom_smd_qrtr_driver);
> This style tends to extend badly. Go with more conventional errors
> out of line style.
>
> if (ret)
> return ret;
>
> ret = register_rpmsg_driver(&qcom_smd_qrtr_driver);
> if (ret) {
> bus_unregister(&qtr_bus);
> return ret;
> }
>
> return 0;
>
Ack.
>> + else
>> + bus_unregister(&qrtr_bus);
>> +
>> + return ret;
>> +}
>> +
>> +static void __exit qcom_smd_qrtr_exit(void)
>> +{
>> + bus_unregister(&qrtr_bus);
>
> Order should be the reverse of what happened in probe so swap these round.
Ack.
>
>> + unregister_rpmsg_driver(&qcom_smd_qrtr_driver);
>> +}
>> +
>> +subsys_initcall(qcom_smd_qrtr_init);
>> +module_exit(qcom_smd_qrtr_exit);
>>
>> MODULE_ALIAS("rpmsg:IPCRTR");
>> MODULE_DESCRIPTION("Qualcomm IPC-Router SMD interface driver");
>>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] iio: Add Qualcomm Sensor Manager drivers
2025-04-06 16:29 ` Jonathan Cameron
@ 2025-04-10 12:31 ` Yassine Oudjana
2025-04-12 11:21 ` Jonathan Cameron
0 siblings, 1 reply; 21+ messages in thread
From: Yassine Oudjana @ 2025-04-10 12:31 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Sverdlin,
Sean Nyekjaer, Javier Carrasco, Matti Vaittinen, Antoniu Miclaus,
Ramona Gradinariu, Yo-Jung (Leo) Lin, Andy Shevchenko,
Neil Armstrong, Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
On 06/04/2025 7:29 pm, Jonathan Cameron wrote:
> On Sun, 06 Apr 2025 14:08:03 +0000
> Yassine Oudjana <y.oudjana@protonmail.com> wrote:
>
>> Add drivers 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 | 18 +
>> drivers/iio/accel/Kconfig | 10 +
>> drivers/iio/accel/Makefile | 2 +
>> 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 | 589 ++++++++++++++++
>> drivers/iio/common/qcom_smgr/qmi/Makefile | 3 +
>> drivers/iio/common/qcom_smgr/qmi/sns_smgr.c | 711 ++++++++++++++++++++
>> drivers/iio/common/qcom_smgr/qmi/sns_smgr.h | 163 +++++
>> drivers/iio/gyro/Kconfig | 10 +
>> drivers/iio/gyro/Makefile | 2 +
>> drivers/iio/gyro/qcom_smgr_gyro.c | 138 ++++
>> drivers/iio/magnetometer/Kconfig | 9 +
>> drivers/iio/magnetometer/Makefile | 2 +
>> drivers/iio/magnetometer/qcom_smgr_mag.c | 138 ++++
>> drivers/iio/pressure/Kconfig | 10 +
>> drivers/iio/pressure/Makefile | 1 +
>> drivers/iio/pressure/qcom_smgr_pressure.c | 106 +++
>> drivers/iio/proximity/Kconfig | 10 +
>> drivers/iio/proximity/Makefile | 1 +
>> drivers/iio/proximity/qcom_smgr_prox.c | 106 +++
>> include/linux/iio/common/qcom_smgr.h | 64 ++
>> 25 files changed, 2257 insertions(+)
> Split this up. Common library code first, then
> individual drivers making use of it.
Ack.
>
>
>> create mode 100644 drivers/iio/accel/qcom_smgr_accel.c
>> create mode 100644 drivers/iio/common/qcom_smgr/Kconfig
>> create mode 100644 drivers/iio/common/qcom_smgr/Makefile
>> create mode 100644 drivers/iio/common/qcom_smgr/qcom_smgr.c
>> create mode 100644 drivers/iio/common/qcom_smgr/qmi/Makefile
>> create mode 100644 drivers/iio/common/qcom_smgr/qmi/sns_smgr.c
>> create mode 100644 drivers/iio/common/qcom_smgr/qmi/sns_smgr.h
>> create mode 100644 drivers/iio/gyro/qcom_smgr_gyro.c
>> create mode 100644 drivers/iio/magnetometer/qcom_smgr_mag.c
>> create mode 100644 drivers/iio/pressure/qcom_smgr_pressure.c
>> create mode 100644 drivers/iio/proximity/qcom_smgr_prox.c
>> create mode 100644 include/linux/iio/common/qcom_smgr.h
>
>
>> diff --git a/drivers/iio/accel/qcom_smgr_accel.c b/drivers/iio/accel/qcom_smgr_accel.c
>> new file mode 100644
>> index 000000000000..ce854312d1d9
>> --- /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>
> Given ongoing work, a range on that date to go up to this year
> probably makes sense!
Yeah
>
>> + */
>> +
>> +#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));
> For all error message in probe() use
> return dev_err_probe(&pdev->dev, ERR_PTR(ret), "Failed to setup buffer\n")
> etc.
Ack.
>
>> + 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)
>
> I'm surprised to see a platform device here - will read on but I
> doubt that is the way to go. Maybe an auxbus or similar or
> just squashing this all down to be registered directly by
> the parent driver.
I got the idea from cros_ec_sensors which also deals with a similar
sensor hub paradigm.
>
>
>> +{
>> + 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/qcom_smgr/qcom_smgr.c b/drivers/iio/common/qcom_smgr/qcom_smgr.c
>> new file mode 100644
>> index 000000000000..8d46be11d5b6
>> --- /dev/null
>> +++ b/drivers/iio/common/qcom_smgr/qcom_smgr.c
>> @@ -0,0 +1,589 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Qualcomm Sensor Manager core driver
>> + *
>> + * Copyright (c) 2021, Yassine Oudjana <y.oudjana@protonmail.com>
>
> As above, I'd add a date range to reflect that this is ongoing.
Ack.
>
>> + */
>> +
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/common/qcom_smgr.h>
>> +#include <linux/iio/iio.h>
>
> Be consistent with ordering. Above you have iio as a separate block.
> Either option is fine, but not a mixture.
Ack.
>
>> +#include <linux/math64.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
> Unless there are very strong reasons for of specific code please
> use property.h and the generic firmware accessors.
Ack.
>
>> +#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>
>
>
>> +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);
>> + struct sns_smgr_buffering_report_ind *ind =
>> + (struct sns_smgr_buffering_report_ind *)data;
>
> Casting away a const isn't a good sign. Why do you need to do that?
> const struct sns_smg_buffer_repor_ind *ind = data;
> should be fine I think.
The casted struct was previously not const so I was only casting from
void *. I made it const lately but didn't notice this cast. Will change it.
>
>
>> + struct qcom_smgr_sensor *sensor;
>> + u8 i;
>> +
>> + for (i = 0; i < smgr->sensor_count; ++i) {
>> + sensor = &smgr->sensors[i];
>> +
>> + /* Find sensor matching report */
>> + if (sensor->id != ind->report_id)
>> + continue;
>> +
>> + if (!sensor->iio_dev)
>> + /* Corresponding driver was unloaded. Ignore remaining reports. */
>> + return;
>> +
>> + /*
>> + * Since we are matching report rate with sample rate, we only
>> + * get a single sample in every report.
>> + */
>> + iio_push_to_buffers_with_timestamp(sensor->iio_dev,
>> + ind->samples[0].values,
>> + ind->metadata.timestamp);
> You are using a 64 bit timestamp writer that doesn't know about the endianness of
> that timestamp. I'd not do this. Just write the timestamp in like any normal
> channel and call iio_push_to_buffers().
Will look into it.
>
>> +
>> + break;
>
> return;
>
>> + }
>> +}
>> +
>> +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,
>> + },
>> + {}
> { }
>
> given it's in IIO and I get to pick silly formatting rules.
> More seriously I wanted this consistent so picked a choice mostly at random.
No objections on my side.
>
>> +};
>
>> +
>> +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:
>> + priv->sensor->data_types[0].cur_sample_rate = val;
>> +
>> + /*
>> + * Send new SMGR buffering request with updated rates
>> + * if buffer is enabled
>> + */
>> + if (iio_buffer_enabled(iio_dev))
>> + return iio_dev->setup_ops->postenable(iio_dev);
>
> Generally we'd just refuse to set the sampling frequency.
> This is racy as nothing holds the buffer enabled.
> So I'd do
> if (!iio_device_claim_direct(iio_dev);
> return -EBUSY;
>
> priv->sensor->data_types[0].cur_sample_rate = val;
> iio_device_release_diect(iio_dev);
>
> Change sampling frequency when doing buffered capture is really confusing
> anyway for userspace software as it has no way to know when the
> change occurred so just don't bother supporting that.
Makes sense.
>
>> +
>> + return 0;
>> + }
>> +
>> + return -EINVAL;
> Put this in a default in the switch. That makes it clear we don't
> expect to see anything else.
> Same for other similar cases above.
Ack.
>
>> +}
>> +
>> +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);
>> + const int samp_freq_vals[3] = {
>> + 1, 1, priv->sensor->data_types[0].cur_sample_rate
>> + };
>
> Lifetime of this needs to last beyond the end of this call as some users
> or read_avail hang on to it. Embed the storage in your priv
> structure rather than local in this function.
Ack.
> I'm also a little confused how the maximum comes from something called cur_sample_rate.
This was max_sample_rate previously but at some point while trying to
understand how the sampling rate works on the SMGR side I removed
max_sample_rate and replaced it here with cur_sample_rate without
understanding what I was changing. When I brought back max_sample_rate I
missed this change. Will change it back.
>
>> +
>> + switch (mask) {
>> + case IIO_CHAN_INFO_SAMP_FREQ:
>> + *type = IIO_VAL_INT;
>> + *vals = samp_freq_vals;
>> + *length = ARRAY_SIZE(samp_freq_vals);
>> + return IIO_AVAIL_RANGE;
>> + }
>> +
>> + return -EINVAL;
>> +}
>
> ...
>
>> +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),
>> + {}
> trivial but I'm trying to standardize on
> { }
> for this in IIO.
Ack.
>
>> +};
>> +EXPORT_SYMBOL_GPL(qcom_smgr_iio_ext_info);
>> +
>> +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);
> On error this handle doesn't seem to be released.
Will fix.
>
>> + if (ret < 0) {
>> + dev_err(smgr->dev,
>> + "Failed to initialize sensor manager handle: %d\n",
>> + ret);
>> + return ret;
>
> return dev_err_probe()
> Same in all other similar cases that are only called from probe.
Ack.
>
>> + }
>> +
>> + ret = qcom_smgr_request_all_sensor_info(smgr, &smgr->sensors);
>> + if (ret < 0) {
>> + dev_err(smgr->dev, "Failed to get available sensors: %pe\n",
>> + ERR_PTR(ret));
>> + return ret;
>> + }
>> + 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) {
>> + dev_err(smgr->dev,
>> + "Failed to get sensors from ID 0x%02x: %pe\n",
>> + smgr->sensors[i].id, ERR_PTR(ret));
>> + return ret;
>> + }
>> +
>> + 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);
>> + }
>> +
>> + qcom_smgr_register_sensor(smgr, &smgr->sensors[i]);
> Above I suggest that maybe you should just skip the platform devices and register
> directly with IIO as you find the sensors. So have the struct iio_dev->device
> parent directly off this one.
As I said previously I followed the model used in cros_ec_sensors, and
it made sense to me since I always see platform devices used to
represent firmware-backed devices like this.
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void qcom_smgr_remove(struct qrtr_device *qdev)
>> +{
>> + struct qcom_smgr *smgr = dev_get_drvdata(&qdev->dev);
>> +
>> + qmi_handle_release(&smgr->sns_smgr_hdl);
> If that is all you have, use a devm_add_action_or_reset() to
> register a handler and drop this remove entirely.
Ack.
>
>> +}
>> +
>> +static const struct qrtr_device_id qcom_smgr_qrtr_match[] = {
>> + {
>> + .service = SNS_SMGR_QMI_SVC_ID,
>> + .instance = QRTR_INSTANCE(SNS_SMGR_QMI_SVC_V1,
>> + SNS_SMGR_QMI_INS_ID)
>> + },
>> + {},
> { }
>
> for IIO terminating entries like this.
Ack.
>
>> +};
>> +MODULE_DEVICE_TABLE(qrtr, qcom_smgr_qrtr_match);
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] net: qrtr: Turn QRTR into a bus
2025-04-06 16:01 ` Jonathan Cameron
2025-04-10 12:10 ` Yassine Oudjana
@ 2025-04-10 12:44 ` Yassine Oudjana
2025-04-12 10:59 ` Jonathan Cameron
2025-06-25 22:20 ` Yassine Oudjana
2 siblings, 1 reply; 21+ messages in thread
From: Yassine Oudjana @ 2025-04-10 12:44 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Sverdlin,
Sean Nyekjaer, Javier Carrasco, Matti Vaittinen, Antoniu Miclaus,
Ramona Gradinariu, Yo-Jung (Leo) Lin, Andy Shevchenko,
Neil Armstrong, Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
Missed one comment so sending a second reply.
On 06/04/2025 7:01 pm, Jonathan Cameron wrote:
> On Sun, 06 Apr 2025 14:07:43 +0000
> Yassine Oudjana <y.oudjana@protonmail.com> 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.
>>
>> In smd.c, replace all current occurences of qdev with qsdev in order to
>> distinguish between the newly 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>
> Hi Yassine
>
> Just took a quick look through.
>
> It might make more sense to do this with an auxiliary_bus rather
> than defining a new bus.
I'm not familiar with auxiliary bus, but reading the documentation it
seems to me like it's used like MFD where there is a device that has
multiple functions, just without the subdevices having physical
addresses. QRTR is not really a device but more closely resembles
something like PCI or I2C as a communication interface.
>
> I'd also split out the renames as a precursor patch.
>
> Various other comments inline.
>
> Jonathan
>
>> diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
>> index 00c51cf693f3..e11682fd7960 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;
> How do we get here with non error value given we couldn't before?
>
>
>> }
>> EXPORT_SYMBOL_GPL(qrtr_endpoint_post);
>>
>
>> diff --git a/net/qrtr/smd.c b/net/qrtr/smd.c
>> index c91bf030fbc7..fd5ad6a8d1c3 100644
>> --- a/net/qrtr/smd.c
>> +++ b/net/qrtr/smd.c
>> @@ -7,6 +7,7 @@
>
>> +
>> +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);
>> +}
>
>
>> +void qrtr_driver_unregister(struct qrtr_driver *drv)
>> +{
>> + driver_unregister(&drv->driver);
>> +}
>> +EXPORT_SYMBOL_GPL(qrtr_driver_unregister);
>
> Given this is a 'new thing' maybe namespace it from the start?
> EXPORT_SYMBOL_NS_GPL();
>
>
>> +
>> +static int qcom_smd_qrtr_match_device_by_port(struct device *dev, const void *data)
>> +{
>> + struct qrtr_device *qdev = to_qrtr_device(dev);
>> + unsigned int port = *(unsigned int *)data;
> unsinged int *port = data;
>
> return qdev->port == *port;
>
>> +
>> + 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;
>> +
> Maybe
> *qdev = (struct qrtr_device *) {
> };
> and free new_server after all of these are filled in.
>
>> + qdev->node = new_server->node;
>> + qdev->port = new_server->port;
>> + qdev->service = new_server->service;
>> + qdev->instance = new_server->instance;
>> +
>> + devm_kfree(qsdev->dev, new_server);
>
> As below.
>
>> +
>> + 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;
>> + qdev->dev.driver = NULL;
>
> it's kzalloc'd so no need to set this.
>
>> +
>> + 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);
>> +
>> + devm_kfree(qsdev->dev, del_server);
> If we are always going to free what was alocated in qcom_smd_qrtr_del_device()
> why use devm at all?
>> +
>> + if (dev)
>> + device_unregister(dev);
> If this doesn't match anything I'm guessing it's a bug? So maybe an error message?
>
>> +}
>> +
>> +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(struct qrtr_new_server), GFP_KERNEL);
>
> As below. sizeof(*new_server)
>
>> + if (!new_server)
>> + return -ENOMEM;
>> +
> *new_server = (struct qtr_new_server) {
> .parent = qsdev,
> .ndoe = node,
> ...
> };
>
> perhaps a tiny bit easier to read?
>
>> + new_server->parent = qsdev;
>> + new_server->node = node;
>> + new_server->port = port;
>> + new_server->service = service;
>> + new_server->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(struct qrtr_del_server), GFP_KERNEL);
>
> sizeof(*del_server)
> preferred as then no one has to check types match.
>
>> + 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);
>
> One option that may simplify this is to do the device_unregister() handling
> a devm_action_or_reset() handler that is using the parent device as it's dev
> but unregistering the children. That way the unregister is called in the
> reverse order of setup and you only register a handler for those devices
> registered (rather walking children). I did this in the CXL pmu driver
> for instance.
>
>> +
>> + return 0;
>> +}
>> +
>
>> @@ -82,9 +276,11 @@ 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);
>
> May be worth doing the rename in a precursor patch to simplify a little what is
> in this one.
>
>> +
>> + device_for_each_child(qsdev->dev, NULL, qcom_smd_qrtr_device_unregister);
>>
>> - qrtr_endpoint_unregister(&qdev->ep);
>> + qrtr_endpoint_unregister(&qsdev->ep);
>>
>> dev_set_drvdata(&rpdev->dev, NULL);
>> }
>> @@ -104,7 +300,27 @@ 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)
>> + ret = register_rpmsg_driver(&qcom_smd_qrtr_driver);
> This style tends to extend badly. Go with more conventional errors
> out of line style.
>
> if (ret)
> return ret;
>
> ret = register_rpmsg_driver(&qcom_smd_qrtr_driver);
> if (ret) {
> bus_unregister(&qtr_bus);
> return ret;
> }
>
> return 0;
>
>> + else
>> + bus_unregister(&qrtr_bus);
>> +
>> + return ret;
>> +}
>> +
>> +static void __exit qcom_smd_qrtr_exit(void)
>> +{
>> + bus_unregister(&qrtr_bus);
>
> Order should be the reverse of what happened in probe so swap these round.
>
>> + unregister_rpmsg_driver(&qcom_smd_qrtr_driver);
>> +}
>> +
>> +subsys_initcall(qcom_smd_qrtr_init);
>> +module_exit(qcom_smd_qrtr_exit);
>>
>> MODULE_ALIAS("rpmsg:IPCRTR");
>> MODULE_DESCRIPTION("Qualcomm IPC-Router SMD interface driver");
>>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] net: qrtr: Turn QRTR into a bus
2025-04-10 12:10 ` Yassine Oudjana
@ 2025-04-12 10:58 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2025-04-12 10:58 UTC (permalink / raw)
To: Yassine Oudjana
Cc: Lars-Peter Clausen, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Sverdlin,
Sean Nyekjaer, Javier Carrasco, Matti Vaittinen, Antoniu Miclaus,
Ramona Gradinariu, Yo-Jung (Leo) Lin, Andy Shevchenko,
Neil Armstrong, Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
On Thu, 10 Apr 2025 12:10:54 +0000
Yassine Oudjana <y.oudjana@protonmail.com> wrote:
> On 06/04/2025 7:01 pm, Jonathan Cameron wrote:
> > On Sun, 06 Apr 2025 14:07:43 +0000
> > Yassine Oudjana <y.oudjana@protonmail.com> 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.
> >>
> >> In smd.c, replace all current occurences of qdev with qsdev in order to
> >> distinguish between the newly 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>
> > Hi Yassine
> >
> > Just took a quick look through.
> >
> > It might make more sense to do this with an auxiliary_bus rather
> > than defining a new bus.
> >
> > I'd also split out the renames as a precursor patch.
> >
> > Various other comments inline.
> >
> > Jonathan
> >
> >> diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
> >> index 00c51cf693f3..e11682fd7960 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;
> > How do we get here with non error value given we couldn't before?
>
> We don't, but we may have errors in ret other than -EINVAL returned by
> the newly added add_device and del_device which we should propagate.
Ah. Got it (I misread that!). Personally I'd go for setting ret in the
other error paths explicitly to -EINVAL. Mixing two styles of handling
where you have some paths setting ret and some not is rather confusing to read.
> >> +
> >> + 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;
> >> +
> > Maybe
> > *qdev = (struct qrtr_device *) {
> > };
>
> (struct qrtr_device)
oops. Indeed that!
Jonathan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] net: qrtr: Turn QRTR into a bus
2025-04-10 12:44 ` Yassine Oudjana
@ 2025-04-12 10:59 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2025-04-12 10:59 UTC (permalink / raw)
To: Yassine Oudjana
Cc: Lars-Peter Clausen, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Sverdlin,
Sean Nyekjaer, Javier Carrasco, Matti Vaittinen, Antoniu Miclaus,
Ramona Gradinariu, Yo-Jung (Leo) Lin, Andy Shevchenko,
Neil Armstrong, Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
On Thu, 10 Apr 2025 12:44:25 +0000
Yassine Oudjana <y.oudjana@protonmail.com> wrote:
> Missed one comment so sending a second reply.
>
> On 06/04/2025 7:01 pm, Jonathan Cameron wrote:
> > On Sun, 06 Apr 2025 14:07:43 +0000
> > Yassine Oudjana <y.oudjana@protonmail.com> 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.
> >>
> >> In smd.c, replace all current occurences of qdev with qsdev in order to
> >> distinguish between the newly 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>
> > Hi Yassine
> >
> > Just took a quick look through.
> >
> > It might make more sense to do this with an auxiliary_bus rather
> > than defining a new bus.
>
> I'm not familiar with auxiliary bus, but reading the documentation it
> seems to me like it's used like MFD where there is a device that has
> multiple functions, just without the subdevices having physical
> addresses. QRTR is not really a device but more closely resembles
> something like PCI or I2C as a communication interface.
Fair enough. If this has real bus like properties then it may make
sense to go with a new explicit bus.
Jonathan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] iio: Add Qualcomm Sensor Manager drivers
2025-04-10 12:31 ` Yassine Oudjana
@ 2025-04-12 11:21 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2025-04-12 11:21 UTC (permalink / raw)
To: Yassine Oudjana
Cc: Lars-Peter Clausen, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Sverdlin,
Sean Nyekjaer, Javier Carrasco, Matti Vaittinen, Antoniu Miclaus,
Ramona Gradinariu, Yo-Jung (Leo) Lin, Andy Shevchenko,
Neil Armstrong, Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
> >> +
> >> +static void qcom_smgr_accel_remove(struct platform_device *pdev)
> >
> > I'm surprised to see a platform device here - will read on but I
> > doubt that is the way to go. Maybe an auxbus or similar or
> > just squashing this all down to be registered directly by
> > the parent driver.
> I got the idea from cros_ec_sensors which also deals with a similar
> sensor hub paradigm.
Generally the use of platform drivers for subfunctions of something
is now not considered the way to go. Here there seems to be little
point in spinning out another layer of devices.
> >
> >> +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);
> >> + struct sns_smgr_buffering_report_ind *ind =
> >> + (struct sns_smgr_buffering_report_ind *)data;
> >
> > Casting away a const isn't a good sign. Why do you need to do that?
> > const struct sns_smg_buffer_repor_ind *ind = data;
> > should be fine I think.
>
> The casted struct was previously not const so I was only casting from
> void *. I made it const lately but didn't notice this cast. Will change it.
Ok. But never a reason to cast from a void *. The C spec says that
happens implicitly just fine.
> >> + ret = qcom_smgr_request_all_sensor_info(smgr, &smgr->sensors);
> >> + if (ret < 0) {
> >> + dev_err(smgr->dev, "Failed to get available sensors: %pe\n",
> >> + ERR_PTR(ret));
> >> + return ret;
> >> + }
> >> + 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) {
> >> + dev_err(smgr->dev,
> >> + "Failed to get sensors from ID 0x%02x: %pe\n",
> >> + smgr->sensors[i].id, ERR_PTR(ret));
> >> + return ret;
> >> + }
> >> +
> >> + 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);
> >> + }
> >> +
> >> + qcom_smgr_register_sensor(smgr, &smgr->sensors[i]);
> > Above I suggest that maybe you should just skip the platform devices and register
> > directly with IIO as you find the sensors. So have the struct iio_dev->device
> > parent directly off this one.
>
> As I said previously I followed the model used in cros_ec_sensors, and
> it made sense to me since I always see platform devices used to
> represent firmware-backed devices like this.
In this case you end up with
parent device
|
|____________________
| | |
ChildA ChildB ChildC (all platform devices)
| | |
IIODevA IIODevB IIODEVC
Today we'd probably do those child devices using auxiliary devices but
aside from that, the only reason to do this is you want to have separate
drivers for each child.
You can just do
parent device
|
|_______________________
| | |
IIODevA IIODevB IIODevC
for the case where there no separate child drivers.
That is the parent driver can just instantiate the IIO Bus Devices
directly (it's kind of a Class really but a bus for historical reasons).
Various IMUs do this when they have separately controlled sampling frequencies
for different types of sensor or even separate fifos. (They are really
sensor hubs, just connected of SPI or similar).
So it's up to you whether you want the separate per channel type devices and
to handle the potential races around those going away.
qcom_smgr_buffering_report_handler() for instance needs a lock to
stop the child driver being unbound between the checks on iio_dev and the
use of it. With one driver that complexity doesn't occur.
See for example drivers/iio/st_lsm6dsx/ that does it this way.
Also possible would be a single IIO device with multiple buffers.
We only have a few of those though so you might run into missing bits of ABI
taking that path.
Jonathan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] iio: Add Qualcomm Sensor Manager drivers
2025-04-06 14:08 ` [PATCH 3/3] iio: Add Qualcomm Sensor Manager drivers Yassine Oudjana
2025-04-06 16:29 ` Jonathan Cameron
@ 2025-06-18 19:19 ` Luca Weiss
2025-06-25 17:09 ` Yassine Oudjana
1 sibling, 1 reply; 21+ messages in thread
From: Luca Weiss @ 2025-06-18 19:19 UTC (permalink / raw)
To: Yassine Oudjana, Jonathan Cameron, Lars-Peter Clausen,
Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Sverdlin, Sean Nyekjaer, Javier Carrasco,
Matti Vaittinen, Antoniu Miclaus, Ramona Gradinariu,
Yo-Jung (Leo) Lin, Andy Shevchenko, Neil Armstrong,
Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar
Cc: Yassine Oudjana, linux-kernel, linux-iio, linux-arm-msm, netdev,
linux-kbuild
Hi Yassine!
On 06-04-2025 4:08 p.m., Yassine Oudjana wrote:
> Add drivers 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>
<snip>
> +static const char *const qcom_smgr_sensor_type_platform_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 void qcom_smgr_unregister_sensor(void *data)
> +{
> + struct platform_device *pdev = data;
> +
> + platform_device_unregister(pdev);
> +}
> +
> +static int qcom_smgr_register_sensor(struct qcom_smgr *smgr,
> + struct qcom_smgr_sensor *sensor)
> +{
> + struct platform_device *pdev;
> + const char *name = qcom_smgr_sensor_type_platform_names[sensor->type];
On msm8226 lg-lenok I get NULL here leading to a crash with the next call.
I get sensor->type=0 for some heart rate sensor on that watch. I've
added this patch on top to fix that (excuse the formatting):
<snip>
> diff --git a/drivers/iio/common/qcom_smgr/qmi/sns_smgr.h b/drivers/iio/common/qcom_smgr/qmi/sns_smgr.h
> new file mode 100644
> index 000000000000..a741dfd87452
> --- /dev/null
> +++ b/drivers/iio/common/qcom_smgr/qmi/sns_smgr.h
> @@ -0,0 +1,163 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __SSC_SNS_SMGR_H__
> +#define __SSC_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_QMI_SVC_V1 1
> +#define SNS_SMGR_QMI_INS_ID 50
This instance ID needs to be 0 on msm8974 and msm8226, so I assume we
don't want to make this a define but just add the 50 and the 0 as-is to
the match table?
Regards
Luca
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] iio: Add Qualcomm Sensor Manager drivers
2025-06-18 19:19 ` Luca Weiss
@ 2025-06-25 17:09 ` Yassine Oudjana
0 siblings, 0 replies; 21+ messages in thread
From: Yassine Oudjana @ 2025-06-25 17:09 UTC (permalink / raw)
To: Luca Weiss
Cc: Jonathan Cameron, Lars-Peter Clausen, Bjorn Andersson,
Konrad Dybcio, Manivannan Sadhasivam, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Sverdlin, Sean Nyekjaer, Javier Carrasco,
Matti Vaittinen, Antoniu Miclaus, Ramona Gradinariu,
Yo-Jung (Leo) Lin, Andy Shevchenko, Neil Armstrong,
Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
On Wednesday, June 18th, 2025 at 8:19 PM, Luca Weiss <luca@lucaweiss.eu> wrote:
> Hi Yassine!
>
> On 06-04-2025 4:08 p.m., Yassine Oudjana wrote:
>
> > Add drivers 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
>
>
> <snip>
>
> > +static const char *const qcom_smgr_sensor_type_platform_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 void qcom_smgr_unregister_sensor(void *data)
> > +{
> > + struct platform_device *pdev = data;
> > +
> > + platform_device_unregister(pdev);
> > +}
> > +
> > +static int qcom_smgr_register_sensor(struct qcom_smgr *smgr,
> > + struct qcom_smgr_sensor *sensor)
> > +{
> > + struct platform_device *pdev;
> > + const char *name = qcom_smgr_sensor_type_platform_names[sensor->type];
>
>
> On msm8226 lg-lenok I get NULL here leading to a crash with the next call.
>
> I get sensor->type=0 for some heart rate sensor on that watch. I've
>
> added this patch on top to fix that (excuse the formatting):
I don't see your patch, but I already have a fix and will include it in the next
iteration.
>
> <snip>
>
> > diff --git a/drivers/iio/common/qcom_smgr/qmi/sns_smgr.h b/drivers/iio/common/qcom_smgr/qmi/sns_smgr.h
> > new file mode 100644
> > index 000000000000..a741dfd87452
> > --- /dev/null
> > +++ b/drivers/iio/common/qcom_smgr/qmi/sns_smgr.h
> > @@ -0,0 +1,163 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only /
> > +
> > +#ifndef SSC_SNS_SMGR_H
> > +#define SSC_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_QMI_SVC_V1 1
> > +#define SNS_SMGR_QMI_INS_ID 50
>
> This instance ID needs to be 0 on msm8974 and msm8226, so I assume we
> don't want to make this a define but just add the 50 and the 0 as-is to
> the match table?
Yes that is a better idea.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] net: qrtr: Turn QRTR into a bus
2025-04-06 16:01 ` Jonathan Cameron
2025-04-10 12:10 ` Yassine Oudjana
2025-04-10 12:44 ` Yassine Oudjana
@ 2025-06-25 22:20 ` Yassine Oudjana
2 siblings, 0 replies; 21+ messages in thread
From: Yassine Oudjana @ 2025-06-25 22:20 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Bjorn Andersson, Konrad Dybcio,
Manivannan Sadhasivam, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Sverdlin,
Sean Nyekjaer, Javier Carrasco, Matti Vaittinen, Antoniu Miclaus,
Ramona Gradinariu, Yo-Jung (Leo) Lin, Andy Shevchenko,
Neil Armstrong, Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
On Sunday, April 6th, 2025 at 5:01 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On Sun, 06 Apr 2025 14:07:43 +0000
> Yassine Oudjana y.oudjana@protonmail.com 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.
> >
> > In smd.c, replace all current occurences of qdev with qsdev in order to
> > distinguish between the newly 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
>
> Hi Yassine
>
> Just took a quick look through.
>
> It might make more sense to do this with an auxiliary_bus rather
> than defining a new bus.
>
> I'd also split out the renames as a precursor patch.
>
> Various other comments inline.
>
> Jonathan
<...>
> > + 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);
>
>
> One option that may simplify this is to do the device_unregister() handling
> a devm_action_or_reset() handler that is using the parent device as it's dev
> but unregistering the children. That way the unregister is called in the
> reverse order of setup and you only register a handler for those devices
> registered (rather walking children). I did this in the CXL pmu driver
> for instance.
Not sure I understand this correctly. This function is called for all children when
the parent (the bus) is removed in order to unregister them, so its called for all
registered devices under the parent. It's just a wrapper for device_unregister so
that it can be used with device_for_each_child. If I register a handler with
devm_add_action_or_reset using the parent device then it seems to me like I will
have to add a new function used as handler for that which in turn goes over the
children and unregisters them (we always unregister all children since the parent
will be no more) then I will only be adding an extra layer. I checked the CXL PMU
driver but I only found devm_add_action_or_reset used for cleaning up objects
associated with the device, not removing child devices.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] net: qrtr: Define macro to convert QMI version and instance to QRTR instance
2025-04-09 14:54 ` Konrad Dybcio
@ 2025-07-05 18:29 ` Yassine Oudjana
2025-07-07 17:06 ` Simon Horman
0 siblings, 1 reply; 21+ messages in thread
From: Yassine Oudjana @ 2025-07-05 18:29 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Jonathan Cameron, Lars-Peter Clausen, Bjorn Andersson,
Konrad Dybcio, Manivannan Sadhasivam, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Sverdlin, Sean Nyekjaer, Javier Carrasco,
Matti Vaittinen, Antoniu Miclaus, Ramona Gradinariu,
Yo-Jung (Leo) Lin, Andy Shevchenko, Neil Armstrong,
Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
On Wednesday, April 9th, 2025 at 3:54 PM, Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote:
> On 4/6/25 4:07 PM, Yassine Oudjana wrote:
>
> > 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 | 2 ++
> > 2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/soc/qcom/qmi_interface.c b/drivers/soc/qcom/qmi_interface.c
> > index bc6d6379d8b1..cb57b7e1f252 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 4d7f25c64c56..10c89a35cbb9 100644
> > --- a/include/linux/soc/qcom/qrtr.h
> > +++ b/include/linux/soc/qcom/qrtr.h
> > @@ -13,6 +13,8 @@ struct qrtr_device {
> >
> > #define to_qrtr_device(d) container_of(d, struct qrtr_device, dev)
> >
> > +#define QRTR_INSTANCE(qmi_version, qmi_instance) (qmi_version | qmi_instance << 8)
>
>
> Please use FIELD_PREP + GENMASK to avoid potential overflows
>
> Konrad
Since I'm using this macro in initializing QRTR match tables I am unable to use
FIELD_PREP. When I do, I get such errors:
In file included from ../arch/arm64/include/asm/sysreg.h:1108,
from ../arch/arm64/include/asm/memory.h:223,
from ../arch/arm64/include/asm/pgtable-prot.h:8,
from ../arch/arm64/include/asm/sparsemem.h:8,
from ../include/linux/numa.h:23,
from ../include/linux/cpumask.h:17,
from ../include/linux/smp.h:13,
from ../include/linux/lockdep.h:14,
from ../include/linux/mutex.h:17,
from ../include/linux/kernfs.h:11,
from ../include/linux/sysfs.h:16,
from ../include/linux/iio/buffer.h:9,
from ../drivers/iio/common/qcom_smgr/qcom_smgr.c:8:
../include/linux/bitfield.h:114:9: error: braced-group within expression allowed only inside a function
114 | ({ \
| ^
../include/linux/soc/qcom/qrtr.h:21:10: note: in expansion of macro 'FIELD_PREP'
21 | (FIELD_PREP(GENMASK(7, 0), qmi_version) | FIELD_PREP(GENMASK(15, 8), qmi_instance))
| ^~~~~~~~~~
../drivers/iio/common/qcom_smgr/qcom_smgr.c:825:29: note: in expansion of macro 'QRTR_INSTANCE'
825 | .instance = QRTR_INSTANCE(SNS_SMGR_QMI_SVC_V1,
| ^~~~~~~~~~~~~
../include/linux/bitfield.h:114:9: error: braced-group within expression allowed only inside a function
114 | ({ \
| ^
../include/linux/soc/qcom/qrtr.h:21:51: note: in expansion of macro 'FIELD_PREP'
21 | (FIELD_PREP(GENMASK(7, 0), qmi_version) | FIELD_PREP(GENMASK(15, 8), qmi_instance))
| ^~~~~~~~~~
../drivers/iio/common/qcom_smgr/qcom_smgr.c:825:29: note: in expansion of macro 'QRTR_INSTANCE'
825 | .instance = QRTR_INSTANCE(SNS_SMGR_QMI_SVC_V1,
| ^~~~~~~~~~~~~
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] net: qrtr: Define macro to convert QMI version and instance to QRTR instance
2025-07-05 18:29 ` Yassine Oudjana
@ 2025-07-07 17:06 ` Simon Horman
2025-07-09 7:44 ` Yassine Oudjana
0 siblings, 1 reply; 21+ messages in thread
From: Simon Horman @ 2025-07-07 17:06 UTC (permalink / raw)
To: Yassine Oudjana
Cc: Konrad Dybcio, Jonathan Cameron, Lars-Peter Clausen,
Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Sverdlin, Sean Nyekjaer, Javier Carrasco,
Matti Vaittinen, Antoniu Miclaus, Ramona Gradinariu,
Yo-Jung (Leo) Lin, Andy Shevchenko, Neil Armstrong,
Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
On Sat, Jul 05, 2025 at 06:29:39PM +0000, Yassine Oudjana wrote:
> On Wednesday, April 9th, 2025 at 3:54 PM, Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote:
>
> > On 4/6/25 4:07 PM, Yassine Oudjana wrote:
...
> > > diff --git a/include/linux/soc/qcom/qrtr.h b/include/linux/soc/qcom/qrtr.h
> > > index 4d7f25c64c56..10c89a35cbb9 100644
> > > --- a/include/linux/soc/qcom/qrtr.h
> > > +++ b/include/linux/soc/qcom/qrtr.h
> > > @@ -13,6 +13,8 @@ struct qrtr_device {
> > >
> > > #define to_qrtr_device(d) container_of(d, struct qrtr_device, dev)
> > >
> > > +#define QRTR_INSTANCE(qmi_version, qmi_instance) (qmi_version | qmi_instance << 8)
> >
> >
> > Please use FIELD_PREP + GENMASK to avoid potential overflows
> >
> > Konrad
>
> Since I'm using this macro in initializing QRTR match tables I am unable to use
> FIELD_PREP. When I do, I get such errors:
Does using FIELD_PREP_CONST, say in a QRTR_INSTANCE_CONST variant, help?
>
> In file included from ../arch/arm64/include/asm/sysreg.h:1108,
> from ../arch/arm64/include/asm/memory.h:223,
> from ../arch/arm64/include/asm/pgtable-prot.h:8,
> from ../arch/arm64/include/asm/sparsemem.h:8,
> from ../include/linux/numa.h:23,
> from ../include/linux/cpumask.h:17,
> from ../include/linux/smp.h:13,
> from ../include/linux/lockdep.h:14,
> from ../include/linux/mutex.h:17,
> from ../include/linux/kernfs.h:11,
> from ../include/linux/sysfs.h:16,
> from ../include/linux/iio/buffer.h:9,
> from ../drivers/iio/common/qcom_smgr/qcom_smgr.c:8:
> ../include/linux/bitfield.h:114:9: error: braced-group within expression allowed only inside a function
> 114 | ({ \
> | ^
> ../include/linux/soc/qcom/qrtr.h:21:10: note: in expansion of macro 'FIELD_PREP'
> 21 | (FIELD_PREP(GENMASK(7, 0), qmi_version) | FIELD_PREP(GENMASK(15, 8), qmi_instance))
> | ^~~~~~~~~~
> ../drivers/iio/common/qcom_smgr/qcom_smgr.c:825:29: note: in expansion of macro 'QRTR_INSTANCE'
> 825 | .instance = QRTR_INSTANCE(SNS_SMGR_QMI_SVC_V1,
> | ^~~~~~~~~~~~~
...
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] net: qrtr: Define macro to convert QMI version and instance to QRTR instance
2025-07-07 17:06 ` Simon Horman
@ 2025-07-09 7:44 ` Yassine Oudjana
2025-07-09 11:52 ` Simon Horman
0 siblings, 1 reply; 21+ messages in thread
From: Yassine Oudjana @ 2025-07-09 7:44 UTC (permalink / raw)
To: Simon Horman
Cc: Konrad Dybcio, Jonathan Cameron, Lars-Peter Clausen,
Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Sverdlin, Sean Nyekjaer, Javier Carrasco,
Matti Vaittinen, Antoniu Miclaus, Ramona Gradinariu,
Yo-Jung (Leo) Lin, Andy Shevchenko, Neil Armstrong,
Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
Sent with Proton Mail secure email.
On Monday, July 7th, 2025 at 6:06 PM, Simon Horman <horms@kernel.org> wrote:
> On Sat, Jul 05, 2025 at 06:29:39PM +0000, Yassine Oudjana wrote:
>
> > On Wednesday, April 9th, 2025 at 3:54 PM, Konrad Dybcio konrad.dybcio@oss.qualcomm.com wrote:
> >
> > > On 4/6/25 4:07 PM, Yassine Oudjana wrote:
>
>
> ...
>
> > > > diff --git a/include/linux/soc/qcom/qrtr.h b/include/linux/soc/qcom/qrtr.h
> > > > index 4d7f25c64c56..10c89a35cbb9 100644
> > > > --- a/include/linux/soc/qcom/qrtr.h
> > > > +++ b/include/linux/soc/qcom/qrtr.h
> > > > @@ -13,6 +13,8 @@ struct qrtr_device {
> > > >
> > > > #define to_qrtr_device(d) container_of(d, struct qrtr_device, dev)
> > > >
> > > > +#define QRTR_INSTANCE(qmi_version, qmi_instance) (qmi_version | qmi_instance << 8)
> > >
> > > Please use FIELD_PREP + GENMASK to avoid potential overflows
> > >
> > > Konrad
> >
> > Since I'm using this macro in initializing QRTR match tables I am unable to use
> > FIELD_PREP. When I do, I get such errors:
>
>
> Does using FIELD_PREP_CONST, say in a QRTR_INSTANCE_CONST variant, help?
That works, but do we want to have two variants? Or in this case maybe
I should leave qmi_interface.c untouched and define the macro only for use
in match tables?
>
> > In file included from ../arch/arm64/include/asm/sysreg.h:1108,
> > from ../arch/arm64/include/asm/memory.h:223,
> > from ../arch/arm64/include/asm/pgtable-prot.h:8,
> > from ../arch/arm64/include/asm/sparsemem.h:8,
> > from ../include/linux/numa.h:23,
> > from ../include/linux/cpumask.h:17,
> > from ../include/linux/smp.h:13,
> > from ../include/linux/lockdep.h:14,
> > from ../include/linux/mutex.h:17,
> > from ../include/linux/kernfs.h:11,
> > from ../include/linux/sysfs.h:16,
> > from ../include/linux/iio/buffer.h:9,
> > from ../drivers/iio/common/qcom_smgr/qcom_smgr.c:8:
> > ../include/linux/bitfield.h:114:9: error: braced-group within expression allowed only inside a function
> > 114 | ({ \
> > | ^
> > ../include/linux/soc/qcom/qrtr.h:21:10: note: in expansion of macro 'FIELD_PREP'
> > 21 | (FIELD_PREP(GENMASK(7, 0), qmi_version) | FIELD_PREP(GENMASK(15, 8), qmi_instance))
> > | ^~~~~~~~~~
> > ../drivers/iio/common/qcom_smgr/qcom_smgr.c:825:29: note: in expansion of macro 'QRTR_INSTANCE'
> > 825 | .instance = QRTR_INSTANCE(SNS_SMGR_QMI_SVC_V1,
> > | ^~~~~~~~~~~~~
>
>
> ...
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] net: qrtr: Define macro to convert QMI version and instance to QRTR instance
2025-07-09 7:44 ` Yassine Oudjana
@ 2025-07-09 11:52 ` Simon Horman
0 siblings, 0 replies; 21+ messages in thread
From: Simon Horman @ 2025-07-09 11:52 UTC (permalink / raw)
To: Yassine Oudjana
Cc: Konrad Dybcio, Jonathan Cameron, Lars-Peter Clausen,
Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Sverdlin, Sean Nyekjaer, Javier Carrasco,
Matti Vaittinen, Antoniu Miclaus, Ramona Gradinariu,
Yo-Jung (Leo) Lin, Andy Shevchenko, Neil Armstrong,
Barnabás Czémán, Danila Tikhonov,
Antoni Pokusinski, Vasileios Amoiridis, Petar Stoykov,
shuaijie wang, Yasin Lee, Borislav Petkov (AMD), Dave Hansen,
Tony Luck, Pawan Gupta, Ingo Molnar, Yassine Oudjana,
linux-kernel, linux-iio, linux-arm-msm, netdev, linux-kbuild
On Wed, Jul 09, 2025 at 07:44:49AM +0000, Yassine Oudjana wrote:
>
>
>
>
>
> Sent with Proton Mail secure email.
>
> On Monday, July 7th, 2025 at 6:06 PM, Simon Horman <horms@kernel.org> wrote:
>
> > On Sat, Jul 05, 2025 at 06:29:39PM +0000, Yassine Oudjana wrote:
> >
> > > On Wednesday, April 9th, 2025 at 3:54 PM, Konrad Dybcio konrad.dybcio@oss.qualcomm.com wrote:
> > >
> > > > On 4/6/25 4:07 PM, Yassine Oudjana wrote:
> >
> >
> > ...
> >
> > > > > diff --git a/include/linux/soc/qcom/qrtr.h b/include/linux/soc/qcom/qrtr.h
> > > > > index 4d7f25c64c56..10c89a35cbb9 100644
> > > > > --- a/include/linux/soc/qcom/qrtr.h
> > > > > +++ b/include/linux/soc/qcom/qrtr.h
> > > > > @@ -13,6 +13,8 @@ struct qrtr_device {
> > > > >
> > > > > #define to_qrtr_device(d) container_of(d, struct qrtr_device, dev)
> > > > >
> > > > > +#define QRTR_INSTANCE(qmi_version, qmi_instance) (qmi_version | qmi_instance << 8)
> > > >
> > > > Please use FIELD_PREP + GENMASK to avoid potential overflows
> > > >
> > > > Konrad
> > >
> > > Since I'm using this macro in initializing QRTR match tables I am unable to use
> > > FIELD_PREP. When I do, I get such errors:
> >
> >
> > Does using FIELD_PREP_CONST, say in a QRTR_INSTANCE_CONST variant, help?
>
> That works, but do we want to have two variants? Or in this case maybe
> I should leave qmi_interface.c untouched and define the macro only for use
> in match tables?
FWIIW, my order of preference would be:
1. Two variants, declared next to each other
2. One variant (using FIELD_PREP_CONST)
3. Leave qmi_interface.c untouched
...
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-07-09 11:52 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-06 14:07 [PATCH 0/3] QRTR bus and Qualcomm Sensor Manager IIO drivers Yassine Oudjana
2025-04-06 14:07 ` [PATCH 1/3] net: qrtr: Turn QRTR into a bus Yassine Oudjana
2025-04-06 16:01 ` Jonathan Cameron
2025-04-10 12:10 ` Yassine Oudjana
2025-04-12 10:58 ` Jonathan Cameron
2025-04-10 12:44 ` Yassine Oudjana
2025-04-12 10:59 ` Jonathan Cameron
2025-06-25 22:20 ` Yassine Oudjana
2025-04-06 14:07 ` [PATCH 2/3] net: qrtr: Define macro to convert QMI version and instance to QRTR instance Yassine Oudjana
2025-04-09 14:54 ` Konrad Dybcio
2025-07-05 18:29 ` Yassine Oudjana
2025-07-07 17:06 ` Simon Horman
2025-07-09 7:44 ` Yassine Oudjana
2025-07-09 11:52 ` Simon Horman
2025-04-06 14:08 ` [PATCH 3/3] iio: Add Qualcomm Sensor Manager drivers Yassine Oudjana
2025-04-06 16:29 ` Jonathan Cameron
2025-04-10 12:31 ` Yassine Oudjana
2025-04-12 11:21 ` Jonathan Cameron
2025-06-18 19:19 ` Luca Weiss
2025-06-25 17:09 ` Yassine Oudjana
2025-04-08 10:27 ` [PATCH 0/3] QRTR bus and Qualcomm Sensor Manager IIO drivers Luca Weiss
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).