iio: add Open Sensor Fusion UART support
Subsystems (3)
v10 cover iio: add Open Sensor Fusion UART support
Specification status: OSF-D2H 0.0 spec-1 has completed project technical stability review, has been adopted by the project owner, and is published as a fixed specification.
Canonical specification:
https://github.com/opensensorfusion/opensensorfusion-protocol/blob/ca9cdea1ae550c2b4d6f29f87877adae99470744/spec/osf-d2h-0.0.md
Errata process:
https://github.com/opensensorfusion/opensensorfusion-protocol/blob/ca9cdea1ae550c2b4d6f29f87877adae99470744/errata/README.md
Current adoption/publication record:
https://github.com/opensensorfusion/opensensorfusion-protocol/blob/0cabccf63ac01d0eb82dd9882739ff58eed4a76c/reviews/publication-record-20260918.md
The specification was frozen before adoption/publication, so its embedded status snapshot is intentionally historical. The dated publication record above establishes the current adopted/public state. This is project review and adoption, not an external maintainer approval.
This series adds the Open Sensor Fusion UART receive path and IIO devices discovered from capability reports. It exposes accelerometer, gyroscope, magnetometer and temperature data through RAW/SCALE and buffered scans. Device Tree describes the sensor hub; its individual streams are discovered at runtime.
The receiver validates supported descriptors and sample scales, keeps discovery open after empty or unsupported initial inventories, and checks repeated inventories before allowing new data to use registered metadata. A supported descriptor changing meaning faults the session until an explicit teardown and rebind. Existing v9 cache, scan-layout and buffer lifetime fixes are retained.
Based on Jonathan Cameron's IIO testing branch:69fa76f0af3414cc189c3b0b807cb59e327ecc00
Changes since v9:
- Publish and reference the reviewed/adopted OSF-D2H 0.0 specification, errata process and dated publication evidence.
- Tolerate reserved padding, validate descriptor/sample scales, keep discovery open after empty/unsupported reports, and compare repeated capability reports.
- Fail closed on changed descriptor meaning instead of publishing data with stale metadata.
- Add focused KUnit coverage for these lifecycle and validation cases.
- Use managed UART/IIO/power teardown and dev_warn_probe() for the controller baud-rate warning, addressing Andy's probe-path feedback.
- Split the former combined UART/core/IIO driver patch into transport/core, IIO registration, core KUnit, and IIO KUnit patches following review.
- Use validated/unvalidated terminology for framing/CRC results throughout the stream/core/transport code, tests and diagnostics. CRC detects accidental corruption; it does not provide cryptographic authentication.
Prior validation on the identical source tree:
identical source tree evidence reused; no builds rerun for DCO packaging.
- All eight intermediate apply/config/relevant builds.
- Independent transport/core and IIO module link/MODPOST.
- GCC/Clang W=1 vmlinux and modules.
- GCC and Clang KUnit: core 16 + IIO 2, all 18 pass in each run.
- Core-only intermediate KUnit: all 16 pass.
- ARM64 Image, selected OSF module and Pi4 DTB.
- Targeted DT binding/style and IIO documentation.
The terminology-only diff was verified mechanically. Wire semantics and data-path behavior are unchanged; the receive diagnostic key is validated=. Hardware testing was not repeated for this terminology revision or this post-DCO packaging audit; previous hardware evidence remains historical.
Human DCO is complete. No email has been sent.
Jinseob Kim (8):
dt-bindings: iio: add Open Sensor Fusion device
Documentation: iio: add Open Sensor Fusion driver overview
iio: osf: add protocol decoding
iio: osf: add validated stream parser
iio: osf: add UART transport and core receive path
iio: osf: add IIO devices from capability reports
iio: osf: add core KUnit tests
iio: osf: add IIO KUnit tests
.../bindings/iio/opensensorfusion,osf.yaml | 52 + .../devicetree/bindings/vendor-prefixes.yaml | 2 + Documentation/iio/index.rst | 1 + Documentation/iio/open-sensor-fusion.rst | 77 ++ MAINTAINERS | 8 + drivers/iio/Kconfig | 1 + drivers/iio/Makefile | 1 + drivers/iio/opensensorfusion/Kconfig | 27 + drivers/iio/opensensorfusion/Makefile | 7 + drivers/iio/opensensorfusion/osf_core.c | 465 ++++++++ drivers/iio/opensensorfusion/osf_core.h | 75 ++ drivers/iio/opensensorfusion/osf_core_test.c | 1046 +++++++++++++++++ drivers/iio/opensensorfusion/osf_iio.c | 338 ++++++ drivers/iio/opensensorfusion/osf_iio.h | 22 + drivers/iio/opensensorfusion/osf_iio_test.c | 325 +++++ drivers/iio/opensensorfusion/osf_protocol.c | 215 ++++ drivers/iio/opensensorfusion/osf_protocol.h | 101 ++ drivers/iio/opensensorfusion/osf_serdev.c | 148 +++ drivers/iio/opensensorfusion/osf_stream.c | 231 ++++ drivers/iio/opensensorfusion/osf_stream.h | 53 + 20 files changed, 3195 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml create mode 100644 Documentation/iio/open-sensor-fusion.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_core_test.c 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_iio_test.c 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
base-commit: 69fa76f0af3414cc189c3b0b807cb59e327ecc00
Signature · 1 line
2.43.0
PATCH v10 1/8 dt-bindings: iio: add Open Sensor Fusion device
Add a binding for the generic Open Sensor Fusion host interface.
Open Sensor Fusion devices report capabilities and samples over an OSF protocol stream. Sensor channels are discovered at runtime from capability reports instead of being described individually in Device Tree.
The protocol version is discovered at runtime from the OSF frame header. OSF GREEN is a product identity, and OSF0 is a wire-format magic value, so neither is used as the Linux compatible string.
--- .../bindings/iio/opensensorfusion,osf.yaml | 52 +++++++++++++++++++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + MAINTAINERS | 6 +++ 3 files changed, 60 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml
Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml new file +52 -0
@@ -0,0 +1,52 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/opensensorfusion,osf.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Open Sensor Fusion Sensor Aggregation Hub + +maintainers: + - Jinseob Kim <[email protected]> + +description: | + This binding documents the generic Open Sensor Fusion host interface. Open + Sensor Fusion is a sensor aggregation hub. The hub exposes an OSF protocol + data stream over its host interface and reports capabilities and samples for + multiple sensor classes. The actual sensor channels are discovered at runtime + from OSF capability reports instead of describing them in Device Tree. The + protocol version is discovered at runtime. + + Public project documentation is available at: + + https://github.com/opensensorfusion + + OSF0, protocol_major, and protocol_minor are wire-protocol details + exchanged in OSF frames. + +allOf: + - $ref: /schemas/serial/serial-peripheral-props.yaml# + +properties: + compatible: + const: opensensorfusion,osf + + vcc-supply: + description: + Regulator supplying power to the Open Sensor Fusion device. + +required: + - compatible + - vcc-supply + +unevaluatedProperties: false + +examples: + - | + serial { + sensor { + compatible = "opensensorfusion,osf"; + vcc-supply = <&vcc_sensor>; + }; + }; +...
Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0
@@ -1267,6 +1267,8 @@ patternProperties: description: OpenPandora GmbH "^openrisc,.*": description: OpenRISC.io + "^opensensorfusion,.*": + description: Open Sensor Fusion "^openwrt,.*": description: OpenWrt "^option,.*":
MAINTAINERS +6 -0
@@ -20511,6 +20511,12 @@ F: Documentation/process/maintainer-devicetree.rst F: arch/*/boot/dts/ F: include/dt-bindings/ +OPEN SENSOR FUSION +M: Jinseob Kim <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml +K: opensensorfusion + OPENCOMPUTE PTP CLOCK DRIVER M: Vadim Fedorenko <[email protected]> L: [email protected]
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v10 1/8] dt-bindings: iio: add Open Sensor Fusion device
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] dt-bindings: generic compatible string without hardware identity --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: generic compatible string without hardware identity --
commit 3fe2d4345189668d8fcaa5a3baad45cea6d48946
Author: Jinseob Kim <[email protected]>
dt-bindings: iio: add Open Sensor Fusion device
This commit adds a binding for the generic Open Sensor Fusion host interface. It defines a generic protocol stream over which capabilities and sensor channels are discovered at runtime, intentionally omitting the specific product identity (OSF GREEN) from the compatible string.
Quoted 5 lines
Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml new file +0
-0
[ ... ]
Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml line 32
Quoted 3 lines
Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml new file +3
-0+properties: + compatible: + const: opensensorfusion,osf
[Severity: Medium]
Does restricting the compatible string to this single generic protocol value prevent the kernel from handling future hardware-specific quirks, power sequencing requirements, or errata?
The commit message explicitly states that the product identity (OSF GREEN) was intentionally omitted. Device Tree principles generally require describing the specific hardware rather than just the generic software protocol.
Would it be better to require a hardware-specific compatible string here alongside the generic one, to ensure the binding can accommodate variations in future silicon?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Jonathan Cameron
maintainerRe: [PATCH v10 1/8] dt-bindings: iio: add Open Sensor Fusion device
In reply to Jinseob KimQuoted 10 lines · On Sat, 19 Sep 2026 03:24:39 +0900 Jinseob Kim <[email protected]> wrote:
Add a binding for the generic Open Sensor Fusion host interface.
Open Sensor Fusion devices report capabilities and samples over an OSF protocol stream. Sensor channels are discovered at runtime from capability reports instead of being described individually in Device Tree.
The protocol version is discovered at runtime from the OSF frame header. OSF GREEN is a product identity, and OSF0 is a wire-format magic value, so neither is used as the Linux compatible string.
Sashiko raises a reasonable question about what you'd do if it turned out your current OSF GREEN doesn't quite comply with the specification or needs some additional quirks. Does it make sense to have a more specific compatible with a fallback to the generic one?
Or given this is mostly about device firmware maybe this is fine.
Either way - would be good to add a note on why that doesn't make sense in this patch description.
Thanks,
Jonathan
Re: [PATCH v10 1/8] dt-bindings: iio: add Open Sensor Fusion device
Quoted 4 lines
Sashiko raises a reasonable question about what you'd do if it turned out your current OSF GREEN doesn't quite comply with the specification or needs some additional quirks. Does it make sense to have a more specific compatible with a fallback to the generic one?
Hi Jonathan
With that in mind, I agree that a specific compatible with the generic OSF interface as a fallback is the better model.
OSF GREEN is one hardware variant in a family of boards with different hardware capabilities and configurations. For example, GREEN does not include RTK-GPS hardware, while another variant does. The firmware still exposes the common OSF host interface and runtime capability discovery, but the underlying boards are distinct hardware implementations.
For the next revision I'll describe the current board as:
compatible = "opensensorfusion,osf-green",
"opensensorfusion,osf";
The first string identifies the specific hardware, while the second remains the common host-interface fallback. Future hardware variants can get their own specific compatibles as they are supported, without encoding their sensor inventory or protocol version into Device Tree.
OSF0 and the protocol major/minor will remain runtime wire-protocol details.
thanks,
Jinseob
2026년 9월 20일 (일) 오전 10:14, Jonathan Cameron <[email protected]>님이 작성:
Quoted 27 lines
Quoted 10 lines · On Sat, 19 Sep 2026 03:24:39 +0900 Jinseob Kim <[email protected]> wrote:
Add a binding for the generic Open Sensor Fusion host interface.
Open Sensor Fusion devices report capabilities and samples over an OSF protocol stream. Sensor channels are discovered at runtime from capability reports instead of being described individually in Device Tree.
The protocol version is discovered at runtime from the OSF frame header. OSF GREEN is a product identity, and OSF0 is a wire-format magic value, so neither is used as the Linux compatible string.
Sashiko raises a reasonable question about what you'd do if it turned out your current OSF GREEN doesn't quite comply with the specification or needs some additional quirks. Does it make sense to have a more specific compatible with a fallback to the generic one?
Or given this is mostly about device firmware maybe this is fine.
Either way - would be good to add a note on why that doesn't make sense in this patch description.
Thanks,
Jonathan
PATCH v10 2/8 Documentation: iio: add Open Sensor Fusion driver overview
In reply to Jinseob KimDocument the supported OSF receiver profile, runtime sensor discovery, IIO channel units, scale validation and session lifetime.
Link the project-maintained fixed wire specification instead of duplicating its layouts and compatibility policy. Explain host IIO timestamps and the explicit rebind needed after a changed inventory.
--- Documentation/iio/index.rst | 1 + Documentation/iio/open-sensor-fusion.rst | 77 ++++++++++++++++++++++++ MAINTAINERS | 1 + 3 files changed, 79 insertions(+) create mode 100644 Documentation/iio/open-sensor-fusion.rst
Documentation/iio/index.rst +1 -0
@@ -40,4 +40,5 @@ Industrial I/O Kernel Drivers adxl345 bno055 ep93xx_adc + open-sensor-fusion opt4060
Documentation/iio/open-sensor-fusion.rst new file +77 -0
77 lines not on this page · raw message
MAINTAINERS +1 -0
@@ -20515,6 +20515,7 @@ OPEN SENSOR FUSION M: Jinseob Kim <[email protected]> S: Maintained F: Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml +F: Documentation/iio/open-sensor-fusion.rst K: opensensorfusion OPENCOMPUTE PTP CLOCK DRIVER
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH v10 2/8] Documentation: iio: add Open Sensor Fusion driver overview
Quoted 9 lines · On Sat, 19 Sep 2026 03:24:40 +0900 Jinseob Kim <[email protected]> wrote:
Document the supported OSF receiver profile, runtime sensor discovery, IIO channel units, scale validation and session lifetime.
Link the project-maintained fixed wire specification instead of duplicating its layouts and compatibility policy. Explain host IIO timestamps and the explicit rebind needed after a changed inventory.
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>
Hi Jinseob,
Just a couple of minor things inline.
Thanks,
Jonathan
Quoted 39 lines
--- Documentation/iio/index.rst | 1 + Documentation/iio/open-sensor-fusion.rst | 77 ++++++++++++++++++++++++ MAINTAINERS | 1 + 3 files changed, 79 insertions(+) create mode 100644 Documentation/iio/open-sensor-fusion.rstDocumentation/iio/index.rst +1
-0@@ -40,4 +40,5 @@ Industrial I/O Kernel Drivers adxl345 bno055 ep93xx_adc + open-sensor-fusion opt4060Documentation/iio/open-sensor-fusion.rst new file +16
-0@@ -0,0 +1,77 @@ +.. SPDX-License-Identifier: GPL-2.0-only + +Open Sensor Fusion +================== + +Open Sensor Fusion is a sensor aggregation hub interface. The Linux IIO driver +receives device-to-host frames over UART and discovers sensor channels from +capability reports. Device Tree describes the hub using the +``opensensorfusion,osf`` compatible; individual sensors are discovered at +runtime. See the binding in +``Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml``. + +The OSF Device-to-Host 0.0 specification is maintained by the Open Sensor Fusion +project. The canonical specification_ defines the wire layout, compatibility +rules, reserved fields and physical units. This document describes the Linux +receiver profile and its mapping to IIO.
Documentation/iio/open-sensor-fusion.rst line 26
Quoted 10 lines
Documentation/iio/open-sensor-fusion.rst new file +10
-0+ +.. _specification: https://github.com/opensensorfusion/opensensorfusion-protocol/blob/ca9cdea1ae550c2b4d6f29f87877adae99470744/spec/osf-d2h-0.0.md + +Supported receive profile +------------------------- + +The driver supports the fixed OSF0 frame layout with protocol major version 0. +Compatible minor versions use the same known message layouts. The decoder +requires exact lengths for known messages, validates CRC and bounds before +interpreting payloads, and tolerates reserved padding. Unsupported majors and
If you spin again "Unsupported major versions and.."
Quoted 20 lines
Documentation/iio/open-sensor-fusion.rst new file +20
-0+unknown message types are ignored. Unsupported capability entries are skipped +individually. A CRC-valid application rejection consumes the complete frame; +an invalid frame candidate is resynchronized without trusting its payload length. + +The receive path handles sensor samples, complete capability reports and +device status. Its limits are 4096 bytes per frame and 16 supported sensor +descriptors. These are implementation limits, not protocol-wide maxima. + +IIO mapping +----------- + +The supported sensor classes map to: + +* accelerometer: ``IIO_ACCEL`` X/Y/Z, in m/s^2; +* gyroscope: ``IIO_ANGL_VEL`` X/Y/Z, in rad/s; +* magnetometer: ``IIO_MAGN`` X/Y/Z, in gauss; +* temperature: ``IIO_TEMP``, in millidegrees Celsius. + +Each registered channel exposes signed ``RAW`` data and descriptor-derived +``SCALE``. Multiplying RAW by SCALE gives a value in the channel's IIO unit.
Documentation/iio/open-sensor-fusion.rst
Quoted 10 lines
Documentation/iio/open-sensor-fusion.rst new file +10
-0+Buffered samples use signed 32-bit values with native CPU endianness. Active +channels are packed into the scan, with initialized padding before an optional +64-bit timestamp. + +Discovery and session lifetime +------------------------------ + +The driver validates all supported descriptors before registering IIO devices. +An empty or entirely unsupported initial report leaves discovery open for a +later supported report.
So if the device can send multiple discovery reports, this implies if we recognise something in the first one we will stop looking at later ones? That seems odd given if we don't recognise anything we keep looking. If I'm reading too much into this text, then please tweak it to make it clear why this doesn't matter.
Quoted 34 lines
Supported descriptors must advertise a nonzero scale.
Documentation/iio/open-sensor-fusion.rst new file +21
-0+A sample's channel count, format and scale must match its registered descriptor +before it can update the latest RAW cache or be published to a buffer. A +rejected sample preserves previously accepted data. + +Repeated reports are compared by sensor type and index, independently of entry +order. Reserved padding, advisory flags and unsupported additions do not change +the supported inventory. A supported key appearing or disappearing, or a +change in its channel count, format or scale, faults the bound session. + +After a session fault, new cache updates and buffer publication stop, and +direct RAW reads fail. Existing IIO devices remain until driver teardown. +An equivalent report does not clear the fault; explicit unbind/rebind starts +fresh discovery. No fault-specific userspace ABI is exposed. + +Timestamps +---------- + +Buffered samples use the selected IIO host clock when they are pushed into +the buffer. The device timestamp is not correlated with that clock and is not +used as the IIO buffer timestamp. Device timestamp or sequence discontinuities +alone do not start a new host session.MAINTAINERS +1
-0@@ -20515,6 +20515,7 @@ OPEN SENSOR FUSION M: Jinseob Kim <[email protected]> S: Maintained F: Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml +F: Documentation/iio/open-sensor-fusion.rst K: opensensorfusion OPENCOMPUTE PTP CLOCK DRIVER
Re: [PATCH v10 2/8] Documentation: iio: add Open Sensor Fusion driver overview
Hi Jonathan,
both points make sense.
The capability reports are complete inventory snapshots rather than incremental fragments. An empty or unsupported-only initial report does not establish a supported inventory, so discovery remains open. Once a complete report with supported descriptors establishes the inventory for the host session, later reports are still processed:
equivalent inventories are idempotent, while a supported key appearing/disappearing or changing its shape, format, or scale is treated as an inventory change and faults the session.
I'll clarify that distinction in the next revision, and also change “Unsupported majors” to “Unsupported major versions”.
Thanks,
Jinseob
2026년 9월 20일 (일) 오전 10:12, Jonathan Cameron <[email protected]>님이 작성:
Quoted 146 lines
Quoted 9 lines · On Sat, 19 Sep 2026 03:24:40 +0900 Jinseob Kim <[email protected]> wrote:
Document the supported OSF receiver profile, runtime sensor discovery, IIO channel units, scale validation and session lifetime.
Link the project-maintained fixed wire specification instead of duplicating its layouts and compatibility policy. Explain host IIO timestamps and the explicit rebind needed after a changed inventory.
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>Hi Jinseob,
Just a couple of minor things inline.
Thanks,
Jonathan
Quoted 39 lines
--- Documentation/iio/index.rst | 1 + Documentation/iio/open-sensor-fusion.rst | 77 ++++++++++++++++++++++++ MAINTAINERS | 1 + 3 files changed, 79 insertions(+) create mode 100644 Documentation/iio/open-sensor-fusion.rstDocumentation/iio/index.rst +1
-0@@ -40,4 +40,5 @@ Industrial I/O Kernel Drivers adxl345 bno055 ep93xx_adc + open-sensor-fusion opt4060Documentation/iio/open-sensor-fusion.rst new file +16
-0@@ -0,0 +1,77 @@ +.. SPDX-License-Identifier: GPL-2.0-only + +Open Sensor Fusion +================== + +Open Sensor Fusion is a sensor aggregation hub interface. The Linux IIO driver +receives device-to-host frames over UART and discovers sensor channels from +capability reports. Device Tree describes the hub using the +``opensensorfusion,osf`` compatible; individual sensors are discovered at +runtime. See the binding in +``Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml``. + +The OSF Device-to-Host 0.0 specification is maintained by the Open Sensor Fusion +project. The canonical specification_ defines the wire layout, compatibility +rules, reserved fields and physical units. This document describes the Linux +receiver profile and its mapping to IIO.Documentation/iio/open-sensor-fusion.rst line 26
Quoted 10 lines
Documentation/iio/open-sensor-fusion.rst new file +10
-0+ +.. _specification: https://github.com/opensensorfusion/opensensorfusion-protocol/blob/ca9cdea1ae550c2b4d6f29f87877adae99470744/spec/osf-d2h-0.0.md + +Supported receive profile +------------------------- + +The driver supports the fixed OSF0 frame layout with protocol major version 0. +Compatible minor versions use the same known message layouts. The decoder +requires exact lengths for known messages, validates CRC and bounds before +interpreting payloads, and tolerates reserved padding. Unsupported majors andIf you spin again "Unsupported major versions and.."
Quoted 20 lines
Documentation/iio/open-sensor-fusion.rst new file +20
-0+unknown message types are ignored. Unsupported capability entries are skipped +individually. A CRC-valid application rejection consumes the complete frame; +an invalid frame candidate is resynchronized without trusting its payload length. + +The receive path handles sensor samples, complete capability reports and +device status. Its limits are 4096 bytes per frame and 16 supported sensor +descriptors. These are implementation limits, not protocol-wide maxima. + +IIO mapping +----------- + +The supported sensor classes map to: + +* accelerometer: ``IIO_ACCEL`` X/Y/Z, in m/s^2; +* gyroscope: ``IIO_ANGL_VEL`` X/Y/Z, in rad/s; +* magnetometer: ``IIO_MAGN`` X/Y/Z, in gauss; +* temperature: ``IIO_TEMP``, in millidegrees Celsius. + +Each registered channel exposes signed ``RAW`` data and descriptor-derived +``SCALE``. Multiplying RAW by SCALE gives a value in the channel's IIO unit.Documentation/iio/open-sensor-fusion.rst
Quoted 10 lines
Documentation/iio/open-sensor-fusion.rst new file +10
-0+Buffered samples use signed 32-bit values with native CPU endianness. Active +channels are packed into the scan, with initialized padding before an optional +64-bit timestamp. + +Discovery and session lifetime +------------------------------ + +The driver validates all supported descriptors before registering IIO devices. +An empty or entirely unsupported initial report leaves discovery open for a +later supported report.So if the device can send multiple discovery reports, this implies if we recognise something in the first one we will stop looking at later ones? That seems odd given if we don't recognise anything we keep looking. If I'm reading too much into this text, then please tweak it to make it clear why this doesn't matter.
Quoted 34 lines
Supported descriptors must advertise a nonzero scale.
Documentation/iio/open-sensor-fusion.rst new file +21
-0+A sample's channel count, format and scale must match its registered descriptor +before it can update the latest RAW cache or be published to a buffer. A +rejected sample preserves previously accepted data. + +Repeated reports are compared by sensor type and index, independently of entry +order. Reserved padding, advisory flags and unsupported additions do not change +the supported inventory. A supported key appearing or disappearing, or a +change in its channel count, format or scale, faults the bound session. + +After a session fault, new cache updates and buffer publication stop, and +direct RAW reads fail. Existing IIO devices remain until driver teardown. +An equivalent report does not clear the fault; explicit unbind/rebind starts +fresh discovery. No fault-specific userspace ABI is exposed. + +Timestamps +---------- + +Buffered samples use the selected IIO host clock when they are pushed into +the buffer. The device timestamp is not correlated with that clock and is not +used as the IIO buffer timestamp. Device timestamp or sequence discontinuities +alone do not start a new host session.MAINTAINERS +1
-0@@ -20515,6 +20515,7 @@ OPEN SENSOR FUSION M: Jinseob Kim <[email protected]> S: Maintained F: Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml +F: Documentation/iio/open-sensor-fusion.rst K: opensensorfusion OPENCOMPUTE PTP CLOCK DRIVER
PATCH v10 3/8 iio: osf: add protocol decoding
In reply to Jinseob KimAdd helpers for decoding Open Sensor Fusion frame headers and supported message payloads.
Validate the fixed OSF0 envelope, payload bounds and CRC before exposing decoded frame contents. Require exact known payload lengths and decode capability entries structurally so the core can apply support policy. Tolerate reserved padding as required by the fixed protocol contract.
Use explicit little-endian wire storage sizes and designated initializers for decoded output structures.
--- MAINTAINERS | 1 + drivers/iio/opensensorfusion/osf_protocol.c | 215 ++++++++++++++++++++ drivers/iio/opensensorfusion/osf_protocol.h | 101 +++++++++ 3 files changed, 317 insertions(+) create mode 100644 drivers/iio/opensensorfusion/osf_protocol.c create mode 100644 drivers/iio/opensensorfusion/osf_protocol.h
MAINTAINERS +1 -0
@@ -20516,6 +20516,7 @@ M: Jinseob Kim <[email protected]> S: Maintained F: Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml F: Documentation/iio/open-sensor-fusion.rst +F: drivers/iio/opensensorfusion/osf_protocol.* K: opensensorfusion OPENCOMPUTE PTP CLOCK DRIVER
drivers/iio/opensensorfusion/osf_protocol.c new file +215 -0
215 lines not on this page · raw message
drivers/iio/opensensorfusion/osf_protocol.h new file +101 -0
101 lines not on this page · raw message
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH v10 3/8] iio: osf: add protocol decoding
Quoted 8 lines · On Sat, 19 Sep 2026 03:24:41 +0900 Jinseob Kim <[email protected]> wrote:
Add helpers for decoding Open Sensor Fusion frame headers and supported message payloads.
Validate the fixed OSF0 envelope, payload bounds and CRC before exposing decoded frame contents. Require exact known payload lengths and decode capability entries structurally so the core can apply support policy. Tolerate reserved padding as required by the fixed protocol contract.
Quoted 5 lines
Use explicit little-endian wire storage sizes and designated initializers for decoded output structures.
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>
Hi Jinseob
A few things inline. Some are about what I'd kind of expect from how specs are often defined to leave a bit of flexibility and reduce the need to update drivers for new stuff being added.
If you don't want to go that way I don't really mind.
Otherwise main thing here is why carry reserved data to next layer given it is meant to be ignored. Drop it down here in the protocol decode.
Thanks,
Jonathan
Quoted 36 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +30
-0+ +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; + +if (!buf || !frame || !frame_len) +return -EINVAL ; + +if (len <OSF_FRAME_MIN_LEN ) +return -EMSGSIZE ; + +if (get_unaligned_le32 (buf) !=OSF_FRAME_MAGIC ) +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)
drivers/iio/opensensorfusion/osf_protocol.c line 63
Quoted 10 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +10
-0+return -EBADMSG ; + + frame->protocol_major = buf[4 ]; + 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 );
As below. I'm not sure what benefit of keeping reserved around is.
drivers/iio/opensensorfusion/osf_protocol.c line 65
Quoted 2 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +2
-0+ frame->payload = buf +OSF_FRAME_HEADER_LEN ; + frame->crc = actual_crc;
The above fully assigns frame so I'd do similar to you have elsewhere
*frame = (struct osf_frame) {
.protocol_major = ...
...
};Both makes it more readable and makes it clear you aren't leaving any existing data in place.
Quoted 18 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +17
-0+ *frame_len = total_len; + +return 0 ; +} + +int osf_protocol_decode_capability_report (const struct osf_frame *frame, +struct osf_capability_report *report) +{ +u16 capability_count; +size_t expected_len; +const u8 *payload; + +if (!frame || !report || !frame->payload) +return -EINVAL ; + +if (frame->message_type !=OSF_MSG_CAPABILITY_REPORT ) +return -EPROTO ;
drivers/iio/opensensorfusion/osf_protocol.c line 180
Quoted 10 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +10
-0+ +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)
I don't hugely mind as it's your code + spec to maintain but generally for a spec with records like this I'd expect it to be possible to extend
the structure without it being a breaking spec change. As such
I'd kind of expect the check to be that the payload_len was at least as big as expected len. Larger would be fine but we'd ignore anything there.
Quoted 21 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +21
-0+return -EMSGSIZE ; + + *report = (struct osf_capability_report ) { + .capability_count = capability_count, + .entries = payload +OSF_CAP_REPORT_BASE_LEN , + }; + +return 0 ; +} + +int osf_protocol_decode_capability_entry (const struct osf_capability_report + *report,u16 index, +struct osf_capability_entry *entry) +{ +const u8 *payload; + +if (!report || !report->entries || !entry) +return -EINVAL ; + +if (index >= report->capability_count) +return -ERANGE ;
drivers/iio/opensensorfusion/osf_protocol.c line 211
Quoted 10 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +10
-0+ + payload = report->entries + index *OSF_CAP_SENSOR_ENTRY_LEN ; + *entry = (struct osf_capability_entry ) { + .sensor_type =get_unaligned_le16 (payload), + .sensor_index =get_unaligned_le16 (payload +2 ), + .channel_count =get_unaligned_le16 (payload +4 ), + .sample_format =get_unaligned_le16 (payload +6 ), + .scale_nano =get_unaligned_le32 (payload +8 ), + .flags =get_unaligned_le32 (payload +12 ), + .reserved =get_unaligned_le32 (payload +16 ),
Entirely correctly I think you don't do any checks or reads of reserved values. As such why copy them around? I'd just drop that field from your decoded structures. Not a huge saving but why carry irrelevant data around!
Jonathan
Quoted 4 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +4
-0+ }; + +return 0 ; +}
Re: [PATCH v10 3/8] iio: osf: add protocol decoding
Quoted 7 lines
The above fully assigns frame so I'd do similar to you have elsewhere *frame = (struct
osf_frame) {
.protocol_major = ...
...
};
Both makes it more readable and makes it clear you aren't leaving any existing data in place.
Agreed. I'll change this to a designated initializer in the next revision.
Quoted 6 lines
I don't hugely mind as it's your code + spec to maintain but generally for a spec with records like this I'd expect it to be possible to extend the structure without it being a breaking spec change. As such I'd kind of expect the check to be that the
payload_lenwas at least as big as expected len. Larger would be fine but we'd ignore anything there.
I'd prefer to keep the exact-length check for the existing OSF0 message IDs. The fixed OSF0 specification deliberately keeps the baseline message layouts exact, with optional extensions using newly allocated message IDs rather than trailing data on an existing message ID.
So I don't plan to change this to an at-least-length check in the next revision.
Quoted 3 lines
Entirely correctly I think you don't do any checks or reads of reserved values. As such why copy them around? I'd just drop that field from your decoded structures. Not a huge saving but why carry irrelevant data around!
Agreed. I'll remove the reserved fields from the decoded structures. The reserved bytes will still be covered by the CRC and tolerated on receive, but they don't need to be propagated past the protocol decoder.
Thanks,
Jinseob
2026년 9월 20일 (일) 오전 10:27, Jonathan Cameron <[email protected]>님이 작성:
Quoted 171 lines
Quoted 8 lines · On Sat, 19 Sep 2026 03:24:41 +0900 Jinseob Kim <[email protected]> wrote:
Add helpers for decoding Open Sensor Fusion frame headers and supported message payloads.
Validate the fixed OSF0 envelope, payload bounds and CRC before exposing decoded frame contents. Require exact known payload lengths and decode capability entries structurally so the core can apply support policy. Tolerate reserved padding as required by the fixed protocol contract.
Quoted 5 lines
Use explicit little-endian wire storage sizes and designated initializers for decoded output structures.
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>Hi Jinseob
A few things inline. Some are about what I'd kind of expect from how specs are often defined to leave a bit of flexibility and reduce the need to update drivers for new stuff being added.
If you don't want to go that way I don't really mind.
Otherwise main thing here is why carry reserved data to next layer given it is meant to be ignored. Drop it down here in the protocol decode.
Thanks,
Jonathan
Quoted 36 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +30
-0+ +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; + +if (!buf || !frame || !frame_len) +return -EINVAL ; + +if (len <OSF_FRAME_MIN_LEN ) +return -EMSGSIZE ; + +if (get_unaligned_le32 (buf) !=OSF_FRAME_MAGIC ) +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)drivers/iio/opensensorfusion/osf_protocol.c line 63
Quoted 10 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +10
-0+return -EBADMSG ; + + frame->protocol_major = buf[4 ]; + 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 );As below. I'm not sure what benefit of keeping reserved around is.
drivers/iio/opensensorfusion/osf_protocol.c line 65
Quoted 2 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +2
-0+ frame->payload = buf +OSF_FRAME_HEADER_LEN ; + frame->crc = actual_crc;The above fully assigns frame so I'd do similar to you have elsewhere
*frame = (struct osf_frame) { .protocol_major = ... ... };Both makes it more readable and makes it clear you aren't leaving any existing data in place.
Quoted 18 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +17
-0+ *frame_len = total_len; + +return 0 ; +} + +int osf_protocol_decode_capability_report (const struct osf_frame *frame, +struct osf_capability_report *report) +{ +u16 capability_count; +size_t expected_len; +const u8 *payload; + +if (!frame || !report || !frame->payload) +return -EINVAL ; + +if (frame->message_type !=OSF_MSG_CAPABILITY_REPORT ) +return -EPROTO ;drivers/iio/opensensorfusion/osf_protocol.c line 180
Quoted 10 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +10
-0+ +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)I don't hugely mind as it's your code + spec to maintain but generally for a spec with records like this I'd expect it to be possible to extend
the structure without it being a breaking spec change. As such
I'd kind of expect the check to be that thepayload_lenwas at least as big as expected len. Larger would be fine but we'd ignore anything there.Quoted 21 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +21
-0+return -EMSGSIZE ; + + *report = (struct osf_capability_report ) { + .capability_count = capability_count, + .entries = payload +OSF_CAP_REPORT_BASE_LEN , + }; + +return 0 ; +} + +int osf_protocol_decode_capability_entry (const struct osf_capability_report + *report,u16 index, +struct osf_capability_entry *entry) +{ +const u8 *payload; + +if (!report || !report->entries || !entry) +return -EINVAL ; + +if (index >= report->capability_count) +return -ERANGE ;drivers/iio/opensensorfusion/osf_protocol.c line 211
Quoted 10 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +10
-0+ + payload = report->entries + index *OSF_CAP_SENSOR_ENTRY_LEN ; + *entry = (struct osf_capability_entry ) { + .sensor_type =get_unaligned_le16 (payload), + .sensor_index =get_unaligned_le16 (payload +2 ), + .channel_count =get_unaligned_le16 (payload +4 ), + .sample_format =get_unaligned_le16 (payload +6 ), + .scale_nano =get_unaligned_le32 (payload +8 ), + .flags =get_unaligned_le32 (payload +12 ), + .reserved =get_unaligned_le32 (payload +16 ),Entirely correctly I think you don't do any checks or reads of reserved values. As such why copy them around? I'd just drop that field from your decoded structures. Not a huge saving but why carry irrelevant data around!
Jonathan
Quoted 4 lines
drivers/iio/opensensorfusion/osf_protocol.c new file +4
-0+ }; + +return 0 ; +}
PATCH v10 4/8 iio: osf: add validated stream parser
In reply to Jinseob KimAdd a UART byte-stream parser for Open Sensor Fusion frames.
The parser searches for the OSF0 wire magic, keeps partial frames buffered, checks header length and payload bounds, and passes complete candidate frames to a registered frame callback.
Candidates rejected before validation drop only the current head byte before resynchronizing, so a corrupted unvalidated payload length cannot make the parser skip later valid frames. CRC-valid validated frames are consumed in full and classified as handled, ignored, or rejected.
Use a direct callback member with an opaque context and keep explicit statistics for validated outcomes and framing failures.
--- MAINTAINERS | 1 + drivers/iio/opensensorfusion/osf_stream.c | 231 ++++++++++++++++++++++ drivers/iio/opensensorfusion/osf_stream.h | 53 +++++ 3 files changed, 285 insertions(+) create mode 100644 drivers/iio/opensensorfusion/osf_stream.c create mode 100644 drivers/iio/opensensorfusion/osf_stream.h
MAINTAINERS +1 -0
@@ -20517,6 +20517,7 @@ S: Maintained F: Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml F: Documentation/iio/open-sensor-fusion.rst F: drivers/iio/opensensorfusion/osf_protocol.* +F: drivers/iio/opensensorfusion/osf_stream.* K: opensensorfusion OPENCOMPUTE PTP CLOCK DRIVER
drivers/iio/opensensorfusion/osf_stream.c new file +231 -0
231 lines not on this page · raw message
drivers/iio/opensensorfusion/osf_stream.h new file +53 -0
53 lines not on this page · raw message
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH v10 4/8] iio: osf: add validated stream parser
Quoted 12 lines · On Sat, 19 Sep 2026 03:24:42 +0900 Jinseob Kim <[email protected]> wrote:
Add a UART byte-stream parser for Open Sensor Fusion frames.
The parser searches for the OSF0 wire magic, keeps partial frames buffered, checks header length and payload bounds, and passes complete candidate frames to a registered frame callback.
Candidates rejected before validation drop only the current head byte before resynchronizing, so a corrupted unvalidated payload length cannot make the parser skip later valid frames. CRC-valid validated frames are consumed in full and classified as handled, ignored, or rejected.
Quoted 5 lines
Use a direct callback member with an opaque context and keep explicit statistics for validated outcomes and framing failures.
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>
Just one trivial thing.
Thanks,
Jonathan
Quoted 19 lines
drivers/iio/opensensorfusion/osf_stream.c new file +12
-0+void osf_stream_init (struct osf_stream *stream, +int (*receive_frame)(void *context,const u8 *buf, +size_t len), +void *frame_context) +{ +if (!stream) +return ; + + stream->receive_frame = receive_frame; + stream->frame_context = frame_context; + stream->len =0 ; +memset (&stream->stats,0 ,sizeof (stream->stats));
drivers/iio/opensensorfusion/osf_stream.c line 186
Quoted 10 lines
drivers/iio/opensensorfusion/osf_stream.c new file +10
-0+} + +void osf_stream_reset (struct osf_stream *stream) +{ +if (!stream) +return ; + + stream->len =0 ; +memset (&stream->stats,0 ,sizeof (stream->stats)); +}
You could reorder these two functions and call stream_reset from stream_init. Would make it clear what getting to a 'clean' state means.
Re: [PATCH v10 4/8] iio: osf: add validated stream parser
Quoted 3 lines
You could reorder these two functions and call
stream_resetfromstream_init. Would make it clear what getting to a 'clean' state means.
Agreed. I'll move osf_stream_reset() before osf_stream_init() and use it from the init path in the next revision.
Thanks,
Jinseob
2026년 9월 20일 (일) 오전 10:31, Jonathan Cameron <[email protected]>님이 작성:
Quoted 64 lines
Quoted 12 lines · On Sat, 19 Sep 2026 03:24:42 +0900 Jinseob Kim <[email protected]> wrote:
Add a UART byte-stream parser for Open Sensor Fusion frames.
The parser searches for the OSF0 wire magic, keeps partial frames buffered, checks header length and payload bounds, and passes complete candidate frames to a registered frame callback.
Candidates rejected before validation drop only the current head byte before resynchronizing, so a corrupted unvalidated payload length cannot make the parser skip later valid frames. CRC-valid validated frames are consumed in full and classified as handled, ignored, or rejected.
Quoted 5 lines
Use a direct callback member with an opaque context and keep explicit statistics for validated outcomes and framing failures.
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>Just one trivial thing.
Thanks,
Jonathan
Quoted 19 lines
drivers/iio/opensensorfusion/osf_stream.c new file +12
-0+void osf_stream_init (struct osf_stream *stream, +int (*receive_frame)(void *context,const u8 *buf, +size_t len), +void *frame_context) +{ +if (!stream) +return ; + + stream->receive_frame = receive_frame; + stream->frame_context = frame_context; + stream->len =0 ; +memset (&stream->stats,0 ,sizeof (stream->stats));drivers/iio/opensensorfusion/osf_stream.c line 186
Quoted 10 lines
drivers/iio/opensensorfusion/osf_stream.c new file +10
-0+} + +void osf_stream_reset (struct osf_stream *stream) +{ +if (!stream) +return ; + + stream->len =0 ; +memset (&stream->stats,0 ,sizeof (stream->stats)); +}You could reorder these two functions and call
stream_resetfromstream_init. Would make it clear what getting to a 'clean' state means.
Jonathan Cameron
maintainerRe: [PATCH v10 4/8] iio: osf: add validated stream parser
Quoted 7 lines · On Sun, 20 Sep 2026 13:12:54 +0900 Kim Jinseob <[email protected]> wrote:
Quoted 3 lines
You could reorder these two functions and call
stream_resetfromstream_init. Would make it clear what getting to a 'clean' state means.Agreed. I'll move
osf_stream_reset()beforeosf_stream_init()and use it from the init path in the next revision.
A few process things. Strong preference is always for reply inline so the context is there next to the reply rather than pulling part of that context to the top of the email.
The other thing is that if your reply doesn't convey much information beyond agreeing to make suggested change, don't send the reply. We all get far too many emails and can check the change log of the next version to see if everything that was not addressed in the earlier thread has been dealt with!
Note I send this sort of email about once a week - so you are far from the first to need a little nudge towards more optimal interactions on the list!
Thanks,
Jonathan
Quoted 70 lines
Thanks,
Jinseob
2026년 9월 20일 (일) 오전 10:31, Jonathan Cameron <[email protected]>님이 작성:
Quoted 64 lines
Quoted 12 lines · On Sat, 19 Sep 2026 03:24:42 +0900 Jinseob Kim <[email protected]> wrote:
Add a UART byte-stream parser for Open Sensor Fusion frames.
The parser searches for the OSF0 wire magic, keeps partial frames buffered, checks header length and payload bounds, and passes complete candidate frames to a registered frame callback.
Candidates rejected before validation drop only the current head byte before resynchronizing, so a corrupted unvalidated payload length cannot make the parser skip later valid frames. CRC-valid validated frames are consumed in full and classified as handled, ignored, or rejected.
Quoted 5 lines
Use a direct callback member with an opaque context and keep explicit statistics for validated outcomes and framing failures.
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>Just one trivial thing.
Thanks,
Jonathan
Quoted 19 lines
drivers/iio/opensensorfusion/osf_stream.c new file +12
-0+void osf_stream_init (struct osf_stream *stream, +int (*receive_frame)(void *context,const u8 *buf, +size_t len), +void *frame_context) +{ +if (!stream) +return ; + + stream->receive_frame = receive_frame; + stream->frame_context = frame_context; + stream->len =0 ; +memset (&stream->stats,0 ,sizeof (stream->stats));drivers/iio/opensensorfusion/osf_stream.c line 186
Quoted 10 lines
drivers/iio/opensensorfusion/osf_stream.c new file +10
-0+} + +void osf_stream_reset (struct osf_stream *stream) +{ +if (!stream) +return ; + + stream->len =0 ; +memset (&stream->stats,0 ,sizeof (stream->stats)); +}You could reorder these two functions and call
stream_resetfromstream_init. Would make it clear what getting to a 'clean' state means.
PATCH v10 5/8 iio: osf: add UART transport and core receive path
In reply to Jinseob KimConnect the stream parser to a serdev receiver and add core frame dispatch with an owned device-status cache. Preserve the distinction between unvalidated candidates and validated application rejections, so malformed status payloads do not cause byte-wise resynchronization.
Open and configure the UART before enabling the supply. A managed release action closes and drains the receive producer before resetting the parser and disabling a successfully enabled supply.
This transport base processes device-status frames and ignores message types without an application handler. It does not register IIO sensor devices. The following patch adds capability discovery and sample publication to IIO, including the publication gate and IIO teardown.
--- MAINTAINERS | 3 +- drivers/iio/Kconfig | 1 + drivers/iio/Makefile | 1 + drivers/iio/opensensorfusion/Kconfig | 12 ++ drivers/iio/opensensorfusion/Makefile | 5 + drivers/iio/opensensorfusion/osf_core.c | 101 +++++++++++++++ drivers/iio/opensensorfusion/osf_core.h | 29 +++++ drivers/iio/opensensorfusion/osf_serdev.c | 143 ++++++++++++++++++++++ 8 files changed, 293 insertions(+), 2 deletions(-) create mode 100644 drivers/iio/opensensorfusion/Kconfig create mode 100644 drivers/iio/opensensorfusion/Makefile create mode 100644 drivers/iio/opensensorfusion/osf_core.c create mode 100644 drivers/iio/opensensorfusion/osf_core.h create mode 100644 drivers/iio/opensensorfusion/osf_serdev.c
MAINTAINERS +1 -2
@@ -20516,8 +20516,7 @@ M: Jinseob Kim <[email protected]> S: Maintained F: Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml F: Documentation/iio/open-sensor-fusion.rst -F: drivers/iio/opensensorfusion/osf_protocol.* -F: drivers/iio/opensensorfusion/osf_stream.* +F: drivers/iio/opensensorfusion/ K: opensensorfusion OPENCOMPUTE PTP CLOCK DRIVER
drivers/iio/Kconfig +1 -0
@@ -102,6 +102,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"
drivers/iio/Makefile +1 -0
@@ -38,6 +38,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/
drivers/iio/opensensorfusion/Kconfig new file +12 -0
@@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0-only + +config OPEN_SENSOR_FUSION + tristate "Open Sensor Fusion UART receiver" + depends on SERIAL_DEV_BUS + select CRC32 + help + Build the Open Sensor Fusion UART receive path. + + The driver receives and validates OSF protocol frames over a serdev + UART and caches device status reported by the sensor hub. + Message types without an application handler are ignored.
drivers/iio/opensensorfusion/Makefile new file +5 -0
@@ -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
drivers/iio/opensensorfusion/osf_core.c new file +101 -0
101 lines not on this page · raw message
drivers/iio/opensensorfusion/osf_core.h new file +29 -0
@@ -0,0 +1,29 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef _OSF_CORE_H +#define _OSF_CORE_H + +#include <linux/types.h> + +#include "osf_protocol.h" + +struct device ; + +struct osf_status_cache { +u32 uptime_s; +u32 status_flags; +u32 error_flags; +u32 dropped_frames; +u64 sequence; +bool valid; +}; + +struct osf_device { +struct device *dev; +struct osf_status_cache status_cache; +u64 last_sequence; +}; + +void osf_core_init (struct osf_device *osf,struct device *dev); +int osf_core_receive_frame (struct osf_device *osf,const u8 *buf,size_t len); + +#endif
drivers/iio/opensensorfusion/osf_serdev.c new file +143 -0
143 lines not on this page · raw message
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH v10 5/8] iio: osf: add UART transport and core receive path
Quoted 9 lines · On Sat, 19 Sep 2026 03:24:43 +0900 Jinseob Kim <[email protected]> wrote:
Connect the stream parser to a serdev receiver and add core frame dispatch with an owned device-status cache. Preserve the distinction between unvalidated candidates and validated application rejections, so malformed status payloads do not cause byte-wise resynchronization.
Open and configure the UART before enabling the supply. A managed release action closes and drains the receive producer before resetting the parser and disabling a successfully enabled supply.
Quoted 7 lines
This transport base processes device-status frames and ignores message types without an application handler. It does not register IIO sensor devices. The following patch adds capability discovery and sample publication to IIO, including the publication gate and IIO teardown.
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>
One question for Uwe if they see it about whether device-id/of.h should be included or we can assume it for serial drivers.
Otherwise, just a couple of minor things inline.
Thanks,
Jonathan
Quoted 21 lines
--- MAINTAINERS | 3 +- drivers/iio/Kconfig | 1 + drivers/iio/Makefile | 1 + drivers/iio/opensensorfusion/Kconfig | 12 ++ drivers/iio/opensensorfusion/Makefile | 5 + drivers/iio/opensensorfusion/osf_core.c | 101 +++++++++++++++ drivers/iio/opensensorfusion/osf_core.h | 29 +++++ drivers/iio/opensensorfusion/osf_serdev.c | 143 ++++++++++++++++++++++ 8 files changed, 293 insertions(+), 2 deletions(-) create mode 100644 drivers/iio/opensensorfusion/Kconfig create mode 100644 drivers/iio/opensensorfusion/Makefile create mode 100644 drivers/iio/opensensorfusion/osf_core.c create mode 100644 drivers/iio/opensensorfusion/osf_core.h create mode 100644 drivers/iio/opensensorfusion/osf_serdev.cMAINTAINERS +0
-0
MAINTAINERS M: Jinseob Kim <[email protected]> · line 20519
Quoted 6 lines
MAINTAINERS +1
-2@@ -20516,8 +20516,7 @@ M: Jinseob Kim <[email protected]> S: Maintained F: Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml F: Documentation/iio/open-sensor-fusion.rst -F: drivers/iio/opensensorfusion/osf_protocol.* -F: drivers/iio/opensensorfusion/osf_stream.* +F: drivers/iio/opensensorfusion/
Push that back to earlier patches.
Quoted 17 lines
MAINTAINERS +0
-0K: opensensorfusiondrivers/iio/opensensorfusion/osf_core.c new file +9
-0@@ -0,0 +1,101 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include <linux/device.h> +#include <linux/errno.h> +#include <linux/types.h> + +#include "osf_core.h" +#include "osf_stream.h" +
drivers/iio/opensensorfusion/osf_core.c line 19
Quoted 10 lines
drivers/iio/opensensorfusion/osf_core.c new file +10
-0+#define OSF_RESERVED_MSG_FIRST 0x7f00 +#define OSF_RESERVED_MSG_LAST 0x7fff +#define OSF_VENDOR_PRIVATE_FIRST 0x8000 + +void osf_core_init (struct osf_device *osf,struct device *dev) +{ + *osf = (struct osf_device ) { + .dev = dev, + }; +}
Unless this gets a lot more complex in later patches, I'd drop the helper.
Quoted 41 lines
drivers/iio/opensensorfusion/osf_core.c new file +41
-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; + + ret =osf_protocol_decode_frame (buf, len, &frame, &frame_len); +if (ret) +return ret; + +if (frame_len != len) +return -EMSGSIZE ; + +if (frame.protocol_major !=OSF_PROTOCOL_MAJOR ) { +dev_dbg_ratelimited (osf->dev, +"ignoring unsupported protocol major %u\n" , + frame.protocol_major); +return OSF_STREAM_FRAME_IGNORED ; + } + +switch (frame.message_type) { +case OSF_MSG_DEVICE_STATUS : + ret =osf_core_handle_device_status (osf, &frame); +break ; +default : +if (frame.message_type >=OSF_RESERVED_MSG_FIRST && + frame.message_type <=OSF_RESERVED_MSG_LAST ) { +dev_dbg_ratelimited (osf->dev, +"ignoring reserved message type %#x\n" , + frame.message_type); +return OSF_STREAM_FRAME_IGNORED ; + } +if (frame.message_type >=OSF_VENDOR_PRIVATE_FIRST ) { +dev_dbg_ratelimited (osf->dev, +"ignoring vendor message type %#x\n" , + frame.message_type); +return OSF_STREAM_FRAME_IGNORED ; + } + + dev_dbg_ratelimited(osf->dev, + "ignoring unsupported message type %#x\n",
drivers/iio/opensensorfusion/osf_core.c line 98
Quoted 10 lines
drivers/iio/opensensorfusion/osf_core.c new file +10
-0+ frame.message_type); +return OSF_STREAM_FRAME_IGNORED ; + } + +/* +* Handler failures are validated application rejections. Keep the +* trusted frame boundary and let the stream consume the complete frame. +*/ +if (ret <0 ) +return OSF_STREAM_FRAME_REJECTED ;
I think there is only one path to this. Move the comment and check up there
+ the return.
Then we never leave the switch statement without returning so this last chunk isn't needed.
Quoted 10 lines
drivers/iio/opensensorfusion/osf_core.c new file +3
-0+ +return ret; +}drivers/iio/opensensorfusion/osf_serdev.c new file +0
-0
drivers/iio/opensensorfusion/osf_serdev.c line 4
Quoted 4 lines
drivers/iio/opensensorfusion/osf_serdev.c new file +4
-0@@ -0,0 +1,143 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include <linux/cleanup.h> +#include <linux/device-id/of.h>
Curious that in all the recent tidying up of this stuff serdev.h didn't include this one. Uwe, if you see this, was that intentional?
Doesn't matter though as if it is something that gets tidied up later then this driver will get covered with any others.
Quoted 17 lines
drivers/iio/opensensorfusion/osf_serdev.c new file +16
-0+#include <linux/device.h> +#include <linux/err.h> +#include <linux/errno.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/regulator/consumer.h> +#include <linux/serdev.h> +#include <linux/slab.h> +#include <linux/types.h> +static int osf_serdev_probe (struct serdev_device *serdev) +{ +struct device *dev = &serdev->dev; +struct osf_serdev *osf_uart; +unsigned int baudrate; +int ret; +
drivers/iio/opensensorfusion/osf_serdev.c line 94
Quoted 10 lines
drivers/iio/opensensorfusion/osf_serdev.c new file +10
-0+ osf_uart =devm_kzalloc (dev,sizeof (*osf_uart),GFP_KERNEL ); +if (!osf_uart) +return -ENOMEM ; + + osf_uart->vcc =devm_regulator_get (dev,"vcc" ); +if (IS_ERR (osf_uart->vcc)) +return dev_err_probe (dev,PTR_ERR (osf_uart->vcc), +"failed to get vcc regulator\n" ); + +mutex_init (&osf_uart->rx_lock);
Can we use devm_mutex_init() here. It doesn't bring huge advantage but it is a cheap win for some debugging cases.
Quoted 31 lines
drivers/iio/opensensorfusion/osf_serdev.c new file +31
-0+ osf_uart->serdev = serdev; +osf_core_init (&osf_uart->osf, dev); +osf_stream_init (&osf_uart->stream, osf_serdev_receive_frame, + &osf_uart->osf); + +serdev_device_set_drvdata (serdev, osf_uart); +serdev_device_set_client_ops (serdev, &osf_serdev_ops); + + ret =serdev_device_open (serdev); +if (ret) +return ret; + + ret =devm_add_action_or_reset (dev, osf_serdev_release, osf_uart); +if (ret) +return ret; + + baudrate =serdev_device_set_baudrate (serdev,OSF_SERDEV_BAUD ); +if (baudrate !=OSF_SERDEV_BAUD ) +/* Keep accepting controller rounding after reporting the mismatch. */ +dev_warn_probe (dev, -EINVAL ,"requested %u baud, controller set %u\n" , +OSF_SERDEV_BAUD , baudrate); + +serdev_device_set_flow_control (serdev, false); + + ret =regulator_enable (osf_uart->vcc); +if (ret) +return dev_err_probe (dev, ret,"failed to enable vcc regulator\n" ); + osf_uart->vcc_enabled = true; + +return 0 ; +}
PATCH v10 6/8 iio: osf: add IIO devices from capability reports
In reply to Jinseob KimRegister IIO devices from supported capability descriptors and expose signed raw samples, descriptor scales and software buffers. Keep early capabilities owned by the core until UART and supply setup is complete, and unregister children after the receive producer has stopped.
Validate nonzero descriptor scales and sample/descriptor equality, leave discovery open after empty or unsupported inventories, and compare repeated descriptors independently of order. Fault a bound session if a descriptor changes meaning, gating cache and buffer publication together.
Add initialized active-scan packing and buffer producer quiescence. The production implementation is now complete; subsequent patches only add the existing core and IIO KUnit suites and their build wiring.
--- drivers/iio/opensensorfusion/Kconfig | 13 +- drivers/iio/opensensorfusion/Makefile | 3 +- drivers/iio/opensensorfusion/osf_core.c | 364 ++++++++++++++++++++++ drivers/iio/opensensorfusion/osf_core.h | 46 +++ drivers/iio/opensensorfusion/osf_iio.c | 338 ++++++++++++++++++++ drivers/iio/opensensorfusion/osf_iio.h | 22 ++ drivers/iio/opensensorfusion/osf_serdev.c | 11 +- 7 files changed, 788 insertions(+), 9 deletions(-) create mode 100644 drivers/iio/opensensorfusion/osf_iio.c create mode 100644 drivers/iio/opensensorfusion/osf_iio.h
drivers/iio/opensensorfusion/Kconfig +8 -5
@@ -1,12 +1,15 @@ # SPDX-License-Identifier: GPL-2.0-only config OPEN_SENSOR_FUSION - tristate "Open Sensor Fusion UART receiver" + tristate "Open Sensor Fusion UART IIO driver" depends on SERIAL_DEV_BUS select CRC32 + select IIO_BUFFER + select IIO_KFIFO_BUF help - Build the Open Sensor Fusion UART receive path. + Build the Open Sensor Fusion UART IIO driver. - The driver receives and validates OSF protocol frames over a serdev - UART and caches device status reported by the sensor hub. - Message types without an application handler are ignored. + The driver receives OSF protocol frames over a serdev UART and + registers IIO devices for supported capability entries. It exposes + accelerometer, gyroscope, magnetometer, and temperature samples + through IIO direct reads and software buffers.
drivers/iio/opensensorfusion/Makefile +2 -1
@@ -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
drivers/iio/opensensorfusion/osf_core.c +364 -0
398 lines not on this page · raw message
drivers/iio/opensensorfusion/osf_core.h +46 -0
70 lines not on this page · raw message
drivers/iio/opensensorfusion/osf_iio.c new file +338 -0
338 lines not on this page · raw message
drivers/iio/opensensorfusion/osf_iio.h new file +22 -0
@@ -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, +u16 channel_count); +bool osf_iio_sensor_supported (u16 sensor_type,u16 channel_count); +const char *osf_iio_sensor_name (u16 sensor_type); + +#endif
drivers/iio/opensensorfusion/osf_serdev.c +8 -3
@@ -23,7 +23,7 @@ struct osf_serdev {bool vcc_enabled;struct osf_device osf;struct osf_stream stream; -/* Protects the parser and all RX counters. */ +/* Protects the parser, all RX counters, and the registration gate. */ struct mutex rx_lock; };
@@ -68,9 +68,10 @@ static void osf_serdev_release(void *data) {struct osf_serdev *osf_uart = data; -/* The TTY controller drains RX work on close; hold no RX lock. */ +/* The TTY controller drains RX work on close; hold no RX or IIO lock. */ serdev_device_close (osf_uart->serdev);osf_stream_reset (&osf_uart->stream); +osf_core_unregister_iio (&osf_uart->osf);if (osf_uart->vcc_enabled)regulator_disable (osf_uart->vcc); }
@@ -121,6 +122,10 @@ static int osf_serdev_probe(struct serdev_device *serdev)return dev_err_probe (dev, ret,"failed to enable vcc regulator\n" ); osf_uart->vcc_enabled = true; +/* No fallible probe steps remain when IIO children become visible. */ + scoped_guard(mutex, &osf_uart->rx_lock) +osf_core_start (&osf_uart->osf); +return 0 ; }
@@ -139,5 +144,5 @@ static struct serdev_device_driver osf_serdev_driver = { };module_serdev_device_driver (osf_serdev_driver); -MODULE_DESCRIPTION ("Open Sensor Fusion UART receiver" ); +MODULE_DESCRIPTION ("Open Sensor Fusion IIO driver" );MODULE_LICENSE ("GPL" );
Signature · 1 line
2.43.0
Jonathan Cameron
maintainerRe: [PATCH v10 6/8] iio: osf: add IIO devices from capability reports
Quoted 5 lines · On Sat, 19 Sep 2026 03:24:44 +0900 Jinseob Kim <[email protected]> wrote:
Register IIO devices from supported capability descriptors and expose signed raw samples, descriptor scales and software buffers. Keep early capabilities owned by the core until UART and supply setup is complete, and unregister children after the receive producer has stopped.
Quoted 8 lines
Validate nonzero descriptor scales and sample/descriptor equality, leave discovery open after empty or unsupported inventories, and compare repeated descriptors independently of order. Fault a bound session if a descriptor changes meaning, gating cache and buffer publication together.
Add initialized active-scan packing and buffer producer quiescence. The production implementation is now complete; subsequent patches only add the existing core and IIO KUnit suites and their build wiring.
No need to talk about what other patches do and that a series is functionally complete in a patch. If you want to do it, under the --- so it doesn't end up in the git log.
Quoted 3 lines
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>
A few things inline. Biggest one is a question (I'm not sure) on whether you can use a claim on buffered mode (which will hold it in that if it succeeds until released) is sufficient to replace the extra lock you have to block that transition.
Thanks,
Jonathan
Quoted 5 lines
drivers/iio/opensensorfusion/osf_iio.c new file +0
-0
...
drivers/iio/opensensorfusion/osf_iio.c line 39
Quoted 4 lines
drivers/iio/opensensorfusion/osf_iio.c new file +4
-0+ +struct osf_iio_scan_3axis { +s32 values[3 ]; +u32 padding;
No needed unless you explicitly write into padding for some reason. C rules add it anyway and then we don't need to make sure it remains correct if this structure changes in future.
drivers/iio/opensensorfusion/osf_iio.c line 45
Quoted 6 lines
drivers/iio/opensensorfusion/osf_iio.c new file +6
-0+aligned_s64 timestamp; +}; + +struct osf_iio_scan_1axis { +s32 value; +u32 padding;
As above, unneeded.
drivers/iio/opensensorfusion/osf_iio.c
Quoted 2 lines
drivers/iio/opensensorfusion/osf_iio.c new file +2
-0+aligned_s64 timestamp; +};
Given I think these are only used in one place, moving them to there would avoid need for named structure types.
Quoted 8 lines
drivers/iio/opensensorfusion/osf_iio.c new file +8
-0+ +#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), \
drivers/iio/opensensorfusion/osf_iio.c line 65
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+ .scan_index = (_idx), \ + .scan_type = { \ + .sign = 's', \ + .realbits = 32, \ + .storagebits = 32, \ + .endianness = IIO_CPU, \ + }, \ + } + +#define OSF_CHAN (_type, _idx) \
Given this is not much used, you could use the IIO_NO_MOD modifier as a gate on setting .modified = (_mod != IIO_NO_MOD) ? 1 : 0,
and have just one macro.
Quoted 78 lines
drivers/iio/opensensorfusion/osf_iio.c new file +78
-0+ { \ + .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 = { \ + .sign = 's', \ + .realbits = 32, \ + .storagebits = 32, \ + .endianness = IIO_CPU, \ + }, \ + } + +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 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), + }, + { + .sensor_type =OSF_SENSOR_GYROSCOPE , + .channel_count =3 , + .name ="osf-gyro" , + .channels = osf_gyro_channels, + .num_channels =ARRAY_SIZE (osf_gyro_channels), + }, + { + .sensor_type =OSF_SENSOR_MAGNETOMETER , + .channel_count =3 , + .name ="osf-magn" , + .channels = osf_mag_channels, + .num_channels =ARRAY_SIZE (osf_mag_channels), + }, + { + .sensor_type =OSF_SENSOR_TEMPERATURE , + .channel_count =1 , + .name ="osf-temp" , + .channels = osf_temp_channels, + .num_channels =ARRAY_SIZE (osf_temp_channels), + }, +}; + +static const struct osf_iio_sensor_spec * +osf_iio_find_sensor_spec (u16 sensor_type,u16 channel_count) +{ +for (unsigned int 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]; + }
drivers/iio/opensensorfusion/osf_iio.c
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+ +return NULL ; +} + +bool osf_iio_sensor_supported (u16 sensor_type,u16 channel_count) +{ +if (osf_iio_find_sensor_spec (sensor_type, channel_count)) +return true; + +return false
return osf_iio_find_sensor_spec(sensor_type, channel_count) != NULL;
drivers/iio/opensensorfusion/osf_iio.c line 202
Quoted 8 lines
drivers/iio/opensensorfusion/osf_iio.c new file +7
-0+} + +static int osf_iio_buffer_postenable (struct iio_dev *indio_dev) +{ +struct osf_iio_state *state =iio_priv (indio_dev); + +guard (mutex)(&state->buffer_lock);
These transistions are already guarded by the core, so see the question later on whether you can just claim buffered mode in the data handler to avoid the need for this. That might not work but the way you did this got me thinking about a more general solution to the problem.
Quoted 18 lines
drivers/iio/opensensorfusion/osf_iio.c new file +16
-0+ state->buffer_active = true; + +return 0 ; +} + +static int osf_iio_buffer_predisable (struct iio_dev *indio_dev) +{ +struct osf_iio_state *state =iio_priv (indio_dev); + +/* Wait for the current push before the IIO core changes its buffers. */ +guard (mutex)(&state->buffer_lock); + state->buffer_active = false; + +return 0 ; +} +
drivers/iio/opensensorfusion/osf_iio.c line 307
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+int osf_iio_push_sample (struct iio_dev *indio_dev,const s32 *values, +u16 channel_count) +{ +struct osf_iio_state *state =iio_priv (indio_dev); +s64 timestamp; + +if (channel_count != state->spec->channel_count) +return -EPROTO ; + +guard (mutex)(&state->buffer_lock);
Using an extra lock is one solution I suppose to the risk of this racing with the buffer tear down and the problems around active_scan_mask being freed. Could you use
if (iio_device_try_claim_buffer_mode(indio_dev))
and later a release on that to achieve the same result?
Only the ade9000 is doing similar at the moment so would need some testing to make sure no lock inversions occur in general but given you don't take any other locks in here it should be fine.
drivers/iio/opensensorfusion/osf_iio.c line 317
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+if (!state->buffer_active || !iio_buffer_enabled (indio_dev)) +return 0 ; + + timestamp =iio_get_time_ns (indio_dev); + +switch (channel_count) { +case 1 : { +struct osf_iio_scan_1axis scan = { + .value = values[0 ], + };
As suggested above, this structure type is only used locally here. So make the definition local as well.
struct {
s32 value;
aligned_s64 timestamp;
} scan = { };drivers/iio/opensensorfusion/osf_iio.c line 324
Quoted 7 lines
drivers/iio/opensensorfusion/osf_iio.c new file +7
-0+ +return iio_push_to_buffers_with_ts (indio_dev, &scan, +sizeof (scan), timestamp); + } +case 3 : { +struct osf_iio_scan_3axis scan = { + .values = { },
Setting values doesn't add anything useful as = { } zeros the whole structure (including any holes - they kernel is built with specific compiler options to ensure that last bit as it wasn't required in older c specs).
drivers/iio/opensensorfusion/osf_iio.c
Quoted 2 lines
drivers/iio/opensensorfusion/osf_iio.c new file +2
-0+ }; +unsigned int channel, index =0 ;
Please split declarations that assign and ones that don't on to separate lines. Ends up a little more readable.
unsigned int index = 0; unsigned int channel;
Quoted 18 lines
drivers/iio/opensensorfusion/osf_iio.c new file +12
-0+ +/* Pack the active channels; unused storage remains initialized. */ +for_each_set_bit (channel, indio_dev->active_scan_mask, channel_count) + scan.values[index++] = values[channel]; + +return iio_push_to_buffers_with_ts (indio_dev, &scan, +sizeof (scan), timestamp); + } +default : +return -EPROTO ; + } +}drivers/iio/opensensorfusion/osf_serdev.c +0
-0
drivers/iio/opensensorfusion/osf_serdev.c struct osf_serdev { · line 27
Quoted 6 lines
drivers/iio/opensensorfusion/osf_serdev.c +1
-1@@ -23,7 +23,7 @@ struct osf_serdev {bool vcc_enabled;struct osf_device osf;struct osf_stream stream; -/* Protects the parser and all RX counters. */ +/* Protects the parser, all RX counters, and the registration gate. */ struct mutex rx_lock;
Just name it 'lock' given it has scope beyond rx. Do that in earlier patch though rather than changing it here.
drivers/iio/opensensorfusion/osf_serdev.c static struct serdev_device_driver osf_serdev_driver = { · line 147
Quoted 9 lines
drivers/iio/opensensorfusion/osf_serdev.c +1
-1};@@ -139,5 +144,5 @@ static struct serdev_device_driver osf_serdev_driver = { };module_serdev_device_driver (osf_serdev_driver); -MODULE_DESCRIPTION ("Open Sensor Fusion UART receiver" ); +MODULE_DESCRIPTION ("Open Sensor Fusion IIO driver" );
Just have that being slightly missleading in earlier patch to reduce churn here.
Quoted 1 line
drivers/iio/opensensorfusion/osf_serdev.c +0
-0MODULE_LICENSE ("GPL" );
Re: [PATCH v10 6/8] iio: osf: add IIO devices from capability reports
Quoted 11 lines
Using an extra lock is one solution I suppose to the risk of this racing with the buffer tear down and the problems around
active_scan_maskbeing freed. Could you useif (
iio_device_try_claim_buffer_mode(indio_dev))and later a release on that to achieve the same result?
Only the ade9000 is doing similar at the moment so would need some testing to make sure no lock inversions occur in general but given you don't take any other locks in here it should be fine.
I think that should be a better fit here. I'll test replacing the private buffer lock and active flag with iio_device_try_claim_buffer_mode() , iio_device_release_buffer_mode(), including the existing buffer disable/unregister race coverage and lockdep checks.
If that confirms the expected serialization against buffer reconfiguration, I'll use that approach in the next revision.
Thanks,
Jinseob
2026년 9월 20일 (일) 오전 11:03, Jonathan Cameron <[email protected]>님이 작성:
Quoted 313 lines
Quoted 5 lines · On Sat, 19 Sep 2026 03:24:44 +0900 Jinseob Kim <[email protected]> wrote:
Register IIO devices from supported capability descriptors and expose signed raw samples, descriptor scales and software buffers. Keep early capabilities owned by the core until UART and supply setup is complete, and unregister children after the receive producer has stopped.
Quoted 8 lines
Validate nonzero descriptor scales and sample/descriptor equality, leave discovery open after empty or unsupported inventories, and compare repeated descriptors independently of order. Fault a bound session if a descriptor changes meaning, gating cache and buffer publication together.
Add initialized active-scan packing and buffer producer quiescence. The production implementation is now complete; subsequent patches only add the existing core and IIO KUnit suites and their build wiring.
No need to talk about what other patches do and that a series is functionally complete in a patch. If you want to do it, under the --- so it doesn't end up in the git log.
Quoted 3 lines
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>A few things inline. Biggest one is a question (I'm not sure) on whether you can use a claim on buffered mode (which will hold it in that if it succeeds until released) is sufficient to replace the extra lock you have to block that transition.
Thanks,
Jonathan
Quoted 5 lines
drivers/iio/opensensorfusion/osf_iio.c new file +0
-0...
drivers/iio/opensensorfusion/osf_iio.c line 39
Quoted 4 lines
drivers/iio/opensensorfusion/osf_iio.c new file +4
-0+ +struct osf_iio_scan_3axis { +s32 values[3 ]; +u32 padding;No needed unless you explicitly write into padding for some reason. C rules add it anyway and then we don't need to make sure it remains correct if this structure changes in future.
drivers/iio/opensensorfusion/osf_iio.c line 45
Quoted 6 lines
drivers/iio/opensensorfusion/osf_iio.c new file +6
-0+aligned_s64 timestamp; +}; + +struct osf_iio_scan_1axis { +s32 value; +u32 padding;As above, unneeded.
drivers/iio/opensensorfusion/osf_iio.c
Quoted 2 lines
drivers/iio/opensensorfusion/osf_iio.c new file +2
-0+aligned_s64 timestamp; +};Given I think these are only used in one place, moving them to there would avoid need for named structure types.
Quoted 8 lines
drivers/iio/opensensorfusion/osf_iio.c new file +8
-0+ +#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), \drivers/iio/opensensorfusion/osf_iio.c line 65
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+ .scan_index = (_idx), \ + .scan_type = { \ + .sign = 's', \ + .realbits = 32, \ + .storagebits = 32, \ + .endianness = IIO_CPU, \ + }, \ + } + +#define OSF_CHAN (_type, _idx) \Given this is not much used, you could use the
IIO_NO_MODmodifier as a gate on setting .modified = (_mod!=IIO_NO_MOD) ? 1 : 0,
and have just one macro.Quoted 78 lines
drivers/iio/opensensorfusion/osf_iio.c new file +78
-0+ { \ + .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 = { \ + .sign = 's', \ + .realbits = 32, \ + .storagebits = 32, \ + .endianness = IIO_CPU, \ + }, \ + } + +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 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), + }, + { + .sensor_type =OSF_SENSOR_GYROSCOPE , + .channel_count =3 , + .name ="osf-gyro" , + .channels = osf_gyro_channels, + .num_channels =ARRAY_SIZE (osf_gyro_channels), + }, + { + .sensor_type =OSF_SENSOR_MAGNETOMETER , + .channel_count =3 , + .name ="osf-magn" , + .channels = osf_mag_channels, + .num_channels =ARRAY_SIZE (osf_mag_channels), + }, + { + .sensor_type =OSF_SENSOR_TEMPERATURE , + .channel_count =1 , + .name ="osf-temp" , + .channels = osf_temp_channels, + .num_channels =ARRAY_SIZE (osf_temp_channels), + }, +}; + +static const struct osf_iio_sensor_spec * +osf_iio_find_sensor_spec (u16 sensor_type,u16 channel_count) +{ +for (unsigned int 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]; + }drivers/iio/opensensorfusion/osf_iio.c
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+ +return NULL ; +} + +bool osf_iio_sensor_supported (u16 sensor_type,u16 channel_count) +{ +if (osf_iio_find_sensor_spec (sensor_type, channel_count)) +return true; + +return falsereturn osf_iio_find_sensor_spec(sensor_type, channel_count) != NULL;drivers/iio/opensensorfusion/osf_iio.c line 202
Quoted 8 lines
drivers/iio/opensensorfusion/osf_iio.c new file +7
-0+} + +static int osf_iio_buffer_postenable (struct iio_dev *indio_dev) +{ +struct osf_iio_state *state =iio_priv (indio_dev); + +guard (mutex)(&state->buffer_lock);These transistions are already guarded by the core, so see the question later on whether you can just claim buffered mode in the data handler to avoid the need for this. That might not work but the way you did this got me thinking about a more general solution to the problem.
Quoted 18 lines
drivers/iio/opensensorfusion/osf_iio.c new file +16
-0+ state->buffer_active = true; + +return 0 ; +} + +static int osf_iio_buffer_predisable (struct iio_dev *indio_dev) +{ +struct osf_iio_state *state =iio_priv (indio_dev); + +/* Wait for the current push before the IIO core changes its buffers. */ +guard (mutex)(&state->buffer_lock); + state->buffer_active = false; + +return 0 ; +} +drivers/iio/opensensorfusion/osf_iio.c line 307
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+int osf_iio_push_sample (struct iio_dev *indio_dev,const s32 *values, +u16 channel_count) +{ +struct osf_iio_state *state =iio_priv (indio_dev); +s64 timestamp; + +if (channel_count != state->spec->channel_count) +return -EPROTO ; + +guard (mutex)(&state->buffer_lock);Using an extra lock is one solution I suppose to the risk of this racing with the buffer tear down and the problems around
active_scan_maskbeing freed. Could you useif (
iio_device_try_claim_buffer_mode(indio_dev))
and later a release on that to achieve the same result?Only the ade9000 is doing similar at the moment so would need some testing to make sure no lock inversions occur in general but given you don't take any other locks in here it should be fine.
drivers/iio/opensensorfusion/osf_iio.c line 317
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+if (!state->buffer_active || !iio_buffer_enabled (indio_dev)) +return 0 ; + + timestamp =iio_get_time_ns (indio_dev); + +switch (channel_count) { +case 1 : { +struct osf_iio_scan_1axis scan = { + .value = values[0 ], + };As suggested above, this structure type is only used locally here. So make the definition local as well.
struct { s32 value; aligned_s64 timestamp; } scan = { };drivers/iio/opensensorfusion/osf_iio.c line 324
Quoted 7 lines
drivers/iio/opensensorfusion/osf_iio.c new file +7
-0+ +return iio_push_to_buffers_with_ts (indio_dev, &scan, +sizeof (scan), timestamp); + } +case 3 : { +struct osf_iio_scan_3axis scan = { + .values = { },Setting values doesn't add anything useful as = { } zeros the whole structure (including any holes - they kernel is built with specific compiler options to ensure that last bit as it wasn't required in older c specs).
drivers/iio/opensensorfusion/osf_iio.c
Quoted 2 lines
drivers/iio/opensensorfusion/osf_iio.c new file +2
-0+ }; +unsigned int channel, index =0 ;Please split declarations that assign and ones that don't on to separate lines. Ends up a little more readable.
unsigned int index = 0; unsigned int channel;Quoted 18 lines
drivers/iio/opensensorfusion/osf_iio.c new file +12
-0+ +/* Pack the active channels; unused storage remains initialized. */ +for_each_set_bit (channel, indio_dev->active_scan_mask, channel_count) + scan.values[index++] = values[channel]; + +return iio_push_to_buffers_with_ts (indio_dev, &scan, +sizeof (scan), timestamp); + } +default : +return -EPROTO ; + } +}drivers/iio/opensensorfusion/osf_serdev.c +0
-0drivers/iio/opensensorfusion/osf_serdev.c struct osf_serdev { · line 27
Quoted 6 lines
drivers/iio/opensensorfusion/osf_serdev.c +1
-1@@ -23,7 +23,7 @@ struct osf_serdev {bool vcc_enabled;struct osf_device osf;struct osf_stream stream; -/* Protects the parser and all RX counters. */ +/* Protects the parser, all RX counters, and the registration gate. */ struct mutex rx_lock;Just name it 'lock' given it has scope beyond rx. Do that in earlier patch though rather than changing it here.
drivers/iio/opensensorfusion/osf_serdev.c static struct serdev_device_driver osf_serdev_driver = { · line 147
Quoted 9 lines
drivers/iio/opensensorfusion/osf_serdev.c +1
-1};@@ -139,5 +144,5 @@ static struct serdev_device_driver osf_serdev_driver = { };module_serdev_device_driver (osf_serdev_driver); -MODULE_DESCRIPTION ("Open Sensor Fusion UART receiver" ); +MODULE_DESCRIPTION ("Open Sensor Fusion IIO driver" );Just have that being slightly missleading in earlier patch to reduce churn here.
Quoted 1 line
drivers/iio/opensensorfusion/osf_serdev.c +0
-0MODULE_LICENSE ("GPL" );
Re: [PATCH v10 6/8] iio: osf: add IIO devices from capability reports
Quoted 7 lines
Using an extra lock is one solution I suppose to the risk of this racing with the buffer tear down and the problems around
active_scan_maskbeing freed. Could you useif (
iio_device_try_claim_buffer_mode(indio_dev))and later a release on that to achieve the same result?
I looked into this against the current IIO core and tested the relevant paths.
iio_device_try_claim_buffer_mode() does hold the IIO mode lock, and iio_update_buffers() takes the same lock, so it would serialize normal buffer reconfiguration.
However, the unregister path does not hold that mode lock while disabling/deactivating the buffers, and that path can free active_scan_mask. Because the OSF receive path can still have an admitted buffer push at that point, the mode claim alone does not appear to provide the same lifetime guarantee as the existing private lock.
I therefore kept the private lock for now. I also reran the disable/unregister race coverage with KASAN and lockdep enabled; those tests pass without diagnostics.
Thanks,
Jinseob
2026년 9월 20일 (일) 오후 1:20, Kim Jinseob <[email protected]>님이 작성:
Quoted 340 lines
Quoted 5 lines
Using an extra lock is one solution I suppose to the risk of this racing with the buffer tear down and the problems around
active_scan_maskbeing freed. Could you useQuoted 7 lines
if (
iio_device_try_claim_buffer_mode(indio_dev))and later a release on that to achieve the same result?
Only the ade9000 is doing similar at the moment so would need some testing to make sure no lock inversions occur in general but given you don't take any other locks in here it should be fine.
I think that should be a better fit here. I'll test replacing the private buffer lock and active flag with
iio_device_try_claim_buffer_mode(),iio_device_release_buffer_mode(), including the existing buffer disable/unregister race coverage and lockdep checks.If that confirms the expected serialization against buffer reconfiguration, I'll use that approach in the next revision.
Thanks,
Jinseob
2026년 9월 20일 (일) 오전 11:03, Jonathan Cameron <[email protected]>님이 작성:
Quoted 313 lines
Quoted 5 lines · On Sat, 19 Sep 2026 03:24:44 +0900 Jinseob Kim <[email protected]> wrote:
Register IIO devices from supported capability descriptors and expose signed raw samples, descriptor scales and software buffers. Keep early capabilities owned by the core until UART and supply setup is complete, and unregister children after the receive producer has stopped.
Quoted 8 lines
Validate nonzero descriptor scales and sample/descriptor equality, leave discovery open after empty or unsupported inventories, and compare repeated descriptors independently of order. Fault a bound session if a descriptor changes meaning, gating cache and buffer publication together.
Add initialized active-scan packing and buffer producer quiescence. The production implementation is now complete; subsequent patches only add the existing core and IIO KUnit suites and their build wiring.
No need to talk about what other patches do and that a series is functionally complete in a patch. If you want to do it, under the --- so it doesn't end up in the git log.
Quoted 3 lines
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>A few things inline. Biggest one is a question (I'm not sure) on whether you can use a claim on buffered mode (which will hold it in that if it succeeds until released) is sufficient to replace the extra lock you have to block that transition.
Thanks,
Jonathan
Quoted 5 lines
drivers/iio/opensensorfusion/osf_iio.c new file +0
-0...
drivers/iio/opensensorfusion/osf_iio.c line 39
Quoted 4 lines
drivers/iio/opensensorfusion/osf_iio.c new file +4
-0+ +struct osf_iio_scan_3axis { +s32 values[3 ]; +u32 padding;No needed unless you explicitly write into padding for some reason. C rules add it anyway and then we don't need to make sure it remains correct if this structure changes in future.
drivers/iio/opensensorfusion/osf_iio.c line 45
Quoted 6 lines
drivers/iio/opensensorfusion/osf_iio.c new file +6
-0+aligned_s64 timestamp; +}; + +struct osf_iio_scan_1axis { +s32 value; +u32 padding;As above, unneeded.
drivers/iio/opensensorfusion/osf_iio.c
Quoted 2 lines
drivers/iio/opensensorfusion/osf_iio.c new file +2
-0+aligned_s64 timestamp; +};Given I think these are only used in one place, moving them to there would avoid need for named structure types.
Quoted 8 lines
drivers/iio/opensensorfusion/osf_iio.c new file +8
-0+ +#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), \drivers/iio/opensensorfusion/osf_iio.c line 65
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+ .scan_index = (_idx), \ + .scan_type = { \ + .sign = 's', \ + .realbits = 32, \ + .storagebits = 32, \ + .endianness = IIO_CPU, \ + }, \ + } + +#define OSF_CHAN (_type, _idx) \Given this is not much used, you could use the
IIO_NO_MODmodifier as a gate on setting .modified = (_mod!=IIO_NO_MOD) ? 1 : 0,
and have just one macro.Quoted 78 lines
drivers/iio/opensensorfusion/osf_iio.c new file +78
-0+ { \ + .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 = { \ + .sign = 's', \ + .realbits = 32, \ + .storagebits = 32, \ + .endianness = IIO_CPU, \ + }, \ + } + +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 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), + }, + { + .sensor_type =OSF_SENSOR_GYROSCOPE , + .channel_count =3 , + .name ="osf-gyro" , + .channels = osf_gyro_channels, + .num_channels =ARRAY_SIZE (osf_gyro_channels), + }, + { + .sensor_type =OSF_SENSOR_MAGNETOMETER , + .channel_count =3 , + .name ="osf-magn" , + .channels = osf_mag_channels, + .num_channels =ARRAY_SIZE (osf_mag_channels), + }, + { + .sensor_type =OSF_SENSOR_TEMPERATURE , + .channel_count =1 , + .name ="osf-temp" , + .channels = osf_temp_channels, + .num_channels =ARRAY_SIZE (osf_temp_channels), + }, +}; + +static const struct osf_iio_sensor_spec * +osf_iio_find_sensor_spec (u16 sensor_type,u16 channel_count) +{ +for (unsigned int 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]; + }drivers/iio/opensensorfusion/osf_iio.c
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+ +return NULL ; +} + +bool osf_iio_sensor_supported (u16 sensor_type,u16 channel_count) +{ +if (osf_iio_find_sensor_spec (sensor_type, channel_count)) +return true; + +return falsereturn osf_iio_find_sensor_spec(sensor_type, channel_count) != NULL;drivers/iio/opensensorfusion/osf_iio.c line 202
Quoted 8 lines
drivers/iio/opensensorfusion/osf_iio.c new file +7
-0+} + +static int osf_iio_buffer_postenable (struct iio_dev *indio_dev) +{ +struct osf_iio_state *state =iio_priv (indio_dev); + +guard (mutex)(&state->buffer_lock);These transistions are already guarded by the core, so see the question later on whether you can just claim buffered mode in the data handler to avoid the need for this. That might not work but the way you did this got me thinking about a more general solution to the problem.
Quoted 18 lines
drivers/iio/opensensorfusion/osf_iio.c new file +16
-0+ state->buffer_active = true; + +return 0 ; +} + +static int osf_iio_buffer_predisable (struct iio_dev *indio_dev) +{ +struct osf_iio_state *state =iio_priv (indio_dev); + +/* Wait for the current push before the IIO core changes its buffers. */ +guard (mutex)(&state->buffer_lock); + state->buffer_active = false; + +return 0 ; +} +drivers/iio/opensensorfusion/osf_iio.c line 307
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+int osf_iio_push_sample (struct iio_dev *indio_dev,const s32 *values, +u16 channel_count) +{ +struct osf_iio_state *state =iio_priv (indio_dev); +s64 timestamp; + +if (channel_count != state->spec->channel_count) +return -EPROTO ; + +guard (mutex)(&state->buffer_lock);Using an extra lock is one solution I suppose to the risk of this racing with the buffer tear down and the problems around
active_scan_maskbeing freed. Could you useif (
iio_device_try_claim_buffer_mode(indio_dev))
and later a release on that to achieve the same result?Only the ade9000 is doing similar at the moment so would need some testing to make sure no lock inversions occur in general but given you don't take any other locks in here it should be fine.
drivers/iio/opensensorfusion/osf_iio.c line 317
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+if (!state->buffer_active || !iio_buffer_enabled (indio_dev)) +return 0 ; + + timestamp =iio_get_time_ns (indio_dev); + +switch (channel_count) { +case 1 : { +struct osf_iio_scan_1axis scan = { + .value = values[0 ], + };As suggested above, this structure type is only used locally here. So make the definition local as well.
struct { s32 value; aligned_s64 timestamp; } scan = { };drivers/iio/opensensorfusion/osf_iio.c line 324
Quoted 7 lines
drivers/iio/opensensorfusion/osf_iio.c new file +7
-0+ +return iio_push_to_buffers_with_ts (indio_dev, &scan, +sizeof (scan), timestamp); + } +case 3 : { +struct osf_iio_scan_3axis scan = { + .values = { },Setting values doesn't add anything useful as = { } zeros the whole structure (including any holes - they kernel is built with specific compiler options to ensure that last bit as it wasn't required in older c specs).
drivers/iio/opensensorfusion/osf_iio.c
Quoted 2 lines
drivers/iio/opensensorfusion/osf_iio.c new file +2
-0+ }; +unsigned int channel, index =0 ;Please split declarations that assign and ones that don't on to separate lines. Ends up a little more readable.
unsigned int index = 0; unsigned int channel;Quoted 18 lines
drivers/iio/opensensorfusion/osf_iio.c new file +12
-0+ +/* Pack the active channels; unused storage remains initialized. */ +for_each_set_bit (channel, indio_dev->active_scan_mask, channel_count) + scan.values[index++] = values[channel]; + +return iio_push_to_buffers_with_ts (indio_dev, &scan, +sizeof (scan), timestamp); + } +default : +return -EPROTO ; + } +}drivers/iio/opensensorfusion/osf_serdev.c +0
-0drivers/iio/opensensorfusion/osf_serdev.c struct osf_serdev { · line 27
Quoted 6 lines
drivers/iio/opensensorfusion/osf_serdev.c +1
-1@@ -23,7 +23,7 @@ struct osf_serdev {bool vcc_enabled;struct osf_device osf;struct osf_stream stream; -/* Protects the parser and all RX counters. */ +/* Protects the parser, all RX counters, and the registration gate. */ struct mutex rx_lock;Just name it 'lock' given it has scope beyond rx. Do that in earlier patch though rather than changing it here.
drivers/iio/opensensorfusion/osf_serdev.c static struct serdev_device_driver osf_serdev_driver = { · line 147
Quoted 9 lines
drivers/iio/opensensorfusion/osf_serdev.c +1
-1};@@ -139,5 +144,5 @@ static struct serdev_device_driver osf_serdev_driver = { };module_serdev_device_driver (osf_serdev_driver); -MODULE_DESCRIPTION ("Open Sensor Fusion UART receiver" ); +MODULE_DESCRIPTION ("Open Sensor Fusion IIO driver" );Just have that being slightly missleading in earlier patch to reduce churn here.
Quoted 1 line
drivers/iio/opensensorfusion/osf_serdev.c +0
-0MODULE_LICENSE ("GPL" );
Jonathan Cameron
maintainerRe: [PATCH v10 6/8] iio: osf: add IIO devices from capability reports
Quoted 10 lines · On Sun, 20 Sep 2026 14:07:34 +0900 Kim Jinseob <[email protected]> wrote:
Quoted 7 lines
Using an extra lock is one solution I suppose to the risk of this racing with the buffer tear down and the problems around
active_scan_maskbeing freed. Could you useif (
iio_device_try_claim_buffer_mode(indio_dev))and later a release on that to achieve the same result?
I looked into this against the current IIO core and tested the relevant paths.
Quoted 7 lines
iio_device_try_claim_buffer_mode()does hold the IIO mode lock, andiio_update_buffers()takes the same lock, so it would serialize normal buffer reconfiguration.However, the unregister path does not hold that mode lock while disabling/deactivating the buffers, and that path can free
active_scan_mask.
Ah. That seems like a hole we should close. This I think is the call to iio_disable_all_buffers() in iio_device_unregister()? The only potential problem there would be lock inversion wrt to the info_exist_lock. That shouldn't be a problem as iio_update_buffers() takes them in the same order as we would be doing here.
So at least a first look, taking mlock inside iio_disable_all_buffers() would seem like an easy change to make.
It would be a lot more logical if that gap had never existed! Thanks for the analysis - I'd be surprised if we don't have other latent bugs around that protection not being there.
Assuming there isn't something I'm missing would you mind sending that as a separate patch that we can ask folk to test ahead of the main series.
Thanks,
Jonathan
Quoted 354 lines
Because the OSF receive path can still have an
admitted buffer push at that point, the mode claim alone does not appear to provide the same lifetime guarantee as the existing private lock.I therefore kept the private lock for now. I also reran the disable/unregister race coverage with KASAN and lockdep enabled; those tests pass without diagnostics.
Thanks,
Jinseob
2026년 9월 20일 (일) 오후 1:20, Kim Jinseob <[email protected]>님이 작성:
Quoted 340 lines
Quoted 5 lines
Using an extra lock is one solution I suppose to the risk of this racing with the buffer tear down and the problems around
active_scan_maskbeing freed. Could you useQuoted 7 lines
if (
iio_device_try_claim_buffer_mode(indio_dev))and later a release on that to achieve the same result?
Only the ade9000 is doing similar at the moment so would need some testing to make sure no lock inversions occur in general but given you don't take any other locks in here it should be fine.
I think that should be a better fit here. I'll test replacing the private buffer lock and active flag with
iio_device_try_claim_buffer_mode(),iio_device_release_buffer_mode(), including the existing buffer disable/unregister race coverage and lockdep checks.If that confirms the expected serialization against buffer reconfiguration, I'll use that approach in the next revision.
Thanks,
Jinseob
2026년 9월 20일 (일) 오전 11:03, Jonathan Cameron <[email protected]>님이 작성:
Quoted 313 lines
Quoted 5 lines · On Sat, 19 Sep 2026 03:24:44 +0900 Jinseob Kim <[email protected]> wrote:
Register IIO devices from supported capability descriptors and expose signed raw samples, descriptor scales and software buffers. Keep early capabilities owned by the core until UART and supply setup is complete, and unregister children after the receive producer has stopped.
Quoted 8 lines
Validate nonzero descriptor scales and sample/descriptor equality, leave discovery open after empty or unsupported inventories, and compare repeated descriptors independently of order. Fault a bound session if a descriptor changes meaning, gating cache and buffer publication together.
Add initialized active-scan packing and buffer producer quiescence. The production implementation is now complete; subsequent patches only add the existing core and IIO KUnit suites and their build wiring.
No need to talk about what other patches do and that a series is functionally complete in a patch. If you want to do it, under the --- so it doesn't end up in the git log.
Quoted 3 lines
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>A few things inline. Biggest one is a question (I'm not sure) on whether you can use a claim on buffered mode (which will hold it in that if it succeeds until released) is sufficient to replace the extra lock you have to block that transition.
Thanks,
Jonathan
Quoted 5 lines
drivers/iio/opensensorfusion/osf_iio.c new file +0
-0...
drivers/iio/opensensorfusion/osf_iio.c line 39
Quoted 4 lines
drivers/iio/opensensorfusion/osf_iio.c new file +4
-0+ +struct osf_iio_scan_3axis { +s32 values[3 ]; +u32 padding;No needed unless you explicitly write into padding for some reason. C rules add it anyway and then we don't need to make sure it remains correct if this structure changes in future.
drivers/iio/opensensorfusion/osf_iio.c line 45
Quoted 6 lines
drivers/iio/opensensorfusion/osf_iio.c new file +6
-0+aligned_s64 timestamp; +}; + +struct osf_iio_scan_1axis { +s32 value; +u32 padding;As above, unneeded.
drivers/iio/opensensorfusion/osf_iio.c
Quoted 2 lines
drivers/iio/opensensorfusion/osf_iio.c new file +2
-0+aligned_s64 timestamp; +};Given I think these are only used in one place, moving them to there would avoid need for named structure types.
Quoted 8 lines
drivers/iio/opensensorfusion/osf_iio.c new file +8
-0+ +#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), \drivers/iio/opensensorfusion/osf_iio.c line 65
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+ .scan_index = (_idx), \ + .scan_type = { \ + .sign = 's', \ + .realbits = 32, \ + .storagebits = 32, \ + .endianness = IIO_CPU, \ + }, \ + } + +#define OSF_CHAN (_type, _idx) \Given this is not much used, you could use the
IIO_NO_MODmodifier as a gate on setting .modified = (_mod!=IIO_NO_MOD) ? 1 : 0,
and have just one macro.Quoted 78 lines
drivers/iio/opensensorfusion/osf_iio.c new file +78
-0+ { \ + .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 = { \ + .sign = 's', \ + .realbits = 32, \ + .storagebits = 32, \ + .endianness = IIO_CPU, \ + }, \ + } + +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 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), + }, + { + .sensor_type =OSF_SENSOR_GYROSCOPE , + .channel_count =3 , + .name ="osf-gyro" , + .channels = osf_gyro_channels, + .num_channels =ARRAY_SIZE (osf_gyro_channels), + }, + { + .sensor_type =OSF_SENSOR_MAGNETOMETER , + .channel_count =3 , + .name ="osf-magn" , + .channels = osf_mag_channels, + .num_channels =ARRAY_SIZE (osf_mag_channels), + }, + { + .sensor_type =OSF_SENSOR_TEMPERATURE , + .channel_count =1 , + .name ="osf-temp" , + .channels = osf_temp_channels, + .num_channels =ARRAY_SIZE (osf_temp_channels), + }, +}; + +static const struct osf_iio_sensor_spec * +osf_iio_find_sensor_spec (u16 sensor_type,u16 channel_count) +{ +for (unsigned int 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]; + }drivers/iio/opensensorfusion/osf_iio.c
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+ +return NULL ; +} + +bool osf_iio_sensor_supported (u16 sensor_type,u16 channel_count) +{ +if (osf_iio_find_sensor_spec (sensor_type, channel_count)) +return true; + +return falsereturn osf_iio_find_sensor_spec(sensor_type, channel_count) != NULL;drivers/iio/opensensorfusion/osf_iio.c line 202
Quoted 8 lines
drivers/iio/opensensorfusion/osf_iio.c new file +7
-0+} + +static int osf_iio_buffer_postenable (struct iio_dev *indio_dev) +{ +struct osf_iio_state *state =iio_priv (indio_dev); + +guard (mutex)(&state->buffer_lock);These transistions are already guarded by the core, so see the question later on whether you can just claim buffered mode in the data handler to avoid the need for this. That might not work but the way you did this got me thinking about a more general solution to the problem.
Quoted 18 lines
drivers/iio/opensensorfusion/osf_iio.c new file +16
-0+ state->buffer_active = true; + +return 0 ; +} + +static int osf_iio_buffer_predisable (struct iio_dev *indio_dev) +{ +struct osf_iio_state *state =iio_priv (indio_dev); + +/* Wait for the current push before the IIO core changes its buffers. */ +guard (mutex)(&state->buffer_lock); + state->buffer_active = false; + +return 0 ; +} +drivers/iio/opensensorfusion/osf_iio.c line 307
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+int osf_iio_push_sample (struct iio_dev *indio_dev,const s32 *values, +u16 channel_count) +{ +struct osf_iio_state *state =iio_priv (indio_dev); +s64 timestamp; + +if (channel_count != state->spec->channel_count) +return -EPROTO ; + +guard (mutex)(&state->buffer_lock);Using an extra lock is one solution I suppose to the risk of this racing with the buffer tear down and the problems around
active_scan_maskbeing freed. Could you useif (
iio_device_try_claim_buffer_mode(indio_dev))
and later a release on that to achieve the same result?Only the ade9000 is doing similar at the moment so would need some testing to make sure no lock inversions occur in general but given you don't take any other locks in here it should be fine.
drivers/iio/opensensorfusion/osf_iio.c line 317
Quoted 10 lines
drivers/iio/opensensorfusion/osf_iio.c new file +10
-0+if (!state->buffer_active || !iio_buffer_enabled (indio_dev)) +return 0 ; + + timestamp =iio_get_time_ns (indio_dev); + +switch (channel_count) { +case 1 : { +struct osf_iio_scan_1axis scan = { + .value = values[0 ], + };As suggested above, this structure type is only used locally here. So make the definition local as well.
struct { s32 value; aligned_s64 timestamp; } scan = { };drivers/iio/opensensorfusion/osf_iio.c line 324
Quoted 7 lines
drivers/iio/opensensorfusion/osf_iio.c new file +7
-0+ +return iio_push_to_buffers_with_ts (indio_dev, &scan, +sizeof (scan), timestamp); + } +case 3 : { +struct osf_iio_scan_3axis scan = { + .values = { },Setting values doesn't add anything useful as = { } zeros the whole structure (including any holes - they kernel is built with specific compiler options to ensure that last bit as it wasn't required in older c specs).
drivers/iio/opensensorfusion/osf_iio.c
Quoted 2 lines
drivers/iio/opensensorfusion/osf_iio.c new file +2
-0+ }; +unsigned int channel, index =0 ;Please split declarations that assign and ones that don't on to separate lines. Ends up a little more readable.
unsigned int index = 0; unsigned int channel;Quoted 18 lines
drivers/iio/opensensorfusion/osf_iio.c new file +12
-0+ +/* Pack the active channels; unused storage remains initialized. */ +for_each_set_bit (channel, indio_dev->active_scan_mask, channel_count) + scan.values[index++] = values[channel]; + +return iio_push_to_buffers_with_ts (indio_dev, &scan, +sizeof (scan), timestamp); + } +default : +return -EPROTO ; + } +}drivers/iio/opensensorfusion/osf_serdev.c +0
-0drivers/iio/opensensorfusion/osf_serdev.c struct osf_serdev { · line 27
Quoted 6 lines
drivers/iio/opensensorfusion/osf_serdev.c +1
-1@@ -23,7 +23,7 @@ struct osf_serdev {bool vcc_enabled;struct osf_device osf;struct osf_stream stream; -/* Protects the parser and all RX counters. */ +/* Protects the parser, all RX counters, and the registration gate. */ struct mutex rx_lock;Just name it 'lock' given it has scope beyond rx. Do that in earlier patch though rather than changing it here.
drivers/iio/opensensorfusion/osf_serdev.c static struct serdev_device_driver osf_serdev_driver = { · line 147
Quoted 9 lines
drivers/iio/opensensorfusion/osf_serdev.c +1
-1};@@ -139,5 +144,5 @@ static struct serdev_device_driver osf_serdev_driver = { };module_serdev_device_driver (osf_serdev_driver); -MODULE_DESCRIPTION ("Open Sensor Fusion UART receiver" ); +MODULE_DESCRIPTION ("Open Sensor Fusion IIO driver" );Just have that being slightly missleading in earlier patch to reduce churn here.
Quoted 1 line
drivers/iio/opensensorfusion/osf_serdev.c +0
-0MODULE_LICENSE ("GPL" );
PATCH v10 7/8 iio: osf: add core KUnit tests
In reply to Jinseob KimAdd the existing core regression cases for frame handling, sample-cache admission, supported capability discovery, repeated descriptors and session faults. The tests exercise valid, ignored and rejected input without changing the production receive or IIO implementation.
Introduce KUnit configuration and build wiring when the core test source is present. IIO-specific tests are added separately in the next patch.
--- drivers/iio/opensensorfusion/Kconfig | 12 + drivers/iio/opensensorfusion/Makefile | 1 + drivers/iio/opensensorfusion/osf_core_test.c | 1046 ++++++++++++++++++ 3 files changed, 1059 insertions(+) create mode 100644 drivers/iio/opensensorfusion/osf_core_test.c
drivers/iio/opensensorfusion/Kconfig +12 -0
@@ -13,3 +13,15 @@ config OPEN_SENSOR_FUSION registers IIO devices for supported capability entries. It exposes accelerometer, gyroscope, magnetometer, and temperature samples through IIO direct reads and software buffers. + +config OPEN_SENSOR_FUSION_KUNIT_TEST + bool "KUnit tests for Open Sensor Fusion" if !KUNIT_ALL_TESTS + depends on OPEN_SENSOR_FUSION=y && KUNIT=y + default KUNIT_ALL_TESTS + help + Build focused unit tests for the Open Sensor Fusion core sample + acceptance, direct-read cache, and capability/session handling. The tests + exercise accepted, ignored, and rejected sample handling without + exposing additional production interfaces. + + If unsure, say N.
drivers/iio/opensensorfusion/Makefile +1 -0
@@ -4,3 +4,4 @@ obj-$(CONFIG_OPEN_SENSOR_FUSION) += open-sensor-fusion.o open-sensor-fusion-y := osf_core.o osf_iio.o osf_protocol.o osf_serdev.o \ osf_stream.o +open-sensor-fusion-$(CONFIG_OPEN_SENSOR_FUSION_KUNIT_TEST) += osf_core_test.o
drivers/iio/opensensorfusion/osf_core_test.c new file +1046 -0
1,046 lines not on this page · raw message
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v10 7/8] iio: osf: add core KUnit tests
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] kunit: UAF in osf_session_producer due to missing kthread lifecycle action - [Medium] kunit…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] kunit: UAF in osf_session_producer due to missing kthread lifecycle action
- [Medium] kunit: mutex re-initialization in osf_test_new_session
--
commit b82aa72688edb8c728c31226b6fec70af9389122
Author: Jinseob Kim <[email protected]>
iio: osf: add core KUnit tests
This commit adds core regression test cases for frame handling, sample-cache admission, and other operations for the Open Sensor Fusion IIO device. It introduces KUnit configuration and build wiring to exercise these paths.
Quoted 5 lines
drivers/iio/opensensorfusion/osf_core_test.c new file +0
-0
[ ... ]
drivers/iio/opensensorfusion/osf_core_test.c line 614
Quoted 4 lines
drivers/iio/opensensorfusion/osf_core_test.c new file +4
-0+static void osf_test_new_session (struct osf_test_context *ctx) +{ +osf_test_cleanup (ctx); +osf_core_init (&ctx->osf, ctx->dev);
[Severity: Medium]
Does this code trigger a debugobjects warning by re-initializing an active mutex?
osf_core_init() performs the following operations:
drivers/iio/opensensorfusion/osf_core.c:osf_core_init() {
*osf = (struct osf_device) {
.dev = dev,
};
mutex_init(&osf->latest_lock);
}By wiping the osf_device structure and calling mutex_init() again on latest_lock without first calling mutex_destroy(), this can cause debugobjects to emit an init active warning.
Could a cleanup of the old mutex be added, or the re-initialization avoided?
Quoted 4 lines
drivers/iio/opensensorfusion/osf_core_test.c new file +4
-0+osf_core_start (&ctx->osf); + ctx->indio_dev =NULL ; + ctx->buffer =NULL ; +}
[ ... ]
Quoted 3 lines
drivers/iio/opensensorfusion/osf_core_test.c new file +3
-0+static void osf_session_fault_buffer_race_test (struct kunit *test) +{ +struct osf_test_context *ctx = test->priv;
[ ... ]
drivers/iio/opensensorfusion/osf_core_test.c line 986
Quoted 2 lines
drivers/iio/opensensorfusion/osf_core_test.c new file +2
-0+ producer =kthread_run (osf_session_producer, race,"osf-session-test" ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, producer);
[Severity: Medium]
Can this kthread leak and cause a use-after-free if the KUnit test is aborted prematurely?
If the test is aborted due to a KUnit timeout or an unhandled signal, the manual kthread_stop() at the end of the test will be bypassed. The KUnit framework will then free the race structure, but osf_session_producer will continue running in the background.
When the background thread subsequently accesses race->rx_lock or race->ctx, it will access freed memory and potentially crash the kernel.
Could kunit_add_action() be used to guarantee the termination of the kthread during test cleanup?
Quoted 1 line
drivers/iio/opensensorfusion/osf_core_test.c new file +1
-0+KUNIT_EXPECT_NE (test,wait_for_completion_timeout (&race->started,HZ ),0UL );
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
Jonathan Cameron
maintainerRe: [PATCH v10 7/8] iio: osf: add core KUnit tests
In reply to Jinseob KimQuoted 10 lines · On Sat, 19 Sep 2026 03:24:45 +0900 Jinseob Kim <[email protected]> wrote:
Add the existing core regression cases for frame handling, sample-cache admission, supported capability discovery, repeated descriptors and session faults. The tests exercise valid, ignored and rejected input without changing the production receive or IIO implementation.
Introduce KUnit configuration and build wiring when the core test source is present. IIO-specific tests are added separately in the next patch.
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>
https://sashiko.dev/#/patchset/cover.1789753020.git.kimjinseob88%40gmail.com
The lock one looks right and needs to be fixed. Not sure on the others (I didn't really look as running out of time today!)
There is a lot here and that is likely to mean it doesn't all get detailed review. That isn't necessarily a problem though and they are generally good to have. Just something to be aware of.
Jonathan
Quoted 1 line
PATCH v10 8/8 iio: osf: add IIO KUnit tests
In reply to Jinseob KimAdd the existing IIO active-scan packing and buffer producer lifetime tests. Extend the test build wiring and help text to cover both suites.
This changes no production source. The complete series retains the production behavior of the combined-driver revision, with CRC terminology clarified.
--- drivers/iio/opensensorfusion/Kconfig | 2 +- drivers/iio/opensensorfusion/Makefile | 2 +- drivers/iio/opensensorfusion/osf_iio_test.c | 325 ++++++++++++++++++++ 3 files changed, 327 insertions(+), 2 deletions(-) create mode 100644 drivers/iio/opensensorfusion/osf_iio_test.c
drivers/iio/opensensorfusion/Kconfig +1 -1
@@ -20,7 +20,7 @@ config OPEN_SENSOR_FUSION_KUNIT_TEST default KUNIT_ALL_TESTS help Build focused unit tests for the Open Sensor Fusion core sample - acceptance, direct-read cache, and capability/session handling. The tests + acceptance, direct-read cache, and IIO buffer paths. The tests exercise accepted, ignored, and rejected sample handling without exposing additional production interfaces.
drivers/iio/opensensorfusion/Makefile +1 -1
@@ -4,4 +4,4 @@ obj-$(CONFIG_OPEN_SENSOR_FUSION) += open-sensor-fusion.o open-sensor-fusion-y := osf_core.o osf_iio.o osf_protocol.o osf_serdev.o \ osf_stream.o -open-sensor-fusion-$(CONFIG_OPEN_SENSOR_FUSION_KUNIT_TEST) += osf_core_test.o +open-sensor-fusion-$(CONFIG_OPEN_SENSOR_FUSION_KUNIT_TEST) += osf_core_test.o osf_iio_test.o
drivers/iio/opensensorfusion/osf_iio_test.c new file +325 -0
325 lines not on this page · raw message
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v10 8/8] iio: osf: add IIO KUnit tests
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] kthread: UAF in osf_iio_test_producer() on KUnit assertion failure - [Medium] mm: missing memory…
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] kthread: UAF in osf_iio_test_producer() on KUnit assertion failure
- [Medium] mm: missing memory barrier in osf_iio_buffer_lifetime_test() causes test timeout
- [Medium] iio: memory and device leak on KUnit assertion failure
--
commit 8655dd7dbcb0e7d37056c4f127c9412727c35c55
Author: Jinseob Kim <[email protected]>
iio: osf: add IIO KUnit tests
Add the existing IIO active-scan packing and buffer producer lifetime tests. Extend the test build wiring and help text to cover both suites.
Quoted 4 lines
drivers/iio/opensensorfusion/osf_iio_test.c new file +0
-0
[ ... ]
Quoted 4 lines
drivers/iio/opensensorfusion/osf_iio_test.c new file +4
-0+static void osf_iio_check_scan (struct kunit *test,struct iio_dev *indio_dev, +const s32 *values,unsigned int channels, +unsigned int mask,bool ts) +{
[ ... ]
drivers/iio/opensensorfusion/osf_iio_test.c line 46
Quoted 3 lines
drivers/iio/opensensorfusion/osf_iio_test.c new file +3
-0+ buffer->scan_timestamp = ts; + ret =iio_update_buffers (indio_dev, buffer,NULL ); +KUNIT_ASSERT_EQ (test, ret,0 );
[Severity: Medium]
If this assertion fails, the test function is aborted instantly.
Since IIO devices and buffers are registered manually using iio_device_alloc() and iio_device_register(), they require explicit unregistration via osf_iio_unregister_sensor().
Will bypassing the explicit cleanup permanently leak the IIO device, kfifo, and associated memory? Should KUnit deferred actions (kunit_add_action) be used for cleanup instead?
[ ... ]
drivers/iio/opensensorfusion/osf_iio_test.c line 182
Quoted 7 lines
drivers/iio/opensensorfusion/osf_iio_test.c new file +7
-0+static int osf_iio_test_producer (void *data) +{ +struct osf_iio_race *race = data; +const s32 values[] = {101 ,-202 ,303 }; + +while (!kthread_should_stop ()) { +osf_iio_push_sample (race->indio_dev, values,ARRAY_SIZE (values));
[Severity: High]
If a KUNIT_ASSERT_* check fails elsewhere in the test, KUnit will abort the test and free the race structure as part of test cleanup.
Because the test aborted, kthread_stop() is never called. Will this leave the producer thread running and accessing the freed race memory, causing a use-after-free regression?
Quoted 4 lines
drivers/iio/opensensorfusion/osf_iio_test.c new file +4
-0+cond_resched (); + } +return 0 ; +}
[ ... ]
drivers/iio/opensensorfusion/osf_iio_test.c line 212
Quoted 7 lines
drivers/iio/opensensorfusion/osf_iio_test.c new file +7
-0+static void osf_iio_expect_quiesce (struct kunit *test,struct osf_iio_race *race) +{ +struct task_struct *config; +unsigned long waited; + + config =kthread_run (osf_iio_test_configure, race,"osf-cfg-test" ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, config);
[Severity: High]
If this assertion fails, the test aborts without stopping the running producer thread. Can this trigger the use-after-free regression mentioned above?
[ ... ]
Quoted 2 lines
drivers/iio/opensensorfusion/osf_iio_test.c new file +2
-0+static void osf_iio_buffer_lifetime_test (struct kunit *test) +{
[ ... ]
drivers/iio/opensensorfusion/osf_iio_test.c line 267
Quoted 3 lines
drivers/iio/opensensorfusion/osf_iio_test.c new file +3
-0+atomic_set (&race->block_store,1 ); + producer =kthread_run (osf_iio_test_producer, race,"osf-rx-test" ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, producer);
[Severity: Medium]
If creating the producer thread fails, the test aborts immediately. As with the assertion in osf_iio_check_scan(), will this leave the IIO sensor permanently registered and cause a memory leak regression?
[ ... ]
drivers/iio/opensensorfusion/osf_iio_test.c line 301
Quoted 8 lines
drivers/iio/opensensorfusion/osf_iio_test.c new file +8
-0+KUNIT_EXPECT_EQ (test,iio_update_buffers (indio_dev, buffer,NULL ),0 ); +/* Retain ownership while testing unregister against an admitted push. */ +get_device (&indio_dev->dev); +reinit_completion (&race->entered); +reinit_completion (&race->release); +reinit_completion (&race->config_started); +reinit_completion (&race->config_done); +atomic_set (&race->block_store,1 );
[Severity: Medium]
Does this code lack a memory barrier before atomic_set(&race->block_store, 1)?
The test uses reinit_completion() which performs a relaxed write to x->done, followed immediately by the atomic_set().
Could the continuous background producer thread observe block_store == 1 and execute complete() before the delayed reinit_completion() write takes effect in the main thread?
If the delayed write overwrites the completion status, will the main thread's subsequent wait_for_completion_timeout() block indefinitely and cause a test timeout regression?
Quoted 4 lines
drivers/iio/opensensorfusion/osf_iio_test.c new file +4
-0+KUNIT_EXPECT_NE (test,wait_for_completion_timeout (&race->entered,HZ ),0UL ); + race->unregister = true; +osf_iio_expect_quiesce (test, race); +kthread_stop (producer);
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
Jonathan Cameron
maintainerRe: [PATCH v10 8/8] iio: osf: add IIO KUnit tests
In reply to Jinseob KimQuoted 9 lines · On Sat, 19 Sep 2026 03:24:46 +0900 Jinseob Kim <[email protected]> wrote:
Add the existing IIO active-scan packing and buffer producer lifetime tests. Extend the test build wiring and help text to cover both suites.
This changes no production source. The complete series retains the production behavior of the combined-driver revision, with CRC terminology clarified.
Assisted-by LLMSigned-off-by Jinseob Kim <[email protected]>
So there are a number of generic tests in here (using osf as a device) that we should probably be looking to pull out as independent self tests if we can. Good to have them as something to build on though and there are of course some driver interactions that matter.
Anyhow, I'd like input from others on how they think we should ensure the coverage you have here is present but avoid duplication on a per driver basis (assuming others decide to also start putting together comprehensive self tests like you have done!).
I'd like to find time to take a close look at these tests but it won't happen for a while as there is simply to much activity on the list + I'm travelling in the next few weeks (including Linux Plumbers in Prague in early October). So it may be that we end up merging the driver ahead of these tests.
Jonathan
Quoted 361 lines
--- drivers/iio/opensensorfusion/Kconfig | 2 +- drivers/iio/opensensorfusion/Makefile | 2 +- drivers/iio/opensensorfusion/osf_iio_test.c | 325 ++++++++++++++++++++ 3 files changed, 327 insertions(+), 2 deletions(-) create mode 100644 drivers/iio/opensensorfusion/osf_iio_test.cdrivers/iio/opensensorfusion/Kconfig +1
-1@@ -20,7 +20,7 @@ config OPEN_SENSOR_FUSION_KUNIT_TEST default KUNIT_ALL_TESTS help Build focused unit tests for the Open Sensor Fusion core sample - acceptance, direct-read cache, and capability/session handling. The tests + acceptance, direct-read cache, and IIO buffer paths. The tests exercise accepted, ignored, and rejected sample handling without exposing additional production interfaces.drivers/iio/opensensorfusion/Makefile +1
-1@@ -4,4 +4,4 @@ obj-$(CONFIG_OPEN_SENSOR_FUSION) += open-sensor-fusion.o open-sensor-fusion-y := osf_core.o osf_iio.o osf_protocol.o osf_serdev.o \ osf_stream.o -open-sensor-fusion-$(CONFIG_OPEN_SENSOR_FUSION_KUNIT_TEST) += osf_core_test.o +open-sensor-fusion-$(CONFIG_OPEN_SENSOR_FUSION_KUNIT_TEST) += osf_core_test.o osf_iio_test.odrivers/iio/opensensorfusion/osf_iio_test.c new file +325
-0@@ -0,0 +1,325 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include <kunit/device.h> +#include <kunit/test.h> + +#include <linux/bitmap.h> +#include <linux/completion.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/iio/buffer.h> +#include <linux/iio/buffer_impl.h> +#include <linux/iio/iio.h> +#include <linux/kthread.h> +#include <linux/string.h> + +#include "osf_core.h" +#include "osf_iio.h" + +static noinline void osf_test_poison_stack(void) +{ +u8 bytes[1024 ]; + +memset (bytes,0xa5 ,sizeof (bytes)); +barrier_data (bytes); +} + +static void osf_iio_check_scan (struct kunit *test,struct iio_dev *indio_dev, +const s32 *values,unsigned int channels, +unsigned int mask,bool ts) +{ +struct iio_buffer *buffer = indio_dev->buffer; +unsigned int data_bytes =hweight32 (mask) *sizeof (s32); +unsigned int ts_offset =ALIGN (data_bytes,8 ); +unsigned int scan_bytes = ts ? ts_offset +8 : data_bytes; +unsigned int offset =0 ; +u8 scan[24 ]__aligned(8) ; +s64 before, after, timestamp; +int ret; + +bitmap_zero ((unsigned long *)buffer->scan_mask,iio_get_masklength (indio_dev)); +for (unsigned int i =0 ; i < channels; i++) +if (mask &BIT (i)) +set_bit (i, (unsigned long *)buffer->scan_mask); + buffer->scan_timestamp = ts; + ret =iio_update_buffers (indio_dev, buffer,NULL ); +KUNIT_ASSERT_EQ (test, ret,0 ); + +kunit_info (test,"device=%s mask=%u ts=%u bytes=%u\n" , + indio_dev->name, mask, ts, scan_bytes); +memset (scan,0xa5 ,sizeof (scan)); + before =iio_get_time_ns (indio_dev); +osf_test_poison_stack (); + ret =osf_iio_push_sample (indio_dev, values, channels); + after =iio_get_time_ns (indio_dev); +KUNIT_EXPECT_EQ (test, ret,0 ); +KUNIT_EXPECT_EQ (test, buffer->bytes_per_datum, scan_bytes); + ret =iio_pop_from_buffer (buffer, scan); +KUNIT_EXPECT_EQ (test, ret,0 ); +if (ret) + goto disable; + +for (unsigned int i =0 ; i < channels; i++) { +if (!(mask &BIT (i))) +continue ; +KUNIT_EXPECT_MEMEQ (test, scan + offset, &values[i],sizeof (s32)); + offset +=sizeof (s32); + } +if (ts) { +for (unsigned int i = data_bytes; i < ts_offset; i++) +KUNIT_EXPECT_EQ (test, scan[i], (u8 )0 ); +memcpy (×tamp, scan + ts_offset,sizeof (timestamp)); +KUNIT_EXPECT_GE (test, timestamp, before); +KUNIT_EXPECT_LE (test, timestamp, after); + } +/* Neither kfifo nor the producer may write outside the consumer stride. */ +for (unsigned int i = scan_bytes; i <sizeof (scan); i++) +KUNIT_EXPECT_EQ (test, scan[i], (u8 )0xa5 ); + +disable: +KUNIT_EXPECT_EQ (test,iio_update_buffers (indio_dev,NULL , buffer),0 ); +} + +/* Protect timestamp alignment holes and every supported scan layout. */ +static void osf_iio_scan_bytes_test (struct kunit *test) +{ +static const u16 types[] = { +OSF_SENSOR_ACCELEROMETER ,OSF_SENSOR_GYROSCOPE , +OSF_SENSOR_MAGNETOMETER ,OSF_SENSOR_TEMPERATURE , + }; +const s32 values[] = {101 ,-202 ,303 }; +struct osf_capability_entry entry = { + .sample_format =OSF_SAMPLE_FORMAT_S32 , + .scale_nano =1000000 , + }; +struct osf_device *osf; +struct device *dev; + + dev =kunit_device_register (test,"osf-iio" ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, dev); + osf =kunit_kzalloc (test,sizeof (*osf),GFP_KERNEL ); +KUNIT_ASSERT_NOT_NULL (test, osf); +osf_core_init (osf, dev); + +for (unsigned int t =0 ; t <ARRAY_SIZE (types); t++) { +unsigned int channels = t ==3 ?1 :3 ; +struct iio_dev *indio_dev; +int ret; + + entry.sensor_type = types[t]; + entry.channel_count = channels; + ret =osf_iio_register_sensor (dev, &entry, osf, &indio_dev); +KUNIT_ASSERT_EQ (test, ret,0 ); +for (unsigned int mask =1 ; mask <BIT (channels); mask++) +for (unsigned int ts =0 ; ts <2 ; ts++) +osf_iio_check_scan (test, indio_dev, values, channels, mask, ts); +osf_iio_unregister_sensor (indio_dev); + } +} + +struct osf_iio_race { +struct iio_buffer_access_funcs access; +const struct iio_buffer_access_funcs *original; +struct iio_dev *indio_dev; +struct completion entered; +struct completion release; +struct completion config_started; +struct completion config_done; +atomic_t block_store; +atomic_t stores; +atomic_t disabled; +atomic_t bad_store; +int enable_error; +int disable_error; +int config_result; +bool unregister; +}; + +static struct osf_iio_race *osf_iio_race_from_buffer (struct iio_buffer *buffer) +{ + return container_of(buffer->access, struct osf_iio_race, access); +} + +static int osf_iio_test_store (struct iio_buffer *buffer,const void *data) +{ +struct osf_iio_race *race =osf_iio_race_from_buffer (buffer); + +if (atomic_xchg (&race->block_store,0 )) { +complete (&race->entered); +if (!wait_for_completion_timeout (&race->release,5 *HZ )) +return -ETIMEDOUT ; + } +if (atomic_read (&race->disabled)) +atomic_inc (&race->bad_store); +atomic_inc (&race->stores); +return race->original->store_to (buffer, data); +} + +static int osf_iio_test_enable (struct iio_buffer *buffer,struct iio_dev *indio_dev) +{ +struct osf_iio_race *race =osf_iio_race_from_buffer (buffer); + +if (race->enable_error) +return race->enable_error; +atomic_set (&race->disabled,0 ); +return 0 ; +} + +static int osf_iio_test_disable (struct iio_buffer *buffer,struct iio_dev *indio_dev) +{ +struct osf_iio_race *race =osf_iio_race_from_buffer (buffer); + +atomic_set (&race->disabled,1 ); +return race->disable_error; +} + +static int osf_iio_test_producer (void *data) +{ +struct osf_iio_race *race = data; +const s32 values[] = {101 ,-202 ,303 }; + +while (!kthread_should_stop ()) { +osf_iio_push_sample (race->indio_dev, values,ARRAY_SIZE (values)); +cond_resched (); + } +return 0 ; +} + +static int osf_iio_test_configure (void *data) +{ +struct osf_iio_race *race = data; + +complete (&race->config_started); +if (race->unregister) { +osf_iio_unregister_sensor (race->indio_dev); + race->config_result =0 ; + }else { + race->config_result =iio_update_buffers (race->indio_dev,NULL , + race->indio_dev->buffer); + } +complete (&race->config_done); +while (!kthread_should_stop ()) +msleep (20 ); +return 0 ; +} + +static void osf_iio_expect_quiesce (struct kunit *test,struct osf_iio_race *race) +{ +struct task_struct *config; +unsigned long waited; + + config =kthread_run (osf_iio_test_configure, race,"osf-cfg-test" ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, config); +KUNIT_EXPECT_NE (test,wait_for_completion_timeout (&race->config_started,HZ ),0UL ); + waited =wait_for_completion_timeout (&race->config_done,msecs_to_jiffies (20 )); +KUNIT_EXPECT_EQ (test, waited,0UL ); +complete (&race->release); +KUNIT_EXPECT_NE (test,wait_for_completion_timeout (&race->config_done,HZ ),0UL ); +kthread_stop (config); +KUNIT_EXPECT_EQ (test, race->config_result,0 ); +} + +/* Run real IIO configuration and kfifo code against an independent producer. */ +static void osf_iio_buffer_lifetime_test (struct kunit *test) +{ +struct osf_capability_entry entry = { + .sensor_type =OSF_SENSOR_ACCELEROMETER , + .channel_count =3 , + .sample_format =OSF_SAMPLE_FORMAT_S32 , + .scale_nano =1000000 , + }; +const s32 values[] = {101 ,-202 ,303 }; +struct task_struct *producer; +struct osf_iio_race *race; +struct osf_device *osf; +struct iio_dev *indio_dev; +struct iio_buffer *buffer; +struct device *dev; +int stores, ret; + + dev =kunit_device_register (test,"osf-race" ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, dev); + osf =kunit_kzalloc (test,sizeof (*osf),GFP_KERNEL ); + race =kunit_kzalloc (test,sizeof (*race),GFP_KERNEL ); +KUNIT_ASSERT_NOT_NULL (test, osf); +KUNIT_ASSERT_NOT_NULL (test, race); +osf_core_init (osf, dev); + ret =osf_iio_register_sensor (dev, &entry, osf, &indio_dev); +KUNIT_ASSERT_EQ (test, ret,0 ); + buffer = indio_dev->buffer; + race->indio_dev = indio_dev; + race->original = buffer->access; + race->access = *buffer->access; + race->access.store_to = osf_iio_test_store; + race->access.enable = osf_iio_test_enable; + race->access.disable = osf_iio_test_disable; + buffer->access = &race->access; +init_completion (&race->entered); +init_completion (&race->release); +init_completion (&race->config_started); +init_completion (&race->config_done); +atomic_set (&race->disabled,1 ); +set_bit (0 , (unsigned long *)buffer->scan_mask); +KUNIT_EXPECT_EQ (test,iio_update_buffers (indio_dev, buffer,NULL ),0 ); + +atomic_set (&race->block_store,1 ); + producer =kthread_run (osf_iio_test_producer, race,"osf-rx-test" ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, producer); +KUNIT_EXPECT_NE (test,wait_for_completion_timeout (&race->entered,HZ ),0UL ); +osf_iio_expect_quiesce (test, race); + +for (unsigned int i =0 ; i <100 ; i++) { +bitmap_zero ((unsigned long *)buffer->scan_mask,iio_get_masklength (indio_dev)); + *(unsigned long *)buffer->scan_mask = (i %7 ) +1 ; + buffer->scan_timestamp = i &1 ; +KUNIT_EXPECT_EQ (test,iio_update_buffers (indio_dev, buffer,NULL ),0 ); +cond_resched (); +KUNIT_EXPECT_EQ (test,iio_update_buffers (indio_dev,NULL , buffer),0 ); + } + + race->enable_error = -EIO ; +KUNIT_EXPECT_EQ (test,iio_update_buffers (indio_dev, buffer,NULL ), -EIO ); + stores =atomic_read (&race->stores); +KUNIT_EXPECT_EQ (test,osf_iio_push_sample (indio_dev, values,3 ),0 ); +KUNIT_EXPECT_EQ (test,atomic_read (&race->stores), stores); + race->enable_error =0 ; +KUNIT_EXPECT_EQ (test,iio_update_buffers (indio_dev, buffer,NULL ),0 ); + race->disable_error = -EIO ; +KUNIT_EXPECT_EQ (test,iio_update_buffers (indio_dev,NULL , buffer), -EIO ); + stores =atomic_read (&race->stores); +KUNIT_EXPECT_EQ (test,osf_iio_push_sample (indio_dev, values,3 ),0 ); +KUNIT_EXPECT_EQ (test,atomic_read (&race->stores), stores); + race->disable_error =0 ; + +KUNIT_EXPECT_EQ (test,iio_update_buffers (indio_dev, buffer,NULL ),0 ); +/* Retain ownership while testing unregister against an admitted push. */ +get_device (&indio_dev->dev); +reinit_completion (&race->entered); +reinit_completion (&race->release); +reinit_completion (&race->config_started); +reinit_completion (&race->config_done); +atomic_set (&race->block_store,1 ); +KUNIT_EXPECT_NE (test,wait_for_completion_timeout (&race->entered,HZ ),0UL ); + race->unregister = true; +osf_iio_expect_quiesce (test, race); +kthread_stop (producer); +KUNIT_EXPECT_EQ (test,atomic_read (&race->bad_store),0 ); +KUNIT_EXPECT_GT (test,atomic_read (&race->stores),0 ); + buffer->access = race->original; +put_device (&indio_dev->dev); +} + +static struct kunit_case osf_iio_test_cases[] = { +KUNIT_CASE (osf_iio_scan_bytes_test), +KUNIT_CASE (osf_iio_buffer_lifetime_test), + { } +}; + +static struct kunit_suite osf_iio_test_suite = { + .name ="osf-iio" , + .test_cases = osf_iio_test_cases, +}; + +kunit_test_suite (osf_iio_test_suite); + +MODULE_LICENSE ("GPL" );
Jonathan Cameron
maintainerQuoted 78 lines · On Sat, 19 Sep 2026 03:24:38 +0900 Jinseob Kim <[email protected]> wrote:
Specification status: OSF-D2H 0.0 spec-1 has completed project technical stability review, has been adopted by the project owner, and is published as a fixed specification.
Canonical specification:
https://github.com/opensensorfusion/opensensorfusion-protocol/blob/ca9cdea1ae550c2b4d6f29f87877adae99470744/spec/osf-d2h-0.0.mdErrata process:
https://github.com/opensensorfusion/opensensorfusion-protocol/blob/ca9cdea1ae550c2b4d6f29f87877adae99470744/errata/README.mdCurrent adoption/publication record:
https://github.com/opensensorfusion/opensensorfusion-protocol/blob/0cabccf63ac01d0eb82dd9882739ff58eed4a76c/reviews/publication-record-20260918.mdThe specification was frozen before adoption/publication, so its embedded status snapshot is intentionally historical. The dated publication record above establishes the current adopted/public state. This is project review and adoption, not an external maintainer approval.
This series adds the Open Sensor Fusion UART receive path and IIO devices discovered from capability reports. It exposes accelerometer, gyroscope, magnetometer and temperature data through RAW/SCALE and buffered scans. Device Tree describes the sensor hub; its individual streams are discovered at runtime.
The receiver validates supported descriptors and sample scales, keeps discovery open after empty or unsupported initial inventories, and checks repeated inventories before allowing new data to use registered metadata. A supported descriptor changing meaning faults the session until an explicit teardown and rebind. Existing v9 cache, scan-layout and buffer lifetime fixes are retained.
Based on Jonathan Cameron's IIO testing branch:
69fa76f0af3414cc189c3b0b807cb59e327ecc00Changes since v9:
- Publish and reference the reviewed/adopted OSF-D2H 0.0 specification, errata process and dated publication evidence.
- Tolerate reserved padding, validate descriptor/sample scales, keep discovery open after empty/unsupported reports, and compare repeated capability reports.
- Fail closed on changed descriptor meaning instead of publishing data with stale metadata.
- Add focused KUnit coverage for these lifecycle and validation cases.
- Use managed UART/IIO/power teardown anddev_warn_probe()for the controller baud-rate warning, addressing Andy's probe-path feedback.
- Split the former combined UART/core/IIO driver patch into transport/core, IIO registration, core KUnit, and IIO KUnit patches following review.
- Use validated/unvalidated terminology for framing/CRC results throughout the stream/core/transport code, tests and diagnostics. CRC detects accidental corruption; it does not provide cryptographic authentication.Prior validation on the identical source tree:
identical source tree evidence reused; no builds rerun for DCO packaging.
- All eight intermediate apply/config/relevant builds.
- Independent transport/core and IIO module link/MODPOST.
- GCC/Clang W=1 vmlinux and modules.
- GCC and Clang KUnit: core 16 + IIO 2, all 18 pass in each run.
- Core-only intermediate KUnit: all 16 pass.
- ARM64 Image, selected OSF module and Pi4 DTB.
- Targeted DT binding/style and IIO documentation.The terminology-only diff was verified mechanically. Wire semantics and data-path behavior are unchanged; the receive diagnostic key is validated=. Hardware testing was not repeated for this terminology revision or this post-DCO packaging audit; previous hardware evidence remains historical.
Human DCO is complete. No email has been sent.
Jinseob Kim (8):
dt-bindings: iio: add Open Sensor Fusion device
Documentation: iio: add Open Sensor Fusion driver overview
iio: osf: add protocol decoding
iio: osf: add validated stream parser
iio: osf: add UART transport and core receive path
iio: osf: add IIO devices from capability reports
iio: osf: add core KUnit tests
iio: osf: add IIO KUnit tests
Quoted 8 lines
.../bindings/iio/opensensorfusion,osf.yaml | 52 + .../devicetree/bindings/vendor-prefixes.yaml | 2 + Documentation/iio/index.rst | 1 + Documentation/iio/open-sensor-fusion.rst | 77 ++ MAINTAINERS | 8 + drivers/iio/Kconfig | 1 + drivers/iio/Makefile | 1 + drivers/iio/opensensorfusion/Kconfig | 27 +
Any reason not to push it one level deeper and put it under IMUs? I'm not that keen to grow the top level menu for this as all the other entries are about type of sensor, not one specific sensor.
Quoted 3 lines
Any reason not to push it one level deeper and put it under IMUs? I'm not that keen to grow the top level menu for this as all the other entries are about type of sensor, not one specific sensor.
Yes, I agree that adding an Open Sensor Fusion entry at the IIO top level is not ideal.
My hesitation with putting it under imu/ is that Open Sensor Fusion is not intended to be an IMU. The current Linux profile exposes accelerometer, gyroscope, magnetometer and temperature data, but the hardware/firmware architecture is a general sensor hub and fusion device. Other OSF hardware variants can aggregate sources such as pressure sensors, GNSS/RTK-GNSS, motor encoders and LiDAR in addition to inertial sensors.
Would drivers/iio/common/opensensorfusion/ be a better fit?
There are already sensor-hub/common implementations such as the ChromeOS EC sensors and Samsung SSP sensor hub under drivers/iio/common/.
That would avoid growing the IIO top-level menu without classifying the device itself as an IMU.
Thanks,
Jinseob
2026년 9월 20일 (일) 오전 11:05, Jonathan Cameron <[email protected]>님이 작성:
Quoted 94 lines
Quoted 78 lines · On Sat, 19 Sep 2026 03:24:38 +0900 Jinseob Kim <[email protected]> wrote:
Specification status: OSF-D2H 0.0 spec-1 has completed project technical stability review, has been adopted by the project owner, and is published as a fixed specification.
Canonical specification:
https://github.com/opensensorfusion/opensensorfusion-protocol/blob/ca9cdea1ae550c2b4d6f29f87877adae99470744/spec/osf-d2h-0.0.mdErrata process:
https://github.com/opensensorfusion/opensensorfusion-protocol/blob/ca9cdea1ae550c2b4d6f29f87877adae99470744/errata/README.mdCurrent adoption/publication record:
https://github.com/opensensorfusion/opensensorfusion-protocol/blob/0cabccf63ac01d0eb82dd9882739ff58eed4a76c/reviews/publication-record-20260918.mdThe specification was frozen before adoption/publication, so its embedded status snapshot is intentionally historical. The dated publication record above establishes the current adopted/public state. This is project review and adoption, not an external maintainer approval.
This series adds the Open Sensor Fusion UART receive path and IIO devices discovered from capability reports. It exposes accelerometer, gyroscope, magnetometer and temperature data through RAW/SCALE and buffered scans. Device Tree describes the sensor hub; its individual streams are discovered at runtime.
The receiver validates supported descriptors and sample scales, keeps discovery open after empty or unsupported initial inventories, and checks repeated inventories before allowing new data to use registered metadata. A supported descriptor changing meaning faults the session until an explicit teardown and rebind. Existing v9 cache, scan-layout and buffer lifetime fixes are retained.
Based on Jonathan Cameron's IIO testing branch:
69fa76f0af3414cc189c3b0b807cb59e327ecc00Changes since v9:
- Publish and reference the reviewed/adopted OSF-D2H 0.0 specification, errata process and dated publication evidence.
- Tolerate reserved padding, validate descriptor/sample scales, keep discovery open after empty/unsupported reports, and compare repeated capability reports.
- Fail closed on changed descriptor meaning instead of publishing data with stale metadata.
- Add focused KUnit coverage for these lifecycle and validation cases.
- Use managed UART/IIO/power teardown anddev_warn_probe()for the controller baud-rate warning, addressing Andy's probe-path feedback.
- Split the former combined UART/core/IIO driver patch into transport/core, IIO registration, core KUnit, and IIO KUnit patches following review.
- Use validated/unvalidated terminology for framing/CRC results throughout the stream/core/transport code, tests and diagnostics. CRC detects accidental corruption; it does not provide cryptographic authentication.Prior validation on the identical source tree:
identical source tree evidence reused; no builds rerun for DCO packaging.
- All eight intermediate apply/config/relevant builds.
- Independent transport/core and IIO module link/MODPOST.
- GCC/Clang W=1 vmlinux and modules.
- GCC and Clang KUnit: core 16 + IIO 2, all 18 pass in each run.
- Core-only intermediate KUnit: all 16 pass.
- ARM64 Image, selected OSF module and Pi4 DTB.
- Targeted DT binding/style and IIO documentation.The terminology-only diff was verified mechanically. Wire semantics and data-path behavior are unchanged; the receive diagnostic key is validated=. Hardware testing was not repeated for this terminology revision or this post-DCO packaging audit; previous hardware evidence remains historical.
Human DCO is complete. No email has been sent.
Jinseob Kim (8):
dt-bindings: iio: add Open Sensor Fusion device
Documentation: iio: add Open Sensor Fusion driver overview
iio: osf: add protocol decoding
iio: osf: add validated stream parser
iio: osf: add UART transport and core receive path
iio: osf: add IIO devices from capability reports
iio: osf: add core KUnit tests
iio: osf: add IIO KUnit testsQuoted 8 lines
.../bindings/iio/opensensorfusion,osf.yaml | 52 + .../devicetree/bindings/vendor-prefixes.yaml | 2 + Documentation/iio/index.rst | 1 + Documentation/iio/open-sensor-fusion.rst | 77 ++ MAINTAINERS | 8 + drivers/iio/Kconfig | 1 + drivers/iio/Makefile | 1 + drivers/iio/opensensorfusion/Kconfig | 27 +Any reason not to push it one level deeper and put it under IMUs? I'm not that keen to grow the top level menu for this as all the other entries are about type of sensor, not one specific sensor.
Jonathan Cameron
maintainerQuoted 15 lines · On Sun, 20 Sep 2026 13:22:58 +0900 Kim Jinseob <[email protected]> wrote:
Quoted 3 lines
Any reason not to push it one level deeper and put it under IMUs? I'm not that keen to grow the top level menu for this as all the other entries are about type of sensor, not one specific sensor.
Yes, I agree that adding an Open Sensor Fusion entry at the IIO top level is not ideal.
My hesitation with putting it under imu/ is that Open Sensor Fusion is not intended to be an IMU. The current Linux profile exposes accelerometer, gyroscope, magnetometer and temperature data, but the hardware/firmware architecture is a general sensor hub and fusion device. Other OSF hardware variants can aggregate sources such as pressure sensors, GNSS/RTK-GNSS, motor encoders and LiDAR in addition to inertial sensors.
Quoted 8 lines
Would
drivers/iio/common/opensensorfusion/be a better fit?There are already sensor-hub/common implementations such as the ChromeOS EC sensors and Samsung SSP sensor hub under
drivers/iio/common/.That would avoid growing the IIO top-level menu without classifying the device itself as an IMU.
Lets just start a new directory (maybe move some others later) for sensorhubs. Common is meant for library type modules rather than this purpose. Often those are parts of sensorhubs.
So how about
driver/iio/sensorhubs/osf/
?
Quoted 99 lines
Thanks,
Jinseob2026년 9월 20일 (일) 오전 11:05, Jonathan Cameron <[email protected]>님이 작성:
Quoted 94 lines
Quoted 78 lines · On Sat, 19 Sep 2026 03:24:38 +0900 Jinseob Kim <[email protected]> wrote:
Specification status: OSF-D2H 0.0 spec-1 has completed project technical stability review, has been adopted by the project owner, and is published as a fixed specification.
Canonical specification:
https://github.com/opensensorfusion/opensensorfusion-protocol/blob/ca9cdea1ae550c2b4d6f29f87877adae99470744/spec/osf-d2h-0.0.mdErrata process:
https://github.com/opensensorfusion/opensensorfusion-protocol/blob/ca9cdea1ae550c2b4d6f29f87877adae99470744/errata/README.mdCurrent adoption/publication record:
https://github.com/opensensorfusion/opensensorfusion-protocol/blob/0cabccf63ac01d0eb82dd9882739ff58eed4a76c/reviews/publication-record-20260918.mdThe specification was frozen before adoption/publication, so its embedded status snapshot is intentionally historical. The dated publication record above establishes the current adopted/public state. This is project review and adoption, not an external maintainer approval.
This series adds the Open Sensor Fusion UART receive path and IIO devices discovered from capability reports. It exposes accelerometer, gyroscope, magnetometer and temperature data through RAW/SCALE and buffered scans. Device Tree describes the sensor hub; its individual streams are discovered at runtime.
The receiver validates supported descriptors and sample scales, keeps discovery open after empty or unsupported initial inventories, and checks repeated inventories before allowing new data to use registered metadata. A supported descriptor changing meaning faults the session until an explicit teardown and rebind. Existing v9 cache, scan-layout and buffer lifetime fixes are retained.
Based on Jonathan Cameron's IIO testing branch:
69fa76f0af3414cc189c3b0b807cb59e327ecc00Changes since v9:
- Publish and reference the reviewed/adopted OSF-D2H 0.0 specification, errata process and dated publication evidence.
- Tolerate reserved padding, validate descriptor/sample scales, keep discovery open after empty/unsupported reports, and compare repeated capability reports.
- Fail closed on changed descriptor meaning instead of publishing data with stale metadata.
- Add focused KUnit coverage for these lifecycle and validation cases.
- Use managed UART/IIO/power teardown anddev_warn_probe()for the controller baud-rate warning, addressing Andy's probe-path feedback.
- Split the former combined UART/core/IIO driver patch into transport/core, IIO registration, core KUnit, and IIO KUnit patches following review.
- Use validated/unvalidated terminology for framing/CRC results throughout the stream/core/transport code, tests and diagnostics. CRC detects accidental corruption; it does not provide cryptographic authentication.Prior validation on the identical source tree:
identical source tree evidence reused; no builds rerun for DCO packaging.
- All eight intermediate apply/config/relevant builds.
- Independent transport/core and IIO module link/MODPOST.
- GCC/Clang W=1 vmlinux and modules.
- GCC and Clang KUnit: core 16 + IIO 2, all 18 pass in each run.
- Core-only intermediate KUnit: all 16 pass.
- ARM64 Image, selected OSF module and Pi4 DTB.
- Targeted DT binding/style and IIO documentation.The terminology-only diff was verified mechanically. Wire semantics and data-path behavior are unchanged; the receive diagnostic key is validated=. Hardware testing was not repeated for this terminology revision or this post-DCO packaging audit; previous hardware evidence remains historical.
Human DCO is complete. No email has been sent.
Jinseob Kim (8):
dt-bindings: iio: add Open Sensor Fusion device
Documentation: iio: add Open Sensor Fusion driver overview
iio: osf: add protocol decoding
iio: osf: add validated stream parser
iio: osf: add UART transport and core receive path
iio: osf: add IIO devices from capability reports
iio: osf: add core KUnit tests
iio: osf: add IIO KUnit testsQuoted 8 lines
.../bindings/iio/opensensorfusion,osf.yaml | 52 + .../devicetree/bindings/vendor-prefixes.yaml | 2 + Documentation/iio/index.rst | 1 + Documentation/iio/open-sensor-fusion.rst | 77 ++ MAINTAINERS | 8 + drivers/iio/Kconfig | 1 + drivers/iio/Makefile | 1 + drivers/iio/opensensorfusion/Kconfig | 27 +Any reason not to push it one level deeper and put it under IMUs? I'm not that keen to grow the top level menu for this as all the other entries are about type of sensor, not one specific sensor.