Re: [PATCH v4 6/8] iio: add the IIO backend framework

["Nuno Sá" <noname.nuno@gmail.com>]

On Wed, 2024-01-10 at 09:16 +0000, Jonathan Cameron wrote:
> On Tue, 09 Jan 2024 13:15:54 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
> 
> > On Tue, 2023-12-26 at 15:59 +0000, Jonathan Cameron wrote:
> > >   
> > > > > > +
> > > > > > +       ret = devm_add_action_or_reset(dev, iio_backend_release,
> > > > > > back);
> > > > > > +       if (ret)
> > > > > > +               return ret;
> > > > > > +
> > > > > > +       link = device_link_add(dev, back->dev,
> > > > > > DL_FLAG_AUTOREMOVE_CONSUMER);
> > > > > > +       if (!link)
> > > > > > +               pr_warn("%s: Could not link to supplier(%s)\n",
> > > > > > dev_name(dev),
> > > > > > +                       dev_name(back->dev));    
> > > > > 
> > > > > Why is that not an error and we try to carry on?    
> > > > 
> > > > I guess having the links are not really mandatory for the whole thing to
> > > > work (more
> > > > like a nice to have). That's also how this is handled in another
> > > > subsystems
> > > > so I
> > > > figured it would be fine.
> > > > 
> > > > But since you are speaking about this... After you pointing me to
> > > > Bartosz's
> > > > talk and
> > > > sawing it (as stuff like this is mentioned), I started to question this.
> > > > The
> > > > goal
> > > > with the above comment is that if you remove the backend, all the
> > > > consumers
> > > > are
> > > > automatically removed (unbound). Just not sure if that's what we always
> > > > want
> > > > (and we
> > > > are already handling the situation where a backend goes away with -
> > > > ENODEV)
> > > > as some
> > > > frontends could still be useful without the backend (I guess that could
> > > > be
> > > > plausible). I think for now we don't really have such usecases so the
> > > > links
> > > > can make
> > > > sense (and we can figure something like optionally creating these links
> > > > if
> > > > we ever
> > > > need too) but having your inputs on this will definitely be valuable.  
> > > 
> > > I'm not keen on both trying to make everything tear down cleanly AND
> > > making
> > > sure
> > > it all works even if we don't. That just adds two code paths to test when
> > > either
> > > should be sufficient on its own.  I don't really mind which.  Bartosz's
> > > stuff  
> > 
> > Agreed...
> > 
> > > is nice, but it may not be the right solution here.   
> > 
> > There's pros and cons on both options... 
> > 
> > For the device links the cons I see is that it depends on patch 3 for it to
> > work
> > (or some other approach if the one in that patch is not good) - not really a
> > real con though :). The biggest concern is (possible) future uses where we
> > end
> > up with cases where removing a backend is not really a "deal breaker". I
> > could
> > think of frontends that have multiple backends (one per data path) and
> > removing
> > one backend would not tear the whole thing down (we would just have one non
> > functional data paths/port where the others are still ok).
> 
> I wouldn't waste time catering to such set ups.  If the whole thing gets
> torn down because one element went away that should be fine.
> To do anything else I'd want to see a real world use case.

Fair enough...

> 
> > 
> > Olivier, for STM usecases, do we always need the backend? I mean, does it
> > make
> > sense to always remove/unbind the frontend in case the backend is unbound?
> > 
> > Maybe some of your usecases already "forces" us with a decision. 
> > 
> > The biggest pro I see is code simplicity. If we can assume the frontend can
> > never exist in case one of the backends is gone, we can:
> > 
> >  * get rid of the sync mutex;
> >  * get rid of the kref and bind the backend object lifetime to the backend
> > device (using devm_kzalloc() instead of kzalloc + refcount.
> > 
> > Basically, we would not need to care about syncing the backend existence
> > with
> > accessing it...
> > To sum up, the device_links approach tends to be similar (not identical) to
> > the
> > previous approach using the component API.
> > 
> > The biggest pro I see in Bartosz's stuff is flexibility. So it should just
> > work
> > in whatever future usecases we might have. I fear that going the
> > device_links
> > road we might end up needing this stuff anyways.
> 
> I'm keen on it if it simplifies code or becomes the dominant paradigm for such
> things in the kernel (so becomes what people expect).  That isn't true yet
> and I doubt it will be particularly soon.  If things head that way we can
> revisit as it would enable things that currently we don't support - nothing
> should break.
> 

Well, If I'm not missing anything, simpler code would be with device_links so I
guess that's your preferred approach for now :). Also fine by me as this is an
in kernel interface so we easily revisit it.

I'll just wait a bit more (before sending v5) to see if Olivier has any
foreseeable usecase where device_links would be an issue.

- Nuno Sá