| Message ID | 20230829-iio-spacex-lsm6ds0-v2-1-584e161b612f@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] RFC: iio: lsm6dsx: Support temperature channel on some devices | expand |
> The ISM330 sensors (wai 0x6b) has a temperature channel that can > be read out. Modify the lsm6dsx core to accomodate temperature > readout on these sensors: > > - Make headspace for three members in the channels and odr_table, > - Support offset > - Add code to avoid configuring the ODR of the temperature > sensor, it has no real ODR control register. > > This is investigated because the hardware has important real-life > use cases using the Linux kernel. Hi Linus, please seem my comments inline below. Regards, Lorenzo > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > The ISM330DHCX is used in the SpaceX Starlink terminals which > are running a realtime patched close-to-mainline Linux kernel so > let's support temperature readout on it because why not. > SpaceX is using the temperature. > --- > Changes in v2: > - Put to RFC because I can't test changes. > - Added some mail addresses to SpaceX to the header. Maybe you > guys can check if this works for you. Or forward to designated > open source ambassador or whatever can help. (Addresses found > in SpaceX code drop.) > - Drop the code with strings for ism330dhc as we concluded that > this is probably ism330dhcx which is already supported. > (Would be nice if SpaceX can confirm.) > - Don't write in nonsense register 0x0a for temperature sensor > - More elaborate code to just avoid writing ODR for the temperature > sensor and instead rely on gyro or accelerometer to drive > the odr > - Link to v1: https://lore.kernel.org/r/20230811-iio-spacex-lsm6ds0-v1-0-e953a440170d@linaro.org > --- > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 24 +++++++- > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 4 ++ > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 79 +++++++++++++++++++++++++- > 3 files changed, 102 insertions(+), 5 deletions(-) > [...] > }, > .drdy_mask = { > .addr = 0x13, > @@ -869,6 +878,17 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { > .odr_avl[6] = { 833000, 0x07 }, > .odr_len = 7, > }, > + [ST_LSM6DSX_ID_TEMP] = { > + /* > + * NOTE: this ODR will be capped and controllerd by the > + * gyro and accelerometer don't have any reg to configure > + * this ODR. > + */ > + .odr_avl[0] = { 12500, 0x01 }, > + .odr_avl[1] = { 26000, 0x02 }, > + .odr_avl[2] = { 52000, 0x03 }, > + .odr_len = 3, please consider we do not support low-power mode iirc (just high-performance - bit 4 in CTRL6_C (15h)), so even enabling accel sensor, the temp sensor will always runs at 52Hz. Here we should add just one entry, like: .odr_avl[0] = { 52000, 0x03 }, .odr_len = 1, > + }, > }, > .fs_table = { > [ST_LSM6DSX_ID_ACC] = { > @@ -937,6 +957,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { > .addr = 0x09, > .mask = GENMASK(7, 4), > }, > + [ST_LSM6DSX_ID_TEMP] = { > + .addr = 0x0A, > + .mask = GENMASK(5, 4), > + }, > }, > .fifo_ops = { > .update_fifo = st_lsm6dsx_update_fifo, > @@ -1618,8 +1642,8 @@ int st_lsm6dsx_check_odr(struct st_lsm6dsx_sensor *sensor, u32 odr, u8 *val) > odr_table = &sensor->hw->settings->odr_table[sensor->id]; > for (i = 0; i < odr_table->odr_len; i++) { > /* > - * ext devices can run at different odr respect to > - * accel sensor > + * ext devices and temp sensor can run at different odr > + * respect to accel sensor > */ > if (odr_table->odr_avl[i].milli_hz >= odr) > break; > @@ -1661,6 +1685,21 @@ st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u32 req_odr) > switch (sensor->id) { > case ST_LSM6DSX_ID_GYRO: > break; > + case ST_LSM6DSX_ID_TEMP: > + /* > + * According to the application note AN5398 Rev 3 > + * for ISM330DHCX, section 10, page 109 > + * the ODR for the temperature sensor is equal to the > + * accelerometer ODR if the gyroscope is powered-down, > + * up to 52 Hz, but constant 52 Hz if the gyroscope > + * is powered on. It never goes above 52 Hz. > + */ > + ref_sensor = iio_priv(hw->iio_devs[ST_LSM6DSX_ID_GYRO]); > + if ((req_odr > 0) && (hw->enable_mask |= BIT(ref_sensor->id))) is there a typo here? > + /* Gyro is on! ODR fixed to 52 Hz */ > + return 0; > + /* We fall through and activate accelerometer if need be */ > + fallthrough; I do not think this approach works since please consider what happens with the sequence of events reported below: - user enables gyro sensor - user enables temp sensor - user disables gyro sensor IIUC the accel sensor is not enabled in this case so even the temp one will be powered-down. Is it correct? I think for the temp sensor we should introduce a dependency from the gyro one, doing something like (just compiled, not tested): diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c index 6a18b363cf73..ccd0d48bfb35 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c @@ -1633,19 +1633,36 @@ int st_lsm6dsx_check_odr(struct st_lsm6dsx_sensor *sensor, u32 odr, u8 *val) } static int -st_lsm6dsx_check_odr_dependency(struct st_lsm6dsx_hw *hw, u32 odr, - enum st_lsm6dsx_sensor_id id) +st_lsm6dsx_check_odr_dependency(struct st_lsm6dsx_sensor *sensor, + enum st_lsm6dsx_sensor_id *odr_map, + int odr_map_size, u32 req_odr) { - struct st_lsm6dsx_sensor *ref = iio_priv(hw->iio_devs[id]); + struct st_lsm6dsx_hw *hw = sensor->hw; + int i; - if (odr > 0) { - if (hw->enable_mask & BIT(id)) - return max_t(u32, ref->odr, odr); - else - return odr; - } else { - return (hw->enable_mask & BIT(id)) ? ref->odr : 0; + for (i = 0; i < odr_map_size; i++) { + enum st_lsm6dsx_sensor_id id = odr_map[i]; + struct st_lsm6dsx_sensor *ref = iio_priv(hw->iio_devs[id]); + u32 odr; + + if (!hw->iio_devs[id] || id == sensor->id) + continue; + + if (req_odr) { + if (hw->enable_mask & BIT(id)) + odr = max_t(u32, ref->odr, req_odr); + else + odr = req_odr; + } else { + odr = hw->enable_mask & BIT(id) ? ref->odr : 0; + } + + if (odr != req_odr) + /* device already configured */ + return -EBUSY; } + + return 0; } static int @@ -1659,14 +1676,30 @@ st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u32 req_odr) int err; switch (sensor->id) { - case ST_LSM6DSX_ID_GYRO: + case ST_LSM6DSX_ID_TEMP: + case ST_LSM6DSX_ID_GYRO: { + enum st_lsm6dsx_sensor_id odr_dep_map[] = { + ST_LSM6DSX_ID_GYRO, + ST_LSM6DSX_ID_TEMP, + }; + + ref_sensor = iio_priv(hw->iio_devs[ST_LSM6DSX_ID_GYRO]); + if (st_lsm6dsx_check_odr_dependency(sensor, odr_dep_map, + ARRAY_SIZE(odr_dep_map), + req_odr)) + return 0; break; + } case ST_LSM6DSX_ID_EXT0: case ST_LSM6DSX_ID_EXT1: case ST_LSM6DSX_ID_EXT2: case ST_LSM6DSX_ID_ACC: { - u32 odr; - int i; + enum st_lsm6dsx_sensor_id odr_dep_map[] = { + ST_LSM6DSX_ID_ACC, + ST_LSM6DSX_ID_EXT0, + ST_LSM6DSX_ID_EXT1, + ST_LSM6DSX_ID_EXT2, + }; /* * i2c embedded controller relies on the accelerometer sensor as @@ -1675,15 +1708,10 @@ st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u32 req_odr) * communicate with i2c slave devices */ ref_sensor = iio_priv(hw->iio_devs[ST_LSM6DSX_ID_ACC]); - for (i = ST_LSM6DSX_ID_ACC; i < ST_LSM6DSX_ID_MAX; i++) { - if (!hw->iio_devs[i] || i == sensor->id) - continue; - - odr = st_lsm6dsx_check_odr_dependency(hw, req_odr, i); - if (odr != req_odr) - /* device already configured */ - return 0; - } + if (st_lsm6dsx_check_odr_dependency(sensor, odr_dep_map, + ARRAY_SIZE(odr_dep_map), + req_odr)) + return 0; break; } default: /* should never occur */ What do you think? If you agree I can post it as preliminary patch (removing temp dependency). Regards, Lorenzo > case ST_LSM6DSX_ID_EXT0: > case ST_LSM6DSX_ID_EXT1: > case ST_LSM6DSX_ID_EXT2: > @@ -1800,6 +1839,10 @@ static int st_lsm6dsx_read_raw(struct iio_dev *iio_dev, > *val2 = sensor->gain; > ret = IIO_VAL_INT_PLUS_NANO; > break; > + case IIO_CHAN_INFO_OFFSET: > + *val = sensor->offset; > + ret = IIO_VAL_INT; > + break; > default: > ret = -EINVAL; > break; > @@ -2106,6 +2149,22 @@ static const struct iio_info st_lsm6dsx_gyro_info = { > .write_raw_get_fmt = st_lsm6dsx_write_raw_get_fmt, > }; > > +static struct attribute *st_lsm6dsx_temp_attributes[] = { > + &iio_dev_attr_sampling_frequency_available.dev_attr.attr, > + NULL, > +}; > + > +static const struct attribute_group st_lsm6dsx_temp_attribute_group = { > + .attrs = st_lsm6dsx_temp_attributes, > +}; > + > +static const struct iio_info st_lsm6dsx_temp_info = { > + .attrs = &st_lsm6dsx_temp_attribute_group, > + .read_raw = st_lsm6dsx_read_raw, > + .write_raw = st_lsm6dsx_write_raw, > + .hwfifo_set_watermark = st_lsm6dsx_set_watermark, > +}; > + > static int st_lsm6dsx_get_drdy_pin(struct st_lsm6dsx_hw *hw, int *drdy_pin) > { > struct device *dev = hw->dev; > @@ -2379,7 +2438,16 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw, > sensor->id = id; > sensor->hw = hw; > sensor->odr = hw->settings->odr_table[id].odr_avl[0].milli_hz; > - sensor->gain = hw->settings->fs_table[id].fs_avl[0].gain; > + if (id == ST_LSM6DSX_ID_TEMP) { > + /* > + * The temperature sensor has a fixed scale and offset such > + * that: temp_C = (raw / 256) + 25 > + */ > + sensor->gain = 3906; > + sensor->offset = 25; > + } else { > + sensor->gain = hw->settings->fs_table[id].fs_avl[0].gain; > + } > sensor->watermark = 1; > > switch (id) { > @@ -2393,6 +2461,11 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw, > scnprintf(sensor->name, sizeof(sensor->name), "%s_gyro", > name); > break; > + case ST_LSM6DSX_ID_TEMP: > + iio_dev->info = &st_lsm6dsx_temp_info; > + scnprintf(sensor->name, sizeof(sensor->name), "%s_temp", > + name); > + break; > default: > return NULL; > } > > --- > base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 > change-id: 20230811-iio-spacex-lsm6ds0-33c9365e94bf > > Best regards, > -- > Linus Walleij <linus.walleij@linaro.org> >
Hi Linus, Thanks for helping get this into mainline :) > Changes in v2: > - Put to RFC because I can't test changes. > - Added some mail addresses to SpaceX to the header. Maybe you > guys can check if this works for you. Or forward to designated > open source ambassador or whatever can help. (Addresses found > in SpaceX code drop.) I am familiar with these changes and can help review and test this. > - Drop the code with strings for ism330dhc as we concluded that > this is probably ism330dhcx which is already supported. > (Would be nice if SpaceX can confirm.) I believe we did use the ism330dhc a long time ago but have since switched to the ism330dhcx. The register maps are compatible, so the old name stuck around. Adding support for ism330dhcx only is fine. > - Don't write in nonsense register 0x0a for temperature sensor > - More elaborate code to just avoid writing ODR for the temperature > sensor and instead rely on gyro or accelerometer to drive > the odr OK, we only use this in batch mode, I'll confirm that removing this write (for non-batch mode) works as expected. > + /* > + * NOTE: this ODR will be capped and controllerd by the > + * gyro and accelerometer don't have any reg to configure > + * this ODR. > + */ > + .odr_avl[0] = { 12500, 0x01 }, > + .odr_avl[1] = { 26000, 0x02 }, > + .odr_avl[2] = { 52000, 0x03 }, > + .odr_len = 3, As per the other thread, the data rate should be 1600, 12500, and 52000. > + /* > + * The temperature sensor has a fixed scale and offset such > + * that: temp_C = (raw / 256) + 25 > + */ > + sensor->gain = 3906; > + sensor->offset = 25; I believe the gain should be set to 3906250 now (1e9 / 256). 3906 was an approximation from an older kernel that used IIO_VAL_INT_PLUS_MICRO instead of IIO_VAL_INT_PLUS_NANO. Also in st_lsm6dsx_alloc_iiodev(), I believe we can/should avoid setting available_scan_masks, the mask (0x7) isn't really valid for the temperature sensor that has only one channel.
Hi Lorenzo, Thanks for reviewing this! Regarding the TODR and ODR_T_BATCH settings: > > + [ST_LSM6DSX_ID_TEMP] = { > > + /* > > + * NOTE: this ODR will be capped and controllerd by the > > + * gyro and accelerometer don't have any reg to configure > > + * this ODR. > > + */ > > + .odr_avl[0] = { 12500, 0x01 }, > > + .odr_avl[1] = { 26000, 0x02 }, > > + .odr_avl[2] = { 52000, 0x03 }, > > + .odr_len = 3, > > please consider we do not support low-power mode iirc (just > high-performance - bit 4 in CTRL6_C (15h)), so even enabling accel > sensor, the temp sensor will always runs at 52Hz. Here we should add > just one entry, like: > > .odr_avl[0] = { 52000, 0x03 }, > .odr_len = 1, I didn't see a way to configure the batch data rate in the IIO driver aside from the "odr_avl" table. It seemed useful to allow reading the gyro/accel at a high rate, such as 416 Hz, while still allowing the lower temperature sub-sampling rates of 1.6 and 12.5 Hz. My original intent in adding the lower ODR table entries here was to reuse the sampling_frequency sysfs attr to configure the batch data rate, since that seems to be what most people would care about. Alternately, we could add a separate "odr_batch_avl" table along with a separate sysfs attr (e.g. "buffer_sampling_frequency"). That would be a lot more work though, especially since the actual ODR for the temp sensor won't be configurable anyway. Note, I don't have any specific need for the lower rates, so if 52 Hz is the only rate supported, that still "works for me".
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h index c19237717e81..4d013889c287 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h @@ -118,6 +118,23 @@ enum st_lsm6dsx_hw_id { .ext_info = st_lsm6dsx_ext_info, \ } +#define ST_LSM6DSX_TEMP(chan_type, addr, scan_idx) \ +{ \ + .type = chan_type, \ + .address = addr, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_OFFSET), \ + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ + .scan_index = scan_idx, \ + .scan_type = { \ + .sign = 's', \ + .realbits = 16, \ + .storagebits = 16, \ + .endianness = IIO_LE, \ + }, \ +} + struct st_lsm6dsx_reg { u8 addr; u8 mask; @@ -320,7 +337,7 @@ struct st_lsm6dsx_settings { struct { const struct iio_chan_spec *chan; int len; - } channels[2]; + } channels[3]; struct { struct st_lsm6dsx_reg irq1; struct st_lsm6dsx_reg irq2; @@ -332,7 +349,7 @@ struct st_lsm6dsx_settings { struct st_lsm6dsx_reg od; } irq_config; struct st_lsm6dsx_reg drdy_mask; - struct st_lsm6dsx_odr_table_entry odr_table[2]; + struct st_lsm6dsx_odr_table_entry odr_table[3]; struct st_lsm6dsx_samples_to_discard samples_to_discard[2]; struct st_lsm6dsx_fs_table_entry fs_table[2]; struct st_lsm6dsx_reg decimator[ST_LSM6DSX_MAX_ID]; @@ -346,6 +363,7 @@ struct st_lsm6dsx_settings { enum st_lsm6dsx_sensor_id { ST_LSM6DSX_ID_GYRO, ST_LSM6DSX_ID_ACC, + ST_LSM6DSX_ID_TEMP, ST_LSM6DSX_ID_EXT0, ST_LSM6DSX_ID_EXT1, ST_LSM6DSX_ID_EXT2, @@ -364,6 +382,7 @@ enum st_lsm6dsx_fifo_mode { * @hw: Pointer to instance of struct st_lsm6dsx_hw. * @gain: Configured sensor sensitivity. * @odr: Output data rate of the sensor [Hz]. + * @offset: Constant offset of the sensor * @samples_to_discard: Number of samples to discard for filters settling time. * @watermark: Sensor watermark level. * @decimator: Sensor decimation factor. @@ -378,6 +397,7 @@ struct st_lsm6dsx_sensor { u32 gain; u32 odr; + u32 offset; u16 samples_to_discard; u16 watermark; diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c index 066fe561c5e8..c588451e2ddf 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c @@ -61,6 +61,7 @@ struct st_lsm6dsx_decimator_entry { enum st_lsm6dsx_fifo_tag { ST_LSM6DSX_GYRO_TAG = 0x01, ST_LSM6DSX_ACC_TAG = 0x02, + ST_LSM6DSX_TEMP_TAG = 0x03, ST_LSM6DSX_TS_TAG = 0x04, ST_LSM6DSX_EXT0_TAG = 0x0f, ST_LSM6DSX_EXT1_TAG = 0x10, @@ -532,6 +533,9 @@ st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag, case ST_LSM6DSX_ACC_TAG: iio_dev = hw->iio_devs[ST_LSM6DSX_ID_ACC]; break; + case ST_LSM6DSX_TEMP_TAG: + iio_dev = hw->iio_devs[ST_LSM6DSX_ID_TEMP]; + break; case ST_LSM6DSX_EXT0_TAG: if (hw->enable_mask & BIT(ST_LSM6DSX_ID_EXT0)) iio_dev = hw->iio_devs[ST_LSM6DSX_ID_EXT0]; diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c index 6a18b363cf73..76847d95c1b6 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c @@ -100,6 +100,11 @@ static const struct iio_chan_spec st_lsm6ds0_gyro_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(3), }; +static const struct iio_chan_spec st_lsm6dsx_temp_channels[] = { + ST_LSM6DSX_TEMP(IIO_TEMP, 0x20, 0), + IIO_CHAN_SOFT_TIMESTAMP(1), +}; + static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { { .reset = { @@ -835,6 +840,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .chan = st_lsm6dsx_gyro_channels, .len = ARRAY_SIZE(st_lsm6dsx_gyro_channels), }, + [ST_LSM6DSX_ID_TEMP] = { + .chan = st_lsm6dsx_temp_channels, + .len = ARRAY_SIZE(st_lsm6dsx_temp_channels), + }, }, .drdy_mask = { .addr = 0x13, @@ -869,6 +878,17 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .odr_avl[6] = { 833000, 0x07 }, .odr_len = 7, }, + [ST_LSM6DSX_ID_TEMP] = { + /* + * NOTE: this ODR will be capped and controllerd by the + * gyro and accelerometer don't have any reg to configure + * this ODR. + */ + .odr_avl[0] = { 12500, 0x01 }, + .odr_avl[1] = { 26000, 0x02 }, + .odr_avl[2] = { 52000, 0x03 }, + .odr_len = 3, + }, }, .fs_table = { [ST_LSM6DSX_ID_ACC] = { @@ -937,6 +957,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x09, .mask = GENMASK(7, 4), }, + [ST_LSM6DSX_ID_TEMP] = { + .addr = 0x0A, + .mask = GENMASK(5, 4), + }, }, .fifo_ops = { .update_fifo = st_lsm6dsx_update_fifo, @@ -1618,8 +1642,8 @@ int st_lsm6dsx_check_odr(struct st_lsm6dsx_sensor *sensor, u32 odr, u8 *val) odr_table = &sensor->hw->settings->odr_table[sensor->id]; for (i = 0; i < odr_table->odr_len; i++) { /* - * ext devices can run at different odr respect to - * accel sensor + * ext devices and temp sensor can run at different odr + * respect to accel sensor */ if (odr_table->odr_avl[i].milli_hz >= odr) break; @@ -1661,6 +1685,21 @@ st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u32 req_odr) switch (sensor->id) { case ST_LSM6DSX_ID_GYRO: break; + case ST_LSM6DSX_ID_TEMP: + /* + * According to the application note AN5398 Rev 3 + * for ISM330DHCX, section 10, page 109 + * the ODR for the temperature sensor is equal to the + * accelerometer ODR if the gyroscope is powered-down, + * up to 52 Hz, but constant 52 Hz if the gyroscope + * is powered on. It never goes above 52 Hz. + */ + ref_sensor = iio_priv(hw->iio_devs[ST_LSM6DSX_ID_GYRO]); + if ((req_odr > 0) && (hw->enable_mask |= BIT(ref_sensor->id))) + /* Gyro is on! ODR fixed to 52 Hz */ + return 0; + /* We fall through and activate accelerometer if need be */ + fallthrough; case ST_LSM6DSX_ID_EXT0: case ST_LSM6DSX_ID_EXT1: case ST_LSM6DSX_ID_EXT2: @@ -1800,6 +1839,10 @@ static int st_lsm6dsx_read_raw(struct iio_dev *iio_dev, *val2 = sensor->gain; ret = IIO_VAL_INT_PLUS_NANO; break; + case IIO_CHAN_INFO_OFFSET: + *val = sensor->offset; + ret = IIO_VAL_INT; + break; default: ret = -EINVAL; break; @@ -2106,6 +2149,22 @@ static const struct iio_info st_lsm6dsx_gyro_info = { .write_raw_get_fmt = st_lsm6dsx_write_raw_get_fmt, }; +static struct attribute *st_lsm6dsx_temp_attributes[] = { + &iio_dev_attr_sampling_frequency_available.dev_attr.attr, + NULL, +}; + +static const struct attribute_group st_lsm6dsx_temp_attribute_group = { + .attrs = st_lsm6dsx_temp_attributes, +}; + +static const struct iio_info st_lsm6dsx_temp_info = { + .attrs = &st_lsm6dsx_temp_attribute_group, + .read_raw = st_lsm6dsx_read_raw, + .write_raw = st_lsm6dsx_write_raw, + .hwfifo_set_watermark = st_lsm6dsx_set_watermark, +}; + static int st_lsm6dsx_get_drdy_pin(struct st_lsm6dsx_hw *hw, int *drdy_pin) { struct device *dev = hw->dev; @@ -2379,7 +2438,16 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw, sensor->id = id; sensor->hw = hw; sensor->odr = hw->settings->odr_table[id].odr_avl[0].milli_hz; - sensor->gain = hw->settings->fs_table[id].fs_avl[0].gain; + if (id == ST_LSM6DSX_ID_TEMP) { + /* + * The temperature sensor has a fixed scale and offset such + * that: temp_C = (raw / 256) + 25 + */ + sensor->gain = 3906; + sensor->offset = 25; + } else { + sensor->gain = hw->settings->fs_table[id].fs_avl[0].gain; + } sensor->watermark = 1; switch (id) { @@ -2393,6 +2461,11 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw, scnprintf(sensor->name, sizeof(sensor->name), "%s_gyro", name); break; + case ST_LSM6DSX_ID_TEMP: + iio_dev->info = &st_lsm6dsx_temp_info; + scnprintf(sensor->name, sizeof(sensor->name), "%s_temp", + name); + break; default: return NULL; }
The ISM330 sensors (wai 0x6b) has a temperature channel that can be read out. Modify the lsm6dsx core to accomodate temperature readout on these sensors: - Make headspace for three members in the channels and odr_table, - Support offset - Add code to avoid configuring the ODR of the temperature sensor, it has no real ODR control register. This is investigated because the hardware has important real-life use cases using the Linux kernel. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- The ISM330DHCX is used in the SpaceX Starlink terminals which are running a realtime patched close-to-mainline Linux kernel so let's support temperature readout on it because why not. SpaceX is using the temperature. --- Changes in v2: - Put to RFC because I can't test changes. - Added some mail addresses to SpaceX to the header. Maybe you guys can check if this works for you. Or forward to designated open source ambassador or whatever can help. (Addresses found in SpaceX code drop.) - Drop the code with strings for ism330dhc as we concluded that this is probably ism330dhcx which is already supported. (Would be nice if SpaceX can confirm.) - Don't write in nonsense register 0x0a for temperature sensor - More elaborate code to just avoid writing ODR for the temperature sensor and instead rely on gyro or accelerometer to drive the odr - Link to v1: https://lore.kernel.org/r/20230811-iio-spacex-lsm6ds0-v1-0-e953a440170d@linaro.org --- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 24 +++++++- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 4 ++ drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 79 +++++++++++++++++++++++++- 3 files changed, 102 insertions(+), 5 deletions(-) --- base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 change-id: 20230811-iio-spacex-lsm6ds0-33c9365e94bf Best regards,