Received-SPF: pass (google.com: domain of linux-kernel+bounces-23240-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <0c0b1954825dc174cab48060e96ddadadc18aefd.camel@gmail.com>
Subject: Re: [PATCH 01/13] spi: add core support for controllers with
 offload capabilities
From: Nuno =?ISO-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
To: David Lechner <dlechner@baylibre.com>, Mark Brown <broonie@kernel.org>, 
 Jonathan Cameron <jic23@kernel.org>, Rob Herring <robh+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley
 <conor+dt@kernel.org>,  Michael Hennerich <michael.hennerich@analog.com>,
 Nuno =?ISO-8859-1?Q?S=E1?= <nuno.sa@analog.com>, Frank Rowand
 <frowand.list@gmail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>, Uwe
 =?ISO-8859-1?Q?Kleine-K=F6nig?= <u.kleine-koenig@pengutronix.de>, Jonathan
 Corbet <corbet@lwn.net>,  linux-spi@vger.kernel.org,
 linux-iio@vger.kernel.org, devicetree@vger.kernel.org, 
 linux-doc@vger.kernel.org, linux-pwm@vger.kernel.org, 
 linux-kernel@vger.kernel.org
Date: Thu, 11 Jan 2024 09:49:08 +0100
In-Reply-To: <20240109-axi-spi-engine-series-3-v1-1-e42c6a986580@baylibre.com>
References: 
	<20240109-axi-spi-engine-series-3-v1-0-e42c6a986580@baylibre.com>
	 <20240109-axi-spi-engine-series-3-v1-1-e42c6a986580@baylibre.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.3 
Precedence: bulk
MIME-Version: 1.0

Hi David,


On Wed, 2024-01-10 at 13:49 -0600, David Lechner wrote:
> This adds a feature for specialized SPI controllers that can record
> a series of SPI transfers, including tx data, cs assertions, delays,
> etc. and then play them back using a hardware trigger without CPU
> intervention.
>=20
> The intended use case for this is with the AXI SPI Engine to capture
> data from ADCs at high rates (MSPS) with a stable sample period.
>=20
> Most of the implementation is controller-specific and will be handled by
> drivers that implement the offload_ops callbacks. The API follows a
> prepare/enable pattern that should be familiar to users of the clk
> subsystem.
>=20
> Consumers of this API will make calls similar to this:
>=20
> =C2=A0=C2=A0=C2=A0 /* in probe() */
> =C2=A0=C2=A0=C2=A0 offload =3D spi_offload_get(spi, 0);
> =C2=A0=C2=A0=C2=A0 ...
>=20
On top of what Mark already stated, and as we already discussed offline, I
personally don't like this provider - consumer interface for the offload. T=
he
first thing is that this is taking into account the possibility of having
multiple offload cores. While the FGPA core was designed with that in mind,=
 we
don't really have any design using multiple offloads in one spi engine (alw=
ays
one). Hence this is all pretty much untested.

If we want to already have this support, my feeling is that we should have =
a
simple integer dt property for the peripheral devices (similar to cs). When=
 a
device is being created/added, the spi core would parse this property and g=
et
it's offload index. The point is that this would all be transparent for spi
devices drivers that would only have to call the SPI API's and the core wou=
ld
make sure the right index is passed to the controller.

But honestly, IMO, I would just keep things simple for now and assume one c=
ore
per engine.

I would probably also prefer to see all the new interfaces part of the
spi_controller struct directly...

- Nuno S=C3=A1