From: "Nuno Sá" <noname.nuno@gmail.com>
To: David Lechner <dlechner@baylibre.com>
Cc: "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 Sá" <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
Subject: Re: [PATCH RFC v2 6/8] spi: axi-spi-engine: add offload support
Date: Wed, 22 May 2024 14:08:12 +0200 [thread overview]
Message-ID: <fbcc8bb3804d0f1f178f695cc16a1aa8dbcda1f1.camel@gmail.com> (raw)
In-Reply-To: <CAMknhBH9y=tOhHrhBCoMOSSVgZDRbX90cfzqX62m6wLYsKDhNg@mail.gmail.com>
On Tue, 2024-05-21 at 09:28 -0500, David Lechner wrote:
> On Tue, May 21, 2024 at 7:27 AM Nuno Sá <noname.nuno@gmail.com> wrote:
> >
> > On Fri, 2024-05-10 at 19:44 -0500, David Lechner wrote:
> > > This implements SPI offload support for the AXI SPI Engine. Currently,
> > > the hardware only supports triggering offload transfers with a hardware
> > > trigger so attempting to use an offload message in the regular SPI
> > > message queue will fail. Also, only allows streaming rx data to an
> > > external sink, so attempts to use a rx_buf in the offload message will
> > > fail.
> > >
> > > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > > ---
> > >
>
> ...
>
> > > +
> > > +static int spi_engine_offload_map_channel(struct spi_device *spi,
> > > + unsigned int id,
> > > + unsigned int channel)
> > > +{
> > > + struct spi_controller *host = spi->controller;
> > > + struct spi_engine *spi_engine = spi_controller_get_devdata(host);
> > > + struct spi_engine_offload *priv;
> > > +
> > > + if (channel >= SPI_ENGINE_MAX_NUM_OFFLOADS)
> > > + return -EINVAL;
> > > +
> > > + priv = &spi_engine->offload_priv[channel];
> > > +
> > > + if (priv->spi)
> > > + return -EBUSY;
> >
> > I wonder if we need to be this strict? Is there any problem by having two
> > devices requesting the same offload engine? I would expect that having multiple
> > peripherals trying to actually use it at the same time (with the prepare()
> > callback) to be problematic but if they play along it could actually work,
> > right? In reality that may never be a realistic usecase so this is likely fine.
> >
>
> I guess not. But to keep it simple for now, yeah, let's wait until we
> have an actual use case.
>
Agreed.
> ...
>
> > > +
> > > +static const struct spi_controller_offload_ops spi_engine_offload_ops = {
> > > + .map_channel = spi_engine_offload_map_channel,
> > > + .prepare = spi_engine_offload_prepare,
> > > + .unprepare = spi_engine_offload_unprepare,
> > > + .hw_trigger_enable = spi_engine_offload_enable,
> > > + .hw_trigger_disable = spi_engine_offload_disable,
> >
> > I guess this is what you and Conor are already somehow discussing but I would
> > expect this to be the actual offload trigger to play a spi transfer. As it
> > stands, it looks weird (or confusing) to have the enable/disable of the engine
> > to act as a trigger...
>
> It isn't acting as the trigger, just configuring the offload instance
> for exclusive use by a hardware trigger.
>
> > Maybe these callbacks could be used to enable/disable the
> > actual trigger of the offload engine (in our current cases, the PWM)? So this
> > would make it easy to move the trigger DT property where it belongs. The DMA one
> > (given it's tight relation with IIO DMA buffers) is another (way more difficult)
> > story I think.
> >
>
> One issue I have with making the actual hardware trigger part of the
> SPI controller is that in some cases, the peripheral could actually be
> the trigger. For example, in the case of a self-clocked ADC where
> there is just a RDY signal from the ADC when sample data is ready to
> be read. In this case would the peripheral have to register a trigger
> enable callback with the controller so that the controller can
> communicate with the peripheral to enable and disable sampling mode,
> and therefore the trigger?
In those cases, the peripheral would not bother in doing any spi hardware triggering
enable/disable (this assumes that we would need explicit interfaces for offload
enable/disable). In DT, the engine would also have no trigger has it's not required
so the controller even conditionally register these callbacks.
As for the DMA, I have no clue how we can have it associated with the controller
given how we want the data to be exported to userspace. Maybe dma-buf could be used
but it won't be easy. TBH, I'm not sure if it's that bad having the DMA associated
with the peripheral since it's purpose is to transfer peripheral DATA (not like a spi
controller DMA used for the actual SPI transfers).
- Nuno Sá
next prev parent reply other threads:[~2024-05-22 12:08 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-11 0:44 [PATCH RFC v2 0/8] spi: axi-spi-engine: add offload support David Lechner
2024-05-11 0:44 ` [PATCH RFC v2 1/8] spi: dt-bindings: spi-peripheral-props: add spi-offloads property David Lechner
2024-05-13 16:46 ` Conor Dooley
2024-05-13 17:06 ` David Lechner
2024-05-14 18:46 ` Conor Dooley
2024-05-14 22:56 ` David Lechner
2024-05-16 21:32 ` Conor Dooley
2024-05-17 16:51 ` David Lechner
2024-05-19 12:53 ` Conor Dooley
2024-05-21 14:54 ` David Lechner
2024-05-22 18:24 ` Conor Dooley
2024-05-23 12:15 ` Nuno Sá
2024-05-23 12:45 ` Mark Brown
2024-05-23 14:31 ` Conor Dooley
2024-05-23 15:05 ` David Lechner
2024-05-26 15:42 ` Conor Dooley
2024-05-26 17:35 ` Conor Dooley
2024-05-29 8:07 ` Nuno Sá
2024-05-29 8:33 ` Conor Dooley
2024-05-30 19:18 ` Conor Dooley
2024-05-30 21:28 ` David Lechner
2024-05-31 12:47 ` Mark Brown
2024-05-31 7:39 ` Nuno Sá
2024-05-30 19:24 ` David Lechner
2024-05-31 7:33 ` Nuno Sá
2024-06-04 19:33 ` Conor Dooley
2024-06-04 19:39 ` David Lechner
2024-06-04 19:42 ` Conor Dooley
2024-06-04 20:04 ` David Lechner
2024-05-23 14:28 ` David Lechner
2024-05-23 14:57 ` Nuno Sá
2024-05-23 15:09 ` David Lechner
2024-05-23 15:30 ` Nuno Sá
2024-05-26 15:45 ` Conor Dooley
2024-05-29 20:10 ` David Lechner
2024-05-30 17:25 ` Conor Dooley
2024-05-30 18:42 ` Conor Dooley
2024-05-11 0:44 ` [PATCH RFC v2 2/8] spi: add basic support for SPI offloading David Lechner
2024-05-21 11:57 ` Nuno Sá
2024-05-11 0:44 ` [PATCH RFC v2 3/8] spi: add support for hardware triggered offload David Lechner
2024-05-11 16:51 ` Jonathan Cameron
2024-05-11 0:44 ` [PATCH RFC v2 4/8] spi: add offload xfer flags David Lechner
2024-05-11 0:44 ` [PATCH RFC v2 5/8] spi: dt-bindings: axi-spi-engine: document spi-offloads David Lechner
2024-05-11 0:44 ` [PATCH RFC v2 6/8] spi: axi-spi-engine: add offload support David Lechner
2024-05-21 12:31 ` Nuno Sá
2024-05-21 14:28 ` David Lechner
2024-05-22 12:08 ` Nuno Sá [this message]
2024-05-11 0:44 ` [PATCH RFC v2 7/8] dt-bindings: iio: adc: adi,ad7944: add SPI offload properties David Lechner
2024-05-11 0:44 ` [PATCH RFC v2 8/8] iio: adc: ad7944: add support for SPI offload David Lechner
2024-05-11 16:58 ` Jonathan Cameron
2024-05-11 18:41 ` David Lechner
2024-05-12 11:52 ` Jonathan Cameron
2024-05-13 15:15 ` David Lechner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fbcc8bb3804d0f1f178f695cc16a1aa8dbcda1f1.camel@gmail.com \
--to=noname.nuno@gmail.com \
--cc=Michael.Hennerich@analog.com \
--cc=broonie@kernel.org \
--cc=conor+dt@kernel.org \
--cc=david@protonic.nl \
--cc=devicetree@vger.kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=kernel@martin.sperl.org \
--cc=krzk+dt@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-spi@vger.kernel.org \
--cc=nuno.sa@analog.com \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).