Re: [PATCH RFC v2 6/7] iio: osf: register IIO devices from capabilities

[Kim Jinseob]

On Thu, May 28, 2026 at 11:30:00PM +0100, Jonathan Cameron wrote:

> Various things inline.

Addressed in RFC v3.

> Why?

Addressed in the v3 patch split. The core/protocol/IIO changes were
reworked so the includes match the code introduced in each patch

> This sounds like it makes a duplicate.  Rename to _isduplicate() or something like that.

Addressed in RFC v3.

> Another place a designated intializer makes sense I think. See below for
> other examples.

Addressed in RFC v3 where it made the code clearer.

> Maybe reorder so yo ucan use a designated initializer.
> Technically it will zero the values before you then fill them, but the
> code is a fair bit simpler.

Addressed in RFC v3 where it made the code clearer.

> I'd flip
>         if (!indio_dev)
>                 return 0; /* Nothing to do */

Addressed in RFC v3.

> Seems like these changes belong in the earlier patch? Can you refactor
> that to avoid having to modify it here.

Addressed in the v3 patch split.

> I'd put the found block here rather than a goto. That simpler
> flow is worth the cost of indent. You can also flip the condition
> giving

Addressed in RFC v3.

> Some of each of these comes from an existing structure reflecting what we
> read from the device, can we use that structure embedded her instead of having
> to copy field by field?

I reviewed this while preparing RFC v3. I kept the cached runtime state
separate from the wire-format decode structures, but reduced the field-by-field
copying where the decoded data did not need to be retained.

> Neither device.h nor kernel.h should be included in a driver, unless you
> are using one of the few definitions that need them. Typically there are more
> focused include like dev_printk.h which should be used instead.

Addressed in RFC v3. I removed broad includes that were not needed.
linux/device.h
remains only where device APIs are used.

> Only seems likely to be used once. So why have an macro?
> Maybe it is useful just to put it next to the others for visual comparison.

Reviewed while preparing RFC v3. I kept the channel helper macros only where
they improved consistency between the channel definitions.

> 0 is a terminating entry so no trailing comma.

Addressed in RFC v3.

> If this isn't needed to silence a compiler or similar warning: we control
> that value in the driver so should know if this can happen or not.
> So probably not needed.

Addressed in RFC v3. I removed unnecessary defensive checks for values the
driver controls.

> If you are unregistering something that is null something went wrong
> probably.  Seems unlikely we need this defence.

Addressed in RFC v3.

> Then this can be done at declaration above.

Addressed in RFC v3.

> Add a comment on what the race is (I assume) that gets us here with that not set.

Addressed in RFC v3. I added a short comment around the iio_buffer_enabled()
case.

I also addressed the Sashiko feedback for this patch in RFC v3 by fixing the
IIO_BUFFER / IIO_KFIFO_BUF dependency handling, rejecting channel_count
mismatches before pushing samples to IIO buffers, and adding locking around
cached latest samples.

Thanks,

Jinseob