Re: [PATCH RFC v2 4/7] iio: osf: add stream parser

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 24 May 2026 17:53:09 +0900
Jinseob Kim <kimjinseob88@gmail.com> wrote:

> Add byte stream assembly and resync for OSF0 frames.
Various minor things inline.

> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
> ---
>  drivers/iio/opensensorfusion/osf_stream.c | 207 ++++++++++++++++++++++
>  drivers/iio/opensensorfusion/osf_stream.h |  31 ++++
>  2 files changed, 238 insertions(+)
>  create mode 100644 drivers/iio/opensensorfusion/osf_stream.c
>  create mode 100644 drivers/iio/opensensorfusion/osf_stream.h
> 
> diff --git a/drivers/iio/opensensorfusion/osf_stream.c b/drivers/iio/opensensorfusion/osf_stream.c
> new file mode 100644
> index 000000000..a2739c987
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_stream.c
> @@ -0,0 +1,207 @@

> +
> +static size_t osf_stream_discard_to_magic(struct osf_stream *stream)
> +{
> +	size_t old_len = stream->len;
> +	size_t match_len;
> +	size_t i;
> +
> +	for (i = 0; i < stream->len; i++) {

	for (size_t i = 0; i < stream->len; i++)

> +		match_len = stream->len - i;
> +		if (match_len > OSF_STREAM_MAGIC_LEN)
> +			match_len = OSF_STREAM_MAGIC_LEN;

		match_len = min(stream->len - i, OSF_STREAM_MAGIC_LEN);

> +
> +		if (osf_stream_magic_match(stream->buf + i, match_len)) {
> +			if (i)
> +				osf_stream_discard(stream, i);
> +			return i;
> +		}
> +	}
> +
> +	stream->len = 0;
> +	return old_len;
> +}
> +
> +static int osf_stream_process(struct osf_stream *stream)
> +{
> +	struct osf_frame frame;
> +	size_t decoded_len;
> +	size_t discarded;
> +	size_t frame_len;
> +	u32 payload_len;
> +	int first_err = 0;
> +	int ret;
> +
> +	while (stream->len) {
> +		discarded = osf_stream_discard_to_magic(stream);
> +		if (discarded) {
> +			stream->stats.bad_magic_resyncs++;
> +			stream->stats.dropped_bytes += discarded;
> +			if (!first_err)
> +				first_err = -EPROTO;
> +		}
> +
> +		if (!stream->len)
> +			break;
> +
> +		if (stream->len < OSF_FRAME_HEADER_LEN) {
> +			stream->stats.partial_frames++;
> +			break;
> +		}
> +
> +		if (get_unaligned_le16(stream->buf + 6) !=
> +		    OSF_FRAME_HEADER_LEN) {

		if (get_unaligned_le16(stream->buf + 6) != OSF_FRAME_HEADER_LEN) {

is fine. We are a bit flexible on line length when it helps readability.

> +			stream->stats.dropped_bytes++;
> +			osf_stream_drop_invalid_head(stream);
> +			if (!first_err)
> +				first_err = -EPROTO;
> +			continue;
> +		}
> +
> +		payload_len = get_unaligned_le32(stream->buf + 10);
> +		if (payload_len > OSF_STREAM_MAX_PAYLOAD_LEN) {
> +			stream->stats.dropped_bytes++;
> +			osf_stream_drop_invalid_head(stream);
> +			if (!first_err)
> +				first_err = -EMSGSIZE;
> +			continue;
> +		}
> +
> +		frame_len = OSF_FRAME_HEADER_LEN + payload_len + OSF_FRAME_CRC_LEN;
> +		if (stream->len < frame_len) {
> +			stream->stats.partial_frames++;
> +			break;
> +		}
> +
> +		ret = osf_protocol_decode_frame(stream->buf, frame_len, &frame,
> +						&decoded_len);
> +		if (ret) {
> +			if (ret == -EBADMSG)
> +				stream->stats.bad_crc_frames++;
> +			stream->stats.dropped_bytes++;
> +			osf_stream_drop_invalid_head(stream);
> +			if (!first_err)
> +				first_err = ret;
> +			continue;
> +		}
> +
> +		if (decoded_len != frame_len) {
> +			stream->stats.dropped_bytes++;
> +			osf_stream_drop_invalid_head(stream);
> +			if (!first_err)
> +				first_err = -EMSGSIZE;
> +			continue;
> +		}
> +
> +		ret = osf_core_receive_frame(stream->osf, stream->buf, frame_len);
> +		if (ret) {
> +			osf_stream_discard(stream, frame_len);
> +			if (!first_err)
> +				first_err = ret;
> +			continue;
> +		}
> +
> +		stream->stats.valid_frames++;
> +		osf_stream_discard(stream, frame_len);
> +	}
> +
> +	return first_err;
> +}

> +int osf_stream_receive_bytes(struct osf_stream *stream, const u8 *buf,
> +			     size_t len)
> +{
> +	size_t copy_len;
> +	size_t space;
> +	int first_err = 0;
> +	int ret;
> +
> +	if (!stream || !stream->osf || (!buf && len))

As in previous patch, how do we get here with any of those being possible?
The last one might make sense as it's about coupling between parameters, but
I'm not seeing the first two as useful unless you clear stream->osf in remove
path or similar.  If you do maybe a comment.

> +		return -EINVAL;
> +
> +	if (!len) {
> +		ret = osf_stream_process(stream);
> +		if (ret && !first_err)
No way for first erro to be set, so same as:
		return osf_stream_process(stream);

If this changes later in this patch series then fine to keep it like this.

> +			first_err = ret;
> +		return first_err;
> +	}
> +
> +	while (len) {
> +		space = OSF_STREAM_MAX_FRAME_LEN - stream->len;
> +		if (!space) {
> +			stream->stats.dropped_bytes++;
> +			osf_stream_discard(stream, 1);
> +			if (!first_err)
> +				first_err = -EMSGSIZE;
> +			continue;
> +		}
> +
> +		copy_len = len < space ? len : space;

		copy_len = min(len, space);

> +		memcpy(stream->buf + stream->len, buf, copy_len);
> +		stream->len += copy_len;
> +		buf += copy_len;
> +		len -= copy_len;
> +
> +		ret = osf_stream_process(stream);
> +		if (ret && !first_err)
> +			first_err = ret;
> +	}
> +
> +	return first_err;
> +}
>