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).
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.
Obviously, the biggest con is code complexity (not that bad though) as we always
need to properly sync any backend callback.
- Nuno Sá
>
