Re: [PATCH v8 5/5] iio: osf: add UART IIO driver

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 20 Aug 2026 14:06:08 +0900
Jinseob Kim <kimjinseob88@gmail.com> wrote:

> Add the Open Sensor Fusion serdev transport, driver core, and IIO
> registration path as one complete driver patch.
> 
> The driver enables the required vcc regulator, receives OSF frames over
> UART, registers IIO devices from capability reports, supports direct raw
> reads from the latest sample cache, and pushes buffered samples into
> software kfifo buffers.

Too much info.  We definitely don't need mention it turns on the power
or that the data goes standard paths.

> 
> Wire the stream parser frame callback to the OSF core, use final Kconfig
> and Makefile contents from the start, check iio_buffer_enabled() before
> pushing samples, and use zero-initialized scan storage with explicit
> timestamp alignment.
> 
> Classify authenticated application outcomes as handled, ignored, or
> rejected so the parser consumes every CRC-valid frame in full. Decode
> capability entries structurally, skip unsupported entries individually,
> and register the supported entries from the same report. Allocate latest
> sample cache slots only for sensors with registered IIO devices.
> 
> Deliver sensor samples to IIO before committing the latest-sample
> cache, so a frame rejected by the registered channel layout or buffer
> path cannot change direct-read state or the last accepted sequence.
> Add focused KUnit coverage for rejected, valid, ignored, and malformed
> sample paths and cache-slot exhaustion.
> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>

A couple of minor things inline. 

Thanks,

Jonathan

> diff --git a/drivers/iio/opensensorfusion/osf_iio.c b/drivers/iio/opensensorfusion/osf_iio.c
> new file mode 100644
> index 000000000000..56030b4d6a9f
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_iio.c
> @@ -0,0 +1,304 @@

...

> +
> +#define OSF_MOD_CHAN(_type, _mod, _idx)                         \
> +	{                                                               \
> +		.type = (_type),                                           \
> +		.modified = 1,                                             \
> +		.channel2 = (_mod),                                        \
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),              \
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),      \
> +		.scan_index = (_idx),                                      \
> +		.scan_type = {                                             \
> +			.sign = 's',                                         \
> +			.realbits = 32,                                      \
> +			.storagebits = 32,                                   \
> +			.endianness = IIO_CPU,                               \
> +		},                                                        \
> +	}
> +
> +#define OSF_CHAN(_type, _idx)                                     \
> +	{                                                               \
> +		.type = (_type),                                           \
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),              \
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),      \
> +		.scan_index = (_idx),                                      \
> +		.scan_type = {                                             \
> +			.sign = 's',                                         \
> +			.realbits = 32,                                      \
> +			.storagebits = 32,                                   \
> +			.endianness = IIO_CPU,                               \
> +		},                                                        \
> +	}

Tidy up the \ as there doesn't seem to be any standard arrangement going on here.

> +int osf_iio_push_sample(struct iio_dev *indio_dev, const s32 *values,
> +			u16 channel_count)
> +{
> +	struct osf_iio_state *state = iio_priv(indio_dev);
> +	s64 timestamp;
> +
> +	if (channel_count != state->spec->channel_count)
> +		return -EPROTO;
> +
> +	if (!iio_buffer_enabled(indio_dev))
> +		return 0;
> +
> +	timestamp = iio_get_time_ns(indio_dev);
> +
> +	switch (channel_count) {
> +	case 1: {
> +		struct osf_iio_scan_1axis scan = { };

Similar to below - you might as well initialize the one value.

> +
> +		scan.value = values[0];
> +		return iio_push_to_buffers_with_ts(indio_dev, &scan,
> +						       sizeof(scan), timestamp);

Check for bits of alignment of code that have become wrong over time.

> +	}
> +	case 3: {
> +		struct osf_iio_scan_3axis scan = { };
> +
> +		scan.values[0] = values[0];
> +		scan.values[1] = values[1];
> +		scan.values[2] = values[2];

Might as well do
		struct osf_iio_scan_3axis scan = {
			.values[0] = values[0],
			.values[1] = values[1],
			.values[2] = values[2],
		};
Similar for other cases.

> +		return iio_push_to_buffers_with_ts(indio_dev, &scan,
> +						       sizeof(scan), timestamp);
> +	}
> +	default:
> +		return -EPROTO;
> +	}
> +}