Re: [PATCH RFC v2 1/8] spi: dt-bindings: spi-peripheral-props: add spi-offloads property

[Conor Dooley]

On Thu, May 23, 2024 at 09:28:54AM -0500, David Lechner wrote:
> On 5/22/24 1:24 PM, Conor Dooley wrote:
> > On Tue, May 21, 2024 at 09:54:39AM -0500, David Lechner wrote:
> >> On Sun, May 19, 2024 at 7:53 AM Conor Dooley <conor@xxxxxxxxxx> wrote:
> >>>
> >>> On Fri, May 17, 2024 at 11:51:58AM -0500, David Lechner wrote:
> >>>> On Thu, May 16, 2024 at 4:32 PM Conor Dooley <conor@xxxxxxxxxx> wrote:
> >>>>> On Tue, May 14, 2024 at 05:56:47PM -0500, David Lechner wrote:
> >>>
> 
> ...
> 
> >> This time, the periodic trigger (PWM) is connected to the pin on the
> >> ADC that triggers a sample conversion (CNV). The ADC has a BUSY output
> >> that will go high at the start of the conversion and go low at the end
> >> of the conversion. The BUSY output of the ADC is wired as the hardware
> >> trigger input of the offload.
> >>
> >> In this case would we still consider the PWM as part of the SPI
> >> controller/offload since it can only be used in conjunction with the
> >> SPI offload? It isn't connected to it at all though.
> > 
> > No, in this case the ADC is a PWM consumer and the offload engine is
> > not. The ADC is a "trigger" provider and the SPI offload engine is a
> > trigger consumer.
> 
> Makes sense.
> 
> ...
> 
> 
> > 
> >>> In fact, looking at the documentation for the "spi engine" some more, I
> >>> am even less convinced that the right place for describing the offload is
> >>> the peripheral as I (finally?) noticed that the registers for the offload
> >>> engine are mapped directly into the "spi engine"'s memory map, rather than
> >>> it being a entirely separate IP in the FPGA fabric.
> >>
> >> True, but we don't use these registers, e.g., to configure the
> >> sampling frequency of a trigger (if it can even be done). That is done
> >> in a completely separate IP block with it's own registers.
> > 
> > Where is the binding for that IP block? I think describing that is
> > pretty key. goto continuation;
> 
> For the real-world case I used to test this series, it is an AXI PWMGEN
> [1] that is providing the trigger event source. It has a typical PWM
> provider binding with #pwm-cells [2].
> 
> Calling this a "trigger" provider to the SPI offload instance just like the
> case above where the ADC is directly connected as the offload trigger makes
> sense to me.
> 
> What I was going for in this patch (slightly revised to use #cells) is that
> this trigger provider, whatever it is, is selected by one of the cells of
> #spi-offload-cells. It doesn't seem like there should be a special case for
> if the trigger provider is a clock or PWM where the SPI controller node
> becomes a consumer of the clock or PWM provider rather than just describing
> the trigger relationship.
> 
> For example, supposing we had an FPGA/HDL that could handle all 3 wiring
> configurations we have discussed so far. A) PWM connected directly to SPI
> offload as trigger, B) PWM connected to CNV of ADC and BSY of ADC connected
> to SPI offload as trigger, C) self clocked ADC with RDY of ADC connected
> to SPI offload as trigger. So the DT would have:
> 
> controller:
> #spi-offload-cells = <2>: /* 1st cell = offload instance
>                            * 2nd cell = trigger provider */
> 
> peripheral (choose one based on actual wiring):
> spi-offloads = <0 0>; /* case A */
> spi-offloads = <0 1>; /* case B */
> spi-offloads = <0 2>; /* case C */
> 
> 
> As to what is the actual consumer of the PWM provider in each of these
> cases...
> 
> * C is easy. There isn't a PWM provider since the ADC is self-clocked.
> * B, as discussed elsewhere is fairly straight forward. The ADC node is
>   the consumer since the PWM is connected directly to the ADC.
> * A is the one we need to figure out. I'm proposing that the PWM consumer
>   should be whatever kind of composite device node we come up with that
>   also solves the issue described below about where does the CRC checker
>   (or whatever) go. I think we are in agreement here at least on the point
>   that it doesn't belong in the SPI controller node?

To be clear, you're saying that we agree that the CRC checker doesnt
belong in the SPI controller node, right?

Attachment: signature.asc
Description: PGP signature