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

On Tue, 4 Aug 2026 at 21:56, Nuno S=C3=A1 <[email protected]> wrote:
>
> Hi Coirul,
>
> Same as Jonathan on my rust capabilities (maybe now I'll have proper
> motivation to learn it :)).
>
> Just one question below...

Hi Nuno,

Thanks for taking the time to look at this. I'm still learning too,
especially on the kernel side.


> > +impl IioDriver for As5600Priv {
> > +    fn read_raw(&self, _chan: *const iio_chan_spec, mask: isize) -> Re=
sult<IioVal> {
> > +        const INFO_RAW: isize =3D iio_chan_info_enum_IIO_CHAN_INFO_RAW=
 as isize;
> > +        const INFO_SCALE: isize =3D iio_chan_info_enum_IIO_CHAN_INFO_S=
CALE as isize;
>
> Likely stupid question but do we always need the above? Some drivers migh=
t have
> several INFO_* bits sets and we often need to nest the match if multiple =
channels have the
> same bits. For those cases having to define all of them like this and
> then doing the match seems not great to me. I guess we can directly have
> ` iio_chan_info_enum_IIO_CHAN_INFO_RAW as isize` in match but not sure
> if it's also that neat...
>
> So bottom line, it would be nice if we could have something nicely
> readable that we could directly pass to match.

On the const definitions, yes, they are necessary in Rust. Your point
about scalability is well taken though. AS5600 is simple enough that I
didn't notice this, but for a driver with multiple channels and
several INFO bits the const list grows quickly. Brandon Saint-John
raised a related concern in v3 [1] and suggested wrapping the
iio_chan_info_enum_* variants in a Rust #[repr] enum. I didn't get to
it in v4 and I'm planning to address it in a future patch.

> On another topic, I believe mask is present in several drivers for
> historic reasons but note that this parameter is not really a mask. So
> other name like 'info' is more appropriate.
>
> - Nuno S=C3=A1

On mask vs info, you're right that mask implies a bitmask operation
when what arrives is a single enum index. I'll rename it to info for
v5.

Coirul

[1] https://lore.kernel.org/all/[email protected]/