| Message ID | 20241015092035.10482-1-hardevsinh.palaniya@siliconsignals.io (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: imu: inv_icm42600: Enable Pedometer Functionality | expand |
Hi Hardevsinh, kernel test robot noticed the following build warnings: [auto build test WARNING on jic23-iio/togreg] [also build test WARNING on linus/master v6.12-rc3 next-20241016] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Hardevsinh-Palaniya/iio-imu-inv_icm42600-Enable-Pedometer-Functionality/20241015-172227 base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg patch link: https://lore.kernel.org/r/20241015092035.10482-1-hardevsinh.palaniya%40siliconsignals.io patch subject: [PATCH] iio: imu: inv_icm42600: Enable Pedometer Functionality config: i386-randconfig-062-20241016 (https://download.01.org/0day-ci/archive/20241016/202410161606.EbqeKmdm-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241016/202410161606.EbqeKmdm-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410161606.EbqeKmdm-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c:704:21: sparse: sparse: incorrect type in assignment (different base types) @@ expected int @@ got restricted __le16 [addressable] [usertype] steps @@ drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c:704:21: sparse: expected int drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c:704:21: sparse: got restricted __le16 [addressable] [usertype] steps vim +704 drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c 685 686 static int inv_icm42600_steps_read_raw(struct iio_dev *indio_dev, 687 struct iio_chan_spec const *chan, 688 int *val, int *val2, long mask) 689 { 690 struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev); 691 __le16 steps; 692 int ret; 693 694 if (mask == IIO_CHAN_INFO_PROCESSED) { 695 ret = iio_device_claim_direct_mode(indio_dev); 696 if (ret) 697 return ret; 698 ret = regmap_bulk_read(st->map, INV_ICM42600_REG_APEX_DATA, &steps, sizeof(steps)); 699 if (ret) 700 return ret; 701 iio_device_release_direct_mode(indio_dev); 702 if (ret) 703 return ret; > 704 *val = steps; 705 return IIO_VAL_INT; 706 } 707 708 return -EINVAL; 709 } 710
Hello, thanks for the patch, but for me there are multiple important issues there. You cannot prevent runtime_suspend to turn the chip off since it is its purpose. If you want to keep chip on when pedometer is enabled, you need to use pm_runtime_resume_and_get() when turning pedometer on, and pm_runtime_mark_last_busy()+pm_runtime_put_autosuspend() when turning pedometer off. This way you ensure the chip stays on while pedometer is turned on. The 2nd important issue, is the ODR frequency for accel with pedometer. Pedometer requires accel to run at 50Hz minimum, otherwise it will not work correctly. I think we need to enforce this inside the driver, otherwise userspace may not understand why pedometer is not working correctly. We could prevent pedometer from running if sampling_frequency is below 50Hz, or force frequency to 50Hz. And when pedometer is running, preventing ODR switch to lower than 50Hz. Anyway, I'm currently working on adding Wake-on-Motion (WoM) feature to the driver. WoM can also be used with pedometer to reduce power consumption. It would be better to postpone this pedometer work and wait for WoM integration before adding pedometer support. Thanks, JB
Hi Jean, Thank you for your suggestions. >thanks for the patch, but for me there are multiple important issues there. > >You cannot prevent runtime_suspend to turn the chip off since it is its purpose. If you want to keep chip on when pedometer is enabled, you need to use >pm_runtime_resume_and_get() when turning pedometer on, and pm_runtime_mark_last_busy()+pm_runtime_put_autosuspend() when turning pedometer off. >This way you ensure the chip stays on while pedometer is turned on. I agree, will change it >The 2nd important issue, is the ODR frequency for accel with pedometer. Pedometer requires accel to run at 50Hz minimum, otherwise it will not work correctly. >I think we need to enforce this inside the driver, otherwise userspace may not understand why pedometer is not working correctly. We could prevent >pedometer from running if sampling_frequency is below 50Hz, or force frequency to 50Hz. And when pedometer is running, preventing ODR switch to lower >than 50Hz. That makes sense. it could create problems >Anyway, I'm currently working on adding Wake-on-Motion (WoM) feature to the driver. WoM can also be used with pedometer to reduce power consumption. >It would be better to postpone this pedometer work and wait for WoM integration before adding pedometer support. That's great to hear Could you please let me know when the integration will be completed? I am also working on the tilt functionality, which is nearly finished. Should I continue with this work and submit a patch for both the pedometer and tilt features after the WoM integration, or would you recommend dropping this plan? Best Regards, Hardev
On Tue, 15 Oct 2024 14:50:03 +0530 Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io> wrote: > Enables pedometer functionality in the ICM42605 IMU sensor. > > The pedometer feature allows for step counting, which is accessible through > a new sysfs entry. Interrupts are triggered when a step event occurs, enabling > step event detection. > > Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io> Some additional comments from a quick read. > --- > drivers/iio/imu/inv_icm42600/inv_icm42600.h | 16 ++ > .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 165 ++++++++++++++++++ > .../iio/imu/inv_icm42600/inv_icm42600_core.c | 36 +++- > 3 files changed, 211 insertions(+), 6 deletions(-) > > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h > index 3a07e43e4cf1..c3471b73152e 100644 > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h > @@ -122,6 +122,7 @@ struct inv_icm42600_sensor_conf { > int filter; > }; > #define INV_ICM42600_SENSOR_CONF_INIT {-1, -1, -1, -1} > +#define INV_ICM42600_SENSOR_CONF_APEX { 2, 0, 9, 6} > > struct inv_icm42600_conf { > struct inv_icm42600_sensor_conf gyro; > @@ -141,6 +142,8 @@ struct inv_icm42600_suspended { > * @chip: chip identifier. > * @name: chip name. > * @map: regmap pointer. > + * @pedometer_enable status of pedometer function > + * @pedometer_value status of steps event occurnce Check the kernel-doc style. Even better run the script over the files you are touching. You are missing : here. > * @vdd_supply: VDD voltage regulator for the chip. > * @vddio_supply: I/O voltage regulator for the chip. > * @orientation: sensor chip orientation relative to main hardware. > @@ -157,6 +160,8 @@ struct inv_icm42600_state { > enum inv_icm42600_chip chip; > const char *name; > struct regmap *map; > + bool pedometer_enable; > + bool pedometer_value; > struct regulator *vdd_supply; > struct regulator *vddio_supply; > struct iio_mount_matrix orientation; > @@ -301,6 +306,15 @@ struct inv_icm42600_sensor_state { > #define INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(_f) \ > FIELD_PREP(GENMASK(3, 0), (_f)) > > +/* Pedometer functionality */ > +#define INV_ICM42600_REG_APEX_CONFIG0 0x0056 > +#define INV_ICM42600_DMP_ODR_50Hz BIT(1) > +#define INV_ICM42600_PED_ENABLE BIT(5) > +#define INV_ICM42600_REG_INT_STATUS3 0x0038 > +#define INV_ICM42600_STEP_DET_INT BIT(5) > +#define INV_ICM42600_REG_APEX_DATA 0x0031 // 2 Byte little-endian /* */ for comments in IIO (and most of the kernel) Also, put it on the line above rather than making such a long line. > + one blank line is enough. > + > #define INV_ICM42600_REG_TMST_CONFIG 0x0054 > #define INV_ICM42600_TMST_CONFIG_MASK GENMASK(4, 0) > #define INV_ICM42600_TMST_CONFIG_TMST_TO_REGS_EN BIT(4) > @@ -373,6 +387,8 @@ struct inv_icm42600_sensor_state { > #define INV_ICM42600_INTF_CONFIG6_I3C_SDR_EN BIT(0) > > /* User bank 4 (MSB 0x40) */ > +#define INV_ICM42600_REG_INT_SOURCE6 0x404D > +#define INV_ICM42600_STEP_DET_INT1_EN BIT(5) > #define INV_ICM42600_REG_INT_SOURCE8 0x404F > #define INV_ICM42600_INT_SOURCE8_FSYNC_IBI_EN BIT(5) > #define INV_ICM42600_INT_SOURCE8_PLL_RDY_IBI_EN BIT(4) > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c > index 56ac19814250..90fe4c9e15ab 100644 > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c > @@ -160,6 +160,13 @@ static const struct iio_chan_spec_ext_info inv_icm42600_accel_ext_infos[] = { > {}, > }; > +static int inv_icm42600_steps_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev); > + __le16 steps; > + int ret; > + > + if (mask == IIO_CHAN_INFO_PROCESSED) { > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + ret = regmap_bulk_read(st->map, INV_ICM42600_REG_APEX_DATA, &steps, sizeof(steps)); > + if (ret) > + return ret; > + iio_device_release_direct_mode(indio_dev); > + if (ret) > + return ret; > + *val = steps; As the bot pointed out, you need an endian conversion here. le16_to_cpu() > + return IIO_VAL_INT; > + } > + > + return -EINVAL; > +} > + > static int inv_icm42600_accel_read_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, > int *val, int *val2, long mask) > @@ -681,6 +721,8 @@ static int inv_icm42600_accel_read_raw(struct iio_dev *indio_dev, > break; > case IIO_TEMP: > return inv_icm42600_temp_read_raw(indio_dev, chan, val, val2, mask); > + case IIO_STEPS: > + return inv_icm42600_steps_read_raw(indio_dev, chan, val, val2, mask); > default: > return -EINVAL; > } > @@ -824,6 +866,126 @@ static int inv_icm42600_accel_hwfifo_flush(struct iio_dev *indio_dev, > return ret; > } > > +/*****************Pedometer Functionality**************/ No to structure comments like this. They add little to readability and have a habit of rapidly becoming wrong. > +static int inv_icm42600_step_en(struct inv_icm42600_state *st, int state) > +{ > + struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_APEX; > + int ret, value; > + > + if (state) { > + > + ret = inv_icm42600_set_accel_conf(st, &conf, NULL); > + if (ret) > + return ret; > + > + ret = regmap_write(st->map, INV_ICM42600_REG_APEX_CONFIG0, > + INV_ICM42600_DMP_ODR_50Hz); > + if (ret) > + return ret; > + > + ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET, > + INV_ICM42600_SIGNAL_PATH_RESET_DMP_MEM_RESET); > + if (ret) > + return ret; > + msleep(1); Document the reason for this value. > + > + ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET, > + INV_ICM42600_SIGNAL_PATH_RESET_DMP_INIT_EN); > + if (ret) > + return ret; > + > + ret = regmap_write(st->map, INV_ICM42600_REG_INT_SOURCE6, > + INV_ICM42600_STEP_DET_INT1_EN); > + if (ret) > + return ret; > + > + value = INV_ICM42600_DMP_ODR_50Hz | INV_ICM42600_PED_ENABLE; > + ret = regmap_write(st->map, INV_ICM42600_REG_APEX_CONFIG0, value); > + if (ret) > + return ret; > + > + st->pedometer_enable = true; return here. Then can drop the else. With two such different paths, even better would be two little functions to handle the two operations (enable + disable) as will make each individually easier to read. > + > + } else { > + > + ret = regmap_write(st->map, INV_ICM42600_REG_APEX_CONFIG0, 0); > + if (ret) > + return ret; > + > + ret = regmap_write(st->map, INV_ICM42600_REG_INT_SOURCE6, 0); > + if (ret) > + return ret; > + > + st->pedometer_enable = false; > + } > + > + return 0; > +} > + > +static int inv_icm42600_write_event_config(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, int state) > +{ > + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev); > + int ret; > + > + if(chan->type != IIO_STEPS) > + return -EINVAL; > + > + mutex_lock(&st->lock); guard() is useful in cases like this. > + > + ret = inv_icm42600_step_en(st, state); > + > + mutex_unlock(&st->lock); > + return ret; > +} > + > +static int inv_icm42600_read_event_config(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir) > +{ > + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev); > + int value; > + > + if (chan->type != IIO_STEPS) > + return -EINVAL; > + > + regmap_read(st->map, INV_ICM42600_REG_APEX_CONFIG0, &value); check return value. > + > + if (value & INV_ICM42600_PED_ENABLE) > + return 1; > + else > + return 0; > +} > + > +static int inv_icm42600_read_event_value(struct iio_dev *indio_dev, This isn't to get if the event happened, it's for reading thresholds etc. Not relevant for a pedometer. > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, > + int *val, int *val2) > +{ > + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev); > + > + mutex_lock(&st->lock); guard() > + > + if (type == IIO_EV_TYPE_CHANGE) { flip logic and can test that before taking the lock > + if (st->pedometer_value == true) { > + *val = 1; > + st->pedometer_value = false; > + } else { > + *val = 0; > + } > + mutex_unlock(&st->lock); > + return IIO_VAL_INT; > + } > + > + mutex_unlock(&st->lock); > + return -EINVAL; > +} > + > static const struct iio_info inv_icm42600_accel_info = { > .read_raw = inv_icm42600_accel_read_raw, > .read_avail = inv_icm42600_accel_read_avail, > @@ -833,6 +995,9 @@ static const struct iio_info inv_icm42600_accel_info = { > .update_scan_mode = inv_icm42600_accel_update_scan_mode, > .hwfifo_set_watermark = inv_icm42600_accel_hwfifo_set_watermark, > .hwfifo_flush_to_buffer = inv_icm42600_accel_hwfifo_flush, > + .write_event_config = inv_icm42600_write_event_config, > + .read_event_config = inv_icm42600_read_event_config, > + .read_event_value = inv_icm42600_read_event_value, > }; > > struct iio_dev *inv_icm42600_accel_init(struct inv_icm42600_state *st) > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c > index c3924cc6190e..4d78cb5ca396 100644 > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c > @@ -15,7 +15,8 @@ > #include <linux/pm_runtime.h> > #include <linux/property.h> > #include <linux/regmap.h> > - Keep the blank line and keep the iio headers in their own section. > +#include <linux/iio/events.h> > +#include <linux/of_irq.h> hmm. Can we not use the generic property accessors? Also you aren't using any interrupt related new stuff in here so I think this is just spurious. > #include <linux/iio/iio.h> > > #include "inv_icm42600.h" > @@ -533,6 +534,19 @@ static irqreturn_t inv_icm42600_irq_handler(int irq, void *_data) > > mutex_lock(&st->lock); Probably worth considering use of guard() in here as a precursor patch. > > + ret = regmap_read(st->map, INV_ICM42600_REG_INT_STATUS3, &status); > + if (ret) > + goto out_unlock; > + > + if (status & INV_ICM42600_STEP_DET_INT) { > + iio_push_event(st->indio_accel, IIO_MOD_EVENT_CODE(IIO_STEPS, 0, > + IIO_NO_MOD, > + IIO_EV_TYPE_CHANGE, > + IIO_EV_DIR_NONE), > + st->timestamp.accel); > + st->pedometer_value = true; > + } > + > ret = regmap_read(st->map, INV_ICM42600_REG_INT_STATUS, &status); > if (ret) > goto out_unlock; > @@ -860,12 +876,20 @@ static int inv_icm42600_runtime_suspend(struct device *dev) > mutex_lock(&st->lock); > > /* disable all sensors */ > - ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF, > - INV_ICM42600_SENSOR_MODE_OFF, false, > - NULL); > - if (ret) > - goto error_unlock; > + if (st->pedometer_enable) { > + ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF, > + INV_ICM42600_SENSOR_MODE_LOW_POWER, > + false, NULL); > + if (ret) > + goto error_unlock; > + } else { > > + ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF, > + INV_ICM42600_SENSOR_MODE_OFF, > + false, NULL); > + if (ret) > + goto error_unlock; > + } > regulator_disable(st->vddio_supply); > > error_unlock:
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h index 3a07e43e4cf1..c3471b73152e 100644 --- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h @@ -122,6 +122,7 @@ struct inv_icm42600_sensor_conf { int filter; }; #define INV_ICM42600_SENSOR_CONF_INIT {-1, -1, -1, -1} +#define INV_ICM42600_SENSOR_CONF_APEX { 2, 0, 9, 6} struct inv_icm42600_conf { struct inv_icm42600_sensor_conf gyro; @@ -141,6 +142,8 @@ struct inv_icm42600_suspended { * @chip: chip identifier. * @name: chip name. * @map: regmap pointer. + * @pedometer_enable status of pedometer function + * @pedometer_value status of steps event occurnce * @vdd_supply: VDD voltage regulator for the chip. * @vddio_supply: I/O voltage regulator for the chip. * @orientation: sensor chip orientation relative to main hardware. @@ -157,6 +160,8 @@ struct inv_icm42600_state { enum inv_icm42600_chip chip; const char *name; struct regmap *map; + bool pedometer_enable; + bool pedometer_value; struct regulator *vdd_supply; struct regulator *vddio_supply; struct iio_mount_matrix orientation; @@ -301,6 +306,15 @@ struct inv_icm42600_sensor_state { #define INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(_f) \ FIELD_PREP(GENMASK(3, 0), (_f)) +/* Pedometer functionality */ +#define INV_ICM42600_REG_APEX_CONFIG0 0x0056 +#define INV_ICM42600_DMP_ODR_50Hz BIT(1) +#define INV_ICM42600_PED_ENABLE BIT(5) +#define INV_ICM42600_REG_INT_STATUS3 0x0038 +#define INV_ICM42600_STEP_DET_INT BIT(5) +#define INV_ICM42600_REG_APEX_DATA 0x0031 // 2 Byte little-endian + + #define INV_ICM42600_REG_TMST_CONFIG 0x0054 #define INV_ICM42600_TMST_CONFIG_MASK GENMASK(4, 0) #define INV_ICM42600_TMST_CONFIG_TMST_TO_REGS_EN BIT(4) @@ -373,6 +387,8 @@ struct inv_icm42600_sensor_state { #define INV_ICM42600_INTF_CONFIG6_I3C_SDR_EN BIT(0) /* User bank 4 (MSB 0x40) */ +#define INV_ICM42600_REG_INT_SOURCE6 0x404D +#define INV_ICM42600_STEP_DET_INT1_EN BIT(5) #define INV_ICM42600_REG_INT_SOURCE8 0x404F #define INV_ICM42600_INT_SOURCE8_FSYNC_IBI_EN BIT(5) #define INV_ICM42600_INT_SOURCE8_PLL_RDY_IBI_EN BIT(4) diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c index 56ac19814250..90fe4c9e15ab 100644 --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c @@ -160,6 +160,13 @@ static const struct iio_chan_spec_ext_info inv_icm42600_accel_ext_infos[] = { {}, }; +static const struct iio_event_spec icm42600_step_event = { + .type = IIO_EV_TYPE_CHANGE, + .dir = IIO_EV_DIR_NONE, + .mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) | + BIT(IIO_EV_INFO_VALUE), +}; + static const struct iio_chan_spec inv_icm42600_accel_channels[] = { INV_ICM42600_ACCEL_CHAN(IIO_MOD_X, INV_ICM42600_ACCEL_SCAN_X, inv_icm42600_accel_ext_infos), @@ -169,6 +176,14 @@ static const struct iio_chan_spec inv_icm42600_accel_channels[] = { inv_icm42600_accel_ext_infos), INV_ICM42600_TEMP_CHAN(INV_ICM42600_ACCEL_SCAN_TEMP), IIO_CHAN_SOFT_TIMESTAMP(INV_ICM42600_ACCEL_SCAN_TIMESTAMP), + { + .type = IIO_STEPS, + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), + .scan_index = -1, + .event_spec = &icm42600_step_event, + .num_event_specs = 1, + }, + }; /* @@ -668,6 +683,31 @@ static int inv_icm42600_accel_write_offset(struct inv_icm42600_state *st, return ret; } +static int inv_icm42600_steps_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev); + __le16 steps; + int ret; + + if (mask == IIO_CHAN_INFO_PROCESSED) { + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + ret = regmap_bulk_read(st->map, INV_ICM42600_REG_APEX_DATA, &steps, sizeof(steps)); + if (ret) + return ret; + iio_device_release_direct_mode(indio_dev); + if (ret) + return ret; + *val = steps; + return IIO_VAL_INT; + } + + return -EINVAL; +} + static int inv_icm42600_accel_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long mask) @@ -681,6 +721,8 @@ static int inv_icm42600_accel_read_raw(struct iio_dev *indio_dev, break; case IIO_TEMP: return inv_icm42600_temp_read_raw(indio_dev, chan, val, val2, mask); + case IIO_STEPS: + return inv_icm42600_steps_read_raw(indio_dev, chan, val, val2, mask); default: return -EINVAL; } @@ -824,6 +866,126 @@ static int inv_icm42600_accel_hwfifo_flush(struct iio_dev *indio_dev, return ret; } +/*****************Pedometer Functionality**************/ +static int inv_icm42600_step_en(struct inv_icm42600_state *st, int state) +{ + struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_APEX; + int ret, value; + + if (state) { + + ret = inv_icm42600_set_accel_conf(st, &conf, NULL); + if (ret) + return ret; + + ret = regmap_write(st->map, INV_ICM42600_REG_APEX_CONFIG0, + INV_ICM42600_DMP_ODR_50Hz); + if (ret) + return ret; + + ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET, + INV_ICM42600_SIGNAL_PATH_RESET_DMP_MEM_RESET); + if (ret) + return ret; + msleep(1); + + ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET, + INV_ICM42600_SIGNAL_PATH_RESET_DMP_INIT_EN); + if (ret) + return ret; + + ret = regmap_write(st->map, INV_ICM42600_REG_INT_SOURCE6, + INV_ICM42600_STEP_DET_INT1_EN); + if (ret) + return ret; + + value = INV_ICM42600_DMP_ODR_50Hz | INV_ICM42600_PED_ENABLE; + ret = regmap_write(st->map, INV_ICM42600_REG_APEX_CONFIG0, value); + if (ret) + return ret; + + st->pedometer_enable = true; + + } else { + + ret = regmap_write(st->map, INV_ICM42600_REG_APEX_CONFIG0, 0); + if (ret) + return ret; + + ret = regmap_write(st->map, INV_ICM42600_REG_INT_SOURCE6, 0); + if (ret) + return ret; + + st->pedometer_enable = false; + } + + return 0; +} + +static int inv_icm42600_write_event_config(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir, int state) +{ + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev); + int ret; + + if(chan->type != IIO_STEPS) + return -EINVAL; + + mutex_lock(&st->lock); + + ret = inv_icm42600_step_en(st, state); + + mutex_unlock(&st->lock); + return ret; +} + +static int inv_icm42600_read_event_config(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir) +{ + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev); + int value; + + if (chan->type != IIO_STEPS) + return -EINVAL; + + regmap_read(st->map, INV_ICM42600_REG_APEX_CONFIG0, &value); + + if (value & INV_ICM42600_PED_ENABLE) + return 1; + else + return 0; +} + +static int inv_icm42600_read_event_value(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir, + enum iio_event_info info, + int *val, int *val2) +{ + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev); + + mutex_lock(&st->lock); + + if (type == IIO_EV_TYPE_CHANGE) { + if (st->pedometer_value == true) { + *val = 1; + st->pedometer_value = false; + } else { + *val = 0; + } + mutex_unlock(&st->lock); + return IIO_VAL_INT; + } + + mutex_unlock(&st->lock); + return -EINVAL; +} + static const struct iio_info inv_icm42600_accel_info = { .read_raw = inv_icm42600_accel_read_raw, .read_avail = inv_icm42600_accel_read_avail, @@ -833,6 +995,9 @@ static const struct iio_info inv_icm42600_accel_info = { .update_scan_mode = inv_icm42600_accel_update_scan_mode, .hwfifo_set_watermark = inv_icm42600_accel_hwfifo_set_watermark, .hwfifo_flush_to_buffer = inv_icm42600_accel_hwfifo_flush, + .write_event_config = inv_icm42600_write_event_config, + .read_event_config = inv_icm42600_read_event_config, + .read_event_value = inv_icm42600_read_event_value, }; struct iio_dev *inv_icm42600_accel_init(struct inv_icm42600_state *st) diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c index c3924cc6190e..4d78cb5ca396 100644 --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c @@ -15,7 +15,8 @@ #include <linux/pm_runtime.h> #include <linux/property.h> #include <linux/regmap.h> - +#include <linux/iio/events.h> +#include <linux/of_irq.h> #include <linux/iio/iio.h> #include "inv_icm42600.h" @@ -533,6 +534,19 @@ static irqreturn_t inv_icm42600_irq_handler(int irq, void *_data) mutex_lock(&st->lock); + ret = regmap_read(st->map, INV_ICM42600_REG_INT_STATUS3, &status); + if (ret) + goto out_unlock; + + if (status & INV_ICM42600_STEP_DET_INT) { + iio_push_event(st->indio_accel, IIO_MOD_EVENT_CODE(IIO_STEPS, 0, + IIO_NO_MOD, + IIO_EV_TYPE_CHANGE, + IIO_EV_DIR_NONE), + st->timestamp.accel); + st->pedometer_value = true; + } + ret = regmap_read(st->map, INV_ICM42600_REG_INT_STATUS, &status); if (ret) goto out_unlock; @@ -860,12 +876,20 @@ static int inv_icm42600_runtime_suspend(struct device *dev) mutex_lock(&st->lock); /* disable all sensors */ - ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF, - INV_ICM42600_SENSOR_MODE_OFF, false, - NULL); - if (ret) - goto error_unlock; + if (st->pedometer_enable) { + ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF, + INV_ICM42600_SENSOR_MODE_LOW_POWER, + false, NULL); + if (ret) + goto error_unlock; + } else { + ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF, + INV_ICM42600_SENSOR_MODE_OFF, + false, NULL); + if (ret) + goto error_unlock; + } regulator_disable(st->vddio_supply); error_unlock:
Enables pedometer functionality in the ICM42605 IMU sensor. The pedometer feature allows for step counting, which is accessible through a new sysfs entry. Interrupts are triggered when a step event occurs, enabling step event detection. Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io> --- drivers/iio/imu/inv_icm42600/inv_icm42600.h | 16 ++ .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 165 ++++++++++++++++++ .../iio/imu/inv_icm42600/inv_icm42600_core.c | 36 +++- 3 files changed, 211 insertions(+), 6 deletions(-)