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

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