Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LIS2DUX12/LIS2DUXS12 driver create badly named event files #20

Open
lorenzodallacasa opened this issue Jun 12, 2024 · 1 comment
Open

Comments

@lorenzodallacasa
Copy link

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

@mariotesi
Copy link
Contributor

Hi,

Yes we know the problem in fact we added a message here in the Kconfig.

Unfortunately the Linux IIO framework still not support all event types that our MEMS sensors are capable to generate so in order to have this event working an IIO patch is requested for extending the iio_chan_type_name_spec array to our custom types names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants