Re: [PATCH v10 5/8] iio: osf: add UART transport and core receive path

[Jonathan Cameron]

On Sat, 19 Sep 2026 03:24:43 +0900
Jinseob Kim <kimjinseob88@xxxxxxxxx> wrote:

> Connect the stream parser to a serdev receiver and add core frame dispatch
> with an owned device-status cache. Preserve the distinction between
> unvalidated candidates and validated application rejections, so
> malformed status payloads do not cause byte-wise resynchronization.
> 
> Open and configure the UART before enabling the supply. A managed release
> action closes and drains the receive producer before resetting the parser
> and disabling a successfully enabled supply.
> 
> This transport base processes device-status frames and ignores message
> types without an application handler. It does not register IIO sensor
> devices. The following patch adds capability discovery and sample
> publication to IIO, including the publication gate and IIO teardown.
> 
> Assisted-by: LLM
> Signed-off-by: Jinseob Kim <kimjinseob88@xxxxxxxxx>
One question for Uwe if they see it about whether device-id/of.h should
be included or we can assume it for serial drivers.

Otherwise, just a couple of minor things inline.

Thanks,

Jonathan

> ---
>  MAINTAINERS                               |   3 +-
>  drivers/iio/Kconfig                       |   1 +
>  drivers/iio/Makefile                      |   1 +
>  drivers/iio/opensensorfusion/Kconfig      |  12 ++
>  drivers/iio/opensensorfusion/Makefile     |   5 +
>  drivers/iio/opensensorfusion/osf_core.c   | 101 +++++++++++++++
>  drivers/iio/opensensorfusion/osf_core.h   |  29 +++++
>  drivers/iio/opensensorfusion/osf_serdev.c | 143 ++++++++++++++++++++++
>  8 files changed, 293 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/iio/opensensorfusion/Kconfig
>  create mode 100644 drivers/iio/opensensorfusion/Makefile
>  create mode 100644 drivers/iio/opensensorfusion/osf_core.c
>  create mode 100644 drivers/iio/opensensorfusion/osf_core.h
>  create mode 100644 drivers/iio/opensensorfusion/osf_serdev.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bad05db854cf..fbbf5f06f1a5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20516,8 +20516,7 @@ M:	Jinseob Kim <kimjinseob88@xxxxxxxxx>
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml
>  F:	Documentation/iio/open-sensor-fusion.rst
> -F:	drivers/iio/opensensorfusion/osf_protocol.*
> -F:	drivers/iio/opensensorfusion/osf_stream.*
> +F:	drivers/iio/opensensorfusion/

Push that back to earlier patches.

>  K:	opensensorfusion

> diff --git a/drivers/iio/opensensorfusion/osf_core.c b/drivers/iio/opensensorfusion/osf_core.c
> new file mode 100644
> index 000000000000..e6812cef4d8c
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_core.c
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/types.h>
> +
> +#include "osf_core.h"
> +#include "osf_stream.h"
> +
> +#define OSF_RESERVED_MSG_FIRST		0x7f00
> +#define OSF_RESERVED_MSG_LAST		0x7fff
> +#define OSF_VENDOR_PRIVATE_FIRST	0x8000
> +
> +void osf_core_init(struct osf_device *osf, struct device *dev)
> +{
> +	*osf = (struct osf_device) {
> +		.dev = dev,
> +	};
> +}
Unless this gets a lot more complex in later patches, I'd drop
the helper.


> +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;
> +
> +	ret = osf_protocol_decode_frame(buf, len, &frame, &frame_len);
> +	if (ret)
> +		return ret;
> +
> +	if (frame_len != len)
> +		return -EMSGSIZE;
> +
> +	if (frame.protocol_major != OSF_PROTOCOL_MAJOR) {
> +		dev_dbg_ratelimited(osf->dev,
> +				    "ignoring unsupported protocol major %u\n",
> +				    frame.protocol_major);
> +		return OSF_STREAM_FRAME_IGNORED;
> +	}
> +
> +	switch (frame.message_type) {
> +	case OSF_MSG_DEVICE_STATUS:
> +		ret = osf_core_handle_device_status(osf, &frame);
> +		break;
> +	default:
> +		if (frame.message_type >= OSF_RESERVED_MSG_FIRST &&
> +		    frame.message_type <= OSF_RESERVED_MSG_LAST) {
> +			dev_dbg_ratelimited(osf->dev,
> +					    "ignoring reserved message type %#x\n",
> +					    frame.message_type);
> +			return OSF_STREAM_FRAME_IGNORED;
> +		}
> +		if (frame.message_type >= OSF_VENDOR_PRIVATE_FIRST) {
> +			dev_dbg_ratelimited(osf->dev,
> +					    "ignoring vendor message type %#x\n",
> +					    frame.message_type);
> +			return OSF_STREAM_FRAME_IGNORED;
> +		}
> +
> +		dev_dbg_ratelimited(osf->dev,
> +				    "ignoring unsupported message type %#x\n",
> +				    frame.message_type);
> +		return OSF_STREAM_FRAME_IGNORED;
> +	}
> +
> +	/*
> +	 * Handler failures are validated application rejections. Keep the
> +	 * trusted frame boundary and let the stream consume the complete frame.
> +	 */
> +	if (ret < 0)
> +		return OSF_STREAM_FRAME_REJECTED;

I think there is only one path to this.  Move the comment and check up there
+ the return.

Then we never leave the switch statement without returning so this last
chunk isn't needed.

> +
> +	return ret;
> +}

> diff --git a/drivers/iio/opensensorfusion/osf_serdev.c b/drivers/iio/opensensorfusion/osf_serdev.c
> new file mode 100644
> index 000000000000..8a747c01ff9d
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_serdev.c
> @@ -0,0 +1,143 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/cleanup.h>
> +#include <linux/device-id/of.h>

Curious that in all the recent tidying up of this stuff serdev.h
didn't include this one.  Uwe, if you see this, was that intentional?

Doesn't matter though as if it is something that gets tidied up later
then this driver will get covered with any others.

> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/serdev.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>

> +static int osf_serdev_probe(struct serdev_device *serdev)
> +{
> +	struct device *dev = &serdev->dev;
> +	struct osf_serdev *osf_uart;
> +	unsigned int baudrate;
> +	int ret;
> +
> +	osf_uart = devm_kzalloc(dev, sizeof(*osf_uart), GFP_KERNEL);
> +	if (!osf_uart)
> +		return -ENOMEM;
> +
> +	osf_uart->vcc = devm_regulator_get(dev, "vcc");
> +	if (IS_ERR(osf_uart->vcc))
> +		return dev_err_probe(dev, PTR_ERR(osf_uart->vcc),
> +				     "failed to get vcc regulator\n");
> +
> +	mutex_init(&osf_uart->rx_lock);

Can we use devm_mutex_init() here. It doesn't bring huge advantage
but it is a cheap win for some debugging cases.


> +	osf_uart->serdev = serdev;
> +	osf_core_init(&osf_uart->osf, dev);
> +	osf_stream_init(&osf_uart->stream, osf_serdev_receive_frame,
> +			&osf_uart->osf);
> +
> +	serdev_device_set_drvdata(serdev, osf_uart);
> +	serdev_device_set_client_ops(serdev, &osf_serdev_ops);
> +
> +	ret = serdev_device_open(serdev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, osf_serdev_release, osf_uart);
> +	if (ret)
> +		return ret;
> +
> +	baudrate = serdev_device_set_baudrate(serdev, OSF_SERDEV_BAUD);
> +	if (baudrate != OSF_SERDEV_BAUD)
> +		/* Keep accepting controller rounding after reporting the mismatch. */
> +		dev_warn_probe(dev, -EINVAL, "requested %u baud, controller set %u\n",
> +			       OSF_SERDEV_BAUD, baudrate);
> +
> +	serdev_device_set_flow_control(serdev, false);
> +
> +	ret = regulator_enable(osf_uart->vcc);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to enable vcc regulator\n");
> +	osf_uart->vcc_enabled = true;
> +
> +	return 0;
> +}