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.