Received-SPF: pass (google.com: domain of linux-kernel+bounces-196416-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <d8d95f957f465148f0ddb6eae87159a2394cf2e9.camel@gmail.com>
Subject: Re: [PATCH RFC v2 1/8] spi: dt-bindings: spi-peripheral-props: add
 spi-offloads property
From: Nuno =?ISO-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
To: Conor Dooley <conor@kernel.org>
Cc: David Lechner <dlechner@baylibre.com>, Mark Brown <broonie@kernel.org>, 
 Jonathan Cameron
	 <jic23@kernel.org>, Rob Herring <robh@kernel.org>, Krzysztof Kozlowski
	 <krzk+dt@kernel.org>, Conor Dooley <conor+dt@kernel.org>, Nuno
 =?ISO-8859-1?Q?S=E1?=
	 <nuno.sa@analog.com>, Michael Hennerich <Michael.Hennerich@analog.com>, 
 Lars-Peter Clausen
	 <lars@metafoo.de>, David Jander <david@protonic.nl>, Martin Sperl
	 <kernel@martin.sperl.org>, linux-spi@vger.kernel.org, 
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, 
	linux-iio@vger.kernel.org
Date: Fri, 31 May 2024 09:39:19 +0200
In-Reply-To: <20240530-petunia-genre-2731493dbd0f@spud>
References: <20240514-aspire-ascension-449556da3615@spud>
	 <CAMknhBFFpEGcMoLo5gsC11Syv+CwUM0mnq1yDMUzL1uutUtB+Q@mail.gmail.com>
	 <20240516-rudder-reburial-dcf300504c0a@spud>
	 <CAMknhBF_s0btus4yqPe-T=F3z7Asi9KkRGsGr7FHDFi=k4EQjw@mail.gmail.com>
	 <20240519-abreast-haziness-096a57ef57d3@spud>
	 <CAMknhBHvEse2FyDoBXR1PvymGpSGq8dotKfm+8XH+0+k+xKtQw@mail.gmail.com>
	 <20240522-gullible-ibuprofen-cf9111c25f6f@spud>
	 <5ad0b5782434eaf4cf565cffb0e4c14b7414ae38.camel@gmail.com>
	 <20240526-peculiar-panama-badda4f02336@spud>
	 <10991373cb9603803df63d8236c475807f6dde68.camel@gmail.com>
	 <20240530-petunia-genre-2731493dbd0f@spud>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.52.1 (3.52.1-1.fc40) 
Precedence: bulk
MIME-Version: 1.0

On Thu, 2024-05-30 at 20:18 +0100, Conor Dooley wrote:
> On Wed, May 29, 2024 at 10:07:37AM +0200, Nuno S=C3=A1 wrote:
> > On Sun, 2024-05-26 at 18:35 +0100, Conor Dooley wrote:
> > > On Thu, May 23, 2024 at 02:15:35PM +0200, Nuno S=C3=A1 wrote:
> > > > On Wed, 2024-05-22 at 19:24 +0100, Conor Dooley wrote:
>=20
> > > > Taking the
> > > > trigger (PWM) as an example and even when it is directly connected =
with the
> > > > offload
> > > > block, the peripheral still needs to know about it. Think of sampli=
ng
> > > > frequency...
> > > > The period of the trigger signal is strictly connected with the sam=
pling
> > > > frequency of
> > > > the peripheral for example. So I see 2 things:
> > > >=20
> > > > 1) Enabling/Disabling the trigger could be easily done from the per=
ipheral
> > > > even
> > > > with
> > > > the resource in the spi engine. I think David already has some code=
 in the
> > > > series
> > > > that would make this trivial and so having the property in the spi =
controller
> > > > brings
> > > > no added complexity.
> > > >=20
> > > > 2) Controlling things like the trigger period/sample_rate. This cou=
ld be
> > > > harder
> > > > to do
> > > > over SPI (or making it generic enough) so we would still need to ha=
ve the
> > > > same
> > > > property on the peripheral (even if not directly connected to it). =
I kind of
> > > > agree
> > > > with David that having the property both in the peripheral and cont=
roller is
> > > > a
> > > > bit
> > > > weird.
> > >=20
> > > Can you explain what you mean by "same property on the peripheral"? I
> > > would expect a peripheral to state its trigger period (just like how =
it
> > > states the max frequency) and for the trigger period not to appear in
> > > the controller.
> > >=20
> >=20
> > Just have the same 'pwms' property on both the controller and periphera=
l...
>=20
> Yeah, no... Opinion unchanged since my last message.
>=20

..

> >=20
>=20
> If only we had another user... I suppose you lads are the market leader
> in these kinds of devices. If I did happen to know if Microchip was
> working on anything similar (which I don't, I work on FPGAs not these
> kinds of devices) I couldn't even tell you. I suppose I could ask around
> and see. Do you know if TI is doing anything along these lines?
>=20

Unfortunately, no idea.

> > > Part of me says "sure, hook the DMAs up to the devices, as that's wha=
t
> > > happens for other IIO devices" but at the same time I recognise that =
the
> > > DMA isn't actually hooked up like that and the other IIO devices I se=
e
> > > like that are all actually on the SoC, rather than connected over SPI=
.
> >=20
> > Yeah, I know... But note (but again, only for ADI designs) that the DMA=
 role is
> > solely for carrying the peripheral data. It is done like this so everyt=
hing works
> > in
> > HW and there's no need for SW to deal with the samples at all. I mean, =
only the
> > userspace app touches the samples.
> >=20
> > TBH, the DMA is the bit that worries me the most as it may be overly co=
mplex to
> > share
> > buffers (using dma-buf or something else) from the spi controller back =
to
> > consumers
> > of it (IIO in this case). And I mean sharing in a way that there's no n=
eed to
> > touch
> > the buffers.
>=20
> <snip>
>=20
> > Maybe having an offload dedicated API (through spi) to get/share a DMA =
handle
> > would
> > be acceptable. Then we could add support to "import" it in the IIO core=
 Then it
> > would be up to the controller to accept or not to share the handle (in =
some cases
> > the
> > controller could really want to have the control of the DMA transfers).
>=20
> Yeah, that is about what I was thinking. I wasn't expecting the spi code
> to grow handing for dmabuf or anything like that, just a way for the
> offload consumer to say "yo, can you tell me what dma buffer I can
> use?". Unless (until?) there's some controller that wants to manage it,
> I think that'd be sufficient?

Yeah, I could see some kind of submit_request() API with some kind of compl=
etion
handler for this. But on the IIO side the DMA code is not that straight (ev=
en getting
more complex with dma-buf's) so I can't really tell how the whole thing wou=
ld look
like. But may be something to look at.

- Nuno S=C3=A1=20