Re: [PATCH V11 6/9] iio: imu: inv_icm42607: Add Temp Support in icm42607

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 10 Jun 2026 12:54:50 -0500
Chris Morgan <macroalpha82@gmail.com> wrote:

> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add functions for reading temperature sensor data.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Hi Chris,

A few consistency things around the channel definitions and some
left over stuff I think from you ripping out the interrupt / buffered support.

Thanks,

Jonathan

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> new file mode 100644
> index 000000000000..55260082a19e
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 InvenSense, Inc.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/iio/iio.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +
> +#include "inv_icm42607.h"
> +#include "inv_icm42607_temp.h"

> +int inv_icm42607_temp_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val, int *val2, long mask)
> +{
> +	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> +	s16 temp;
> +	int ret;
> +
> +	if (chan->type != IIO_TEMP)
> +		return -EINVAL;
This feels little over defensive given the function name. Any bug
that called it anyway would be pretty bad!
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:

The lack of having the shared_by_all stuff in here makes things
a little fragile (once you've added them to the bitmap below) as
which chan->type we get depends on channel registration ordering.

So avoid this being called for those mask elements.  I'll reply
to next patch to call that out in a few mins.

> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;

A driver that isn't yet doing buffered support should not be
claiming modes (as it's always in direct mode).  If this is
serializing for some reason other than avoiding mode transitions,
it should be using the local lock. Seems it is doing that
anyway so just drop this layer of protection.

> +		ret = inv_icm42607_temp_read(st, &temp);
> +		iio_device_release_direct(indio_dev);
> +		if (ret)
> +			return ret;
> +		*val = temp;
> +		return IIO_VAL_INT;
> +	/*
> +	 * T°C = (temp / 128) + 25
> +	 * Tm°C = 1000 * ((temp * 100 / 12800) + 25)
> +	 * scale: 100000 / 12800 ~= 7.8125
> +	 * offset: 3200
> +	 */
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 7;
> +		*val2 = 812500000;
> +		return IIO_VAL_INT_PLUS_NANO;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = 3200;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
> new file mode 100644
> index 000000000000..e03924e30866
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2026 InvenSense, Inc.
> + */
> +
> +#ifndef INV_ICM42607_TEMP_H_
> +#define INV_ICM42607_TEMP_H_
> +
> +#include <linux/bitops.h>
> +
> +struct iio_dev;
> +struct iio_chan_spec;
> +
> +#define INV_ICM42607_TEMP_CHAN(_index)				\
> +{								\
> +	.type = IIO_TEMP,					\
> +	.info_mask_separate =					\
> +		BIT(IIO_CHAN_INFO_RAW) |			\
> +		BIT(IIO_CHAN_INFO_OFFSET) |			\
> +		BIT(IIO_CHAN_INFO_SCALE),			\

Whilst it makes no difference to exposure of sysfs attributes, this
should include the shared_by_all stuff form the other channels.
That is supposed to be in every channel so that we can easily see
what affects each channel. 

> +	.scan_index = _index,					\
> +	.scan_type = {						\
> +		.sign = 's',					\
> +		.realbits = 16,					\
> +		.storagebits = 16,				\
> +	},							\
> +}
> +
> +int inv_icm42607_temp_read_raw(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       int *val, int *val2, long mask);
> +
> +#endif