Re: [PATCH 1/2] iio: lsm6dsx: Support temperature channel

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 5171 bytes --]

> The LSM6DSX (possibly other ST IMUs as well) has a temperature
> channel that can be read out. Modify the lsm6dsx core to
> accomodate temperature readout: make headspace for three
> members in the channels and odr_table, support offset and make
> the available milli_hz frequency resolution optional.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
[...]
>  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,15 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.odr_avl[6] = { 833000, 0x07 },
>  				.odr_len = 7,
>  			},
> +			[ST_LSM6DSX_ID_TEMP] = {
> +				.reg = {
> +					.addr = 0x0A,
> +					.mask = GENMASK(5, 4),
> +				},

looking at the ISM330DHCX datasheet, the temperature sensor ODR is just 52Hz,
while values in 0x0A register are used only for FIFO decimation, they are not
values you can configure the sensor e.g. for read_one_shot().

> +				.odr_avl[0] = {  26000, 0x02 },
> +				.odr_avl[1] = {  52000, 0x03 },
> +				.odr_len = 2,
> +			},
>  		},
>  		.fs_table = {
>  			[ST_LSM6DSX_ID_ACC] = {
> @@ -937,6 +955,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,
> @@ -1661,6 +1683,7 @@ 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:
>  	case ST_LSM6DSX_ID_EXT0:
>  	case ST_LSM6DSX_ID_EXT1:
>  	case ST_LSM6DSX_ID_EXT2:
> @@ -1800,6 +1823,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;
> @@ -2016,9 +2043,11 @@ st_lsm6dsx_sysfs_sampling_frequency_avail(struct device *dev,
>  
>  	odr_table = &sensor->hw->settings->odr_table[sensor->id];
>  	for (i = 0; i < odr_table->odr_len; i++)
> -		len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%03d ",
> -				 odr_table->odr_avl[i].milli_hz / 1000,
> -				 odr_table->odr_avl[i].milli_hz % 1000);
> +		if (odr_table->odr_avl[i].milli_hz) {
> +			len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%03d ",
> +					 odr_table->odr_avl[i].milli_hz / 1000,
> +					 odr_table->odr_avl[i].milli_hz % 1000);
> +		}
>  	buf[len - 1] = '\n';
>  
>  	return len;
> @@ -2106,6 +2135,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 +2424,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;

here ODR should be 52 I guess

> -	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;


I was wondering about the 25 costant value since ISM330DHCX ds does not provide
a typical value. Maybe we should provide a way for the user to configure it?

> +	} else {
> +		sensor->gain = hw->settings->fs_table[id].fs_avl[0].gain;
> +	}
>  	sensor->watermark = 1;
>  
>  	switch (id) {
> @@ -2393,6 +2447,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);

Regards,
Lorenzo

> +		break;
>  	default:
>  		return NULL;
>  	}
> 
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]