Re: [PATCH RFC v3 5/6] iio: osf: add UART serdev transport

[Jonathan Cameron]

On Fri, 29 May 2026 21:10:04 +0900
Jinseob Kim <kimjinseob88@xxxxxxxxx> wrote:

> Register the OSF serdev driver.
> 
> Pass received bytes into the OSF0 stream parser.
> 
> Signed-off-by: Jinseob Kim <kimjinseob88@xxxxxxxxx>

A few minor things inline.

> diff --git a/drivers/iio/opensensorfusion/osf_core.c b/drivers/iio/opensensorfusion/osf_core.c
> new file mode 100644
> index 000000000..c867b3158
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_core.c

> +void osf_core_init(struct osf_device *osf, struct device *dev)
> +{
> +	memset(osf, 0, sizeof(*osf));

Looks to me like it's is already guaranteed to be zero. If so don't
clear it again here.

> +	osf->dev = dev;
> +}

> +
> +static int osf_core_validate_capability_report(const struct osf_frame *frame)
> +{
> +	struct osf_capability_entry entry;
> +	struct osf_capability_report report;
> +	unsigned int i;
> +	int ret;
> +
> +	ret = osf_protocol_decode_capability_report(frame, &report);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < report.capability_count; i++) {

	for (unsigned int i = 0;

Is now acceptable in the kernel so use it where appropriate


> +		ret = osf_protocol_decode_capability_entry(&report, i, &entry);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int osf_core_receive_frame(struct osf_device *osf, const u8 *buf, size_t len)
> +{
> +	struct osf_frame frame;
> +	size_t frame_len;
> +	int ret;
> +
> +	if (!osf || !buf)
> +		return -EINVAL;
> +
> +	ret = osf_protocol_decode_frame(buf, len, &frame, &frame_len);
> +	if (ret)
> +		return ret;
> +
> +	if (frame_len != len)
> +		return -EMSGSIZE;
> +
> +	switch (frame.message_type) {
> +	case OSF_MSG_SENSOR_SAMPLE:
> +		ret = osf_core_validate_sensor_sample(&frame);
> +		break;
> +	case OSF_MSG_DEVICE_STATUS:
> +		ret = osf_core_validate_device_status(&frame);
> +		break;
> +	case OSF_MSG_CAPABILITY_REPORT:
> +		ret = osf_core_validate_capability_report(&frame);
> +		break;
> +	default:
> +		if (frame.message_type >= OSF_RESERVED_MSG_FIRST &&
> +		    frame.message_type <= OSF_RESERVED_MSG_LAST)
> +			ret = 0;
> +		else if (frame.message_type >= OSF_VENDOR_PRIVATE_FIRST)
> +			ret = 0;
> +		else
> +			ret = -EOPNOTSUPP;
I'd return early on error cases so...
> +		break;
> +	}
> +
> +	if (!ret)

.. you can drop this check as you know if you reach here we are in
good path.

It will mean adding a few 
if (ret)
	return ret;
above, but will give a simpler code flow.  Generally, I'd split
the bad path from the good as soon as possible.

> +		osf->last_sequence = frame.sequence;
> +
> +	return ret;
> +}


> +static struct serdev_device_driver osf_serdev_driver = {
> +	.probe = osf_serdev_probe,
> +	.remove = osf_serdev_remove,
> +	.driver = {
> +		.name = "open-sensor-fusion-uart",
> +		.of_match_table = osf_serdev_of_match,
> +	},
> +};
> +

Common convention to have no blank line here to keep that macro
tightly coupled with the structure.

> +module_serdev_device_driver(osf_serdev_driver);
> +
> +MODULE_DESCRIPTION("Open Sensor Fusion IIO driver");
> +MODULE_LICENSE("GPL");