Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Date:   Wed, 6 Dec 2023 17:15:21 +0000
From:   Jonathan Cameron <jic23@kernel.org>
To:     Nuno =?UTF-8?B?U8Oh?= <noname.nuno@gmail.com>
Cc:     Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org>,
        nuno.sa@analog.com, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, linux-iio@vger.kernel.org,
        Olivier MOYSAN <olivier.moysan@foss.st.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Frank Rowand <frowand.list@gmail.com>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Michael Hennerich <Michael.Hennerich@analog.com>
Subject: Re: [PATCH 03/12] iio: add the IIO backend framework
Message-ID: <20231206171521.4133569a@jic23-huawei>
In-Reply-To: <bba767835e775909c6b8a3334cceeb419afef4ca.camel@gmail.com>
References: <20231121-dev-iio-backend-v1-0-6a3d542eba35@analog.com>
        <20231121-dev-iio-backend-v1-3-6a3d542eba35@analog.com>
        <20231204153855.71c9926f@jic23-huawei>
        <bba767835e775909c6b8a3334cceeb419afef4ca.camel@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, 06 Dec 2023 13:05:53 +0100
Nuno S=C3=A1 <noname.nuno@gmail.com> wrote:

> On Mon, 2023-12-04 at 15:38 +0000, Jonathan Cameron wrote:
> > On Tue, 21 Nov 2023 11:20:16 +0100
> > Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:
> >  =20
> > > From: Nuno Sa <nuno.sa@analog.com>
> > >=20
> > > This is a Framework to handle complex IIO aggregate devices.
> > >=20
> > > The typical architecture is to have one device as the frontend device=
 which
> > > can be "linked" against one or multiple backend devices. All the IIO =
and
> > > userspace interface is expected to be registers/managed by the fronte=
nd
> > > device which will callback into the backends when needed (to get/set
> > > some configuration that it does not directly control). =20
> >=20
> > As this is first place backend / frontend terminology used (I think), m=
ake
> > sure to give an example so people understand what sorts of IP / devices=
 thes
> > might be.
> >  =20
> > >=20
> > > The basic framework interface is pretty simple:
> > > =C2=A0- Backends should register themselves with @devm_iio_backend_re=
gister()
> > > =C2=A0- Frontend devices should get backends with @devm_iio_backend_g=
et()
> > >=20
> > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> =20
> >=20
> > Looks good to me in general.=C2=A0 I'll need to have a really close rea=
d though
> > before we merge this as there may be sticky corners! (hopefully not)
> >=20
> >=20
> > ...
> >  =20
> > > +static LIST_HEAD(iio_back_list);
> > > +static DEFINE_MUTEX(iio_back_lock);
> > > +
> > > +/*
> > > + * Helper macros to properly call backend ops. The main point for th=
ese macros
> > > + * is to properly lock the backend mutex on every call plus checking=
 if the
> > > + * backend device is still around (by looking at the *ops pointer). =
=20
> > If just checking if it is around rather thank looking for a bug, then
> > I'd suggest a lighter choice than WARN_ON_x=20
> >  =20
>=20
> Arguably, in here, removing a backend is the user doing something serious=
ly wrong so
> I see the splat with good eyes :D.
>=20
> That said, I'm fine in turning this into a pr_warn_once()...
>=20
> > Btw, there were some interesting discussions on lifetimes and consumer =
/ provider
> > models at plumbers. I think https://www.youtube.com/watch?v=3DbHaMMnIH6=
AM=C2=A0will be
> > the video.=C2=A0=C2=A0 Suggested the approach of not refcounting but in=
stead allowing for
> > a deliberate removal of access similar to your check on ops here (and t=
he one
> > we do in core IIO for similar purposes).=C2=A0 Sounded interesting but =
I've not
> > explored what it would really mean to switch to that model yet. =20
>=20
> Yes, interesting talk indeed. I have been following this issue for some t=
ime now.
> That's why I tried to be careful in the backend stuff (so we don't explod=
e if a
> backend is gone) even though is a much more simpler approach. But the tal=
k mentions
> three solutions and we kind of have both option C (the pointer stuff) and=
 option A
> (consumer removed on provicer unbind)
> in here. option A is being given through device links with the AUTO_REMOV=
E_CONSUMER
> flag.
>=20
> And the talk actually left me thinking on that (as it's discussed in ther=
e. In our
> simpler case (ADI ones), it does make sense to remove the consumer if the=
 provider is
> not there. But if we think in more advanced usecases (or maybe already in=
 the STM
> usecase) where we have a backend per data path. Does it make sense to com=
pletely
> "kill" the consumer if we remove one of the data paths? Starting to think=
 it
> doesn't...

There is a reasonably argument that partial tear down isn't a common case. =
So
may not be worth worrying about.

J
>=20
> - Nuno S=C3=A1
>=20