mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver
@ 2026-05-24  8:53 Jinseob Kim
  2026-05-24  8:53 ` [PATCH RFC v2 1/7] dt-bindings: iio: add Open Sensor Fusion UART device Jinseob Kim
                   ` (7 more replies)
  0 siblings, 8 replies; 26+ messages in thread
From: Jinseob Kim @ 2026-05-24  8:53 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel, Jinseob Kim

This series adds an IIO driver for a UART-attached sensor aggregation
device. The device sends OSF0 binary frames over serdev UART. The driver
uses capability reports to register IIO devices for supported sensors.

v2 preparation changes:

- split the previous RFC driver patch into smaller patches
- add a Device Tree binding
- add an OSF0 protocol reference
- use get_unaligned_le16/32/64() for wire fields
- use IIO timestamp handling for buffered samples
- push decoded samples directly to registered IIO devices

Tested path:

- STM32F405 OSF0 UART stream
- Raspberry Pi 4 serdev
- kernel 6.12.75+rpt-rpi-v8
- IIO devices: osf-accel, osf-gyro, osf-magn, osf-temp
- raw reads from accel, gyro, magn, and temp
- buffer reads from accel, gyro, magn, and temp

No OSF oops, panic, or call trace was observed in the Raspberry Pi
runtime smoke.

Jinseob Kim (7):
  dt-bindings: iio: add Open Sensor Fusion UART device
  Documentation: iio: add Open Sensor Fusion protocol v0 reference
  iio: osf: add protocol v0 decoding
  iio: osf: add stream parser
  iio: osf: add UART serdev transport
  iio: osf: register IIO devices from capabilities
  MAINTAINERS: add Open Sensor Fusion IIO driver

 .../iio/imu/opensensorfusion,osf-uart.yaml    |  33 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 .../iio/open-sensor-fusion-protocol-v0.rst    | 267 +++++++++++++++
 MAINTAINERS                                   |   9 +
 drivers/iio/Kconfig                           |   1 +
 drivers/iio/Makefile                          |   1 +
 drivers/iio/opensensorfusion/Kconfig          |  15 +
 drivers/iio/opensensorfusion/Makefile         |   6 +
 drivers/iio/opensensorfusion/osf_core.c       | 311 ++++++++++++++++++
 drivers/iio/opensensorfusion/osf_core.h       |  67 ++++
 drivers/iio/opensensorfusion/osf_iio.c        | 288 ++++++++++++++++
 drivers/iio/opensensorfusion/osf_iio.h        |  22 ++
 drivers/iio/opensensorfusion/osf_protocol.c   | 220 +++++++++++++
 drivers/iio/opensensorfusion/osf_protocol.h   | 100 ++++++
 drivers/iio/opensensorfusion/osf_serdev.c     | 111 +++++++
 drivers/iio/opensensorfusion/osf_stream.c     | 207 ++++++++++++
 drivers/iio/opensensorfusion/osf_stream.h     |  31 ++
 17 files changed, 1691 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/imu/opensensorfusion,osf-uart.yaml
 create mode 100644 Documentation/iio/open-sensor-fusion-protocol-v0.rst
 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_iio.c
 create mode 100644 drivers/iio/opensensorfusion/osf_iio.h
 create mode 100644 drivers/iio/opensensorfusion/osf_protocol.c
 create mode 100644 drivers/iio/opensensorfusion/osf_protocol.h
 create mode 100644 drivers/iio/opensensorfusion/osf_serdev.c
 create mode 100644 drivers/iio/opensensorfusion/osf_stream.c
 create mode 100644 drivers/iio/opensensorfusion/osf_stream.h

-- 
2.43.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH RFC v2 1/7] dt-bindings: iio: add Open Sensor Fusion UART device
  2026-05-24  8:53 [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver Jinseob Kim
@ 2026-05-24  8:53 ` Jinseob Kim
  2026-05-24 18:59   ` Krzysztof Kozlowski
  2026-05-24 19:34   ` Conor Dooley
  2026-05-24  8:53 ` [PATCH RFC v2 2/7] Documentation: iio: add Open Sensor Fusion protocol v0 reference Jinseob Kim
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 26+ messages in thread
From: Jinseob Kim @ 2026-05-24  8:53 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel, Jinseob Kim

Add a binding for the OSF0 UART-attached sensor aggregation device.

Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
---
 .../iio/imu/opensensorfusion,osf-uart.yaml    | 33 +++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 ++
 2 files changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/imu/opensensorfusion,osf-uart.yaml

diff --git a/Documentation/devicetree/bindings/iio/imu/opensensorfusion,osf-uart.yaml b/Documentation/devicetree/bindings/iio/imu/opensensorfusion,osf-uart.yaml
new file mode 100644
index 000000000..d4a8f6819
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/opensensorfusion,osf-uart.yaml
@@ -0,0 +1,33 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/opensensorfusion,osf-uart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Open Sensor Fusion UART sensor aggregation device
+
+maintainers:
+  - Jinseob Kim <kimjinseob88@gmail.com>
+
+description:
+  UART-attached sensor aggregation device using the OSF0 frame format.
+  The device sends capability, status, and sample frames to the host.
+
+properties:
+  compatible:
+    const: opensensorfusion,osf-uart
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    serial {
+        osf {
+            compatible = "opensensorfusion,osf-uart";
+        };
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 28784d66a..121b83f70 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1239,6 +1239,8 @@ patternProperties:
     description: OpenRISC.io
   "^openwrt,.*":
     description: OpenWrt
+  "^opensensorfusion,.*":
+    description: Open Sensor Fusion project
   "^option,.*":
     description: Option NV
   "^oranth,.*":
-- 
2.43.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH RFC v2 2/7] Documentation: iio: add Open Sensor Fusion protocol v0 reference
  2026-05-24  8:53 [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver Jinseob Kim
  2026-05-24  8:53 ` [PATCH RFC v2 1/7] dt-bindings: iio: add Open Sensor Fusion UART device Jinseob Kim
@ 2026-05-24  8:53 ` Jinseob Kim
  2026-05-28 13:28   ` Jonathan Cameron
  2026-05-24  8:53 ` [PATCH RFC v2 3/7] iio: osf: add protocol v0 decoding Jinseob Kim
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Jinseob Kim @ 2026-05-24  8:53 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel, Jinseob Kim

Document the OSF0 frame format and payloads used by the driver.

Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
---
 .../iio/open-sensor-fusion-protocol-v0.rst    | 267 ++++++++++++++++++
 1 file changed, 267 insertions(+)
 create mode 100644 Documentation/iio/open-sensor-fusion-protocol-v0.rst

diff --git a/Documentation/iio/open-sensor-fusion-protocol-v0.rst b/Documentation/iio/open-sensor-fusion-protocol-v0.rst
new file mode 100644
index 000000000..4800a3ce6
--- /dev/null
+++ b/Documentation/iio/open-sensor-fusion-protocol-v0.rst
@@ -0,0 +1,267 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Open Sensor Fusion protocol v0
+==============================
+
+This document describes the OSF0 UART wire format used by the Linux IIO
+driver. It is not a firmware programming interface.
+
+Device model
+------------
+
+An Open Sensor Fusion UART device is a sensor aggregation device. It sends
+binary frames from the device to the host. The host driver decodes the frames
+and maps supported sensors to IIO devices.
+
+The hardware used for smoke testing is an OSF GREEN prototype with an
+STM32F405RGT6 MCU, an ICM42688P-class IMU, and an MMC5983MA magnetometer. That
+hardware is a test target, not part of the binding ABI.
+
+Transport
+---------
+
+The transport is UART at 115200 baud, 8 data bits, no parity, and 1 stop bit.
+The Linux transport is serdev. The v0 upstream driver covers device-to-host
+frames. Flow control is not used by the tested stream.
+
+Byte order
+----------
+
+All multi-byte integer fields are little-endian. Samples use signed 32-bit
+little-endian integers when ``sample_format`` is ``S32``.
+
+Frame format
+------------
+
+Each frame has a fixed 38-byte header, a payload, and a 4-byte CRC.
+
+.. list-table::
+   :header-rows: 1
+
+   * - Offset
+     - Size
+     - Field
+     - Description
+   * - 0
+     - 4
+     - magic
+     - ASCII ``OSF0``
+   * - 4
+     - 1
+     - protocol_major
+     - Must be ``0``
+   * - 5
+     - 1
+     - protocol_minor
+     - Minor version
+   * - 6
+     - 2
+     - header_len
+     - Must be ``38``
+   * - 8
+     - 2
+     - message_type
+     - Message type
+   * - 10
+     - 4
+     - payload_len
+     - Payload length in bytes
+   * - 14
+     - 8
+     - sequence
+     - Monotonic device sequence
+   * - 22
+     - 8
+     - timestamp_us
+     - Device timestamp in microseconds
+   * - 30
+     - 4
+     - flags
+     - Message flags
+   * - 34
+     - 4
+     - reserved
+     - Must be zero for v0
+   * - 38
+     - payload_len
+     - payload
+     - Message payload
+   * - 38 + payload_len
+     - 4
+     - crc32
+     - CRC32 over header and payload
+
+The frame CRC is IEEE CRC32 as implemented by ``crc32_le()`` with initial
+value ``0xffffffff`` and final XOR value ``0xffffffff``. The CRC field is not
+included in the CRC input.
+
+Message types
+-------------
+
+.. list-table::
+   :header-rows: 1
+
+   * - Value
+     - Name
+     - Direction
+   * - ``0x0001``
+     - ``SENSOR_SAMPLE``
+     - device to host
+   * - ``0x0002``
+     - ``DEVICE_STATUS``
+     - device to host
+   * - ``0x0003``
+     - ``CAPABILITY_REPORT``
+     - device to host
+
+Message types ``0x7f00`` through ``0x7fff`` are reserved. Values at or above
+``0x8000`` are vendor private and are ignored by the upstream driver.
+
+``SENSOR_SAMPLE`` payload
+-------------------------
+
+The base payload size is 16 bytes.
+
+.. list-table::
+   :header-rows: 1
+
+   * - Offset
+     - Size
+     - Field
+     - Description
+   * - 0
+     - 2
+     - sensor_type
+     - Sensor type ID
+   * - 2
+     - 2
+     - sensor_index
+     - Instance index
+   * - 4
+     - 2
+     - channel_count
+     - Number of S32 channels
+   * - 6
+     - 2
+     - sample_format
+     - Must be ``1`` (``S32``)
+   * - 8
+     - 4
+     - scale_nano
+     - Scale factor in nano-units
+   * - 12
+     - 4
+     - reserved
+     - Must be zero for v0
+   * - 16
+     - 4 * channel_count
+     - samples
+     - Signed 32-bit channel samples
+
+``DEVICE_STATUS`` payload
+-------------------------
+
+The payload size is 20 bytes. Fields are ``uptime_s``, ``status_flags``,
+``error_flags``, ``dropped_frames``, and a reserved field. Each field is
+32 bits.
+
+``CAPABILITY_REPORT`` payload
+-----------------------------
+
+The base payload size is 4 bytes. It contains ``capability_count`` and a
+reserved field. Each capability entry is 20 bytes:
+
+.. list-table::
+   :header-rows: 1
+
+   * - Offset
+     - Size
+     - Field
+     - Description
+   * - 0
+     - 2
+     - sensor_type
+     - Sensor type ID
+   * - 2
+     - 2
+     - sensor_index
+     - Instance index
+   * - 4
+     - 2
+     - channel_count
+     - Number of channels
+   * - 6
+     - 2
+     - sample_format
+     - Must be ``1`` (``S32``)
+   * - 8
+     - 4
+     - scale_nano
+     - Scale factor in nano-units
+   * - 12
+     - 4
+     - flags
+     - Capability flags
+   * - 16
+     - 4
+     - reserved
+     - Must be zero for v0
+
+Capability flag bit 0 means enabled by default. Bit 1 means calibrated data can
+be provided by the device. Other bits are invalid for v0.
+
+Sensor type IDs
+---------------
+
+.. list-table::
+   :header-rows: 1
+
+   * - Value
+     - Sensor
+     - IIO mapping
+   * - ``0x0001``
+     - accelerometer
+     - ``IIO_ACCEL``, X/Y/Z
+   * - ``0x0002``
+     - gyroscope
+     - ``IIO_ANGL_VEL``, X/Y/Z
+   * - ``0x0003``
+     - magnetometer
+     - ``IIO_MAGN``, X/Y/Z
+   * - ``0x0004``
+     - barometer
+     - not mapped in the initial driver
+   * - ``0x0005``
+     - temperature
+     - ``IIO_TEMP``
+   * - ``0x0006``
+     - humidity
+     - not mapped in the initial driver
+   * - ``0x0007``
+     - ambient light
+     - not mapped in the initial driver
+   * - ``0x0008``
+     - proximity
+     - not mapped in the initial driver
+
+Scaling
+-------
+
+``scale_nano`` is the per-channel scale value in nano-units. The Linux driver
+maps it to ``IIO_CHAN_INFO_SCALE`` as integer plus nano. The exact physical
+unit depends on the IIO channel type.
+
+Timestamps
+----------
+
+The frame header carries ``timestamp_us``, a device-side timestamp in
+microseconds. v0 transports this value for ordering and diagnostics. The driver
+does not use it as a production-grade host-correlated timestamp. Buffered IIO
+timestamps follow IIO timestamp clock handling.
+
+Non-goals for v0 upstream
+-------------------------
+
+The v0 upstream driver does not include USB transport, fusion output,
+AHRS/Kalman output, calibration command ABI, custom sysfs control surface,
+production timestamp correlation, or runtime capability removal.
-- 
2.43.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH RFC v2 3/7] iio: osf: add protocol v0 decoding
  2026-05-24  8:53 [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver Jinseob Kim
  2026-05-24  8:53 ` [PATCH RFC v2 1/7] dt-bindings: iio: add Open Sensor Fusion UART device Jinseob Kim
  2026-05-24  8:53 ` [PATCH RFC v2 2/7] Documentation: iio: add Open Sensor Fusion protocol v0 reference Jinseob Kim
@ 2026-05-24  8:53 ` Jinseob Kim
  2026-05-28 13:48   ` Jonathan Cameron
  2026-05-24  8:53 ` [PATCH RFC v2 4/7] iio: osf: add stream parser Jinseob Kim
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Jinseob Kim @ 2026-05-24  8:53 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel, Jinseob Kim

Add OSF0 frame and payload decoders for device-to-host messages.

Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
---
 drivers/iio/opensensorfusion/osf_protocol.c | 220 ++++++++++++++++++++
 drivers/iio/opensensorfusion/osf_protocol.h | 100 +++++++++
 2 files changed, 320 insertions(+)
 create mode 100644 drivers/iio/opensensorfusion/osf_protocol.c
 create mode 100644 drivers/iio/opensensorfusion/osf_protocol.h

diff --git a/drivers/iio/opensensorfusion/osf_protocol.c b/drivers/iio/opensensorfusion/osf_protocol.c
new file mode 100644
index 000000000..ac3c37ae2
--- /dev/null
+++ b/drivers/iio/opensensorfusion/osf_protocol.c
@@ -0,0 +1,220 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/crc32.h>
+#include <linux/errno.h>
+#include <linux/bits.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+#include "osf_protocol.h"
+
+#define OSF_CRC32_INIT		GENMASK(31, 0)
+#define OSF_CRC32_XOROUT	GENMASK(31, 0)
+
+static bool osf_sensor_type_valid(u16 sensor_type)
+{
+	return sensor_type >= OSF_SENSOR_ACCELEROMETER &&
+	       sensor_type <= OSF_SENSOR_PROXIMITY;
+}
+
+static u32 osf_crc32_ieee(const u8 *buf, size_t len)
+{
+	return crc32_le(OSF_CRC32_INIT, buf, len) ^ OSF_CRC32_XOROUT;
+}
+
+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;
+
+	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;
+
+	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->reserved = get_unaligned_le32(buf + 34);
+	frame->payload = buf + OSF_FRAME_HEADER_LEN;
+	frame->crc = actual_crc;
+	*frame_len = total_len;
+
+	return 0;
+}
+
+int osf_protocol_decode_sensor_sample(const struct osf_frame *frame,
+				      struct osf_sensor_sample *sample)
+{
+	u16 channel_count;
+	u16 sample_format;
+	u32 expected_len;
+	const u8 *payload;
+
+	if (!frame || !sample || !frame->payload)
+		return -EINVAL;
+
+	if (frame->message_type != OSF_MSG_SENSOR_SAMPLE)
+		return -EPROTO;
+
+	if (frame->payload_len < OSF_SENSOR_SAMPLE_BASE_LEN)
+		return -EMSGSIZE;
+
+	payload = frame->payload;
+	channel_count = get_unaligned_le16(payload + 4);
+	sample_format = get_unaligned_le16(payload + 6);
+
+	if (sample_format != OSF_SAMPLE_FORMAT_S32)
+		return -EPROTO;
+
+	expected_len = OSF_SENSOR_SAMPLE_BASE_LEN + channel_count * sizeof(s32);
+	if (frame->payload_len != expected_len)
+		return -EMSGSIZE;
+
+	sample->sensor_type = get_unaligned_le16(payload);
+	sample->sensor_index = get_unaligned_le16(payload + 2);
+	sample->channel_count = channel_count;
+	sample->sample_format = sample_format;
+	sample->scale_nano = get_unaligned_le32(payload + 8);
+	sample->reserved = get_unaligned_le32(payload + 12);
+	sample->samples = payload + OSF_SENSOR_SAMPLE_BASE_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));
+
+	return 0;
+}
+
+int osf_protocol_decode_device_status(const struct osf_frame *frame,
+				      struct osf_device_status *status)
+{
+	const u8 *payload;
+
+	if (!frame || !status || !frame->payload)
+		return -EINVAL;
+
+	if (frame->message_type != OSF_MSG_DEVICE_STATUS)
+		return -EPROTO;
+
+	if (frame->payload_len != OSF_DEVICE_STATUS_LEN)
+		return -EMSGSIZE;
+
+	payload = frame->payload;
+	status->uptime_s = get_unaligned_le32(payload);
+	status->status_flags = get_unaligned_le32(payload + 4);
+	status->error_flags = get_unaligned_le32(payload + 8);
+	status->dropped_frames = get_unaligned_le32(payload + 12);
+	status->reserved = get_unaligned_le32(payload + 16);
+
+	return 0;
+}
+
+int osf_protocol_decode_capability_report(const struct osf_frame *frame,
+					  struct osf_capability_report *report)
+{
+	u16 capability_count;
+	u32 expected_len;
+	const u8 *payload;
+
+	if (!frame || !report || !frame->payload)
+		return -EINVAL;
+
+	if (frame->message_type != OSF_MSG_CAPABILITY_REPORT)
+		return -EPROTO;
+
+	if (frame->payload_len < OSF_CAP_REPORT_BASE_LEN)
+		return -EMSGSIZE;
+
+	payload = frame->payload;
+	capability_count = get_unaligned_le16(payload);
+	expected_len = OSF_CAP_REPORT_BASE_LEN +
+		       capability_count * OSF_CAP_SENSOR_ENTRY_LEN;
+
+	if (frame->payload_len != expected_len)
+		return -EMSGSIZE;
+
+	report->capability_count = capability_count;
+	report->reserved = get_unaligned_le16(payload + 2);
+	if (report->reserved)
+		return -EPROTO;
+
+	report->entries = payload + OSF_CAP_REPORT_BASE_LEN;
+
+	return 0;
+}
+
+int osf_protocol_decode_capability_entry(const struct osf_capability_report *report,
+					 unsigned int index,
+					 struct osf_capability_entry *entry)
+{
+	const u8 *payload;
+
+	if (!report || !report->entries || !entry)
+		return -EINVAL;
+
+	if (index >= report->capability_count)
+		return -ERANGE;
+
+	payload = report->entries + index * OSF_CAP_SENSOR_ENTRY_LEN;
+	entry->sensor_type = get_unaligned_le16(payload);
+	entry->sensor_index = get_unaligned_le16(payload + 2);
+	entry->channel_count = get_unaligned_le16(payload + 4);
+	entry->sample_format = get_unaligned_le16(payload + 6);
+	entry->scale_nano = get_unaligned_le32(payload + 8);
+	entry->flags = get_unaligned_le32(payload + 12);
+	entry->reserved = get_unaligned_le32(payload + 16);
+
+	if (!osf_sensor_type_valid(entry->sensor_type))
+		return -EPROTO;
+
+	if (entry->sample_format != OSF_SAMPLE_FORMAT_S32)
+		return -EPROTO;
+
+	if (entry->flags & ~OSF_CAPABILITY_FLAGS_MASK)
+		return -EPROTO;
+
+	if (entry->reserved)
+		return -EPROTO;
+
+	return 0;
+}
diff --git a/drivers/iio/opensensorfusion/osf_protocol.h b/drivers/iio/opensensorfusion/osf_protocol.h
new file mode 100644
index 000000000..fd6e9581f
--- /dev/null
+++ b/drivers/iio/opensensorfusion/osf_protocol.h
@@ -0,0 +1,100 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _OSF_PROTOCOL_H
+#define _OSF_PROTOCOL_H
+
+#include <linux/types.h>
+
+#define OSF_PROTOCOL_MAJOR		0
+#define OSF_PROTOCOL_MINOR		0
+#define OSF_FRAME_HEADER_LEN		38
+#define OSF_FRAME_CRC_LEN		4
+#define OSF_FRAME_MIN_LEN		(OSF_FRAME_HEADER_LEN + OSF_FRAME_CRC_LEN)
+
+#define OSF_SENSOR_SAMPLE_BASE_LEN	16
+#define OSF_DEVICE_STATUS_LEN		20
+#define OSF_CAP_REPORT_BASE_LEN		4
+#define OSF_CAP_SENSOR_ENTRY_LEN		20
+#define OSF_CAPABILITY_FLAGS_MASK	0x00000003U
+
+enum osf_message_type {
+	OSF_MSG_SENSOR_SAMPLE		= 0x0001,
+	OSF_MSG_DEVICE_STATUS		= 0x0002,
+	OSF_MSG_CAPABILITY_REPORT	= 0x0003,
+};
+
+enum osf_sensor_type {
+	OSF_SENSOR_ACCELEROMETER		= 0x0001,
+	OSF_SENSOR_GYROSCOPE		= 0x0002,
+	OSF_SENSOR_MAGNETOMETER		= 0x0003,
+	OSF_SENSOR_BAROMETER		= 0x0004,
+	OSF_SENSOR_TEMPERATURE		= 0x0005,
+	OSF_SENSOR_HUMIDITY		= 0x0006,
+	OSF_SENSOR_AMBIENT_LIGHT		= 0x0007,
+	OSF_SENSOR_PROXIMITY		= 0x0008,
+};
+
+enum osf_sample_format {
+	OSF_SAMPLE_FORMAT_S32		= 0x0001,
+};
+
+struct osf_frame {
+	u8 protocol_minor;
+	u16 message_type;
+	u32 payload_len;
+	u64 sequence;
+	u64 timestamp_us;
+	u32 flags;
+	u32 reserved;
+	const u8 *payload;
+	u32 crc;
+};
+
+struct osf_sensor_sample {
+	u16 sensor_type;
+	u16 sensor_index;
+	u16 channel_count;
+	u16 sample_format;
+	u32 scale_nano;
+	u32 reserved;
+	const u8 *samples;
+};
+
+struct osf_device_status {
+	u32 uptime_s;
+	u32 status_flags;
+	u32 error_flags;
+	u32 dropped_frames;
+	u32 reserved;
+};
+
+struct osf_capability_report {
+	u16 capability_count;
+	u16 reserved;
+	const u8 *entries;
+};
+
+struct osf_capability_entry {
+	u16 sensor_type;
+	u16 sensor_index;
+	u16 channel_count;
+	u16 sample_format;
+	u32 scale_nano;
+	u32 flags;
+	u32 reserved;
+};
+
+int osf_protocol_decode_frame(const u8 *buf, size_t len,
+			      struct osf_frame *frame, size_t *frame_len);
+int osf_protocol_decode_sensor_sample(const struct osf_frame *frame,
+				      struct osf_sensor_sample *sample);
+int osf_protocol_decode_device_status(const struct osf_frame *frame,
+				      struct osf_device_status *status);
+int osf_protocol_decode_capability_report(const struct osf_frame *frame,
+					  struct osf_capability_report *report);
+int osf_protocol_decode_capability_entry(const struct osf_capability_report *report,
+					 unsigned int index,
+					 struct osf_capability_entry *entry);
+int osf_protocol_sensor_sample_value(const struct osf_sensor_sample *sample,
+				     unsigned int index, s32 *value);
+
+#endif
-- 
2.43.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH RFC v2 4/7] iio: osf: add stream parser
  2026-05-24  8:53 [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver Jinseob Kim
                   ` (2 preceding siblings ...)
  2026-05-24  8:53 ` [PATCH RFC v2 3/7] iio: osf: add protocol v0 decoding Jinseob Kim
@ 2026-05-24  8:53 ` Jinseob Kim
  2026-05-28 13:58   ` Jonathan Cameron
  2026-05-24  8:53 ` [PATCH RFC v2 5/7] iio: osf: add UART serdev transport Jinseob Kim
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Jinseob Kim @ 2026-05-24  8:53 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel, Jinseob Kim

Add byte stream assembly and resync for OSF0 frames.

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 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+#include "osf_core.h"
+#include "osf_protocol.h"
+#include "osf_stream.h"
+
+#define OSF_STREAM_MAGIC_LEN	4
+#define OSF_STREAM_MAX_PAYLOAD_LEN				\
+	(OSF_STREAM_MAX_FRAME_LEN - OSF_FRAME_HEADER_LEN - OSF_FRAME_CRC_LEN)
+
+static const u8 osf_stream_magic[OSF_STREAM_MAGIC_LEN] = {
+	'O', 'S', 'F', '0',
+};
+
+static void osf_stream_discard(struct osf_stream *stream, size_t count)
+{
+	if (count >= stream->len) {
+		stream->len = 0;
+		return;
+	}
+
+	memmove(stream->buf, stream->buf + count, stream->len - count);
+	stream->len -= count;
+}
+
+static void osf_stream_drop_invalid_head(struct osf_stream *stream)
+{
+	osf_stream_discard(stream, 1);
+}
+
+static bool osf_stream_magic_match(const u8 *buf, size_t len)
+{
+	return !memcmp(buf, osf_stream_magic, len);
+}
+
+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++) {
+		match_len = stream->len - i;
+		if (match_len > OSF_STREAM_MAGIC_LEN)
+			match_len = 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) {
+			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;
+}
+
+void osf_stream_init(struct osf_stream *stream, struct osf_device *osf)
+{
+	if (!stream)
+		return;
+
+	stream->osf = osf;
+	stream->len = 0;
+	memset(&stream->stats, 0, sizeof(stream->stats));
+}
+
+void osf_stream_reset(struct osf_stream *stream)
+{
+	if (stream) {
+		stream->len = 0;
+		memset(&stream->stats, 0, sizeof(stream->stats));
+	}
+}
+
+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))
+		return -EINVAL;
+
+	if (!len) {
+		ret = osf_stream_process(stream);
+		if (ret && !first_err)
+			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;
+		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;
+}
diff --git a/drivers/iio/opensensorfusion/osf_stream.h b/drivers/iio/opensensorfusion/osf_stream.h
new file mode 100644
index 000000000..f7f9477fe
--- /dev/null
+++ b/drivers/iio/opensensorfusion/osf_stream.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _OSF_STREAM_H
+#define _OSF_STREAM_H
+
+#include <linux/types.h>
+
+#define OSF_STREAM_MAX_FRAME_LEN	4096
+
+struct osf_device;
+
+struct osf_stream_stats {
+	u64 valid_frames;
+	u64 bad_magic_resyncs;
+	u64 bad_crc_frames;
+	u64 partial_frames;
+	u64 dropped_bytes;
+};
+
+struct osf_stream {
+	struct osf_device *osf;
+	u8 buf[OSF_STREAM_MAX_FRAME_LEN];
+	size_t len;
+	struct osf_stream_stats stats;
+};
+
+void osf_stream_init(struct osf_stream *stream, struct osf_device *osf);
+void osf_stream_reset(struct osf_stream *stream);
+int osf_stream_receive_bytes(struct osf_stream *stream, const u8 *buf,
+			     size_t len);
+
+#endif
-- 
2.43.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH RFC v2 5/7] iio: osf: add UART serdev transport
  2026-05-24  8:53 [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver Jinseob Kim
                   ` (3 preceding siblings ...)
  2026-05-24  8:53 ` [PATCH RFC v2 4/7] iio: osf: add stream parser Jinseob Kim
@ 2026-05-24  8:53 ` Jinseob Kim
  2026-05-28 14:06   ` Jonathan Cameron
  2026-05-24  8:53 ` [PATCH RFC v2 6/7] iio: osf: register IIO devices from capabilities Jinseob Kim
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Jinseob Kim @ 2026-05-24  8:53 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel, Jinseob Kim

Add the serdev receive path that feeds decoded OSF0 frames to the core.

Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
---
 drivers/iio/Kconfig                       |   1 +
 drivers/iio/Makefile                      |   1 +
 drivers/iio/opensensorfusion/Kconfig      |  15 +++
 drivers/iio/opensensorfusion/Makefile     |   5 +
 drivers/iio/opensensorfusion/osf_core.c   | 107 +++++++++++++++++++++
 drivers/iio/opensensorfusion/osf_core.h   |  18 ++++
 drivers/iio/opensensorfusion/osf_serdev.c | 111 ++++++++++++++++++++++
 7 files changed, 258 insertions(+)
 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/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 661127aed..939f6c546 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -101,6 +101,7 @@ source "drivers/iio/light/Kconfig"
 source "drivers/iio/magnetometer/Kconfig"
 source "drivers/iio/multiplexer/Kconfig"
 source "drivers/iio/orientation/Kconfig"
+source "drivers/iio/opensensorfusion/Kconfig"
 source "drivers/iio/test/Kconfig"
 if IIO_TRIGGER
    source "drivers/iio/trigger/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index cb80ef837..d864fe17b 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -37,6 +37,7 @@ obj-y += light/
 obj-y += magnetometer/
 obj-y += multiplexer/
 obj-y += orientation/
+obj-y += opensensorfusion/
 obj-y += position/
 obj-y += potentiometer/
 obj-y += potentiostat/
diff --git a/drivers/iio/opensensorfusion/Kconfig b/drivers/iio/opensensorfusion/Kconfig
new file mode 100644
index 000000000..360f25b4f
--- /dev/null
+++ b/drivers/iio/opensensorfusion/Kconfig
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config OPEN_SENSOR_FUSION
+	tristate "Open Sensor Fusion UART IIO driver"
+	depends on IIO
+	depends on SERIAL_DEV_BUS
+	select CRC32
+	help
+	  Build the Open Sensor Fusion UART receive path.
+
+	  The driver receives OSF0 frames over a serdev UART.
+	  Frames are decoded and validated before being passed to the
+	  driver core.
+	  This patch only adds the transport path.
+	  IIO device registration is added separately.
diff --git a/drivers/iio/opensensorfusion/Makefile b/drivers/iio/opensensorfusion/Makefile
new file mode 100644
index 000000000..940c82edd
--- /dev/null
+++ b/drivers/iio/opensensorfusion/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-$(CONFIG_OPEN_SENSOR_FUSION) += open-sensor-fusion.o
+
+open-sensor-fusion-y := osf_core.o osf_protocol.o osf_serdev.o osf_stream.o
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
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include "osf_core.h"
+#include "osf_protocol.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)
+{
+	memset(osf, 0, sizeof(*osf));
+	osf->dev = dev;
+}
+
+void osf_core_unregister_iio(struct osf_device *osf)
+{
+}
+
+static int osf_core_validate_sensor_sample(const struct osf_frame *frame)
+{
+	struct osf_sensor_sample sample;
+
+	return osf_protocol_decode_sensor_sample(frame, &sample);
+}
+
+static int osf_core_validate_device_status(const struct osf_frame *frame)
+{
+	struct osf_device_status status;
+	int ret;
+
+	ret = osf_protocol_decode_device_status(frame, &status);
+	if (ret)
+		return ret;
+
+	if (status.reserved)
+		return -EPROTO;
+
+	return 0;
+}
+
+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++) {
+		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;
+		break;
+	}
+
+	if (!ret)
+		osf->last_sequence = frame.sequence;
+
+	return ret;
+}
diff --git a/drivers/iio/opensensorfusion/osf_core.h b/drivers/iio/opensensorfusion/osf_core.h
new file mode 100644
index 000000000..3680c8c9b
--- /dev/null
+++ b/drivers/iio/opensensorfusion/osf_core.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _OSF_CORE_H
+#define _OSF_CORE_H
+
+#include <linux/types.h>
+
+struct device;
+
+struct osf_device {
+	struct device *dev;
+	u64 last_sequence;
+};
+
+void osf_core_init(struct osf_device *osf, struct device *dev);
+void osf_core_unregister_iio(struct osf_device *osf);
+int osf_core_receive_frame(struct osf_device *osf, const u8 *buf, size_t len);
+
+#endif
diff --git a/drivers/iio/opensensorfusion/osf_serdev.c b/drivers/iio/opensensorfusion/osf_serdev.c
new file mode 100644
index 000000000..f121089ed
--- /dev/null
+++ b/drivers/iio/opensensorfusion/osf_serdev.c
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/serdev.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include "osf_core.h"
+#include "osf_stream.h"
+
+#define OSF_SERDEV_BAUD		115200
+
+struct osf_serdev {
+	struct serdev_device *serdev;
+	struct osf_device osf;
+	struct osf_stream stream;
+};
+
+static size_t osf_serdev_receive_buf(struct serdev_device *serdev,
+				     const u8 *buf, size_t count)
+{
+	struct osf_serdev *osf_uart = serdev_device_get_drvdata(serdev);
+	const struct osf_stream_stats *stats;
+	u64 valid_before;
+	int ret;
+
+	valid_before = osf_uart->stream.stats.valid_frames;
+	ret = osf_stream_receive_bytes(&osf_uart->stream, buf, count);
+	stats = &osf_uart->stream.stats;
+
+	if (ret || stats->valid_frames != valid_before)
+		dev_dbg_ratelimited(&serdev->dev,
+				    "rx count=%zu valid=%llu bad_magic=%llu bad_crc=%llu partial=%llu dropped=%llu ret=%d\n",
+				    count,
+				    (unsigned long long)stats->valid_frames,
+				    (unsigned long long)stats->bad_magic_resyncs,
+				    (unsigned long long)stats->bad_crc_frames,
+				    (unsigned long long)stats->partial_frames,
+				    (unsigned long long)stats->dropped_bytes,
+				    ret);
+
+	return count;
+}
+
+static const struct serdev_device_ops osf_serdev_ops = {
+	.receive_buf = osf_serdev_receive_buf,
+};
+
+static int osf_serdev_probe(struct serdev_device *serdev)
+{
+	struct osf_serdev *osf_uart;
+	unsigned int baudrate;
+	int ret;
+
+	osf_uart = devm_kzalloc(&serdev->dev, sizeof(*osf_uart), GFP_KERNEL);
+	if (!osf_uart)
+		return -ENOMEM;
+
+	osf_uart->serdev = serdev;
+	osf_core_init(&osf_uart->osf, &serdev->dev);
+	osf_stream_init(&osf_uart->stream, &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;
+
+	baudrate = serdev_device_set_baudrate(serdev, OSF_SERDEV_BAUD);
+	if (baudrate != OSF_SERDEV_BAUD)
+		dev_warn(&serdev->dev, "requested %u baud, controller set %u\n",
+			 OSF_SERDEV_BAUD, baudrate);
+
+	serdev_device_set_flow_control(serdev, false);
+
+	return 0;
+}
+
+static void osf_serdev_remove(struct serdev_device *serdev)
+{
+	struct osf_serdev *osf_uart = serdev_device_get_drvdata(serdev);
+
+	serdev_device_close(serdev);
+	osf_stream_reset(&osf_uart->stream);
+	osf_core_unregister_iio(&osf_uart->osf);
+}
+
+static const struct of_device_id osf_serdev_of_match[] = {
+	{ .compatible = "opensensorfusion,osf-uart" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, osf_serdev_of_match);
+
+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,
+	},
+};
+
+module_serdev_device_driver(osf_serdev_driver);
+
+MODULE_DESCRIPTION("Open Sensor Fusion IIO driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH RFC v2 6/7] iio: osf: register IIO devices from capabilities
  2026-05-24  8:53 [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver Jinseob Kim
                   ` (4 preceding siblings ...)
  2026-05-24  8:53 ` [PATCH RFC v2 5/7] iio: osf: add UART serdev transport Jinseob Kim
@ 2026-05-24  8:53 ` Jinseob Kim
  2026-05-28 14:30   ` Jonathan Cameron
  2026-05-24  8:53 ` [PATCH RFC v2 7/7] MAINTAINERS: add Open Sensor Fusion IIO driver Jinseob Kim
  2026-06-02 23:59 ` [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver Andy Shevchenko
  7 siblings, 1 reply; 26+ messages in thread
From: Jinseob Kim @ 2026-05-24  8:53 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel, Jinseob Kim

Register supported IIO devices from the first valid capability report.

Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
---
 drivers/iio/opensensorfusion/Kconfig    |  12 +-
 drivers/iio/opensensorfusion/Makefile   |   3 +-
 drivers/iio/opensensorfusion/osf_core.c | 248 ++++++++++++++++++--
 drivers/iio/opensensorfusion/osf_core.h |  49 ++++
 drivers/iio/opensensorfusion/osf_iio.c  | 288 ++++++++++++++++++++++++
 drivers/iio/opensensorfusion/osf_iio.h  |  22 ++
 6 files changed, 593 insertions(+), 29 deletions(-)
 create mode 100644 drivers/iio/opensensorfusion/osf_iio.c
 create mode 100644 drivers/iio/opensensorfusion/osf_iio.h

diff --git a/drivers/iio/opensensorfusion/Kconfig b/drivers/iio/opensensorfusion/Kconfig
index 360f25b4f..004d2aa8a 100644
--- a/drivers/iio/opensensorfusion/Kconfig
+++ b/drivers/iio/opensensorfusion/Kconfig
@@ -5,11 +5,11 @@ config OPEN_SENSOR_FUSION
 	depends on IIO
 	depends on SERIAL_DEV_BUS
 	select CRC32
+	select IIO_KFIFO_BUF
 	help
-	  Build the Open Sensor Fusion UART receive path.
+	  Build the Open Sensor Fusion UART IIO driver.
 
-	  The driver receives OSF0 frames over a serdev UART.
-	  Frames are decoded and validated before being passed to the
-	  driver core.
-	  This patch only adds the transport path.
-	  IIO device registration is added separately.
+	  The driver receives OSF0 frames over a serdev UART and registers
+	  IIO devices for supported capability entries.
+	  Raw reads use the latest decoded samples. Buffered reads use
+	  fixed scan layouts and IIO timestamps.
diff --git a/drivers/iio/opensensorfusion/Makefile b/drivers/iio/opensensorfusion/Makefile
index 940c82edd..b4e03b80c 100644
--- a/drivers/iio/opensensorfusion/Makefile
+++ b/drivers/iio/opensensorfusion/Makefile
@@ -2,4 +2,5 @@
 
 obj-$(CONFIG_OPEN_SENSOR_FUSION) += open-sensor-fusion.o
 
-open-sensor-fusion-y := osf_core.o osf_protocol.o osf_serdev.o osf_stream.o
+open-sensor-fusion-y := osf_core.o osf_iio.o osf_protocol.o osf_serdev.o \
+			 osf_stream.o
diff --git a/drivers/iio/opensensorfusion/osf_core.c b/drivers/iio/opensensorfusion/osf_core.c
index c867b3158..a1e20ede9 100644
--- a/drivers/iio/opensensorfusion/osf_core.c
+++ b/drivers/iio/opensensorfusion/osf_core.c
@@ -5,7 +5,7 @@
 #include <linux/types.h>
 
 #include "osf_core.h"
-#include "osf_protocol.h"
+#include "osf_iio.h"
 
 #define OSF_RESERVED_MSG_FIRST		0x7f00
 #define OSF_RESERVED_MSG_LAST		0x7fff
@@ -19,17 +19,171 @@ void osf_core_init(struct osf_device *osf, struct device *dev)
 
 void osf_core_unregister_iio(struct osf_device *osf)
 {
+	unsigned int i;
+
+	for (i = 0; i < osf->iio_dev_count; i++)
+		osf_iio_unregister_sensor(osf->iio_devs[i].indio_dev);
+
+	osf->iio_dev_count = 0;
+}
+
+static struct iio_dev *osf_core_find_iio_dev(struct osf_device *osf,
+					     u16 sensor_type, u16 sensor_index)
+{
+	const struct osf_iio_binding *binding;
+	unsigned int i;
+
+	for (i = 0; i < osf->iio_dev_count; i++) {
+		binding = &osf->iio_devs[i];
+		if (binding->sensor_type == sensor_type &&
+		    binding->sensor_index == sensor_index)
+			return binding->indio_dev;
+	}
+
+	return NULL;
+}
+
+static struct osf_latest_sample *
+osf_core_find_latest_sample(struct osf_device *osf, u16 sensor_type,
+			    u16 sensor_index)
+{
+	struct osf_latest_sample *latest;
+	unsigned int i;
+
+	for (i = 0; i < osf->latest_sample_count; i++) {
+		latest = &osf->latest_samples[i];
+		if (latest->sensor_type == sensor_type &&
+		    latest->sensor_index == sensor_index)
+			return latest;
+	}
+
+	if (osf->latest_sample_count >= OSF_MAX_CAPABILITIES)
+		return NULL;
+
+	return &osf->latest_samples[osf->latest_sample_count++];
+}
+
+static bool osf_core_capability_duplicate(const struct osf_capability_cache *cache,
+					  unsigned int index)
+{
+	const struct osf_capability_entry *entry = &cache->entries[index];
+	unsigned int i;
+
+	for (i = 0; i < index; i++) {
+		if (!osf_iio_sensor_supported(cache->entries[i].sensor_type,
+					      cache->entries[i].channel_count))
+			continue;
+
+		if (cache->entries[i].sensor_type == entry->sensor_type &&
+		    cache->entries[i].sensor_index == entry->sensor_index)
+			return true;
+	}
+
+	return false;
+}
+
+static int osf_core_register_capabilities(struct osf_device *osf,
+					  const struct osf_capability_cache *cache)
+{
+	struct iio_dev *indio_dev;
+	unsigned int i;
+	int ret;
+
+	if (osf->capability_cache.valid)
+		return 0;
+
+	for (i = 0; i < cache->capability_count; i++) {
+		if (!osf_iio_sensor_supported(cache->entries[i].sensor_type,
+					      cache->entries[i].channel_count))
+			continue;
+
+		if (osf_core_capability_duplicate(cache, i))
+			return -EEXIST;
+	}
+
+	for (i = 0; i < cache->capability_count; i++) {
+		if (!osf_iio_sensor_supported(cache->entries[i].sensor_type,
+					      cache->entries[i].channel_count))
+			continue;
+
+		ret = osf_iio_register_sensor(osf->dev, &cache->entries[i],
+					      osf, &indio_dev);
+		if (ret)
+			goto err_unregister;
+
+		osf->iio_devs[osf->iio_dev_count].sensor_type =
+			cache->entries[i].sensor_type;
+		osf->iio_devs[osf->iio_dev_count].sensor_index =
+			cache->entries[i].sensor_index;
+		osf->iio_devs[osf->iio_dev_count].indio_dev = indio_dev;
+		osf->iio_dev_count++;
+	}
+
+	return 0;
+
+err_unregister:
+	osf_core_unregister_iio(osf);
+
+	return ret;
 }
 
-static int osf_core_validate_sensor_sample(const struct osf_frame *frame)
+static int osf_core_handle_sensor_sample(struct osf_device *osf,
+					 const struct osf_frame *frame)
 {
+	struct osf_latest_sample *latest;
 	struct osf_sensor_sample sample;
+	struct iio_dev *indio_dev;
+	unsigned int i;
+	int ret;
 
-	return osf_protocol_decode_sensor_sample(frame, &sample);
+	ret = osf_protocol_decode_sensor_sample(frame, &sample);
+	if (ret)
+		return ret;
+
+	if (sample.channel_count > OSF_MAX_SAMPLE_CHANNELS)
+		return -E2BIG;
+
+	latest = osf_core_find_latest_sample(osf, sample.sensor_type,
+					     sample.sensor_index);
+	if (!latest)
+		return -E2BIG;
+
+	for (i = 0; i < sample.channel_count; i++) {
+		ret = osf_protocol_sensor_sample_value(&sample, i,
+						       &latest->values[i]);
+		if (ret)
+			return ret;
+	}
+
+	for (; i < OSF_MAX_SAMPLE_CHANNELS; i++)
+		latest->values[i] = 0;
+
+	latest->sensor_type = sample.sensor_type;
+	latest->sensor_index = sample.sensor_index;
+	latest->channel_count = sample.channel_count;
+	latest->sample_format = sample.sample_format;
+	latest->scale_nano = sample.scale_nano;
+	latest->sequence = frame->sequence;
+	latest->timestamp_us = frame->timestamp_us;
+	latest->valid = true;
+	osf->last_sequence = frame->sequence;
+
+	indio_dev = osf_core_find_iio_dev(osf, sample.sensor_type,
+					  sample.sensor_index);
+	if (indio_dev) {
+		ret = osf_iio_push_sample(indio_dev, latest->values,
+					  latest->channel_count);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
 }
 
-static int osf_core_validate_device_status(const struct osf_frame *frame)
+static int osf_core_handle_device_status(struct osf_device *osf,
+					 const struct osf_frame *frame)
 {
+	struct osf_status_cache cache = { };
 	struct osf_device_status status;
 	int ret;
 
@@ -40,12 +194,22 @@ static int osf_core_validate_device_status(const struct osf_frame *frame)
 	if (status.reserved)
 		return -EPROTO;
 
+	cache.uptime_s = status.uptime_s;
+	cache.status_flags = status.status_flags;
+	cache.error_flags = status.error_flags;
+	cache.dropped_frames = status.dropped_frames;
+	cache.sequence = frame->sequence;
+	cache.valid = true;
+	osf->status_cache = cache;
+	osf->last_sequence = frame->sequence;
+
 	return 0;
 }
 
-static int osf_core_validate_capability_report(const struct osf_frame *frame)
+static int osf_core_handle_capability_report(struct osf_device *osf,
+					     const struct osf_frame *frame)
 {
-	struct osf_capability_entry entry;
+	struct osf_capability_cache cache = { };
 	struct osf_capability_report report;
 	unsigned int i;
 	int ret;
@@ -54,12 +218,32 @@ static int osf_core_validate_capability_report(const struct osf_frame *frame)
 	if (ret)
 		return ret;
 
+	if (report.capability_count > OSF_MAX_CAPABILITIES)
+		return -E2BIG;
+
+	if (osf->capability_cache.valid) {
+		osf->last_sequence = frame->sequence;
+		return 0;
+	}
+
 	for (i = 0; i < report.capability_count; i++) {
-		ret = osf_protocol_decode_capability_entry(&report, i, &entry);
+		ret = osf_protocol_decode_capability_entry(&report, i,
+							   &cache.entries[i]);
 		if (ret)
 			return ret;
 	}
 
+	cache.capability_count = report.capability_count;
+	cache.sequence = frame->sequence;
+	cache.valid = true;
+
+	ret = osf_core_register_capabilities(osf, &cache);
+	if (ret)
+		return ret;
+
+	osf->capability_cache = cache;
+	osf->last_sequence = frame->sequence;
+
 	return 0;
 }
 
@@ -81,27 +265,47 @@ int osf_core_receive_frame(struct osf_device *osf, const u8 *buf, size_t len)
 
 	switch (frame.message_type) {
 	case OSF_MSG_SENSOR_SAMPLE:
-		ret = osf_core_validate_sensor_sample(&frame);
-		break;
+		return osf_core_handle_sensor_sample(osf, &frame);
 	case OSF_MSG_DEVICE_STATUS:
-		ret = osf_core_validate_device_status(&frame);
-		break;
+		return osf_core_handle_device_status(osf, &frame);
 	case OSF_MSG_CAPABILITY_REPORT:
-		ret = osf_core_validate_capability_report(&frame);
-		break;
+		return osf_core_handle_capability_report(osf, &frame);
 	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;
-		break;
+			return 0;
+		if (frame.message_type >= OSF_VENDOR_PRIVATE_FIRST)
+			return 0;
+		return -EOPNOTSUPP;
 	}
+}
 
-	if (!ret)
-		osf->last_sequence = frame.sequence;
+int osf_core_read_latest_sample(struct osf_device *osf, u16 sensor_type,
+				u16 sensor_index, unsigned int channel,
+				s32 *value)
+{
+	const struct osf_latest_sample *latest;
+	unsigned int i;
 
-	return ret;
+	if (!osf || !value)
+		return -EINVAL;
+
+	for (i = 0; i < osf->latest_sample_count; i++) {
+		latest = &osf->latest_samples[i];
+		if (latest->sensor_type == sensor_type &&
+		    latest->sensor_index == sensor_index)
+			goto found;
+	}
+
+	return -ENODATA;
+
+found:
+	if (!latest->valid)
+		return -ENODATA;
+	if (channel >= latest->channel_count)
+		return -ENODATA;
+
+	*value = latest->values[channel];
+
+	return 0;
 }
diff --git a/drivers/iio/opensensorfusion/osf_core.h b/drivers/iio/opensensorfusion/osf_core.h
index 3680c8c9b..e5c0d5081 100644
--- a/drivers/iio/opensensorfusion/osf_core.h
+++ b/drivers/iio/opensensorfusion/osf_core.h
@@ -4,15 +4,64 @@
 
 #include <linux/types.h>
 
+#include "osf_protocol.h"
+
+#define OSF_MAX_SAMPLE_CHANNELS	3
+#define OSF_MAX_CAPABILITIES	16
+
 struct device;
+struct iio_dev;
+
+struct osf_latest_sample {
+	u16 sensor_type;
+	u16 sensor_index;
+	u16 channel_count;
+	u16 sample_format;
+	u32 scale_nano;
+	s32 values[OSF_MAX_SAMPLE_CHANNELS];
+	u64 sequence;
+	u64 timestamp_us;
+	bool valid;
+};
+
+struct osf_capability_cache {
+	u16 capability_count;
+	struct osf_capability_entry entries[OSF_MAX_CAPABILITIES];
+	u64 sequence;
+	bool valid;
+};
+
+struct osf_status_cache {
+	u32 uptime_s;
+	u32 status_flags;
+	u32 error_flags;
+	u32 dropped_frames;
+	u64 sequence;
+	bool valid;
+};
+
+struct osf_iio_binding {
+	u16 sensor_type;
+	u16 sensor_index;
+	struct iio_dev *indio_dev;
+};
 
 struct osf_device {
 	struct device *dev;
+	struct osf_latest_sample latest_samples[OSF_MAX_CAPABILITIES];
+	unsigned int latest_sample_count;
+	struct osf_capability_cache capability_cache;
+	struct osf_status_cache status_cache;
+	struct osf_iio_binding iio_devs[OSF_MAX_CAPABILITIES];
+	unsigned int iio_dev_count;
 	u64 last_sequence;
 };
 
 void osf_core_init(struct osf_device *osf, struct device *dev);
 void osf_core_unregister_iio(struct osf_device *osf);
 int osf_core_receive_frame(struct osf_device *osf, const u8 *buf, size_t len);
+int osf_core_read_latest_sample(struct osf_device *osf, u16 sensor_type,
+				u16 sensor_index, unsigned int channel,
+				s32 *value);
 
 #endif
diff --git a/drivers/iio/opensensorfusion/osf_iio.c b/drivers/iio/opensensorfusion/osf_iio.c
new file mode 100644
index 000000000..1f82ec063
--- /dev/null
+++ b/drivers/iio/opensensorfusion/osf_iio.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/kfifo_buf.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include "osf_core.h"
+#include "osf_iio.h"
+
+struct osf_iio_sensor_spec {
+	u16 sensor_type;
+	u16 channel_count;
+	const char *name;
+	const struct iio_chan_spec *channels;
+	unsigned int num_channels;
+	const unsigned long *available_scan_masks;
+};
+
+struct osf_iio_state {
+	const struct osf_iio_sensor_spec *spec;
+	struct iio_buffer *buffer;
+	u32 scale_nano;
+	u16 sensor_index;
+	struct osf_device *osf;
+};
+
+#define OSF_SCAN_TYPE_S32						\
+	{								\
+		.sign = 's',						\
+		.realbits = 32,					\
+		.storagebits = 32,					\
+		.endianness = IIO_CPU,					\
+	}
+
+#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 = OSF_SCAN_TYPE_S32,			\
+	}
+
+#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 = OSF_SCAN_TYPE_S32,			\
+	}
+
+static const struct iio_chan_spec osf_accel_channels[] = {
+	OSF_MOD_CHAN(IIO_ACCEL, IIO_MOD_X, 0),
+	OSF_MOD_CHAN(IIO_ACCEL, IIO_MOD_Y, 1),
+	OSF_MOD_CHAN(IIO_ACCEL, IIO_MOD_Z, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static const struct iio_chan_spec osf_gyro_channels[] = {
+	OSF_MOD_CHAN(IIO_ANGL_VEL, IIO_MOD_X, 0),
+	OSF_MOD_CHAN(IIO_ANGL_VEL, IIO_MOD_Y, 1),
+	OSF_MOD_CHAN(IIO_ANGL_VEL, IIO_MOD_Z, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static const struct iio_chan_spec osf_mag_channels[] = {
+	OSF_MOD_CHAN(IIO_MAGN, IIO_MOD_X, 0),
+	OSF_MOD_CHAN(IIO_MAGN, IIO_MOD_Y, 1),
+	OSF_MOD_CHAN(IIO_MAGN, IIO_MOD_Z, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static const struct iio_chan_spec osf_temp_channels[] = {
+	OSF_CHAN(IIO_TEMP, 0),
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
+static const unsigned long osf_3axis_available_scan_masks[] = {
+	GENMASK(2, 0),
+	0,
+};
+
+static const unsigned long osf_temp_available_scan_masks[] = {
+	BIT(0),
+	0,
+};
+
+static const struct osf_iio_sensor_spec osf_iio_sensor_specs[] = {
+	{
+		.sensor_type = OSF_SENSOR_ACCELEROMETER,
+		.channel_count = 3,
+		.name = "osf-accel",
+		.channels = osf_accel_channels,
+		.num_channels = ARRAY_SIZE(osf_accel_channels),
+		.available_scan_masks = osf_3axis_available_scan_masks,
+	},
+	{
+		.sensor_type = OSF_SENSOR_GYROSCOPE,
+		.channel_count = 3,
+		.name = "osf-gyro",
+		.channels = osf_gyro_channels,
+		.num_channels = ARRAY_SIZE(osf_gyro_channels),
+		.available_scan_masks = osf_3axis_available_scan_masks,
+	},
+	{
+		.sensor_type = OSF_SENSOR_MAGNETOMETER,
+		.channel_count = 3,
+		.name = "osf-magn",
+		.channels = osf_mag_channels,
+		.num_channels = ARRAY_SIZE(osf_mag_channels),
+		.available_scan_masks = osf_3axis_available_scan_masks,
+	},
+	{
+		.sensor_type = OSF_SENSOR_TEMPERATURE,
+		.channel_count = 1,
+		.name = "osf-temp",
+		.channels = osf_temp_channels,
+		.num_channels = ARRAY_SIZE(osf_temp_channels),
+		.available_scan_masks = osf_temp_available_scan_masks,
+	},
+};
+
+static const struct osf_iio_sensor_spec *
+osf_iio_find_sensor_spec(u16 sensor_type, u16 channel_count)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(osf_iio_sensor_specs); i++) {
+		if (osf_iio_sensor_specs[i].sensor_type == sensor_type &&
+		    osf_iio_sensor_specs[i].channel_count == channel_count)
+			return &osf_iio_sensor_specs[i];
+	}
+
+	return NULL;
+}
+
+bool osf_iio_sensor_supported(u16 sensor_type, u16 channel_count)
+{
+	return !!osf_iio_find_sensor_spec(sensor_type, channel_count);
+}
+
+const char *osf_iio_sensor_name(u16 sensor_type)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(osf_iio_sensor_specs); i++) {
+		if (osf_iio_sensor_specs[i].sensor_type == sensor_type)
+			return osf_iio_sensor_specs[i].name;
+	}
+
+	return NULL;
+}
+
+static int osf_iio_read_raw(struct iio_dev *indio_dev,
+			    const struct iio_chan_spec *chan, int *val,
+			    int *val2, long mask)
+{
+	struct osf_iio_state *state = iio_priv(indio_dev);
+	s32 raw;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (!state->osf)
+			return -ENODATA;
+
+		if (chan->scan_index < 0)
+			return -EINVAL;
+
+		ret = osf_core_read_latest_sample(state->osf,
+						  state->spec->sensor_type,
+						  state->sensor_index,
+						  chan->scan_index, &raw);
+		if (ret)
+			return ret;
+
+		*val = raw;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = state->scale_nano / NANO;
+		*val2 = state->scale_nano % NANO;
+		return IIO_VAL_INT_PLUS_NANO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info osf_iio_info = {
+	.read_raw = osf_iio_read_raw,
+};
+
+int osf_iio_register_sensor(struct device *dev,
+			    const struct osf_capability_entry *entry,
+			    struct osf_device *osf, struct iio_dev **indio_dev)
+{
+	const struct osf_iio_sensor_spec *spec;
+	struct osf_iio_state *state;
+	struct iio_dev *iio_dev;
+	int ret;
+
+	spec = osf_iio_find_sensor_spec(entry->sensor_type,
+					entry->channel_count);
+	if (!spec)
+		return -EOPNOTSUPP;
+
+	if (entry->sample_format != OSF_SAMPLE_FORMAT_S32)
+		return -EOPNOTSUPP;
+
+	iio_dev = iio_device_alloc(dev, sizeof(*state));
+	if (!iio_dev)
+		return -ENOMEM;
+
+	state = iio_priv(iio_dev);
+	state->spec = spec;
+	state->scale_nano = entry->scale_nano;
+	state->sensor_index = entry->sensor_index;
+	state->osf = osf;
+
+	iio_dev->name = spec->name;
+	iio_dev->info = &osf_iio_info;
+	iio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
+	iio_dev->channels = spec->channels;
+	iio_dev->num_channels = spec->num_channels;
+	iio_dev->available_scan_masks = spec->available_scan_masks;
+
+	state->buffer = iio_kfifo_allocate();
+	if (!state->buffer) {
+		ret = -ENOMEM;
+		goto err_free_iio;
+	}
+
+	ret = iio_device_attach_buffer(iio_dev, state->buffer);
+	if (ret)
+		goto err_free_buffer;
+
+	ret = iio_device_register(iio_dev);
+	if (ret)
+		goto err_free_buffer;
+
+	if (indio_dev)
+		*indio_dev = iio_dev;
+
+	return 0;
+
+err_free_buffer:
+	iio_kfifo_free(state->buffer);
+err_free_iio:
+	iio_device_free(iio_dev);
+
+	return ret;
+}
+
+void osf_iio_unregister_sensor(struct iio_dev *indio_dev)
+{
+	struct osf_iio_state *state;
+
+	if (!indio_dev)
+		return;
+
+	state = iio_priv(indio_dev);
+	iio_device_unregister(indio_dev);
+	iio_kfifo_free(state->buffer);
+	iio_device_free(indio_dev);
+}
+
+int osf_iio_push_sample(struct iio_dev *indio_dev, const s32 *values,
+			unsigned int channel_count)
+{
+	s64 timestamp;
+
+	if (!iio_buffer_enabled(indio_dev))
+		return 0;
+
+	timestamp = iio_get_time_ns(indio_dev);
+
+	return iio_push_to_buffers_with_ts_unaligned(indio_dev, values,
+						     channel_count * sizeof(*values),
+						     timestamp);
+}
diff --git a/drivers/iio/opensensorfusion/osf_iio.h b/drivers/iio/opensensorfusion/osf_iio.h
new file mode 100644
index 000000000..d90c58fc4
--- /dev/null
+++ b/drivers/iio/opensensorfusion/osf_iio.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _OSF_IIO_H
+#define _OSF_IIO_H
+
+#include <linux/types.h>
+
+#include "osf_protocol.h"
+
+struct device;
+struct iio_dev;
+struct osf_device;
+
+int osf_iio_register_sensor(struct device *dev,
+			    const struct osf_capability_entry *entry,
+			    struct osf_device *osf, struct iio_dev **indio_dev);
+void osf_iio_unregister_sensor(struct iio_dev *indio_dev);
+int osf_iio_push_sample(struct iio_dev *indio_dev, const s32 *values,
+			unsigned int channel_count);
+bool osf_iio_sensor_supported(u16 sensor_type, u16 channel_count);
+const char *osf_iio_sensor_name(u16 sensor_type);
+
+#endif
-- 
2.43.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH RFC v2 7/7] MAINTAINERS: add Open Sensor Fusion IIO driver
  2026-05-24  8:53 [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver Jinseob Kim
                   ` (5 preceding siblings ...)
  2026-05-24  8:53 ` [PATCH RFC v2 6/7] iio: osf: register IIO devices from capabilities Jinseob Kim
@ 2026-05-24  8:53 ` Jinseob Kim
  2026-05-25 12:11   ` Krzysztof Kozlowski
  2026-05-28 14:31   ` Jonathan Cameron
  2026-06-02 23:59 ` [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver Andy Shevchenko
  7 siblings, 2 replies; 26+ messages in thread
From: Jinseob Kim @ 2026-05-24  8:53 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel, Jinseob Kim

Add file patterns for the driver, binding, and protocol reference.

Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c2c6d7927..9238701f4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19977,6 +19977,15 @@ F:	Documentation/networking/oa-tc6-framework.rst
 F:	drivers/net/ethernet/oa_tc6.c
 F:	include/linux/oa_tc6.h
 
+OPEN SENSOR FUSION IIO DRIVER
+M:	Jinseob Kim <kimjinseob88@gmail.com>
+L:	linux-iio@vger.kernel.org
+L:	devicetree@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/imu/opensensorfusion,osf-uart.yaml
+F:	Documentation/iio/open-sensor-fusion-protocol-v0.rst
+F:	drivers/iio/opensensorfusion/
+
 OPEN FIRMWARE AND FLATTENED DEVICE TREE
 M:	Rob Herring <robh@kernel.org>
 M:	Saravana Kannan <saravanak@kernel.org>
-- 
2.43.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 1/7] dt-bindings: iio: add Open Sensor Fusion UART device
  2026-05-24  8:53 ` [PATCH RFC v2 1/7] dt-bindings: iio: add Open Sensor Fusion UART device Jinseob Kim
@ 2026-05-24 18:59   ` Krzysztof Kozlowski
  2026-05-24 19:34   ` Conor Dooley
  1 sibling, 0 replies; 26+ messages in thread
From: Krzysztof Kozlowski @ 2026-05-24 18:59 UTC (permalink / raw)
  To: Jinseob Kim, Jonathan Cameron, linux-iio
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

On 24/05/2026 10:53, Jinseob Kim wrote:
> Add a binding for the OSF0 UART-attached sensor aggregation device.
> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
> ---
>  .../iio/imu/opensensorfusion,osf-uart.yaml    | 33 +++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |  2 ++
>  2 files changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/imu/opensensorfusion,osf-uart.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/opensensorfusion,osf-uart.yaml b/Documentation/devicetree/bindings/iio/imu/opensensorfusion,osf-uart.yaml
> new file mode 100644
> index 000000000..d4a8f6819
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/imu/opensensorfusion,osf-uart.yaml
> @@ -0,0 +1,33 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/imu/opensensorfusion,osf-uart.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Open Sensor Fusion UART sensor aggregation device
> +
> +maintainers:
> +  - Jinseob Kim <kimjinseob88@gmail.com>
> +
> +description:
> +  UART-attached sensor aggregation device using the OSF0 frame format.
> +  The device sends capability, status, and sample frames to the host.
> +
> +properties:
> +  compatible:
> +    const: opensensorfusion,osf-uart

1. No such vendor. You need to test against entire bindings.
2. Looks heavily incomplete - no resources. Compatible and hardware
description looks also incomplete - what is osf? It's exactly ONE
device? Why do you use bus suffix? Please start with writing bindings or
DTS101 slides.

> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    serial {
> +        osf {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).

> +            compatible = "opensensorfusion,osf-uart";



Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 1/7] dt-bindings: iio: add Open Sensor Fusion UART device
  2026-05-24  8:53 ` [PATCH RFC v2 1/7] dt-bindings: iio: add Open Sensor Fusion UART device Jinseob Kim
  2026-05-24 18:59   ` Krzysztof Kozlowski
@ 2026-05-24 19:34   ` Conor Dooley
  2026-05-25  2:06     ` Kim Jinseob
  1 sibling, 1 reply; 26+ messages in thread
From: Conor Dooley @ 2026-05-24 19:34 UTC (permalink / raw)
  To: Jinseob Kim
  Cc: Jonathan Cameron, linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

On Sun, May 24, 2026 at 05:53:06PM +0900, Jinseob Kim wrote:
> Add a binding for the OSF0 UART-attached sensor aggregation device.
> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>

Most of my v1 review was ignored it seems. Please go back to v1 and
respond to the points I made.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 1/7] dt-bindings: iio: add Open Sensor Fusion UART device
  2026-05-24 19:34   ` Conor Dooley
@ 2026-05-25  2:06     ` Kim Jinseob
  0 siblings, 0 replies; 26+ messages in thread
From: Kim Jinseob @ 2026-05-25  2:06 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Jonathan Cameron, linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

> Most of my v1 review was ignored it seems. Please go back to v1 and
> respond to the points I made.

I missed the v1 DT binding feedback when preparing v2. I will go back to
the v1 thread and reply there before the next revision.

Jinseob
> Thanks,
> Conor.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 7/7] MAINTAINERS: add Open Sensor Fusion IIO driver
  2026-05-24  8:53 ` [PATCH RFC v2 7/7] MAINTAINERS: add Open Sensor Fusion IIO driver Jinseob Kim
@ 2026-05-25 12:11   ` Krzysztof Kozlowski
  2026-05-28 14:31   ` Jonathan Cameron
  1 sibling, 0 replies; 26+ messages in thread
From: Krzysztof Kozlowski @ 2026-05-25 12:11 UTC (permalink / raw)
  To: Jinseob Kim, Jonathan Cameron, linux-iio
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

On 24/05/2026 10:53, Jinseob Kim wrote:
> Add file patterns for the driver, binding, and protocol reference.
> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
> ---
>  MAINTAINERS | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c2c6d7927..9238701f4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19977,6 +19977,15 @@ F:	Documentation/networking/oa-tc6-framework.rst
>  F:	drivers/net/ethernet/oa_tc6.c
>  F:	include/linux/oa_tc6.h
>  
> +OPEN SENSOR FUSION IIO DRIVER
> +M:	Jinseob Kim <kimjinseob88@gmail.com>
> +L:	linux-iio@vger.kernel.org
> +L:	devicetree@vger.kernel.org

Drop both lists.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 2/7] Documentation: iio: add Open Sensor Fusion protocol v0 reference
  2026-05-24  8:53 ` [PATCH RFC v2 2/7] Documentation: iio: add Open Sensor Fusion protocol v0 reference Jinseob Kim
@ 2026-05-28 13:28   ` Jonathan Cameron
       [not found]     ` <CALMSew+3RVXxLJYtr3HkV7UeAf6Mqx6PpA2CehChoVaMFddpJw@mail.gmail.com>
  0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Cameron @ 2026-05-28 13:28 UTC (permalink / raw)
  To: Jinseob Kim
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

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

> Document the OSF0 frame format and payloads used by the driver.
> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
> ---
>  .../iio/open-sensor-fusion-protocol-v0.rst    | 267 ++++++++++++++++++
>  1 file changed, 267 insertions(+)
>  create mode 100644 Documentation/iio/open-sensor-fusion-protocol-v0.rst
> 
> diff --git a/Documentation/iio/open-sensor-fusion-protocol-v0.rst b/Documentation/iio/open-sensor-fusion-protocol-v0.rst
> new file mode 100644
> index 000000000..4800a3ce6
> --- /dev/null
> +++ b/Documentation/iio/open-sensor-fusion-protocol-v0.rst
> @@ -0,0 +1,267 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +Open Sensor Fusion protocol v0
> +==============================
> +
> +This document describes the OSF0 UART wire format used by the Linux IIO
> +driver. It is not a firmware programming interface.

Can we have some background information. Where does this OSF thing come from?
Is it a general standard or something you are personally developing?
Some links would definitely help.

> +
> +Device model
> +------------
> +
> +An Open Sensor Fusion UART device is a sensor aggregation device. It sends
> +binary frames from the device to the host. The host driver decodes the frames
> +and maps supported sensors to IIO devices.
> +
> +The hardware used for smoke testing is an OSF GREEN prototype with an
> +STM32F405RGT6 MCU, an ICM42688P-class IMU, and an MMC5983MA magnetometer. That
> +hardware is a test target, not part of the binding ABI.
> +
> +Transport
> +---------
> +
> +The transport is UART at 115200 baud, 8 data bits, no parity, and 1 stop bit.
> +The Linux transport is serdev. The v0 upstream driver covers device-to-host
> +frames. Flow control is not used by the tested stream.
> +
> +Byte order
> +----------
> +
> +All multi-byte integer fields are little-endian. Samples use signed 32-bit
> +little-endian integers when ``sample_format`` is ``S32``.

s32 given this is kernel code and i'm not sure what else this is referring to.



> +``SENSOR_SAMPLE`` payload
> +-------------------------
> +
> +The base payload size is 16 bytes.

Not sure that is meaninful given expectation that there will be channels.
I'd describe it as a payload header, or express this as 16 + 4 * channel_count

> +
> +.. list-table::
> +   :header-rows: 1
> +
> +   * - Offset
> +     - Size
> +     - Field
> +     - Description
> +   * - 0
> +     - 2
> +     - sensor_type
> +     - Sensor type ID
> +   * - 2
> +     - 2
> +     - sensor_index
> +     - Instance index
> +   * - 4
> +     - 2
> +     - channel_count
> +     - Number of S32 channels
> +   * - 6
> +     - 2
> +     - sample_format
> +     - Must be ``1`` (``S32``)

Is this spec defined, or just what the driver supports?
I think this doc needs to distinguish between those two cases
more clearly.

> +   * - 8
> +     - 4
> +     - scale_nano
> +     - Scale factor in nano-units
> +   * - 12
> +     - 4
> +     - reserved
> +     - Must be zero for v0
> +   * - 16
> +     - 4 * channel_count
> +     - samples
> +     - Signed 32-bit channel samples
> +
> +``DEVICE_STATUS`` payload
> +-------------------------
> +
> +The payload size is 20 bytes. Fields are ``uptime_s``, ``status_flags``,
> +``error_flags``, ``dropped_frames``, and a reserved field. Each field is
> +32 bits.
> +
> +``CAPABILITY_REPORT`` payload
> +-----------------------------
> +
> +The base payload size is 4 bytes. It contains ``capability_count`` and a
> +reserved field. Each capability entry is 20 bytes:

Probably want a separate table for that header.

> +
> +.. list-table::
> +   :header-rows: 1
> +
> +   * - Offset
> +     - Size
> +     - Field
> +     - Description
> +   * - 0
> +     - 2
> +     - sensor_type
> +     - Sensor type ID
> +   * - 2
> +     - 2
> +     - sensor_index
> +     - Instance index
> +   * - 4
> +     - 2
> +     - channel_count
> +     - Number of channels
> +   * - 6
> +     - 2
> +     - sample_format
> +     - Must be ``1`` (``S32``)
> +   * - 8
> +     - 4
> +     - scale_nano
> +     - Scale factor in nano-units
> +   * - 12
> +     - 4
> +     - flags
> +     - Capability flags
> +   * - 16
> +     - 4
> +     - reserved
> +     - Must be zero for v0
> +
> +Capability flag bit 0 means enabled by default. Bit 1 means calibrated data can
> +be provided by the device. Other bits are invalid for v0.
> +

> +Scaling
> +-------
> +
> +``scale_nano`` is the per-channel scale value in nano-units. The Linux driver
> +maps it to ``IIO_CHAN_INFO_SCALE`` as integer plus nano. The exact physical
> +unit depends on the IIO channel type.
> +
> +Timestamps
> +----------
> +
> +The frame header carries ``timestamp_us``, a device-side timestamp in
> +microseconds. v0 transports this value for ordering and diagnostics. The driver
> +does not use it as a production-grade host-correlated timestamp. Buffered IIO
> +timestamps follow IIO timestamp clock handling.

Is there likely to be a non trivial delay?  If so we should figure out how to use
that timestamp to get something more useful.

> +
> +Non-goals for v0 upstream
> +-------------------------
> +
> +The v0 upstream driver does not include USB transport, fusion output,
> +AHRS/Kalman output, calibration command ABI, custom sysfs control surface,

What is AHRS?  I'd spell it out.

> +production timestamp correlation, or runtime capability removal.
This is where an external link would be helpful. Lets us have some visibility
of what is coming.

Thanks,

Jonathan



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 3/7] iio: osf: add protocol v0 decoding
  2026-05-24  8:53 ` [PATCH RFC v2 3/7] iio: osf: add protocol v0 decoding Jinseob Kim
@ 2026-05-28 13:48   ` Jonathan Cameron
       [not found]     ` <CALMSew+wUH1H-2MTtexCFgyD4Y+upFuSfFzTWBn3VGN6CWYFNQ@mail.gmail.com>
  0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Cameron @ 2026-05-28 13:48 UTC (permalink / raw)
  To: Jinseob Kim
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

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

> Add OSF0 frame and payload decoders for device-to-host messages.

https://sashiko.dev/#/patchset/20260524085312.15369-1-kimjinseob88%40gmail.com
Has some perhaps useful feedback. Please make sure to either exclude
them as false positives or address for next version.

Quite a bit of the feedback in here is about which checks are actually
useful. Normal kernel practice is assume we didn't shoot outselves in the
foot but hardware and userspace may have done. So we defend only against
things in their control.

Jonathan

> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
> ---
>  drivers/iio/opensensorfusion/osf_protocol.c | 220 ++++++++++++++++++++
>  drivers/iio/opensensorfusion/osf_protocol.h | 100 +++++++++
>  2 files changed, 320 insertions(+)
>  create mode 100644 drivers/iio/opensensorfusion/osf_protocol.c
>  create mode 100644 drivers/iio/opensensorfusion/osf_protocol.h
> 
> diff --git a/drivers/iio/opensensorfusion/osf_protocol.c b/drivers/iio/opensensorfusion/osf_protocol.c
> new file mode 100644
> index 000000000..ac3c37ae2
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_protocol.c
> @@ -0,0 +1,220 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/crc32.h>
> +#include <linux/errno.h>
> +#include <linux/bits.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +
> +#include "osf_protocol.h"
> +
> +#define OSF_CRC32_INIT		GENMASK(31, 0)
> +#define OSF_CRC32_XOROUT	GENMASK(31, 0)
> +
> +static bool osf_sensor_type_valid(u16 sensor_type)
> +{
> +	return sensor_type >= OSF_SENSOR_ACCELEROMETER &&
> +	       sensor_type <= OSF_SENSOR_PROXIMITY;

ah. I talk about sanity checking this below for purposes of ensuring
we can use the ennum type.  Looks like you have it all done already :)

> +}
> +
> +static u32 osf_crc32_ieee(const u8 *buf, size_t len)
> +{
> +	return crc32_le(OSF_CRC32_INIT, buf, len) ^ OSF_CRC32_XOROUT;
> +}
> +
> +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;
> +
> +	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;
> +
> +	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->reserved = get_unaligned_le32(buf + 34);

Seems little point in storing reserved.  If you want to check it is spec
compliant do it here.

> +	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;
Similar on whether this defensive code makes sense (see below and note I review
upwards in drivers as they make sense to me better that way!)


> +
> +	if (index >= sample->channel_count)
> +		return -ERANGE;
> +
> +	*value = (s32)get_unaligned_le32(sample->samples + index * sizeof(s32));

I guess it might change in future, but for now why not make samples an s32 *

> +
> +	return 0;
> +}

> +int osf_protocol_decode_capability_report(const struct osf_frame *frame,
> +					  struct osf_capability_report *report)
> +{
> +	u16 capability_count;
> +	u32 expected_len;
> +	const u8 *payload;
> +
> +	if (!frame || !report || !frame->payload)
> +		return -EINVAL;

Similar sanity check questions.

> +
> +	if (frame->message_type != OSF_MSG_CAPABILITY_REPORT)
> +		return -EPROTO;

If that happens seems like something went very wrong elsewhere so
seems unlikely defense makes sense here.


> +
> +	if (frame->payload_len < OSF_CAP_REPORT_BASE_LEN)
> +		return -EMSGSIZE;
This one does belong in here.
> +
> +	payload = frame->payload;
> +	capability_count = get_unaligned_le16(payload);
> +	expected_len = OSF_CAP_REPORT_BASE_LEN +
> +		       capability_count * OSF_CAP_SENSOR_ENTRY_LEN;
> +
> +	if (frame->payload_len != expected_len)
> +		return -EMSGSIZE;
> +
> +	report->capability_count = capability_count;
> +	report->reserved = get_unaligned_le16(payload + 2);
> +	if (report->reserved)
> +		return -EPROTO;
This check is fine as picks up on bad hardware, but why keep
the value of reserved?
> +
> +	report->entries = payload + OSF_CAP_REPORT_BASE_LEN;
> +
> +	return 0;
> +}
> +
> +int osf_protocol_decode_capability_entry(const struct osf_capability_report *report,
> +					 unsigned int index,
> +					 struct osf_capability_entry *entry)
> +{
> +	const u8 *payload;
> +
> +	if (!report || !report->entries || !entry)
> +		return -EINVAL;

I'm not sure if there is a way to get here with any of those failing.
Generally for kernel code we defend against this sort of thing at higher levels.

Same applies to all this parser. 
> +
> +	if (index >= report->capability_count)

Likewise, I'd expect this to only be called on one that exists. This
defensive stuff costs us in complexity so only do it if needed. Do however
defend against values that are coming from the device where possible to 
ensure they are consistent.

> +		return -ERANGE;
> +
> +	payload = report->entries + index * OSF_CAP_SENSOR_ENTRY_LEN;

As below - if entries had the right type this would be a simple + index
I think.

> +	entry->sensor_type = get_unaligned_le16(payload);

I mention this below - nicer to use the enum type if possible.

> +	entry->sensor_index = get_unaligned_le16(payload + 2);
> +	entry->channel_count = get_unaligned_le16(payload + 4);
> +	entry->sample_format = get_unaligned_le16(payload + 6);
> +	entry->scale_nano = get_unaligned_le32(payload + 8);
> +	entry->flags = get_unaligned_le32(payload + 12);
> +	entry->reserved = get_unaligned_le32(payload + 16);
Why keep it?

I think you are setting them all, so would be neater as a designated
inializer:
	*entry = (struct osf_capbility_entry) {
		.sensor_type = get_unaligned_le16(payload),
		.sensor_index = get_unaligned_le16(payload + 2),
	};

Same applies to some of the other structures filled in here.

> +
> +	if (!osf_sensor_type_valid(entry->sensor_type))
> +		return -EPROTO;
> +
> +	if (entry->sample_format != OSF_SAMPLE_FORMAT_S32)
> +		return -EPROTO;
> +
> +	if (entry->flags & ~OSF_CAPABILITY_FLAGS_MASK)
> +		return -EPROTO;
> +
> +	if (entry->reserved)
> +		return -EPROTO;
> +
> +	return 0;
> +}
> diff --git a/drivers/iio/opensensorfusion/osf_protocol.h b/drivers/iio/opensensorfusion/osf_protocol.h
> new file mode 100644
> index 000000000..fd6e9581f
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_protocol.h
> @@ -0,0 +1,100 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef _OSF_PROTOCOL_H
> +#define _OSF_PROTOCOL_H
> +
> +#include <linux/types.h>
> +
> +#define OSF_PROTOCOL_MAJOR		0
> +#define OSF_PROTOCOL_MINOR		0
> +#define OSF_FRAME_HEADER_LEN		38
> +#define OSF_FRAME_CRC_LEN		4
> +#define OSF_FRAME_MIN_LEN		(OSF_FRAME_HEADER_LEN + OSF_FRAME_CRC_LEN)
> +
> +#define OSF_SENSOR_SAMPLE_BASE_LEN	16
> +#define OSF_DEVICE_STATUS_LEN		20
> +#define OSF_CAP_REPORT_BASE_LEN		4
> +#define OSF_CAP_SENSOR_ENTRY_LEN		20
> +#define OSF_CAPABILITY_FLAGS_MASK	0x00000003U
> +
> +enum osf_message_type {
> +	OSF_MSG_SENSOR_SAMPLE		= 0x0001,
> +	OSF_MSG_DEVICE_STATUS		= 0x0002,
> +	OSF_MSG_CAPABILITY_REPORT	= 0x0003,
> +};
> +
> +enum osf_sensor_type {
> +	OSF_SENSOR_ACCELEROMETER		= 0x0001,
> +	OSF_SENSOR_GYROSCOPE		= 0x0002,
> +	OSF_SENSOR_MAGNETOMETER		= 0x0003,
> +	OSF_SENSOR_BAROMETER		= 0x0004,
> +	OSF_SENSOR_TEMPERATURE		= 0x0005,
> +	OSF_SENSOR_HUMIDITY		= 0x0006,
> +	OSF_SENSOR_AMBIENT_LIGHT		= 0x0007,
> +	OSF_SENSOR_PROXIMITY		= 0x0008,
> +};
> +
> +enum osf_sample_format {
> +	OSF_SAMPLE_FORMAT_S32		= 0x0001,
> +};
> +
> +struct osf_frame {
> +	u8 protocol_minor;
> +	u16 message_type;
> +	u32 payload_len;
> +	u64 sequence;
> +	u64 timestamp_us;
> +	u32 flags;
> +	u32 reserved;
Not obvious why to keep this.

> +	const u8 *payload;
> +	u32 crc;
> +};
> +
> +struct osf_sensor_sample {
> +	u16 sensor_type;
> +	u16 sensor_index;
> +	u16 channel_count;
> +	u16 sample_format;

We don't need to match types with original data so
nice to use the named enums if possible.  Will need
to check limits though when decoding.

> +	u32 scale_nano;
> +	u32 reserved;
why store reserved in these?
> +	const u8 *samples;

Compared to capabilities this one seems trickier for
types given it's variable size. Still nice to use something
that indicates the type if we can.

> +};
> +
> +struct osf_device_status {
> +	u32 uptime_s;
> +	u32 status_flags;
> +	u32 error_flags;
> +	u32 dropped_frames;
> +	u32 reserved;
> +};
> +
> +struct osf_capability_report {
> +	u16 capability_count;
> +	u16 reserved;
> +	const u8 *entries;
Are these not struct osf_capability_entry?
If they are then use that not a u8.

I'd not bother with a const marking.
With that type fixed you should be able to use
__counted_by_ptr(capability_count)  to make that relationship
clear.

> +};
> +
> +struct osf_capability_entry {
> +	u16 sensor_type;
> +	u16 sensor_index;
> +	u16 channel_count;
> +	u16 sample_format;
> +	u32 scale_nano;
> +	u32 flags;
> +	u32 reserved;
> +};


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 4/7] iio: osf: add stream parser
  2026-05-24  8:53 ` [PATCH RFC v2 4/7] iio: osf: add stream parser Jinseob Kim
@ 2026-05-28 13:58   ` Jonathan Cameron
  2026-05-29 12:44     ` Kim Jinseob
  0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Cameron @ 2026-05-28 13:58 UTC (permalink / raw)
  To: Jinseob Kim
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

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;
> +}
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 5/7] iio: osf: add UART serdev transport
  2026-05-24  8:53 ` [PATCH RFC v2 5/7] iio: osf: add UART serdev transport Jinseob Kim
@ 2026-05-28 14:06   ` Jonathan Cameron
  2026-05-29 12:47     ` Kim Jinseob
  0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Cameron @ 2026-05-28 14:06 UTC (permalink / raw)
  To: Jinseob Kim
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

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

> Add the serdev receive path that feeds decoded OSF0 frames to the core.
> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
Various things inline.

> diff --git a/drivers/iio/opensensorfusion/Kconfig b/drivers/iio/opensensorfusion/Kconfig
> new file mode 100644
> index 000000000..360f25b4f
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/Kconfig
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +config OPEN_SENSOR_FUSION
> +	tristate "Open Sensor Fusion UART IIO driver"
> +	depends on IIO
> +	depends on SERIAL_DEV_BUS
> +	select CRC32
> +	help
> +	  Build the Open Sensor Fusion UART receive path.
> +
> +	  The driver receives OSF0 frames over a serdev UART.
> +	  Frames are decoded and validated before being passed to the
> +	  driver core.
> +	  This patch only adds the transport path.
> +	  IIO device registration is added separately.

Why is help text talking about a patch?

> 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
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +
> +#include "osf_core.h"
> +#include "osf_protocol.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)
> +{
> +	memset(osf, 0, sizeof(*osf));

You zero them memory before passing to this.  That seems like a sensible
pattern in which case this memset is unneeded.

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

> +
> +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;

If these can happen add a comment on why. If not remove them as overly cautious
checking.

> +
> +	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);

Perhaps check ret in each of these and return early if set. Then we only
do the shared path below on success.

> +		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;

Given there is nothing else to do on error, return -EOPNOTSUPP; perhaps.

> +		break;
> +	}
> +
> +	if (!ret)
> +		osf->last_sequence = frame.sequence;
> +
> +	return ret;
> +}

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


> +
> +static int osf_serdev_probe(struct serdev_device *serdev)
> +{
> +	struct osf_serdev *osf_uart;
> +	unsigned int baudrate;
> +	int ret;
> +
> +	osf_uart = devm_kzalloc(&serdev->dev, sizeof(*osf_uart), GFP_KERNEL);
> +	if (!osf_uart)
> +		return -ENOMEM;
> +
> +	osf_uart->serdev = serdev;
> +	osf_core_init(&osf_uart->osf, &serdev->dev);
> +	osf_stream_init(&osf_uart->stream, &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;
> +
> +	baudrate = serdev_device_set_baudrate(serdev, OSF_SERDEV_BAUD);
> +	if (baudrate != OSF_SERDEV_BAUD)
> +		dev_warn(&serdev->dev, "requested %u baud, controller set %u\n",
> +			 OSF_SERDEV_BAUD, baudrate);
> +
> +	serdev_device_set_flow_control(serdev, false);
> +
> +	return 0;
> +}
> +
> +static void osf_serdev_remove(struct serdev_device *serdev)
> +{
> +	struct osf_serdev *osf_uart = serdev_device_get_drvdata(serdev);
> +
> +	serdev_device_close(serdev);
> +	osf_stream_reset(&osf_uart->stream);
> +	osf_core_unregister_iio(&osf_uart->osf);

Given these don't match up with things in probe() please add some comments
to explain what they are undoing.

> +}


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 6/7] iio: osf: register IIO devices from capabilities
  2026-05-24  8:53 ` [PATCH RFC v2 6/7] iio: osf: register IIO devices from capabilities Jinseob Kim
@ 2026-05-28 14:30   ` Jonathan Cameron
  2026-05-29 12:49     ` Kim Jinseob
  0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Cameron @ 2026-05-28 14:30 UTC (permalink / raw)
  To: Jinseob Kim
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

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

> Register supported IIO devices from the first valid capability report.
> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
Various things inline.


> diff --git a/drivers/iio/opensensorfusion/osf_core.c b/drivers/iio/opensensorfusion/osf_core.c
> index c867b3158..a1e20ede9 100644
> --- a/drivers/iio/opensensorfusion/osf_core.c
> +++ b/drivers/iio/opensensorfusion/osf_core.c
> @@ -5,7 +5,7 @@
>  #include <linux/types.h>
>  
>  #include "osf_core.h"
> -#include "osf_protocol.h"

Why?

> +#include "osf_iio.h"
>  
>  #define OSF_RESERVED_MSG_FIRST		0x7f00
>  #define OSF_RESERVED_MSG_LAST		0x7fff

> +
> +static bool osf_core_capability_duplicate(const struct osf_capability_cache *cache,

This sounds like it makes a duplicate.  Rename to _isduplicate() or something like that.

> +					  unsigned int index)
> +{
> +	const struct osf_capability_entry *entry = &cache->entries[index];
> +	unsigned int i;
> +
> +	for (i = 0; i < index; i++) {
> +		if (!osf_iio_sensor_supported(cache->entries[i].sensor_type,
> +					      cache->entries[i].channel_count))
> +			continue;
> +
> +		if (cache->entries[i].sensor_type == entry->sensor_type &&
> +		    cache->entries[i].sensor_index == entry->sensor_index)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static int osf_core_register_capabilities(struct osf_device *osf,
> +					  const struct osf_capability_cache *cache)
> +{
> +	struct iio_dev *indio_dev;
> +	unsigned int i;
> +	int ret;
> +
> +	if (osf->capability_cache.valid)
> +		return 0;
> +
> +	for (i = 0; i < cache->capability_count; i++) {
> +		if (!osf_iio_sensor_supported(cache->entries[i].sensor_type,
> +					      cache->entries[i].channel_count))
> +			continue;
> +
> +		if (osf_core_capability_duplicate(cache, i))
> +			return -EEXIST;
> +	}
> +
> +	for (i = 0; i < cache->capability_count; i++) {
> +		if (!osf_iio_sensor_supported(cache->entries[i].sensor_type,
> +					      cache->entries[i].channel_count))
> +			continue;
> +
> +		ret = osf_iio_register_sensor(osf->dev, &cache->entries[i],
> +					      osf, &indio_dev);
> +		if (ret)
> +			goto err_unregister;
> +
> +		osf->iio_devs[osf->iio_dev_count].sensor_type =
> +			cache->entries[i].sensor_type;
> +		osf->iio_devs[osf->iio_dev_count].sensor_index =
> +			cache->entries[i].sensor_index;
> +		osf->iio_devs[osf->iio_dev_count].indio_dev = indio_dev;

Another place a designated intializer makes sense I think. See below for
other examples.

> +		osf->iio_dev_count++;
> +	}
> +
> +	return 0;
> +
> +err_unregister:
> +	osf_core_unregister_iio(osf);
> +
> +	return ret;
>  }
>  
> -static int osf_core_validate_sensor_sample(const struct osf_frame *frame)
> +static int osf_core_handle_sensor_sample(struct osf_device *osf,
> +					 const struct osf_frame *frame)
>  {
> +	struct osf_latest_sample *latest;
>  	struct osf_sensor_sample sample;
> +	struct iio_dev *indio_dev;
> +	unsigned int i;
> +	int ret;
>  
> -	return osf_protocol_decode_sensor_sample(frame, &sample);
> +	ret = osf_protocol_decode_sensor_sample(frame, &sample);
> +	if (ret)
> +		return ret;
> +
> +	if (sample.channel_count > OSF_MAX_SAMPLE_CHANNELS)
> +		return -E2BIG;
> +
> +	latest = osf_core_find_latest_sample(osf, sample.sensor_type,
> +					     sample.sensor_index);
> +	if (!latest)
> +		return -E2BIG;
> +
> +	for (i = 0; i < sample.channel_count; i++) {
> +		ret = osf_protocol_sensor_sample_value(&sample, i,
> +						       &latest->values[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	for (; i < OSF_MAX_SAMPLE_CHANNELS; i++)
> +		latest->values[i] = 0;
> +
> +	latest->sensor_type = sample.sensor_type;
> +	latest->sensor_index = sample.sensor_index;
> +	latest->channel_count = sample.channel_count;
> +	latest->sample_format = sample.sample_format;
> +	latest->scale_nano = sample.scale_nano;
> +	latest->sequence = frame->sequence;
> +	latest->timestamp_us = frame->timestamp_us;
> +	latest->valid = true;

Maybe reorder so yo ucan use a designated initializer.
Technically it will zero the values before you then fill them, but the
code is a fair bit simpler.

> +	osf->last_sequence = frame->sequence;
> +
> +	indio_dev = osf_core_find_iio_dev(osf, sample.sensor_type,
> +					  sample.sensor_index);
> +	if (indio_dev) {
I'd flip
	if (!indio_dev)
		return 0; /* Nothing to do */

> +		ret = osf_iio_push_sample(indio_dev, latest->values,
> +					  latest->channel_count);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
>  }
>  
> -static int osf_core_validate_device_status(const struct osf_frame *frame)
> +static int osf_core_handle_device_status(struct osf_device *osf,
> +					 const struct osf_frame *frame)
>  {
> +	struct osf_status_cache cache = { };
>  	struct osf_device_status status;
>  	int ret;
>  
> @@ -40,12 +194,22 @@ static int osf_core_validate_device_status(const struct osf_frame *frame)
>  	if (status.reserved)
>  		return -EPROTO;
>  
> +	cache.uptime_s = status.uptime_s;
> +	cache.status_flags = status.status_flags;
> +	cache.error_flags = status.error_flags;
> +	cache.dropped_frames = status.dropped_frames;
> +	cache.sequence = frame->sequence;
> +	cache.valid = true;
> +	osf->status_cache = cache;
	osf->status_cache = (struct osf_status_cache) {
		.uptime_s = status.update_s;
	};



> +	osf->last_sequence = frame->sequence;
> +
>  	return 0;
>  }
>  

>  
> @@ -81,27 +265,47 @@ int osf_core_receive_frame(struct osf_device *osf, const u8 *buf, size_t len)
>  
>  	switch (frame.message_type) {
>  	case OSF_MSG_SENSOR_SAMPLE:
> -		ret = osf_core_validate_sensor_sample(&frame);
> -		break;
> +		return osf_core_handle_sensor_sample(osf, &frame);

Seems like these changes belong in the earlier patch? Can you refactor
that to avoid having to modify it here.

>  	case OSF_MSG_DEVICE_STATUS:
> -		ret = osf_core_validate_device_status(&frame);
> -		break;
> +		return osf_core_handle_device_status(osf, &frame);
>  	case OSF_MSG_CAPABILITY_REPORT:
> -		ret = osf_core_validate_capability_report(&frame);
> -		break;
> +		return osf_core_handle_capability_report(osf, &frame);
>  	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;
> -		break;
> +			return 0;
> +		if (frame.message_type >= OSF_VENDOR_PRIVATE_FIRST)
> +			return 0;
> +		return -EOPNOTSUPP;
>  	}
> +}
>  
> -	if (!ret)
> -		osf->last_sequence = frame.sequence;
> +int osf_core_read_latest_sample(struct osf_device *osf, u16 sensor_type,
> +				u16 sensor_index, unsigned int channel,
> +				s32 *value)
> +{
> +	const struct osf_latest_sample *latest;
> +	unsigned int i;
>  
> -	return ret;
> +	if (!osf || !value)
> +		return -EINVAL;
> +
> +	for (i = 0; i < osf->latest_sample_count; i++) {
> +		latest = &osf->latest_samples[i];
> +		if (latest->sensor_type == sensor_type &&
> +		    latest->sensor_index == sensor_index)
> +			goto found;

I'd put the found block here rather than a goto. That simpler
flow is worth the cost of indent. You can also flip the condition
giving

	for(unsigned int i = 0; i < osf->latest_sample_count; i++) {
		latest = &osf->latest_samples[i];
		if (latest->sensor_type != sensor_type ||
		    latest->sensor_index != sensor_index)
			continue;

		if (!latest->valid)
			return -ENODATA;
		if (channel >= latest->channel_count)
			return -ENODATA;
	
		*value = latest->values[channel];

		return 0;
	}

	return -ENODATA;

> +	}
> +
> +	return -ENODATA;
> +
> +found:
> +	if (!latest->valid)
> +		return -ENODATA;
> +	if (channel >= latest->channel_count)
> +		return -ENODATA;
> +
> +	*value = latest->values[channel];
> +
> +	return 0;
>  }
> diff --git a/drivers/iio/opensensorfusion/osf_core.h b/drivers/iio/opensensorfusion/osf_core.h
> index 3680c8c9b..e5c0d5081 100644
> --- a/drivers/iio/opensensorfusion/osf_core.h
> +++ b/drivers/iio/opensensorfusion/osf_core.h
> @@ -4,15 +4,64 @@
>  
>  #include <linux/types.h>
>  
> +#include "osf_protocol.h"
> +
> +#define OSF_MAX_SAMPLE_CHANNELS	3
> +#define OSF_MAX_CAPABILITIES	16
> +
>  struct device;
> +struct iio_dev;
> +
> +struct osf_latest_sample {
> +	u16 sensor_type;
> +	u16 sensor_index;
> +	u16 channel_count;
> +	u16 sample_format;
> +	u32 scale_nano;
> +	s32 values[OSF_MAX_SAMPLE_CHANNELS];
> +	u64 sequence;
> +	u64 timestamp_us;

Some of each of these comes from an existing structure reflecting what we
read from the device, can we use that structure embedded her instead of having
to copy field by field?

> +	bool valid;
> +};
> +
> +struct osf_capability_cache {
> +	u16 capability_count;
> +	struct osf_capability_entry entries[OSF_MAX_CAPABILITIES];
> +	u64 sequence;
> +	bool valid;
> +};
> +
> +struct osf_status_cache {
> +	u32 uptime_s;
> +	u32 status_flags;
> +	u32 error_flags;
> +	u32 dropped_frames;
> +	u64 sequence;
> +	bool valid;
> +};

> diff --git a/drivers/iio/opensensorfusion/osf_iio.c b/drivers/iio/opensensorfusion/osf_iio.c
> new file mode 100644
> index 000000000..1f82ec063
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_iio.c
> @@ -0,0 +1,288 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/bitops.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/kfifo_buf.h>
> +#include <linux/kernel.h>

Neither device.h nor kernel.h should be included in a driver, unless you
are using one of the few definitions that need them. Typically there are more
focused include like dev_printk.h which should be used instead.

> +#include <linux/types.h>
> +#include <linux/units.h>

> +
> +#define OSF_SCAN_TYPE_S32						\
> +	{								\
> +		.sign = 's',						\
> +		.realbits = 32,					\
> +		.storagebits = 32,					\
> +		.endianness = IIO_CPU,					\
> +	}
> +
> +#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 = OSF_SCAN_TYPE_S32,			\
> +	}
> +
> +#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 = OSF_SCAN_TYPE_S32,			\
> +	}
Only seems likely to be used once. So why have an macro?
Maybe it is useful just to put it next to the others for visual comparison.



> +
> +static const struct iio_chan_spec osf_temp_channels[] = {
> +	OSF_CHAN(IIO_TEMP, 0),
> +	IIO_CHAN_SOFT_TIMESTAMP(1),
> +};
> +
> +static const unsigned long osf_3axis_available_scan_masks[] = {
> +	GENMASK(2, 0),
> +	0,

As below.

> +};
> +
> +static const unsigned long osf_temp_available_scan_masks[] = {
> +	BIT(0),
> +	0,
0 is a terminating entry so no trailing comma.
> +};


> +
> +static int osf_iio_read_raw(struct iio_dev *indio_dev,
> +			    const struct iio_chan_spec *chan, int *val,
> +			    int *val2, long mask)
> +{
> +	struct osf_iio_state *state = iio_priv(indio_dev);
> +	s32 raw;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (!state->osf)
> +			return -ENODATA;
> +
> +		if (chan->scan_index < 0)
> +			return -EINVAL;

If this isn't needed to silence a compiler or similar warning: we control
that value in the driver so should know if this can happen or not.
So probably not needed.

> +
> +		ret = osf_core_read_latest_sample(state->osf,
> +						  state->spec->sensor_type,
> +						  state->sensor_index,
> +						  chan->scan_index, &raw);
> +		if (ret)
> +			return ret;
> +
> +		*val = raw;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = state->scale_nano / NANO;
> +		*val2 = state->scale_nano % NANO;
> +		return IIO_VAL_INT_PLUS_NANO;
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +
> +void osf_iio_unregister_sensor(struct iio_dev *indio_dev)
> +{
> +	struct osf_iio_state *state;
> +
> +	if (!indio_dev)

If you are unregistering something that is null something went wrong
probably.  Seems unlikely we need this defence.

> +		return;
> +
> +	state = iio_priv(indio_dev);

Then this can be done at declaration above.

> +	iio_device_unregister(indio_dev);
> +	iio_kfifo_free(state->buffer);
> +	iio_device_free(indio_dev);
> +}
> +
> +int osf_iio_push_sample(struct iio_dev *indio_dev, const s32 *values,
> +			unsigned int channel_count)
> +{
> +	s64 timestamp;
> +
> +	if (!iio_buffer_enabled(indio_dev))

Add a comment on what the race is (I assume) that gets us here with that not set.

> +		return 0;
> +
> +	timestamp = iio_get_time_ns(indio_dev);
> +
> +	return iio_push_to_buffers_with_ts_unaligned(indio_dev, values,
> +						     channel_count * sizeof(*values),
> +						     timestamp);
> +}


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 7/7] MAINTAINERS: add Open Sensor Fusion IIO driver
  2026-05-24  8:53 ` [PATCH RFC v2 7/7] MAINTAINERS: add Open Sensor Fusion IIO driver Jinseob Kim
  2026-05-25 12:11   ` Krzysztof Kozlowski
@ 2026-05-28 14:31   ` Jonathan Cameron
  2026-05-29 12:51     ` Kim Jinseob
  1 sibling, 1 reply; 26+ messages in thread
From: Jonathan Cameron @ 2026-05-28 14:31 UTC (permalink / raw)
  To: Jinseob Kim
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

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

> Add file patterns for the driver, binding, and protocol reference.
> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
> ---
>  MAINTAINERS | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c2c6d7927..9238701f4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19977,6 +19977,15 @@ F:	Documentation/networking/oa-tc6-framework.rst
>  F:	drivers/net/ethernet/oa_tc6.c
>  F:	include/linux/oa_tc6.h
>  
> +OPEN SENSOR FUSION IIO DRIVER
> +M:	Jinseob Kim <kimjinseob88@gmail.com>
> +L:	linux-iio@vger.kernel.org
> +L:	devicetree@vger.kernel.org
As noted, no need for lists. The appropriate catch all listings in 
maintainers cover IIO and DT-bindings.

> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/imu/opensensorfusion,osf-uart.yaml
Bring this in with the first file, then update as you add additional files.

> +F:	Documentation/iio/open-sensor-fusion-protocol-v0.rst
> +F:	drivers/iio/opensensorfusion/
> +
>  OPEN FIRMWARE AND FLATTENED DEVICE TREE
>  M:	Rob Herring <robh@kernel.org>
>  M:	Saravana Kannan <saravanak@kernel.org>


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 4/7] iio: osf: add stream parser
  2026-05-28 13:58   ` Jonathan Cameron
@ 2026-05-29 12:44     ` Kim Jinseob
  0 siblings, 0 replies; 26+ messages in thread
From: Kim Jinseob @ 2026-05-29 12:44 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

>
> On Thu, May 28, 2026 at 10:58:00PM +0100, Jonathan Cameron wrote:
> > Various minor things inline.
>
> Addressed in RFC v3.
>
> I applied the stream parser cleanups in the new series.
>
> >         for (size_t i = 0; i < stream->len; i++)
> >                 match_len = min(stream->len - i, OSF_STREAM_MAGIC_LEN);
>
> Addressed in RFC v3.
>
> >                 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.
>
> Addressed in RFC v3 where it improved readability.
>
> > 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.
>
> Addressed in RFC v3. I removed the unnecessary internal-only checks and kept
> the boundary check for the invalid buffer/length relationship.
>
> > 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.
>
> Addressed in RFC v3.
>
> >    copy_len = min(len, space);
>
> Addressed in RFC v3.
>
> I also reviewed the Sashiko feedback for this patch while preparing RFC v3.
> Normal chunked reception is no longer counted as an error-style partial frame
> condition, and the stream/core path was kept consistent with the revised
> validation boundary used in the rest of the series.
>
> Thanks,


>
> Jinseob

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 5/7] iio: osf: add UART serdev transport
  2026-05-28 14:06   ` Jonathan Cameron
@ 2026-05-29 12:47     ` Kim Jinseob
  0 siblings, 0 replies; 26+ messages in thread
From: Kim Jinseob @ 2026-05-29 12:47 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

On Thu, May 28, 2026 at 11:06:00PM +0100, Jonathan Cameron wrote:

> Various things inline.

Addressed in RFC v3.

> Why is help text talking about a patch?

Addressed in RFC v3. I updated the Kconfig help text so it describes the
driver rather than the patch split.

> You zero them memory before passing to this.  That seems like a sensible
> pattern in which case this memset is unneeded.

Addressed in RFC v3. The unnecessary memset in osf_core_init() was removed.

> If these can happen add a comment on why. If not remove them as overly cautious
> checking.

Addressed in RFC v3. I removed the internal-only NULL checks in the core
receive path while keeping the protocol and stream boundary checks.

> Perhaps check ret in each of these and return early if set. Then we only
> do the shared path below on success.

Addressed in RFC v3. The receive path now uses a simpler early-return style.

> Given there is nothing else to do on error, return -EOPNOTSUPP; perhaps.

Addressed in RFC v3.

> Given these don't match up with things in probe() please add some comments
> to explain what they are undoing.

Addressed in RFC v3. I added a short remove-path ordering comment before the
serdev close/reset/unregister sequence.

Thanks

Jinseob

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 6/7] iio: osf: register IIO devices from capabilities
  2026-05-28 14:30   ` Jonathan Cameron
@ 2026-05-29 12:49     ` Kim Jinseob
  0 siblings, 0 replies; 26+ messages in thread
From: Kim Jinseob @ 2026-05-29 12:49 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

On Thu, May 28, 2026 at 11:30:00PM +0100, Jonathan Cameron wrote:

> Various things inline.

Addressed in RFC v3.

> Why?

Addressed in the v3 patch split. The core/protocol/IIO changes were
reworked so the includes match the code introduced in each patch

> This sounds like it makes a duplicate.  Rename to _isduplicate() or something like that.

Addressed in RFC v3.

> Another place a designated intializer makes sense I think. See below for
> other examples.

Addressed in RFC v3 where it made the code clearer.

> Maybe reorder so yo ucan use a designated initializer.
> Technically it will zero the values before you then fill them, but the
> code is a fair bit simpler.

Addressed in RFC v3 where it made the code clearer.

> I'd flip
>         if (!indio_dev)
>                 return 0; /* Nothing to do */

Addressed in RFC v3.

> Seems like these changes belong in the earlier patch? Can you refactor
> that to avoid having to modify it here.

Addressed in the v3 patch split.

> I'd put the found block here rather than a goto. That simpler
> flow is worth the cost of indent. You can also flip the condition
> giving

Addressed in RFC v3.

> Some of each of these comes from an existing structure reflecting what we
> read from the device, can we use that structure embedded her instead of having
> to copy field by field?

I reviewed this while preparing RFC v3. I kept the cached runtime state
separate from the wire-format decode structures, but reduced the field-by-field
copying where the decoded data did not need to be retained.

> Neither device.h nor kernel.h should be included in a driver, unless you
> are using one of the few definitions that need them. Typically there are more
> focused include like dev_printk.h which should be used instead.

Addressed in RFC v3. I removed broad includes that were not needed.
linux/device.h
remains only where device APIs are used.

> Only seems likely to be used once. So why have an macro?
> Maybe it is useful just to put it next to the others for visual comparison.

Reviewed while preparing RFC v3. I kept the channel helper macros only where
they improved consistency between the channel definitions.

> 0 is a terminating entry so no trailing comma.

Addressed in RFC v3.

> If this isn't needed to silence a compiler or similar warning: we control
> that value in the driver so should know if this can happen or not.
> So probably not needed.

Addressed in RFC v3. I removed unnecessary defensive checks for values the
driver controls.

> If you are unregistering something that is null something went wrong
> probably.  Seems unlikely we need this defence.

Addressed in RFC v3.

> Then this can be done at declaration above.

Addressed in RFC v3.

> Add a comment on what the race is (I assume) that gets us here with that not set.

Addressed in RFC v3. I added a short comment around the iio_buffer_enabled()
case.

I also addressed the Sashiko feedback for this patch in RFC v3 by fixing the
IIO_BUFFER / IIO_KFIFO_BUF dependency handling, rejecting channel_count
mismatches before pushing samples to IIO buffers, and adding locking around
cached latest samples.

Thanks,

Jinseob

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 7/7] MAINTAINERS: add Open Sensor Fusion IIO driver
  2026-05-28 14:31   ` Jonathan Cameron
@ 2026-05-29 12:51     ` Kim Jinseob
  2026-05-29 13:07       ` Jonathan Cameron
  0 siblings, 1 reply; 26+ messages in thread
From: Kim Jinseob @ 2026-05-29 12:51 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

On Thu, May 28, 2026 at 11:31:00PM +0100, Jonathan Cameron wrote:

> As noted, no need for lists. The appropriate catch all listings in
> maintainers cover IIO and DT-bindings.

Addressed in RFC v3.

I dropped the explicit linux-iio and devicetree list entries from the new
MAINTAINERS section.

> Bring this in with the first file, then update as you add additional files.

Addressed in RFC v3.

I removed the standalone MAINTAINERS patch and folded the MAINTAINERS updates
into the patches that add the corresponding files.

Thanks

Jinseob

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 2/7] Documentation: iio: add Open Sensor Fusion protocol v0 reference
       [not found]     ` <CALMSew+3RVXxLJYtr3HkV7UeAf6Mqx6PpA2CehChoVaMFddpJw@mail.gmail.com>
@ 2026-05-29 12:52       ` Kim Jinseob
  0 siblings, 0 replies; 26+ messages in thread
From: Kim Jinseob @ 2026-05-29 12:52 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

Sorry, resending in plain text.

> Can we have some background information. Where does this OSF thing come from?
> Is it a general standard or something you are personally developing?
> Some links would definitely help.

Addressed in RFC v3.

I added a Background section and public project links. I also clarified that
OSF0 is not a general standard, but the current wire format used by the Open
Sensor Fusion project and by this RFC driver.

The public links are now included in the documentation and cover letter:

https://www.opensensorfusion.org/
https://github.com/opensensorfusion
https://github.com/opensensorfusion/opensensorfusion-hardware
https://github.com/opensensorfusion/opensensorfusion-linux

> s32 given this is kernel code and i'm not sure what else this is referring to.

Addressed in RFC v3. I changed the kernel-facing wording to use s32.

> Not sure that is meaninful given expectation that there will be channels.
> I'd describe it as a payload header, or express this as 16 + 4 * channel_count

Addressed in RFC v3. The SENSOR_SAMPLE payload is now described as a 16-byte
payload header followed by 4 * channel_count bytes of s32 channel data.

> Is this spec defined, or just what the driver supports?
> I think this doc needs to distinguish between those two cases
> more clearly.

Addressed in RFC v3 I separated the OSF0 wire format description from the
subset currently supported by this RFC driver.


> Is there likely to be a non trivial delay?  If so we should figure out how to use
> that timestamp to get something more useful.

Addressed in RFC v3. I clarified that timestamp_us is a device-side timestamp
and that the current RFC driver does not claim production-grade host/device
timestamp correlation.

> What is AHRS?  I'd spell it out.

Addressed in RFC v3 as Attitude and Heading Reference System (AHRS).

Thanks

Jinseob

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 3/7] iio: osf: add protocol v0 decoding
       [not found]     ` <CALMSew+wUH1H-2MTtexCFgyD4Y+upFuSfFzTWBn3VGN6CWYFNQ@mail.gmail.com>
@ 2026-05-29 12:53       ` Kim Jinseob
  0 siblings, 0 replies; 26+ messages in thread
From: Kim Jinseob @ 2026-05-29 12:53 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

Sorry, resending in plain text.

On Thu, May 28, 2026 at 10:49:00PM +0100, Jonathan Cameron wrote:
> https://sashiko.dev/#/patchset/20260524085312.15369-1-kimjinseob88%40gmail.com
> Has some perhaps useful feedback. Please make sure to either exclude
> them as false positives or address for next version.
>
> Quite a bit of the feedback in here is about which checks are actually
> useful. Normal kernel practice is assume we didn't shoot outselves in the
> foot but hardware and userspace may have done. So we defend only against
> things in their control.

Addressed in RFC v3.

I reworked the decoder to focus validation on device-controlled fields. The
v3 decoder now validates sensor_type, sample_format, channel_count, reserved
fields, and payload length overflow cases.

I also removed unnecessary internal-only caller checks where the call path
already guarantees the arguments, while keeping protocol and stream boundary
checks.

> Similar on whether this defensive code makes sense

Addressed in RFC v3. I removed the remaining internal-only defensive checks
where the caller already guarantees valid arguments.

> I guess it might change in future, but for now why not make samples an s32 *

For the variable sample data I kept the wire-format decoding explicit because
the payload is little-endian data from the device and may not be naturally
aligned. The v3 code validates the format and channel count before the sample
data is consumed.

> Similar sanity check questions.
>
> If that happens seems like something went very wrong elsewhere so
> seems unlikely defense makes sense here.

Addressed in RFC v3. I trimmed the redundant internal consistency checks and
kept the checks that validate frame and payload data received from the device.

> This one does belong in here.

Kept in RFC v3. Payload length validation remains part of the decoder.

> This check is fine as picks up on bad hardware, but why keep
> the value of reserved?

Addressed in RFC v3. Reserved fields are now validate-only. The decoder checks
them for protocol compliance but does not keep them in the decoded driver
structures.

> I'm not sure if there is a way to get here with any of those failing.
> Generally for kernel code we defend against this sort of thing at higher levels.
>
> Same applies to all this parser.
>
> Likewise, I'd expect this to only be called on one that exists. This
> defensive stuff costs us in complexity so only do it if needed. Do however
> defend against values that are coming from the device where possible to
> ensure they are consistent.

Addressed in RFC v3. I removed unnecessary internal-only checks and kept the
validation for values that come from the device.

> Why keep it?

Addressed in RFC v3. Reserved values are no longer stored after validation.

> I think you are setting them all, so would be neater as a designated
> inializer:
>         *entry = (struct osf_capbility_entry) {
>                 .sensor_type = get_unaligned_le16(payload),
>                 .sensor_index = get_unaligned_le16(payload + 2),
>         };
>
> Same applies to some of the other structures filled in here.

Addressed in RFC v3 where it made the code clearer. I also reduced the amount
of field-by-field copying where the decoded data did not need to be retained.

> We don't need to match types with original data so
> nice to use the named enums if possible.  Will need
> to check limits though when decoding.

Addressed in RFC v3 by validating decoded values before accepting them from
the device. The decoder now rejects unsupported sensor_type, sample_format, and
channel_count combinations.

> Compared to capabilities this one seems trickier for
> types given it's variable size. Still nice to use something
> that indicates the type if we can.

For the variable sample data I kept the wire-format decoding explicit because
the payload is little-endian data from the device and may not be naturally
aligned.

> Are these not struct osf_capability_entry?
> If they are then use that not a u8.
>
> I'd not bother with a const marking.
> With that type fixed you should be able to use
> __counted_by_ptr(capability_count)  to make that relationship
> clear.

I reviewed this while preparing RFC v3. The capability payload is still decoded
from the wire format, but v3 now validates the capability entry contents more
strictly and avoids keeping reserved fields after validation.

Thanks,

Jinseob

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 7/7] MAINTAINERS: add Open Sensor Fusion IIO driver
  2026-05-29 12:51     ` Kim Jinseob
@ 2026-05-29 13:07       ` Jonathan Cameron
  0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2026-05-29 13:07 UTC (permalink / raw)
  To: Kim Jinseob
  Cc: linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

On Fri, 29 May 2026 21:51:06 +0900
Kim Jinseob <kimjinseob88@gmail.com> wrote:

> On Thu, May 28, 2026 at 11:31:00PM +0100, Jonathan Cameron wrote:
> 
> > As noted, no need for lists. The appropriate catch all listings in
> > maintainers cover IIO and DT-bindings.  
> 
> Addressed in RFC v3.
> 
> I dropped the explicit linux-iio and devicetree list entries from the new
> MAINTAINERS section.
> 
> > Bring this in with the first file, then update as you add additional files.  
> 
> Addressed in RFC v3.
> 
> I removed the standalone MAINTAINERS patch and folded the MAINTAINERS updates
> into the patches that add the corresponding files.
> 
> Thanks
> 
> Jinseob

Hi.

A few kernel process things.
1) Don't reply just to say you have address comments - if you are saying that
   for some things in an email, then crop the rest of the quoted text.
   Keep thanks, and how you change it for the next version of the patch, mostly
   in the change log.
2) Slow down a bit - I'd leave at least a week on the list between early versions
   of a patch set of this complexity.  You might have received some more review
   before writing v3 or discussion might be continued on any feedback you commented
   on (I didn't spot any but maybe lack of cropping meant I missed it?)


Thanks,

Jonathan


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver
  2026-05-24  8:53 [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver Jinseob Kim
                   ` (6 preceding siblings ...)
  2026-05-24  8:53 ` [PATCH RFC v2 7/7] MAINTAINERS: add Open Sensor Fusion IIO driver Jinseob Kim
@ 2026-06-02 23:59 ` Andy Shevchenko
  7 siblings, 0 replies; 26+ messages in thread
From: Andy Shevchenko @ 2026-06-02 23:59 UTC (permalink / raw)
  To: Jinseob Kim
  Cc: Jonathan Cameron, linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel

On Sun, May 24, 2026 at 05:53:05PM +0900, Jinseob Kim wrote:
> This series adds an IIO driver for a UART-attached sensor aggregation
> device. The device sends OSF0 binary frames over serdev UART. The driver
> uses capability reports to register IIO devices for supported sensors.
> 
> v2 preparation changes:
> 
> - split the previous RFC driver patch into smaller patches
> - add a Device Tree binding
> - add an OSF0 protocol reference
> - use get_unaligned_le16/32/64() for wire fields
> - use IIO timestamp handling for buffered samples
> - push decoded samples directly to registered IIO devices
> 
> Tested path:
> 
> - STM32F405 OSF0 UART stream
> - Raspberry Pi 4 serdev
> - kernel 6.12.75+rpt-rpi-v8
> - IIO devices: osf-accel, osf-gyro, osf-magn, osf-temp
> - raw reads from accel, gyro, magn, and temp
> - buffer reads from accel, gyro, magn, and temp
> 
> No OSF oops, panic, or call trace was observed in the Raspberry Pi
> runtime smoke.

My comments from the previous version seem still valid.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2026-06-02 23:59 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-24  8:53 [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver Jinseob Kim
2026-05-24  8:53 ` [PATCH RFC v2 1/7] dt-bindings: iio: add Open Sensor Fusion UART device Jinseob Kim
2026-05-24 18:59   ` Krzysztof Kozlowski
2026-05-24 19:34   ` Conor Dooley
2026-05-25  2:06     ` Kim Jinseob
2026-05-24  8:53 ` [PATCH RFC v2 2/7] Documentation: iio: add Open Sensor Fusion protocol v0 reference Jinseob Kim
2026-05-28 13:28   ` Jonathan Cameron
     [not found]     ` <CALMSew+3RVXxLJYtr3HkV7UeAf6Mqx6PpA2CehChoVaMFddpJw@mail.gmail.com>
2026-05-29 12:52       ` Kim Jinseob
2026-05-24  8:53 ` [PATCH RFC v2 3/7] iio: osf: add protocol v0 decoding Jinseob Kim
2026-05-28 13:48   ` Jonathan Cameron
     [not found]     ` <CALMSew+wUH1H-2MTtexCFgyD4Y+upFuSfFzTWBn3VGN6CWYFNQ@mail.gmail.com>
2026-05-29 12:53       ` Kim Jinseob
2026-05-24  8:53 ` [PATCH RFC v2 4/7] iio: osf: add stream parser Jinseob Kim
2026-05-28 13:58   ` Jonathan Cameron
2026-05-29 12:44     ` Kim Jinseob
2026-05-24  8:53 ` [PATCH RFC v2 5/7] iio: osf: add UART serdev transport Jinseob Kim
2026-05-28 14:06   ` Jonathan Cameron
2026-05-29 12:47     ` Kim Jinseob
2026-05-24  8:53 ` [PATCH RFC v2 6/7] iio: osf: register IIO devices from capabilities Jinseob Kim
2026-05-28 14:30   ` Jonathan Cameron
2026-05-29 12:49     ` Kim Jinseob
2026-05-24  8:53 ` [PATCH RFC v2 7/7] MAINTAINERS: add Open Sensor Fusion IIO driver Jinseob Kim
2026-05-25 12:11   ` Krzysztof Kozlowski
2026-05-28 14:31   ` Jonathan Cameron
2026-05-29 12:51     ` Kim Jinseob
2026-05-29 13:07       ` Jonathan Cameron
2026-06-02 23:59 ` [PATCH RFC v2 0/7] iio: add Open Sensor Fusion UART driver Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome