On Sun, 15 Sep 2019 14:24:57 +0200 Sean Nyekjaer <sean@xxxxxxxxxx> wrote: > On 15/09/2019 14.20, Jonathan Cameron wrote: > > On Mon, 9 Sep 2019 13:51:13 +0200 > > Sean Nyekjaer <sean@xxxxxxxxxx> wrote: > > > >> On 09/09/2019 13.28, Sean Nyekjaer wrote: > >>> Report iio motion events to iio subsystem > >>> > >>> Signed-off-by: Sean Nyekjaer <sean@xxxxxxxxxx> > >>> --- > >>> Changes since v4: > >>> * Updated bitmask as pr Jonathans comments > >>> > >>> Changes since v5: > >>> * None > >>> > >>> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 5 ++ > >>> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 70 ++++++++++++++++++++ > >>> 2 files changed, 75 insertions(+) > >>> > >> [...] > >>> > >>> +void st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw, int data) > >>> +{ > >>> + s64 timestamp = iio_get_time_ns(hw->iio_devs[ST_LSM6DSX_ID_ACC]); > >>> + > >>> + if (data & hw->settings->event_settings.wakeup_src_z_mask) > >>> + iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC], > >>> + IIO_MOD_EVENT_CODE(IIO_ACCEL, > >>> + 0, > >>> + IIO_MOD_Z, > >>> + IIO_EV_TYPE_THRESH, > >>> + IIO_EV_DIR_EITHER), > >>> + timestamp); > >>> + > >>> + if (data & hw->settings->event_settings.wakeup_src_x_mask) > >>> + iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC], > >>> + IIO_MOD_EVENT_CODE(IIO_ACCEL, > >>> + 0, > >>> + IIO_MOD_Y, > >>> + IIO_EV_TYPE_THRESH, > >>> + IIO_EV_DIR_EITHER), > >>> + timestamp); > >>> + > >>> + if (data & hw->settings->event_settings.wakeup_src_x_mask) > >>> + iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC], > >>> + IIO_MOD_EVENT_CODE(IIO_ACCEL, > >>> + 0, > >>> + IIO_MOD_X, > >>> + IIO_EV_TYPE_THRESH, > >>> + IIO_EV_DIR_EITHER), > >>> + timestamp); > >>> +} > >>> + > >> > >> I was looking at this again, and if the user enables events for channel > >> x, we continue to report events for y, z. > >> Is it okay or is it better to filter them out? > > Better to filter them out. It'll be a bit of a surprise for userspace > > otherwise. > > > > Thanks, > > > > Jonathan > > > Okay, but keep in mind that we can't distinguish which channel we're > waking up to. So even if some channel is disabled, we still wake up on > it ... Hmm. Alternative is to not filter it, but make sure that the enable and disable interfaces treat all of them as one control. In theory I think the ABI would allow for events/in_accel_thresh_both_en in a similar fashion to the shared_by_type of the info elements for the channel, but given I'm not sure anything implements it, it's likely to be a hole in userspace code. We do have devices that do in_accel_thresh_both_en when the events are rising and falling separately but they two can't be enabled independently. This is kind of similar to that but on the channel rather than the direction. Jonathan > > /Sean