Re: [PATCH RFC v3 3/6] iio: osf: add protocol v0 decoding

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Fri, May 29, 2026 at 09:10:02PM +0900, Jinseob Kim wrote:
> Add OSF0 frame validation and payload decoders.

> Extend MAINTAINERS to cover the protocol decoder.

Unneeded detail in the commit message. It's implied that MAINTAINERS is updated
accordingly.

...

> +int osf_protocol_decode_frame(const u8 *buf, size_t len,
> +			      struct osf_frame *frame, size_t *frame_len)
> +{
> +	u32 expected_crc;
> +	u32 actual_crc;
> +	u32 payload_len;
> +	size_t total_len;
> +	u8 major;
> +
> +	if (!buf || !frame || !frame_len)
> +		return -EINVAL;
> +
> +	if (len < OSF_FRAME_MIN_LEN)
> +		return -EMSGSIZE;

> +	if (buf[0] != 'O' || buf[1] != 'S' || buf[2] != 'F' || buf[3] != '0')
> +		return -EPROTO;

It's FourCC, define it as integer and compare as integer

#define ..._MAGIC	0xhhhhhhhh	/* OSF0 */

	if (get_unaligned_le32() != _MAGIC)


> +	major = buf[4];
> +	if (major != OSF_PROTOCOL_MAJOR)
> +		return -EPROTO;
> +
> +	if (get_unaligned_le16(buf + 6) != OSF_FRAME_HEADER_LEN)
> +		return -EPROTO;
> +
> +	payload_len = get_unaligned_le32(buf + 10);
> +	if (payload_len > len - OSF_FRAME_MIN_LEN)
> +		return -EMSGSIZE;
> +
> +	if (get_unaligned_le32(buf + 34))
> +		return -EPROTO;
> +
> +	total_len = OSF_FRAME_HEADER_LEN + payload_len + OSF_FRAME_CRC_LEN;
> +	expected_crc = osf_crc32_ieee(buf, OSF_FRAME_HEADER_LEN + payload_len);
> +	actual_crc = get_unaligned_le32(buf + OSF_FRAME_HEADER_LEN + payload_len);
> +
> +	if (actual_crc != expected_crc)
> +		return -EBADMSG;
> +
> +	frame->protocol_minor = buf[5];
> +	frame->message_type = get_unaligned_le16(buf + 8);
> +	frame->payload_len = payload_len;
> +	frame->sequence = get_unaligned_le64(buf + 14);
> +	frame->timestamp_us = get_unaligned_le64(buf + 22);
> +	frame->flags = get_unaligned_le32(buf + 30);
> +	frame->payload = buf + OSF_FRAME_HEADER_LEN;
> +	frame->crc = actual_crc;
> +	*frame_len = total_len;
> +
> +	return 0;
> +}

...

> +int osf_protocol_sensor_sample_value(const struct osf_sensor_sample *sample,
> +				     unsigned int index, s32 *value)
> +{
> +	if (!sample || !sample->samples || !value)
> +		return -EINVAL;
> +
> +	if (index >= sample->channel_count)
> +		return -ERANGE;
> +
> +	*value = (s32)get_unaligned_le32(sample->samples + index * sizeof(s32));

Why casting?

> +	return 0;
> +}

...

> +#define OSF_CAPABILITY_FLAGS_MASK	0x00000003U

GENMASK() ?


-- 
With Best Regards,
Andy Shevchenko