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

Muchamad Coirul Anwar <[email protected]>
Newsgroups org.kernel.vger.linux-iio,org.kernel.vger.linux-i2c,org.kernel.vger.linux-kernel
Message-ID <CAO26r3Rxe1kVSU_JeZetvTF9Z3ChoxYj5x8rHNFwcdX+mHLuiw@mail.gmail.com>
On Tue, 4 Aug 2026 at 21:56, Nuno Sá <[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) -> 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;
>
> Likely stupid question but do we always need the above? Some drivers might 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á

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]/
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.