Re: [PATCH 3/3] iio: Add Qualcomm Sensor Manager drivers

[Yassine Oudjana]

On Wednesday, June 18th, 2025 at 8:19 PM, Luca Weiss <luca@xxxxxxxxxxxx> 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@xxxxxxxxxxxxxx
> 
> 
> <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.