Linux I2C development
 help / color / mirror / Atom feed
* [RFC PATCH v4 0/3] iio: position: add Rust driver for ams AS5600
@ 2026-07-07 15:15 Muchamad Coirul Anwar
  2026-07-07 15:15 ` [RFC PATCH v4 1/3] i2c: rust: implement SMBus read abstraction via kernel::io::Io for I2cClient Muchamad Coirul Anwar
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Muchamad Coirul Anwar @ 2026-07-07 15:15 UTC (permalink / raw)
  To: jic23, lars
  Cc: linux-iio, linux-kernel, linux-i2c, andi.shyti, wsa+renesas,
	ojeda, dakr, igor.korotin, branstj, Muchamad Coirul Anwar

This is v4 of the Rust driver for the ams AS5600 12-bit magnetic rotary
position sensor. v3 hardened the IIO abstraction layer and introduced
kernel synchronisation primitives. This revision simplifies the driver
substantially based on review feedback, removes components that were
over-engineered for a single-bus 12-bit sensor, and validates the result
with comprehensive hardware testing.

Apologies for the delay.

Link: https://lore.kernel.org/linux-iio/20260419151327.26306-1-muchamadcoirulanwar@gmail.com/

Changes since RFC v3:

  IIO abstraction (rust/kernel/iio.rs):
  - Removed DirectModeGuard entirely (struct, C helpers, binding file)
    per Jonathan's review of patch 2. This driver operates in
    INDIO_DIRECT_MODE only and has no buffer or trigger support, so
    claiming direct mode is redundant. If buffer support is added later,
    the guard can be reintroduced at the driver level rather than the
    trampoline.
  - Deleted rust/helpers/iio.c (no remaining consumers).
  - Fixed SAFETY comment: iio_device_alloc uses kzalloc internally, so
    priv_ points to zeroed memory, not uninitialised memory as the
    previous comment stated.
  - Added i32::try_from(size_of::<T>()) guard to prevent silent
    truncation when passing priv_size to iio_device_alloc.
  - No other functional changes to the abstraction.

  I2C abstraction (rust/kernel/i2c.rs):
  - Provided full io_read/io_write method bodies for IoCapable<u8> and
    IoCapable<u16> per Brandon's note that IoCapable is no longer a
    marker trait on current tree. The empty impls from v3 would not
    compile.
  - Added clarifying comment on maxsize(): this is the SMBus command
    byte range (0x00-0xFF), not the 7-bit I2C device address.

  Driver (drivers/iio/position/as5600.rs):
  - Removed circuit breaker (DeviceState enum, handle_io_error) per
    Jonathan's review of patch 3. The AS5600 sits on a short local bus
    with no hot-plug; if the bus fails, propagating the error directly
    is simpler and more predictable than internal state machines.
  - Replaced As5600Io(*mut i2c_client) with ARef<I2cClient> per
    Brandon's suggestion. This eliminates manual unsafe Send/Sync impls;
    ARef provides proper reference counting and the compiler can verify
    the bounds itself.
  - Removed the generic type parameter from As5600Priv and As5600HwState
    per Brandon's feedback. A concrete type is sufficient for a
    single-bus sensor.
  - Moved channel spec from KBox<[iio_chan_spec; 1]> (heap) to a module-
    level static per Jonathan's review of patch 2, matching what all C
    IIO drivers do and eliminating the KBox lifetime concern noted in v3.
  - Replaced two sequential try_read8 calls (high byte + low byte) with
    a single try_read16 word read followed by swap_bytes() per
    Jonathan's review of patch 3. The AS5600 stores angle big-endian
    across 0x0C-0x0D; SMBus word reads return little-endian, so a byte
    swap recovers the correct value.
    In my v3 reply I said I would use i2c_smbus_read_word_swapped. I
    apologise for not following through. Adding a standalone swapped
    method alongside the Io trait felt premature while the question of
    whether this driver should use regmap-rs instead remains open. For
    this RFC with a single word read, the in-driver swap is retained
    with a TODO comment. Happy to add the wrapper in v5 if preferred.
  - Added AS5600_RAW_ANGLE_MASK via genmask_u16(0..=11) for 12-bit
    angle extraction.
  - Updated datasheet URL to ams-osram.com per Brandon's note (old link
    broken).
  - Added depends on IIO to Kconfig (won't link without it).
  - Kconfig and Makefile entries merged into driver patch per Jonathan's
    review of patch 4.

  Known limitations (to be addressed before mainline):
  - No power management (suspend/resume) hooks.
  - write_raw and buffer/trigger support deferred to future work.
  - No i2c_smbus_read_word_swapped wrapper yet (see above).

Changes since RFC v2:
  (see v3 cover letter for full changelog)

Changes since RFC v1:
  - Moved magnet validation from probe() to read_raw().
  - Added minimal Rust IIO abstractions.
  - Added OF device table for devicetree matching.

Design notes:

  The IIO abstraction does NOT use devres (devm_iio_device_alloc). The
  Rust Drop implementation controls the cleanup sequence directly:
  iio_device_unregister -> drop_in_place(T) -> iio_device_free. This
  avoids ordering conflicts between Rust ownership semantics and the C
  devres teardown sequence.

  Module ownership is enforced via the second parameter of
  __iio_device_register(indio_dev, module), not via iio_info.owner.

Muchamad Coirul Anwar (3):
  i2c: rust: implement SMBus read abstraction via kernel::io::Io for
    I2cClient
  rust: add minimal IIO subsystem abstractions
  iio: position: add Rust driver for ams AS5600

 drivers/iio/position/Kconfig    |  14 ++
 drivers/iio/position/Makefile   |   1 +
 drivers/iio/position/as5600.rs  | 181 ++++++++++++++++++++
 rust/bindings/bindings_helper.h |   2 +
 rust/kernel/error.rs            |   1 +
 rust/kernel/i2c.rs              |  89 ++++++++++
 rust/kernel/iio.rs              | 295 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 8 files changed, 585 insertions(+)
 create mode 100644 drivers/iio/position/as5600.rs
 create mode 100644 rust/kernel/iio.rs

Signed-off-by: Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>
---
Tested on BeagleBone Black (AM335x), kernel 7.1.0-g97e88f52769a (build
88), AS5600 on i2c-2 (0x36) at 3.3V, 6mm diametric neodymium magnet.
Full test session: 2026-06-30 12:00 UTC.

Build: make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- modules
       (zero warnings, zero errors)
       Module size: 15108 bytes (as5600.ko)

Functional tests:

  1. Probe and registration:
     $ sudo insmod as5600.ko
     $ echo "as5600 0x36" > /sys/bus/i2c/devices/i2c-2/new_device
     $ cat /sys/bus/iio/devices/iio:device0/name
     as5600
     $ ls /sys/bus/i2c/devices/2-0036/driver
     2-0036  bind  module  uevent  unbind

  2. Raw angle and scale (magnet present, rotated by hand):
     $ cat /sys/bus/iio/devices/iio:device0/in_angl_raw
     233
     $ cat /sys/bus/iio/devices/iio:device0/in_angl_scale
     0.001533981
     Multiple reads while rotating: 233, 898, 1873, 1543, 468, 63, 252
     All values within expected 0-4095 range (12-bit).
     Computed: 898 * 0.001533981 = 1.378 rad, about 78.9 degrees.

  3. Magnet removed (MD bit clear, error propagation without circuit
     breaker):
     $ cat /sys/bus/iio/devices/iio:device0/in_angl_raw
     cat: '/sys/.../in_angl_raw': No data available
     Driver reads STATUS, finds MD=0, returns -ENODATA directly. No
     internal state machine. Next read with magnet present succeeds
     immediately without recovery sequence.

  4. Unbind/rebind lifecycle (PinnedDrop with ARef cleanup):
     $ echo "2-0036" > /sys/bus/i2c/devices/2-0036/driver/unbind
     $ ls /sys/bus/iio/devices/iio:device0
     ls: cannot access '...': No such file or directory
     $ echo "2-0036" > /sys/bus/i2c/drivers/as5600/bind
     dmesg confirms full PinnedDrop sequence on unbind:
       iio_device_unregister -> drop_in_place(Mutex<ARef>) -> iio_device_free
     Re-probe succeeds, device functional again.
     $ dmesg | grep -i "oops\|panic\|bug:"
     (empty)

  5. Concurrent stress (Mutex serialization under contention):
     $ for i in $(seq 1 10); do
         timeout 10 sh -c 'while true; do
           cat .../in_angl_raw > /dev/null 2>&1; done' &
       done; wait
     10 parallel readers hammering in_angl_raw for 10 seconds.
     All workers exit cleanly via timeout (exit 124).
     Repeated 3 times, same result each time.
     $ dmesg | grep -i "oops\|panic\|bug:\|rcu"
     (empty, only boot-time RCU init messages present)
     No corrupted values, no races, no use-after-free.

  6. Unbind/rebind race (concurrent I/O during device removal):
     Terminal 1: continuous cat .../in_angl_raw in background
     Terminal 2: 20x unbind/bind cycle with 100ms sleep between
     This exercises the iio_device_unregister drain path. The IIO core
     waits for in-flight read_raw callbacks to complete before
     PinnedDrop proceeds with drop_in_place.
     $ dmesg | grep -i "oops\|panic\|bug:"
     (empty, all 20 cycles complete without crash)

  7. Lifecycle stress (20x probe/remove under I/O load):
     20 complete insmod/new_device/delete_device/rmmod cycles with 10
     concurrent readers active during each cycle. Each cycle allocates
     a new iio_dev (confirmed by unique indio_dev addresses in dmesg)
     and frees it via PinnedDrop. No memory leaks, no KASAN reports,
     no dangling pointers.

-- 
2.50.0


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

* [RFC PATCH v4 1/3] i2c: rust: implement SMBus read abstraction via kernel::io::Io for I2cClient
  2026-07-07 15:15 [RFC PATCH v4 0/3] iio: position: add Rust driver for ams AS5600 Muchamad Coirul Anwar
@ 2026-07-07 15:15 ` Muchamad Coirul Anwar
  2026-07-11 10:05   ` Igor Korotin
  2026-07-11 12:08   ` Danilo Krummrich
  2026-07-07 15:15 ` [RFC PATCH v4 2/3] rust: add minimal IIO subsystem abstractions Muchamad Coirul Anwar
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 10+ messages in thread
From: Muchamad Coirul Anwar @ 2026-07-07 15:15 UTC (permalink / raw)
  To: jic23, lars
  Cc: linux-iio, linux-kernel, linux-i2c, andi.shyti, wsa+renesas,
	ojeda, dakr, igor.korotin, branstj, Muchamad Coirul Anwar

Implement the Io trait for I2cClient, providing SMBus byte and word
read/write operations with automatic offset validation via io_addr().

I2cClient implements the generic Io trait rather than exposing
standalone SMBus methods, following the direction established in [1]
and [2]. The underlying calls are still i2c_smbus_read_byte_data and
i2c_smbus_read_word_data.

I2cClient now implements IoCapable<u8> and IoCapable<u16> with
maxsize=256 (SMBus command byte range 0x00-0xFF, not the 7-bit
device address which is handled by the I2C core at adapter level).

Link: https://lore.kernel.org/rust-for-linux/20260131-i2c-adapter-v1-4-5a436e34cd1a@gmail.com/
Link: https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/commit/?h=driver-core-testing&id=121d87b28e1d9061d3aaa156c43a627d3cb5e620
Signed-off-by: Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>
---
 rust/kernel/i2c.rs | 89 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 624b971ca8b0..31c7216d0299 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -14,6 +14,7 @@
     devres::Devres,
     driver,
     error::*,
+    io::{Io, IoCapable},
     of,
     prelude::*,
     sync::aref::{
@@ -601,3 +602,91 @@ unsafe impl Send for Registration {}
 // SAFETY: `Registration` offers no interior mutability (no mutation through &self
 // and no mutable access is exposed)
 unsafe impl Sync for Registration {}
+
+impl<Ctx: device::DeviceContext> IoCapable<u8> for I2cClient<Ctx> {
+    unsafe fn io_read(&self, address: usize) -> u8 {
+        // SAFETY: `self.as_raw()` returns a valid `struct i2c_client` pointer
+        // (type invariant). `address` was pre-validated by io_addr() before
+        // this function is called (trait contract).
+        let ret = unsafe { bindings::i2c_smbus_read_byte_data(self.as_raw(), address as u8) };
+
+        // NOTE: Error is lost here. This is only called via try_read() which
+        // first validates bounds via io_addr(). For I2C, the caller should
+        // always use try_read8() which provides proper error handling.
+        ret as u8
+    }
+
+    unsafe fn io_write(&self, value: u8, address: usize) {
+        // SAFETY: `self.as_raw()` returns a valid `struct i2c_client` pointer.
+        // `address` pre-validated by io_addr().
+        unsafe { bindings::i2c_smbus_write_byte_data(self.as_raw(), address as u8, value) };
+        // NOTE: Return value is ignored. `IoCapable` trait signature does not
+        // support error returns. Use with caution.
+    }
+}
+
+impl<Ctx: device::DeviceContext> IoCapable<u16> for I2cClient<Ctx> {
+    unsafe fn io_read(&self, address: usize) -> u16 {
+        // SAFETY: `self.as_raw()` returns a valid `struct i2c_client` pointer.
+        // `address` pre-validated by io_addr().
+        let ret = unsafe { bindings::i2c_smbus_read_word_data(self.as_raw(), address as u8) };
+
+        // NOTE: Error is lost here. See u8 implementation note.
+        ret as u16
+    }
+
+    unsafe fn io_write(&self, value: u16, address: usize) {
+        // SAFETY: `self.as_raw()` returns a valid `struct i2c_client` pointer.
+        // `address` pre-validated by io_addr().
+        unsafe { bindings::i2c_smbus_write_word_data(self.as_raw(), address as u8, value) };
+        // NOTE: Return value is ignored.
+    }
+}
+
+impl<Ctx: device::DeviceContext> Io for I2cClient<Ctx> {
+    #[inline]
+    fn addr(&self) -> usize {
+        0
+    }
+
+    /// SMBus command byte range: 0x00-0xFF (256 possible register addresses).
+    /// This is NOT the 7-bit device address; that is handled by the I2C core.
+    #[inline]
+    fn maxsize(&self) -> usize {
+        256
+    }
+
+    #[inline]
+    fn try_read8(&self, offset: usize) -> Result<u8>
+    where
+        Self: IoCapable<u8>,
+    {
+        let reg = self.io_addr::<u8>(offset)? as u8;
+        // SAFETY: `self.as_raw()` returns a valid pointer to a `struct i2c_client`
+        // as guaranteed by the type invariant of `I2cClient`. `reg` is bounds-checked
+        // by `io_addr()` above (offset + 1 <= 256).
+        let ret = unsafe { bindings::i2c_smbus_read_byte_data(self.as_raw(), reg) };
+        if ret < 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(ret as u8)
+        }
+    }
+
+    #[inline]
+    fn try_read16(&self, offset: usize) -> Result<u16>
+    where
+        Self: IoCapable<u16>,
+    {
+        let reg = self.io_addr::<u16>(offset)? as u8;
+        // SAFETY: `self.as_raw()` returns a valid pointer to a `struct i2c_client`
+        // as guaranteed by the type invariant of `I2cClient`. `reg` is bounds-checked
+        // by `io_addr()` above (offset + 2 <= 256).
+        let ret = unsafe { bindings::i2c_smbus_read_word_data(self.as_raw(), reg) };
+        if ret < 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(ret as u16)
+        }
+    }
+}
-- 
2.50.0


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

* [RFC PATCH v4 2/3] rust: add minimal IIO subsystem abstractions
  2026-07-07 15:15 [RFC PATCH v4 0/3] iio: position: add Rust driver for ams AS5600 Muchamad Coirul Anwar
  2026-07-07 15:15 ` [RFC PATCH v4 1/3] i2c: rust: implement SMBus read abstraction via kernel::io::Io for I2cClient Muchamad Coirul Anwar
@ 2026-07-07 15:15 ` Muchamad Coirul Anwar
  2026-07-11 12:12   ` Danilo Krummrich
  2026-07-07 15:15 ` [RFC PATCH v4 3/3] iio: position: add Rust driver for ams AS5600 Muchamad Coirul Anwar
  2026-07-08 10:36 ` [RFC PATCH v4 0/3] " Miguel Ojeda
  3 siblings, 1 reply; 10+ messages in thread
From: Muchamad Coirul Anwar @ 2026-07-07 15:15 UTC (permalink / raw)
  To: jic23, lars
  Cc: linux-iio, linux-kernel, linux-i2c, andi.shyti, wsa+renesas,
	ojeda, dakr, igor.korotin, branstj, Muchamad Coirul Anwar

Add safe Rust wrappers for the Linux IIO (Industrial I/O) subsystem:

- IioVal enum with NonZeroI32 for division-by-zero prevention on
  IIO_VAL_FRACTIONAL
- IioDriver trait with read_raw callback (requires Send + Sync)
- Device<T, State> with typestate (Unregistered -> Registered) to
  prevent double-registration at compile time
- PinnedDrop for guaranteed cleanup: iio_device_unregister ->
  drop_in_place(T) -> iio_device_free
- Compile-time const VTABLE (iio_info)
- C-to-Rust FFI trampoline for read_raw dispatch

The abstraction uses iio_device_alloc (not devm_*) so that the Rust
Drop implementation has full control over the cleanup sequence.
Module ownership is enforced via __iio_device_register(indio_dev, module).

Signed-off-by: Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>
---
 rust/bindings/bindings_helper.h |   2 +
 rust/kernel/error.rs            |   1 +
 rust/kernel/iio.rs              | 293 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 4 files changed, 298 insertions(+)
 create mode 100644 rust/kernel/iio.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 446dbeaf0866..1ff262063556 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -61,6 +61,8 @@
 #include <linux/firmware.h>
 #include <linux/fs.h>
 #include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
 #include <linux/interrupt.h>
 #include <linux/io-pgtable.h>
 #include <linux/ioport.h>
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index a56ba6309594..5dc917d92151 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -86,6 +86,7 @@ macro_rules! declare_err {
     declare_err!(EIOCBQUEUED, "iocb queued, will get completion event.");
     declare_err!(ERECALLCONFLICT, "Conflict with recalled state.");
     declare_err!(ENOGRACE, "NFS file lock reclaim refused.");
+    declare_err!(ENODATA, "No data available.");
 }
 
 /// Generic integer kernel error.
diff --git a/rust/kernel/iio.rs b/rust/kernel/iio.rs
new file mode 100644
index 000000000000..816bf370df86
--- /dev/null
+++ b/rust/kernel/iio.rs
@@ -0,0 +1,293 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2026 Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>
+//! IIO subsystem abstractions.
+//!
+//! Minimal safe Rust wrappers for the Linux IIO (Industrial I/O) subsystem.
+//! Provides [`Device`] for allocating and registering an IIO device, and the
+//! [`IioDriver`] trait for implementing `read_raw` callbacks in safe Rust.
+
+use crate::{
+    bindings::{
+        __iio_device_register,
+        iio_chan_spec,
+        iio_dev,
+        iio_device_alloc,
+        iio_device_free,
+        iio_device_unregister,
+        iio_info,
+        INDIO_DIRECT_MODE, //
+    },
+    device,
+    error::{
+        code::*,
+        to_result,
+        Result, //
+    },
+    prelude::*,
+    ThisModule, //
+};
+use core::{
+    ffi::c_int,
+    marker::PhantomData,
+    mem::{
+        forget,
+        size_of,
+        zeroed, //
+    },
+    num::NonZeroI32,
+    pin::Pin,
+    ptr::drop_in_place, //
+};
+
+use pin_init::{pin_data, pinned_drop};
+
+/// IIO value type: single integer (`IIO_VAL_INT`).
+pub const IIO_VAL_INT: c_int = crate::bindings::IIO_VAL_INT as c_int;
+/// IIO value type: integer plus micro part (`IIO_VAL_INT_PLUS_MICRO`).
+pub const IIO_VAL_INT_PLUS_MICRO: c_int = crate::bindings::IIO_VAL_INT_PLUS_MICRO as c_int;
+/// IIO value type: integer plus nano part (`IIO_VAL_INT_PLUS_NANO`).
+pub const IIO_VAL_INT_PLUS_NANO: c_int = crate::bindings::IIO_VAL_INT_PLUS_NANO as c_int;
+/// IIO value type: fractional (`IIO_VAL_FRACTIONAL`).
+pub const IIO_VAL_FRACTIONAL: c_int = crate::bindings::IIO_VAL_FRACTIONAL as c_int;
+
+/// Represents the return value of a `read_raw` operation.
+///
+/// Each variant corresponds to an `IIO_VAL_*` constant and tells the
+/// IIO core how to format `val` and `val2` for userspace.
+pub enum IioVal {
+    /// A single integer value.
+    Int(i32),
+    /// A fractional value represented as `val / val2`.
+    /// The denominator is `NonZeroI32` to prevent division-by-zero in
+    /// `iio_format_value()`.
+    Fractional(i32, NonZeroI32),
+    /// An integer plus a micro (1e-6) fractional part: `val.val2`.
+    IntPlusMicro(i32, i32),
+    /// An integer plus a nano (1e-9) fractional part: `val.val2`.
+    IntPlusNano(i32, i32),
+}
+
+/// Trait to be implemented by IIO driver private data.
+///
+/// Implementors supply the `read_raw` callback invoked by the IIO core
+/// when userspace reads a channel attribute (e.g. `in_angl_raw`).
+///
+/// The `Send + Sync` bounds ensure the compiler rejects driver types with
+/// thread-unsafe interior mutability (e.g. `Cell`), since the IIO core may
+/// invoke `read_raw` concurrently from multiple sysfs readers.
+pub trait IioDriver: Send + Sync {
+    /// Called by the IIO core when userspace reads a channel attribute.
+    ///
+    /// `chan` is the channel being read; `mask` selects the attribute
+    /// (e.g. `IIO_CHAN_INFO_RAW`, `IIO_CHAN_INFO_SCALE`).
+    fn read_raw(&self, chan: *const iio_chan_spec, mask: isize) -> Result<IioVal>;
+
+    /// Returns the channel specifications for this driver.
+    ///
+    /// The default implementation returns an empty slice.
+    fn channels(&self) -> &[iio_chan_spec] {
+        &[]
+    }
+}
+
+/// C-compatible trampoline for the `iio_info.read_raw` callback.
+///
+/// # Safety
+///
+/// This function is only called by the IIO core with valid pointers:
+/// - `indio_dev` is a valid `iio_dev` allocated by `iio_device_alloc`.
+/// - `chan` points to a valid channel spec from the device's channel array.
+/// - `val` and `val2` are valid pointers for writing the result.
+unsafe extern "C" fn read_raw_callback<T: IioDriver>(
+    indio_dev: *mut iio_dev,
+    chan: *const iio_chan_spec,
+    val: *mut c_int,
+    val2: *mut c_int,
+    mask: isize,
+) -> c_int {
+    // SAFETY: `indio_dev` is valid and was allocated with space for `T` in its
+    // private data area. The `priv_` field was initialized in `Device::build_device()`.
+    let priv_ptr = unsafe { (*indio_dev).priv_ as *mut T };
+    // SAFETY: `priv_ptr` points to a valid, initialized instance of `T` that
+    // lives as long as the `iio_dev` allocation.
+    let driver = unsafe { &*priv_ptr };
+
+    match driver.read_raw(chan, mask) {
+        Ok(IioVal::Int(v)) => {
+            // SAFETY: `val` is a valid pointer provided by the IIO core.
+            unsafe {
+                *val = v;
+            }
+            IIO_VAL_INT
+        }
+        Ok(IioVal::Fractional(v, v2)) => {
+            // SAFETY: `val` and `val2` are valid pointers provided by the IIO core.
+            unsafe {
+                *val = v;
+                *val2 = v2.get();
+            }
+            IIO_VAL_FRACTIONAL
+        }
+        Ok(IioVal::IntPlusMicro(v, v2)) => {
+            // SAFETY: `val` and `val2` are valid pointers provided by the IIO core.
+            unsafe {
+                *val = v;
+                *val2 = v2;
+            }
+            IIO_VAL_INT_PLUS_MICRO
+        }
+        Ok(IioVal::IntPlusNano(v, v2)) => {
+            // SAFETY: `val` and `val2` are valid pointers provided by the IIO core.
+            unsafe {
+                *val = v;
+                *val2 = v2;
+            }
+            IIO_VAL_INT_PLUS_NANO
+        }
+        Err(e) => e.to_errno(),
+    }
+}
+
+// Device<T, State>: IIO device wrapper with typestate.
+
+/// Marker type for an unregistered IIO device.
+pub struct Unregistered;
+/// Marker type for a registered IIO device.
+pub struct Registered;
+
+/// A wrapped IIO device managing its C `struct iio_dev` lifetime.
+///
+/// Uses `iio_device_alloc` for allocation (no devres involvement) and
+/// manual cleanup via `Drop`: `iio_device_unregister` -> `drop_in_place`
+/// for `T` -> `iio_device_free`.
+#[pin_data(PinnedDrop)]
+pub struct Device<T: IioDriver, State = Unregistered> {
+    indio_dev: *mut iio_dev,
+    registered: bool,
+    _p: PhantomData<(T, State)>,
+}
+
+// SAFETY: `Device` only contains a raw pointer to a kernel-managed `iio_dev`.
+// The IIO core serializes access to the device, and `T` is required to be `Send`.
+unsafe impl<T: IioDriver, S> Send for Device<T, S> {}
+// SAFETY: All `&self` access to the `iio_dev` is read-only or goes through the
+// IIO core which provides its own synchronization. `T` is required to be `Sync`.
+unsafe impl<T: IioDriver, S> Sync for Device<T, S> {}
+
+#[pinned_drop]
+impl<T: IioDriver, S> PinnedDrop for Device<T, S> {
+    fn drop(self: Pin<&mut Self>) {
+        if self.registered {
+            // SAFETY: The device was successfully registered via
+            // `__iio_device_register`. Unregistering drains all pending
+            // callbacks, ensuring no `read_raw` is in flight after this.
+            unsafe { iio_device_unregister(self.indio_dev) };
+        }
+
+        // SAFETY: `priv_` was fully initialized in `build_device` via
+        // `init.__pinned_init(priv_ptr)`. `drop_in_place` runs `T`'s destructor
+        // (including any pinned fields like Mutex). After that, `iio_device_free`
+        // calls `put_device` which decrements the kref. The underlying `iio_dev`
+        // memory is only freed when kref reaches 0.
+        unsafe {
+            let priv_ptr = (*self.indio_dev).priv_ as *mut T;
+            drop_in_place(priv_ptr);
+            iio_device_free(self.indio_dev);
+        }
+    }
+}
+
+impl<T: IioDriver> Device<T> {
+    // SAFETY: The remaining fields of `iio_info` are pointers and function
+    // pointers. Zeroed values are NULL, and the IIO core checks for NULL
+    // before invoking callbacks or dereferencing attribute group pointers.
+    const VTABLE: iio_info = iio_info {
+        read_raw: Some(read_raw_callback::<T>),
+        ..unsafe { zeroed() }
+    };
+
+    /// Allocates a new IIO device with the given driver data.
+    ///
+    /// Uses `iio_device_alloc` (not `devm_*`) so that the Rust `Drop`
+    /// implementation has full control over the cleanup sequence.
+    /// The device is not yet registered; call [`register`](Self::register)
+    /// to make it visible to userspace.
+    pub fn build_device<E>(
+        dev: &device::Device,
+        name: &'static CStr,
+        init: impl PinInit<T, E>,
+    ) -> Result<Self>
+    where
+        Error: From<E>,
+    {
+        let priv_size = i32::try_from(size_of::<T>()).map_err(|_| EINVAL)?;
+
+        // SAFETY: `dev.as_raw()` returns a valid `struct device` pointer.
+        // `iio_device_alloc` allocates an `iio_dev` with `sizeof(T)` bytes of
+        // private data. Returns NULL on failure.
+        let indio_dev = unsafe { iio_device_alloc(dev.as_raw(), priv_size) };
+        if indio_dev.is_null() {
+            return Err(ENOMEM);
+        }
+
+        // SAFETY: `indio_dev` is valid and freshly allocated. `priv_` points to
+        // zeroed memory (kzalloc'd by iio_device_alloc). `PinInit::__pinned_init`
+        // overwrites it in place without reading previous contents.
+        let priv_ptr = unsafe { (*indio_dev).priv_ as *mut T };
+        let init_result = unsafe { init.__pinned_init(priv_ptr) };
+        if let Err(e) = init_result {
+            // SAFETY: `pin_init` guarantees partial-init rollback internally.
+            // `priv_` memory was not fully initialized, so we only free the
+            // container without running `T`'s destructor.
+            unsafe { iio_device_free(indio_dev) };
+            return Err(Error::from(e));
+        }
+
+        // SAFETY: `priv_ptr` is now fully initialized. We set up the IIO
+        // device fields:
+        // - `name` is a `'static` C string that outlives the device.
+        // - `VTABLE` is a `'static` const and outlives the device.
+        // - `channels()` returns a reference to data owned by `T` in `priv_`,
+        //   which remains at a fixed address because `priv_` is heap-allocated
+        //   inside `iio_dev`.
+        unsafe {
+            (*indio_dev).name = name.as_char_ptr();
+            (*indio_dev).info = &Self::VTABLE;
+
+            let chans = (*priv_ptr).channels();
+            (*indio_dev).channels = chans.as_ptr();
+            (*indio_dev).num_channels = chans.len() as _;
+            (*indio_dev).modes = INDIO_DIRECT_MODE as i32;
+        }
+
+        Ok(Self {
+            indio_dev,
+            registered: false,
+            _p: PhantomData,
+        })
+    }
+
+    /// Registers the IIO device, making it visible to userspace via sysfs.
+    ///
+    /// On success, channel attributes like `in_angl_raw` become readable.
+    /// On failure the device stays unregistered and will be freed when
+    /// this [`Device`] is dropped.
+    #[inline]
+    pub fn register(self, module: &'static ThisModule) -> Result<Device<T, Registered>> {
+        // SAFETY: `self.indio_dev` is a valid, fully initialized `iio_dev`.
+        // `module.as_ptr()` provides the module owner for proper refcounting.
+        let ret = unsafe { __iio_device_register(self.indio_dev, module.as_ptr()) };
+        to_result(ret)?;
+
+        let registered_dev = Device {
+            indio_dev: self.indio_dev,
+            registered: true,
+            _p: PhantomData,
+        };
+
+        // Prevent `self`'s Drop from running. Ownership of `indio_dev`
+        // has been transferred to `registered_dev`.
+        forget(self);
+        Ok(registered_dev)
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 9512af7156df..f565fc549b8c 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -78,6 +78,8 @@
 #[cfg(CONFIG_I2C = "y")]
 pub mod i2c;
 pub mod id_pool;
+#[cfg(CONFIG_IIO)]
+pub mod iio;
 #[doc(hidden)]
 pub mod impl_flags;
 pub mod init;
-- 
2.50.0


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

* [RFC PATCH v4 3/3] iio: position: add Rust driver for ams AS5600
  2026-07-07 15:15 [RFC PATCH v4 0/3] iio: position: add Rust driver for ams AS5600 Muchamad Coirul Anwar
  2026-07-07 15:15 ` [RFC PATCH v4 1/3] i2c: rust: implement SMBus read abstraction via kernel::io::Io for I2cClient Muchamad Coirul Anwar
  2026-07-07 15:15 ` [RFC PATCH v4 2/3] rust: add minimal IIO subsystem abstractions Muchamad Coirul Anwar
@ 2026-07-07 15:15 ` Muchamad Coirul Anwar
  2026-07-08 10:36 ` [RFC PATCH v4 0/3] " Miguel Ojeda
  3 siblings, 0 replies; 10+ messages in thread
From: Muchamad Coirul Anwar @ 2026-07-07 15:15 UTC (permalink / raw)
  To: jic23, lars
  Cc: linux-iio, linux-kernel, linux-i2c, andi.shyti, wsa+renesas,
	ojeda, dakr, igor.korotin, branstj, Muchamad Coirul Anwar

Add a Rust driver for the ams AS5600 12-bit magnetic rotary position
sensor. The driver exposes in_angl_raw and in_angl_scale via the IIO
sysfs interface.

Features:
- ARef<I2cClient> for safe refcounted I2C client access
- Mutex-serialized status + angle read sequence
- Static channel spec (module-level const)
- No magnet validation at probe (deferred to read_raw per IIO convention)
- Error propagation via ? operator (no recovery state machine)

The byte order for the AS5600's big-endian registers is handled via
swap_bytes() in-driver. This is equivalent to C's
i2c_smbus_read_word_swapped(). The long-term solution is regmap-rs
where endianness is configured once at the transport level.

Tested on BeagleBone Black (AM335x) with AS5600 on i2c-2 (0x36).

Signed-off-by: Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>
---
 drivers/iio/position/Kconfig   |  14 +++
 drivers/iio/position/Makefile  |   1 +
 drivers/iio/position/as5600.rs | 181 +++++++++++++++++++++++++++++++++
 3 files changed, 196 insertions(+)
 create mode 100644 drivers/iio/position/as5600.rs

diff --git a/drivers/iio/position/Kconfig b/drivers/iio/position/Kconfig
index 1576a6380b53..573d241676bf 100644
--- a/drivers/iio/position/Kconfig
+++ b/drivers/iio/position/Kconfig
@@ -6,6 +6,20 @@
 
 menu "Linear and angular position sensors"
 
+config AS5600
+	tristate "ams AS5600 magnetic rotary position sensor"
+	depends on I2C && IIO && RUST
+	help
+	  Say Y here to build support for the ams AS5600 12-bit
+	  magnetic rotary position sensor with IIO channel support
+	  (in_angl_raw and in_angl_scale).
+
+	  This is a Rust driver that exposes the 12-bit raw angle
+	  and radian scale via the IIO subsystem.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called as5600.
+
 config IQS624_POS
 	tristate "Azoteq IQS624/625 angular position sensors"
 	depends on MFD_IQS62X || COMPILE_TEST
diff --git a/drivers/iio/position/Makefile b/drivers/iio/position/Makefile
index d70902f2979d..2d26f6d6ace3 100644
--- a/drivers/iio/position/Makefile
+++ b/drivers/iio/position/Makefile
@@ -4,5 +4,6 @@
 
 # When adding new entries keep the list in alphabetical order
 
+obj-$(CONFIG_AS5600) += as5600.o
 obj-$(CONFIG_HID_SENSOR_CUSTOM_INTEL_HINGE) += hid-sensor-custom-intel-hinge.o
 obj-$(CONFIG_IQS624_POS)	+= iqs624-pos.o
diff --git a/drivers/iio/position/as5600.rs b/drivers/iio/position/as5600.rs
new file mode 100644
index 000000000000..7445398c86b9
--- /dev/null
+++ b/drivers/iio/position/as5600.rs
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright (C) 2026 Muchamad Coirul Anwar <muchamadcoirulanwar@gmail.com>
+//! Driver for ams AS5600 12-bit magnetic rotary position sensor.
+//!
+//! Datasheet: https://look.ams-osram.com/m/7059eac7531a86fd/original/AS5600-DS000365.pdf
+
+use kernel::{
+    bindings::{
+        iio_chan_info_enum_IIO_CHAN_INFO_RAW,
+        iio_chan_info_enum_IIO_CHAN_INFO_SCALE,
+        iio_chan_spec,
+        iio_chan_type_IIO_ANGL, //
+    },
+    bits::{
+        bit_u8,
+        genmask_u16, //
+    },
+    device::Core,
+    error::code::{
+        EINVAL,
+        ENODATA, //
+    },
+    i2c::{
+        DeviceId,
+        Driver,
+        I2cClient,
+        IdTable, //
+    },
+    i2c_device_table,
+    iio::{
+        Device,
+        IioDriver,
+        IioVal,
+        Registered, //
+    },
+    io::Io,
+    module_i2c_driver,
+    of,
+    of_device_table,
+    prelude::*, //
+    sync::{
+        aref::ARef,
+        new_mutex,
+        Mutex, //
+    },
+};
+
+const AS5600_REG_STATUS: u8 = 0x0B;
+const AS5600_REG_RAW_ANGLE_H: u8 = 0x0C;
+
+const AS5600_STATUS_MD: u8 = bit_u8(5);
+const AS5600_RAW_ANGLE_MASK: u16 = genmask_u16(0..=11);
+
+module_i2c_driver! {
+    type: As5600,
+    name: "as5600",
+    authors: ["Muchamad Coirul Anwar"],
+    description: "I2C Driver for ams OSRAM AS5600 Magnetic Rotary Position Sensor",
+    license: "GPL",
+}
+
+i2c_device_table!(
+    I2C_TABLE,
+    MODULE_I2C_TABLE,
+    <As5600 as Driver>::IdInfo,
+    [(DeviceId::new(c"as5600"), ())]
+);
+
+of_device_table!(
+    OF_TABLE,
+    MODULE_OF_TABLE,
+    <As5600 as Driver>::IdInfo,
+    [(of::DeviceId::new(c"ams,as5600"), ())]
+);
+
+struct As5600Channels([iio_chan_spec; 1]);
+
+// SAFETY: `iio_chan_spec` is a plain C struct (all fields are integers/pointers)
+// with no interior mutability. The static is only read after initialization,
+// making shared access safe.
+unsafe impl Sync for As5600Channels {}
+
+static AS5600_CHANNELS: As5600Channels = As5600Channels({
+    // SAFETY: `iio_chan_spec` is a repr(C) struct where all-zeroes is valid
+    // (integers default to 0, pointers to NULL).
+    let mut chan: iio_chan_spec = unsafe { core::mem::zeroed() };
+    chan.type_ = iio_chan_type_IIO_ANGL;
+    // TODO: Use kernel::bits equivalent once bit_usize exists
+    chan.info_mask_separate = (1usize << iio_chan_info_enum_IIO_CHAN_INFO_RAW)
+        | (1usize << iio_chan_info_enum_IIO_CHAN_INFO_SCALE);
+    [chan]
+});
+
+#[pin_data]
+struct As5600Priv {
+    #[pin]
+    io_lock: Mutex<As5600HwState>,
+}
+
+struct As5600HwState {
+    client: ARef<I2cClient>,
+}
+
+impl IioDriver for As5600Priv {
+    fn read_raw(&self, _chan: *const iio_chan_spec, mask: isize) -> Result<IioVal> {
+        const INFO_RAW: isize = iio_chan_info_enum_IIO_CHAN_INFO_RAW as isize;
+        const INFO_SCALE: isize = iio_chan_info_enum_IIO_CHAN_INFO_SCALE as isize;
+        match mask {
+            // IIO_CHAN_INFO_RAW: read the 12-bit raw angle value.
+            INFO_RAW => {
+                let hw = self.io_lock.lock();
+
+                // Read status register to verify magnet presence before
+                // reading the angle.
+                let status = hw.client.try_read8(AS5600_REG_STATUS as usize)?;
+
+                // Check magnet presence (MD bit). Without a magnet the angle
+                // register contains stale/invalid data.
+                if (status & AS5600_STATUS_MD) == 0 {
+                    return Err(ENODATA);
+                }
+
+                // Word read at register 0x0C: SMBus read_word_data returns LE,
+                // AS5600 stores angle big-endian, so swap_bytes() is needed.
+                // Mutex ensures status + angle read is atomic.
+                // NOTE: Equivalent to C's i2c_smbus_read_word_swapped().
+                // Long-term, regmap-rs with val_format_endian=Big handles
+                // this transparently at configuration level.
+                let raw = hw.client.try_read16(AS5600_REG_RAW_ANGLE_H as usize)?;
+                let angle = raw.swap_bytes() & AS5600_RAW_ANGLE_MASK;
+                Ok(IioVal::Int(angle as i32))
+            }
+            // IIO_CHAN_INFO_SCALE: radians per LSB, 2*pi / 4096 = 0.001533981.
+            INFO_SCALE => {
+                Ok(IioVal::IntPlusNano(0, 1533981))
+            }
+            _ => Err(EINVAL),
+        }
+    }
+
+    fn channels(&self) -> &[iio_chan_spec] {
+        &AS5600_CHANNELS.0
+    }
+}
+
+#[pin_data]
+struct As5600 {
+    #[pin]
+    _iio_dev: Device<As5600Priv, Registered>,
+}
+
+impl Driver for As5600 {
+    type IdInfo = ();
+    type Data<'bound> = As5600;
+
+    const I2C_ID_TABLE: Option<IdTable<Self::IdInfo>> = Some(&I2C_TABLE);
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+
+    #[allow(refining_impl_trait)]
+    fn probe<'bound>(
+        dev: &'bound I2cClient<Core<'_>>,
+        _id_info: Option<&'bound Self::IdInfo>,
+    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
+        try_pin_init!(As5600 {
+            _iio_dev: {
+                let client: ARef<I2cClient> = dev.into();
+
+                let priv_init = pin_init!(As5600Priv {
+                    io_lock <- new_mutex!(As5600HwState {
+                       client
+                    }),
+                });
+
+                let iio_dev = Device::build_device(dev.as_ref(), c"as5600", priv_init)?;
+                let registered = iio_dev.register(&crate::THIS_MODULE)?;
+                dev_dbg!(dev.as_ref(), "AS5600 magnetic position sensor ready\n");
+                registered
+            }
+        })
+    }
+}
-- 
2.50.0


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

* Re: [RFC PATCH v4 0/3] iio: position: add Rust driver for ams AS5600
  2026-07-07 15:15 [RFC PATCH v4 0/3] iio: position: add Rust driver for ams AS5600 Muchamad Coirul Anwar
                   ` (2 preceding siblings ...)
  2026-07-07 15:15 ` [RFC PATCH v4 3/3] iio: position: add Rust driver for ams AS5600 Muchamad Coirul Anwar
@ 2026-07-08 10:36 ` Miguel Ojeda
  2026-07-08 12:37   ` Muchamad Coirul Anwar
  3 siblings, 1 reply; 10+ messages in thread
From: Miguel Ojeda @ 2026-07-08 10:36 UTC (permalink / raw)
  To: Muchamad Coirul Anwar
  Cc: jic23, lars, linux-iio, linux-kernel, linux-i2c, andi.shyti,
	wsa+renesas, ojeda, dakr, igor.korotin, branstj, rust-for-linux

On Tue, Jul 7, 2026 at 5:17 PM Muchamad Coirul Anwar
<muchamadcoirulanwar@gmail.com> wrote:
>
> This is v4 of the Rust driver for the ams AS5600 12-bit magnetic rotary
> position sensor. v3 hardened the IIO abstraction layer and introduced
> kernel synchronisation primitives. This revision simplifies the driver
> substantially based on review feedback, removes components that were
> over-engineered for a single-bus 12-bit sensor, and validates the result
> with comprehensive hardware testing.

Please Cc rust-for-linux as well for patch series that contain Rust -- thanks!

Cheers,
Miguel

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

* Re: [RFC PATCH v4 0/3] iio: position: add Rust driver for ams AS5600
  2026-07-08 10:36 ` [RFC PATCH v4 0/3] " Miguel Ojeda
@ 2026-07-08 12:37   ` Muchamad Coirul Anwar
  0 siblings, 0 replies; 10+ messages in thread
From: Muchamad Coirul Anwar @ 2026-07-08 12:37 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: jic23, lars, linux-iio, linux-kernel, linux-i2c, andi.shyti,
	wsa+renesas, ojeda, dakr, igor.korotin, branstj, rust-for-linux

> On Tue, Jul 7, 2026 at 5:17 PM Muchamad Coirul Anwar
> <muchamadcoirulanwar@gmail.com> wrote:
> >
> > This is v4 of the Rust driver for the ams AS5600 12-bit magnetic rotary
> > position sensor. v3 hardened the IIO abstraction layer and introduced
> > kernel synchronisation primitives. This revision simplifies the driver
> > substantially based on review feedback, removes components that were
> > over-engineered for a single-bus 12-bit sensor, and validates the result
> > with comprehensive hardware testing.
>
> Please Cc rust-for-linux as well for patch series that contain Rust -- thanks!
>
> Cheers,
> Miguel

Thanks Miguel, I missed it.

Coirul

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

* Re: [RFC PATCH v4 1/3] i2c: rust: implement SMBus read abstraction via kernel::io::Io for I2cClient
  2026-07-07 15:15 ` [RFC PATCH v4 1/3] i2c: rust: implement SMBus read abstraction via kernel::io::Io for I2cClient Muchamad Coirul Anwar
@ 2026-07-11 10:05   ` Igor Korotin
  2026-07-11 12:05     ` Danilo Krummrich
  2026-07-11 12:08   ` Danilo Krummrich
  1 sibling, 1 reply; 10+ messages in thread
From: Igor Korotin @ 2026-07-11 10:05 UTC (permalink / raw)
  To: jic23, lars
  Cc: linux-iio, linux-kernel, linux-i2c, andi.shyti, wsa+renesas,
	ojeda, dakr, branstj, Muchamad Coirul Anwar

Thanks for reworking this to use `Io` as agreed -- the direction is right.
But I think the fix is incomplete, and it points at something I'd like
Danilo's take on.

try_read8()/try_read16() are overridden here with real errno handling,
bypassing IoCapable::io_read entirely -- necessary, since io_read's
signature (-> T, not Result<T>) can't carry a negative errno from
i2c_smbus_read_byte_data.

But that override only covers two of Io's entry points. The generic
try_read<T, L>, try_write<T, L>, try_update<T, L, F>, and try_write_reg
all still route straight through IoCapable::io_read/io_write and aren't
overridden here. Concretely:

- client.try_read::<u8, _>(offset) (as opposed to try_read8()) silently
  casts a negative errno to a garbage u8 on failure -- the exact bug
  try_read8() exists to avoid, reachable through a different, still-public
  method on the same type.
- try_write8()/try_write16() aren't overridden at all, so any write
  through them (or the generic try_write) discards the real SMBus return
  value and reports Ok(()) even when the transaction failed on the bus
  (NACK, arbitration loss, timeout).
- try_update() combines both problems in a single read-modify-write.

This driver only calls try_read8()/try_read16(), so AS5600 itself isn't
affected in practice. But the abstraction being introduced here would be
unsound the moment any write-capable consumer reaches for it -- and I
don't think patching try_write8/try_update one at a time is the right
fix, since it just leaves the same trap for whichever method nobody's
gotten around to overriding yet.

Danilo -- since using Io for I2C was originally your suggestion, I'd like
your read on this before we go further: IoCapable::io_read/io_write are
infallible by signature, which holds for MMIO/PCI-config (bounds-checked
implies success) but doesn't hold for a bus transaction -- I2C can
genuinely fail per-transfer regardless of address validity. Given that,
is Io/IoCapable the right abstraction for I2cClient to implement at all,
or does I2C need its own fallible-native interface rather than overriding
pieces of this one?

Cheers
Igor

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

* Re: [RFC PATCH v4 1/3] i2c: rust: implement SMBus read abstraction via kernel::io::Io for I2cClient
  2026-07-11 10:05   ` Igor Korotin
@ 2026-07-11 12:05     ` Danilo Krummrich
  0 siblings, 0 replies; 10+ messages in thread
From: Danilo Krummrich @ 2026-07-11 12:05 UTC (permalink / raw)
  To: Igor Korotin
  Cc: jic23, lars, linux-iio, linux-kernel, linux-i2c, andi.shyti,
	wsa+renesas, ojeda, branstj, Muchamad Coirul Anwar

On Sat Jul 11, 2026 at 12:05 PM CEST, Igor Korotin wrote:
> Thanks for reworking this to use `Io` as agreed -- the direction is right.
> But I think the fix is incomplete, and it points at something I'd like
> Danilo's take on.

The reason this should go through the generic I/O backend infrastructure is that
we want this to be able take advantage of the register!() framework.

You are right that currently fallible I/O is not supported. However, this should
easily be fixable by rebasing on top of the latest I/O work [1], plus the
adjustments in [2].

With this, the posted code becomes something like this:

	pub struct I2cBackend;
	
	pub struct I2cView<'a, T: ?Sized> {
	    client: &'a I2cClient<Bound>,
	    ptr: *mut T,
	}
	
	impl<T: ?Sized> Copy for I2cView<'_, T> {}
	
	impl<T: ?Sized> Clone for I2cView<'_, T> { ... }
	
	impl IoBackend for I2cBackend {
	    type View<'a, T: ?Sized + KnownSize> = I2cView<'a, T>;
	
	    fn as_ptr<'a, T: ?Sized + KnownSize>(view: Self::View<'a, T>) -> *mut T {
	        view.ptr  // fake pointer for the register offset
	    }
	
	    unsafe fn project_view<'a, T, U>(
	        view: Self::View<'a, T>,
	        ptr: *mut U,
	    ) -> Self::View<'a, U> {
	        I2cView { client: view.client, ptr }
	    }
	}
	
	impl FallibleIoCapable<u8> for I2cBackend {
	    fn io_try_read<'a>(view: I2cView<'a, u8>) -> Result<u8> {
	        ...
	        view.client.smbus_read_byte_data(offset)
	    }
	
	    fn io_try_write<'a>(view: I2cView<'a, u8>, value: u8) -> Result {
	        ...
	        view.client.smbus_write_byte_data(offset, value)
	    }
	}

[1] https://lore.kernel.org/driver-core/20260706-io_projection-v6-0-72cd5d055d54@garyguo.net/
[2] FallibleIoCapable:

diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 7c9f7b85bca3..d97f58c968e7 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -275,6 +276,36 @@ pub trait IoCapable<T>: IoBackend {
     fn io_write<'a>(view: Self::View<'a, T>, value: T);
 }

+/// Fallible counterpart of [`IoCapable`] for I/O backends where operations can fail at the
+/// transport level (e.g. I2C, SPI).
+///
+/// Infallible backends ([`IoCapable`] implementors) get this for free via blanket implementation.
+/// Fallible-only backends implement this trait directly without implementing [`IoCapable`]; the
+/// infallible [`Io::read`], [`Io::write`], and [`Io::update`] methods will then be unavailable,
+/// enforcing that callers use the `try_*` variants instead.
+pub trait FallibleIoCapable<T>: IoBackend {
+    /// Performs an I/O read of type `T` at `view` and returns the result, or an error if the
+    /// transport-level operation fails.
+    fn io_try_read<'a>(view: Self::View<'a, T>) -> Result<T>;
+
+    /// Performs an I/O write of `value` at `view`, or returns an error if the transport-level
+    /// operation fails.
+    fn io_try_write<'a>(view: Self::View<'a, T>, value: T) -> Result;
+}
+
+impl<B: IoCapable<T>, T> FallibleIoCapable<T> for B {
+    #[inline(always)]
+    fn io_try_read<'a>(view: Self::View<'a, T>) -> Result<T> {
+        Ok(Self::io_read(view))
+    }
+
+    #[inline(always)]
+    fn io_try_write<'a>(view: Self::View<'a, T>, value: T) -> Result {
+        Self::io_write(view, value);
+        Ok(())
+    }
+}
+
 /// Trait indicating that an I/O backend supports memory copy operations.
 pub trait IoCopyable: IoBackend {
     /// Copy contents of `view` to `buffer`.
@@ -644,7 +675,7 @@ fn copy_to_slice(self, data: &mut [u8])
     fn try_read8(self, offset: usize) -> Result<u8>
     where
         usize: IoLoc<Self::Target, u8, IoType = u8>,
-        Self::Backend: IoCapable<u8>,
+        Self::Backend: FallibleIoCapable<u8>,
     {
         self.try_read(offset)
     }
@@ -654,7 +685,7 @@ fn try_read8(self, offset: usize) -> Result<u8>
     fn try_read16(self, offset: usize) -> Result<u16>
     where
         usize: IoLoc<Self::Target, u16, IoType = u16>,
-        Self::Backend: IoCapable<u16>,
+        Self::Backend: FallibleIoCapable<u16>,
     {
         self.try_read(offset)
     }
@@ -664,7 +695,7 @@ fn try_read16(self, offset: usize) -> Result<u16>
     fn try_read32(self, offset: usize) -> Result<u32>
     where
         usize: IoLoc<Self::Target, u32, IoType = u32>,
-        Self::Backend: IoCapable<u32>,
+        Self::Backend: FallibleIoCapable<u32>,
     {
         self.try_read(offset)
     }
@@ -674,7 +705,7 @@ fn try_read32(self, offset: usize) -> Result<u32>
     fn try_read64(self, offset: usize) -> Result<u64>
     where
         usize: IoLoc<Self::Target, u64, IoType = u64>,
-        Self::Backend: IoCapable<u64>,
+        Self::Backend: FallibleIoCapable<u64>,
     {
         self.try_read(offset)
     }
@@ -684,7 +715,7 @@ fn try_read64(self, offset: usize) -> Result<u64>
     fn try_write8(self, value: u8, offset: usize) -> Result
     where
         usize: IoLoc<Self::Target, u8, IoType = u8>,
-        Self::Backend: IoCapable<u8>,
+        Self::Backend: FallibleIoCapable<u8>,
     {
         self.try_write(offset, value)
     }
@@ -694,7 +725,7 @@ fn try_write8(self, value: u8, offset: usize) -> Result
     fn try_write16(self, value: u16, offset: usize) -> Result
     where
         usize: IoLoc<Self::Target, u16, IoType = u16>,
-        Self::Backend: IoCapable<u16>,
+        Self::Backend: FallibleIoCapable<u16>,
     {
         self.try_write(offset, value)
     }
@@ -704,7 +735,7 @@ fn try_write16(self, value: u16, offset: usize) -> Result
     fn try_write32(self, value: u32, offset: usize) -> Result
     where
         usize: IoLoc<Self::Target, u32, IoType = u32>,
-        Self::Backend: IoCapable<u32>,
+        Self::Backend: FallibleIoCapable<u32>,
     {
         self.try_write(offset, value)
     }
@@ -714,7 +745,7 @@ fn try_write32(self, value: u32, offset: usize) -> Result
     fn try_write64(self, value: u64, offset: usize) -> Result
     where
         usize: IoLoc<Self::Target, u64, IoType = u64>,
-        Self::Backend: IoCapable<u64>,
+        Self::Backend: FallibleIoCapable<u64>,
     {
         self.try_write(offset, value)
     }
@@ -826,10 +857,10 @@ fn write64(self, value: u64, offset: usize)
     fn try_read<T, L>(self, location: L) -> Result<T>
     where
         L: IoLoc<Self::Target, T>,
-        Self::Backend: IoCapable<L::IoType>,
+        Self::Backend: FallibleIoCapable<L::IoType>,
     {
         let view = io_view::<Self, L::IoType>(self, location.offset())?;
-        Ok(Self::Backend::io_read(view).into())
+        Ok(Self::Backend::io_try_read(view)?.into())
     }

     /// Generic fallible write with runtime bounds check.
@@ -859,12 +890,11 @@ fn try_read<T, L>(self, location: L) -> Result<T>
     fn try_write<T, L>(self, location: L, value: T) -> Result
     where
         L: IoLoc<Self::Target, T>,
-        Self::Backend: IoCapable<L::IoType>,
+        Self::Backend: FallibleIoCapable<L::IoType>,
     {
         let view = io_view::<Self, L::IoType>(self, location.offset())?;
         let io_value = value.into();
-        Self::Backend::io_write(view, io_value);
-        Ok(())
+        Self::Backend::io_try_write(view, io_value)
     }

     /// Generic fallible write of a fully-located register value.
@@ -904,7 +934,7 @@ fn try_write_reg<T, L, V>(self, value: V) -> Result
     where
         L: IoLoc<Self::Target, T>,
         V: LocatedRegister<Self::Target, Location = L, Value = T>,
-        Self::Backend: IoCapable<L::IoType>,
+        Self::Backend: FallibleIoCapable<L::IoType>,
     {
         let (location, value) = value.into_io_op();

@@ -937,16 +967,14 @@ fn try_write_reg<T, L, V>(self, value: V) -> Result
     fn try_update<T, L, F>(self, location: L, f: F) -> Result
     where
         L: IoLoc<Self::Target, T>,
-        Self::Backend: IoCapable<L::IoType>,
+        Self::Backend: FallibleIoCapable<L::IoType>,
         F: FnOnce(T) -> T,
     {
         let view = io_view::<Self, L::IoType>(self, location.offset())?;

-        let value: T = Self::Backend::io_read(view).into();
+        let value: T = Self::Backend::io_try_read(view)?.into();
         let io_value = f(value).into();
-        Self::Backend::io_write(view, io_value);
-
-        Ok(())
+        Self::Backend::io_try_write(view, io_value)
     }

     /// Generic infallible read with compile-time bounds check.

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

* Re: [RFC PATCH v4 1/3] i2c: rust: implement SMBus read abstraction via kernel::io::Io for I2cClient
  2026-07-07 15:15 ` [RFC PATCH v4 1/3] i2c: rust: implement SMBus read abstraction via kernel::io::Io for I2cClient Muchamad Coirul Anwar
  2026-07-11 10:05   ` Igor Korotin
@ 2026-07-11 12:08   ` Danilo Krummrich
  1 sibling, 0 replies; 10+ messages in thread
From: Danilo Krummrich @ 2026-07-11 12:08 UTC (permalink / raw)
  To: Muchamad Coirul Anwar
  Cc: jic23, lars, linux-iio, linux-kernel, linux-i2c, andi.shyti,
	wsa+renesas, ojeda, igor.korotin, branstj

On Tue Jul 7, 2026 at 5:15 PM CEST, Muchamad Coirul Anwar wrote:
> +impl<Ctx: device::DeviceContext> IoCapable<u8> for I2cClient<Ctx> {

Independent from [1], this must not be implemented for any DeviceContext. I/O
operations require at least the Bound the device context.

[1 https://lore.kernel.org/all/DJVQ852J7SOH.26YBIJTQ9B66G@kernel.org/]

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

* Re: [RFC PATCH v4 2/3] rust: add minimal IIO subsystem abstractions
  2026-07-07 15:15 ` [RFC PATCH v4 2/3] rust: add minimal IIO subsystem abstractions Muchamad Coirul Anwar
@ 2026-07-11 12:12   ` Danilo Krummrich
  0 siblings, 0 replies; 10+ messages in thread
From: Danilo Krummrich @ 2026-07-11 12:12 UTC (permalink / raw)
  To: Muchamad Coirul Anwar
  Cc: jic23, lars, linux-iio, linux-kernel, linux-i2c, andi.shyti,
	wsa+renesas, ojeda, igor.korotin, branstj

On Tue Jul 7, 2026 at 5:15 PM CEST, Muchamad Coirul Anwar wrote:
> Add safe Rust wrappers for the Linux IIO (Industrial I/O) subsystem:

Does IIO's iio_device_unregister() synchronize against in-flight IOCTLs?

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

end of thread, other threads:[~2026-07-11 12:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07 15:15 [RFC PATCH v4 0/3] iio: position: add Rust driver for ams AS5600 Muchamad Coirul Anwar
2026-07-07 15:15 ` [RFC PATCH v4 1/3] i2c: rust: implement SMBus read abstraction via kernel::io::Io for I2cClient Muchamad Coirul Anwar
2026-07-11 10:05   ` Igor Korotin
2026-07-11 12:05     ` Danilo Krummrich
2026-07-11 12:08   ` Danilo Krummrich
2026-07-07 15:15 ` [RFC PATCH v4 2/3] rust: add minimal IIO subsystem abstractions Muchamad Coirul Anwar
2026-07-11 12:12   ` Danilo Krummrich
2026-07-07 15:15 ` [RFC PATCH v4 3/3] iio: position: add Rust driver for ams AS5600 Muchamad Coirul Anwar
2026-07-08 10:36 ` [RFC PATCH v4 0/3] " Miguel Ojeda
2026-07-08 12:37   ` Muchamad Coirul Anwar

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