st-mems-android-linux-drivers-iio icon indicating copy to clipboard operation
st-mems-android-linux-drivers-iio copied to clipboard

LIS2DUX12/LIS2DUXS12 driver create badly named event files

Open lorenzodallacasa opened this issue 1 year ago • 1 comments

Hi, I'm using LIS2DUX12 accelerometer in a Linux environment with kernel version 5.10 and I'm testing event monitoring with iio_event_monitor tool. I managed to verify the functionality of a few events (TAP, DTAP, FF and MLC) but I couldn't test the behavior of the SIGN_MOTION event because I'm having a hard time trying to enable that event from C code. This is because the file responsible to track the event enable/disable status has a particularly bad name: in my case it is in_???'L0_thresh_rising_en. This made me realize that a few other event files were also incorrect because they are named like in_(null)0_thresh_rising_en.

So I decided to take a look inside the kernel and the driver to better understand what is going on and I found out that the driver is using custom enum type values to initialize the structure struct iio_chan_spec of these sensor.

/* stm_iio_types.h */
/* Linux IIO driver custom types */
enum {
	STM_IIO_LAST = 0x3f,
	STM_IIO_SIGN_MOTION = STM_IIO_LAST - 6,
	STM_IIO_STEP_COUNTER = STM_IIO_LAST - 5,
	STM_IIO_TILT = STM_IIO_LAST - 4,
	STM_IIO_TAP = STM_IIO_LAST - 3,
	STM_IIO_TAP_TAP = STM_IIO_LAST - 2,
	STM_IIO_WRIST_TILT_GESTURE = STM_IIO_LAST - 1,
	STM_IIO_GESTURE = STM_IIO_LAST,
};
/* st_lis2duxs12.h */
#define ST_LIS2DUXS12_EVENT_CHANNEL(ctype, etype)	\
{							\
	.type = ctype,					\
	.modified = 0,					\
	.scan_index = -1,				\
	.indexed = -1,					\
	.event_spec = &st_lis2duxs12_##etype##_event,	\
	.num_event_specs = 1,				\
}
/* st_lis2duxs12_basicfunc.c */
static const struct iio_chan_spec st_lis2duxs12_ff_channels[] = {
	ST_LIS2DUXS12_EVENT_CHANNEL(STM_IIO_GESTURE, thr),
	IIO_CHAN_SOFT_TIMESTAMP(1),
};
static const struct iio_chan_spec st_lis2duxs12_tap_channels[] = {
	ST_LIS2DUXS12_EVENT_CHANNEL(STM_IIO_TAP, thr),
	IIO_CHAN_SOFT_TIMESTAMP(1),
};
static const struct iio_chan_spec st_lis2duxs12_dtap_channels[] = {
	ST_LIS2DUXS12_EVENT_CHANNEL(STM_IIO_TAP_TAP, thr),
	IIO_CHAN_SOFT_TIMESTAMP(1),
};
static const struct iio_chan_spec st_lis2duxs12_ttap_channels[] = {
	ST_LIS2DUXS12_EVENT_CHANNEL(STM_IIO_GESTURE, thr),
	IIO_CHAN_SOFT_TIMESTAMP(1),
};

For my understanding, this is a problem because when the kernel generates the event names for these sensors/devices, it tries to get a char pointer from the iio_chan_type_name_spec[] variable that has only IIO_MASSCONCENTRATION (=34) items which is less than the first STM custom type STM_IIO_SIGN_MOTION (=57). So our index/type is out of bounds.

/* industrial-core.c */
static const char * const iio_chan_type_name_spec[] = {
	[IIO_VOLTAGE] = "voltage",
	[IIO_CURRENT] = "current",
	[IIO_POWER] = "power",
	[IIO_ACCEL] = "accel",
	[IIO_ANGL_VEL] = "anglvel",
	[IIO_MAGN] = "magn",
	[IIO_LIGHT] = "illuminance",
	[IIO_INTENSITY] = "intensity",
	[IIO_PROXIMITY] = "proximity",
	[IIO_TEMP] = "temp",
	[IIO_INCLI] = "incli",
	[IIO_ROT] = "rot",
	[IIO_ANGL] = "angl",
	[IIO_TIMESTAMP] = "timestamp",
	[IIO_CAPACITANCE] = "capacitance",
	[IIO_ALTVOLTAGE] = "altvoltage",
	[IIO_CCT] = "cct",
	[IIO_PRESSURE] = "pressure",
	[IIO_HUMIDITYRELATIVE] = "humidityrelative",
	[IIO_ACTIVITY] = "activity",
	[IIO_STEPS] = "steps",
	[IIO_ENERGY] = "energy",
	[IIO_DISTANCE] = "distance",
	[IIO_VELOCITY] = "velocity",
	[IIO_CONCENTRATION] = "concentration",
	[IIO_RESISTANCE] = "resistance",
	[IIO_PH] = "ph",
	[IIO_UVINDEX] = "uvindex",
	[IIO_ELECTRICALCONDUCTIVITY] = "electricalconductivity",
	[IIO_COUNT] = "count",
	[IIO_INDEX] = "index",
	[IIO_GRAVITY]  = "gravity",
	[IIO_POSITIONRELATIVE]  = "positionrelative",
	[IIO_PHASE] = "phase",
	[IIO_MASSCONCENTRATION] = "massconcentration",
};

I think this is the point where the empty string messes things up:

/* industrial-core.c */
/* int __iio_device_attr_init() */
case IIO_SEPARATE:
	if (chan->indexed)
		name = kasprintf(GFP_KERNEL, "%s_%s%d_%s",
				iio_direction[chan->output],
				iio_chan_type_name_spec[chan->type],
				chan->channel,
				full_postfix);

Am I understanding this behavior correctly? How can we fix this?

Thanks

lorenzodallacasa avatar Jun 12 '24 13:06 lorenzodallacasa