Linux-IIO Archive mirror
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
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 1/8] spi: dt-bindings: spi-peripheral-props: add spi-offloads property
Date: Tue, 14 May 2024 19:46:51 +0100	[thread overview]
Message-ID: <20240514-aspire-ascension-449556da3615@spud> (raw)
In-Reply-To: <CAMknhBE5XJzhdJ=PQUXiubw_CiCLcn1jihiscnQZUzDWMASPKw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3906 bytes --]

On Mon, May 13, 2024 at 12:06:17PM -0500, David Lechner wrote:
> On Mon, May 13, 2024 at 11:46 AM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Fri, May 10, 2024 at 07:44:24PM -0500, David Lechner wrote:
> > > This adds a new property to the spi-peripheral-props binding for use
> > > with peripherals connected to controllers that support offloading.
> > >
> > > Here, offloading means that the controller has the ability to perform
> > > complex SPI transactions without CPU intervention in some shape or form.
> > >
> > > This property will be used to assign controller offload resources to
> > > each peripheral that needs them. What these resources are will be
> > > defined by each specific controller binding.
> > >
> > > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > > ---
> > >
> > > v2 changes:
> > >
> > > In v1, instead of generic SPI bindings, there were only controller-
> > > specific bindings, so this is a new patch.
> > >
> > > In the previous version I also had an offloads object node that described
> > > what the offload capabilities were but it was suggested that this was
> > > not necessary/overcomplicated. So I've gone to the other extreme and
> > > made it perhaps over-simplified now by requiring all information about
> > > how each offload is used to be encoded in a single u32.
> >
> > The property is a u32-array, so I guess, not a single u32?
> 
> It is an array to handle cases where a peripheral might need more than
> one offload. But the idea was it put everything about each individual
> offload in a single u32. e.g. 0x0101 could be offload 1 with hardware
> trigger 1 and 0x0201 could be offload 1 with hardware trigger 2. Then
> a peripheral could have spi-offloads = <0x0101>, <0x0201>; if it
> needed to select between both triggers at runtime.
> 
> >
> > > We could of course consider using #spi-offload-cells instead for
> > > allowing encoding multiple parameters for each offload instance if that
> > > would be preferable.
> >
> > A -cells property was my gut reaction to what you'd written here and
> > seems especially appropriate if there's any likelihood of some future
> > device using some external resources for spi-offloading.
> > However, -cells properties go in providers, not consumers, so it wouldn't
> > end up in spi-periph-props.yaml, but rather in the controller binding,
> > and instead there'd be a cell array type property in here. I think you
> > know that though and I'm interpreting what's been written rather than
> > what you meant.
> 
> Indeed you guess correctly. So the next question is if it should be
> the kind of #-cells that implies a phandle like most providers or
> without phandles like #address-cells?

I'm trying to understand if the offload could ever refer to something
beyond the controller that you'd need the phandle for. I think it would
be really helpful to see an example dt of a non-trivial example for how
this would work. The example in the ad7944 patch has a stub controller
node & the clocks/dmas in the peripheral node so it is difficult to
reason about the spi-offloads property there.

> Asking because I got pushback on
> v1 for using a phandle with offloads (although in that case, the
> phandle was for the offload instance itself instead for the SPI
> controller, so maybe this is different in this case?).

Do you have a link to this v1 pushback? I had looked at the v1's binding
comments and didn't see that type of property being resisted - although
I did see some resistance to the spi peripheral node containing any of
the information about the offloads it had been assigned and instead
doing that mapping in the controller so that the cs was sufficient. I
don't think that'd work with the scenario you describe above though
where there could be two different triggers per device tho.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2024-05-14 18:46 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 [this message]
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á
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=20240514-aspire-ascension-449556da3615@spud \
    --to=conor@kernel.org \
    --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).