Re: [RFC PATCH v3 3/4] iio: position: add Rust driver for ams AS5600

[Brandon Saint-John]

On Sun, 24 May 2026 20:28:22 +0700 Muchamad Coirul Anwar <muchamadcoirulanwar@xxxxxxxxx> wrote:

> +//! Driver for ams AS5600 12-bit magnetic rotary position sensor.
> +//!
> +//! Datasheet: https://ams.com/documents/20143/36005/AS5600_DS000365_5-00.pdf

Small nitpick, but going to the above link doesn't resolve to a pdf.
The link that works as of today for me is:

https://look.ams-osram.com/m/7059eac7531a86fd/original/AS5600-DS000365.pdf

> +fn err_enodata() -> Error {
> +    Error::from_errno(-(ENODATA as i32))
> +}

Later on it's probably better to add ENODATA to kernel::error::code instead of the helper.

> +#[derive(Clone, Copy)]
> +struct As5600Io(*mut i2c_client);
> +

You can replace the *mut i2c_client with an ARef<I2cClient>. The
kernel::impl_device_context_into_aref! macro is run on &I2cClient<Core>
so you can call ARef::from on dev instead of casting it to the raw pointer
to hold it.

Then it saves you from repeating a few different parts, like redoing unsafe impls,
recasting back to I2cClient<Core> in try_readN, etc.

> +impl IoCapable<u8> for As5600Io {}
> +impl IoCapable<u16> for As5600Io {}

None of the read_u16 or IoCapable<u16> are used at this point, so those traits/methods
could be dropped.

As a side note, in the most recent rust-next branch, there are a few changes with IoCapable
so maybe worth rebasing at some point to get those changes. IoCapable isn't a marker trait
anymore so I get compile errors trying to rebase there.

> +#[pin_data]
> +struct As5600Priv<T> {
> +    #[pin]
> +    io_lock: Mutex<As5600HwState<T>>,
> +    channels: KBox<[iio_chan_spec; 1]>,
> +}
> +
> +/// Encapsulates the I/O interface and its runtime health state.
> +///
> +/// This prevents operations on a known-dead bus (Circuit Breaker pattern).
> +struct As5600HwState<T> {
> +    io: T,
> +    state: DeviceState,
> +}

Could As5600Priv/HwState hold the As5600Io directly instead of a generic T?
As5600Priv was not generic over T in v2, and since As5600Io implements Io/IoCapable
and the sensor only uses the I2C bus it doesn't seem like it needs to be generic,
at least at the moment.

> +impl<T: Io + IoCapable<u8> + Unpin> IioDriver for As5600Priv<T> {
> +    fn read_raw(&self, _chan: *const iio_chan_spec, mask: isize) -> Result<IioVal> {
> +        match mask {
> +            // IIO_CHAN_INFO_RAW — read the 12-bit raw angle value.
> +            m if m == iio_chan_info_enum_IIO_CHAN_INFO_RAW as isize => {

Ideally in the future, the iio_chan_info_enum_* variants can be wrapped in an Rust
enum with an #[repr] attribute. At this stage, match isn't as useful as it could be
since you still need to call "if m == ...".

Sent using hkml (https://github.com/sjp38/hackermail)