Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Message-ID: <bba767835e775909c6b8a3334cceeb419afef4ca.camel@gmail.com>
Subject: Re: [PATCH 03/12] iio: add the IIO backend framework
From:   Nuno =?ISO-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
To:     Jonathan Cameron <jic23@kernel.org>,
        Nuno Sa via B4 Relay 
        <devnull+nuno.sa.analog.com@kernel.org>
Cc:     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>
Date:   Wed, 06 Dec 2023 13:05:53 +0100
In-Reply-To: <20231204153855.71c9926f@jic23-huawei>
References: <20231121-dev-iio-backend-v1-0-6a3d542eba35@analog.com>
         <20231121-dev-iio-backend-v1-3-6a3d542eba35@analog.com>
         <20231204153855.71c9926f@jic23-huawei>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

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 w=
hich
> > can be "linked" against one or multiple backend devices. All the IIO an=
d
> > userspace interface is expected to be registers/managed by the frontend
> > device which will callback into the backends when needed (to get/set
> > some configuration that it does not directly control).
>=20
> As this is first place backend / frontend terminology used (I think), mak=
e
> sure to give an example so people understand what sorts of IP / devices t=
hes
> might be.
>=20
> >=20
> > The basic framework interface is pretty simple:
> > =C2=A0- Backends should register themselves with @devm_iio_backend_regi=
ster()
> > =C2=A0- Frontend devices should get backends with @devm_iio_backend_get=
()
> >=20
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
>=20
> Looks good to me in general.=C2=A0 I'll need to have a really close read =
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 thes=
e macros
> > + * is to properly lock the backend mutex on every call plus checking i=
f the
> > + * backend device is still around (by looking at the *ops pointer).
> 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

Arguably, in here, removing a backend is the user doing something seriously=
 wrong so
I see the splat with good eyes :D.

That said, I'm fine in turning this into a pr_warn_once()...

> Btw, there were some interesting discussions on lifetimes and consumer / =
provider
> models at plumbers. I think https://www.youtube.com/watch?v=3DbHaMMnIH6AM=
=C2=A0will be
> the video.=C2=A0=C2=A0 Suggested the approach of not refcounting but inst=
ead allowing for
> a deliberate removal of access similar to your check on ops here (and the=
 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.

Yes, interesting talk indeed. I have been following this issue for some tim=
e now.
That's why I tried to be careful in the backend stuff (so we don't explode =
if a
backend is gone) even though is a much more simpler approach. But the talk =
mentions
three solutions and we kind of have both option C (the pointer stuff) and o=
ption A
(consumer removed on provicer unbind)
in here. option A is being given through device links with the AUTO_REMOVE_=
CONSUMER
flag.

And the talk actually left me thinking on that (as it's discussed in there.=
 In our
simpler case (ADI ones), it does make sense to remove the consumer if the p=
rovider is
not there. But if we think in more advanced usecases (or maybe already in t=
he STM
usecase) where we have a backend per data path. Does it make sense to compl=
etely
"kill" the consumer if we remove one of the data paths? Starting to think i=
t
doesn't...

- Nuno S=C3=A1