On Thu, 14 Dec 2023 19:03:28 +0200 Ceclan Dumitru <mitrutzceclan@xxxxxxxxx> wrote: > On 12/14/23 18:12, David Lechner wrote: > > On Thu, Dec 14, 2023 at 1:43 PM Ceclan Dumitru <mitrutzceclan@xxxxxxxxx> wrote: > >> On 12/12/23 17:09, David Lechner wrote: > >>> On Tue, Dec 12, 2023 at 11:45 AM Dumitru Ceclan <mitrutzceclan@xxxxxxxxx> wrote: > > >> ... > >> > >>>> + interrupts: > >>>> + maxItems: 1 > >>> > >>> Shouldn't this be 2? The datasheet says there is a "Data Output Ready" > >>> signal on the DOUT/RDY pin and an "Error Output" on the SYNC/ERROR > >>> pin. Although I could see how RDY could be considered part of the SPI > >>> bus. In any case, a description explaining what the interrupt is would > >>> be useful. > >>> > >> > >> I do not see how there could be 2 interrupts. DOUT/RDY is used as an > >> interrupt when waiting for a conversion to finalize. > >> > >> Sync and Error are sepparate pins, Sync(if enabled) works only as an > >> input that resets the modulator and the digital filter. > > > > I only looked at the AD7172-2 datasheet and pin 15 is labeled > > SYNC/ERROR. Maybe they are separate pins on other chips? > > Yep, sorry, missed that. All other supported models have them separate. > > > >> > >> Error can be configured as input, output or ERROR output (OR between all > >> internal error sources). > >> > >> Would this be alright > >> interrupts: > >> > >> description: Conversion completion interrupt. > >> Pin is shared with SPI DOUT. > >> maxItems: 1 > > > > Since ERROR is an output, I would expect it to be an interrupt. The > > RDY output, on the other hand, would be wired to a SPI controller with > > the SPI_READY feature (I use the Linux flag name here because I'm not > > aware of a corresponding devicetree flag). So I don't think the RDY > > signal would be an interrupt. > > > > Error does not have the purpose to be an interrupt. The only interrupt > used from this chip is the one from the DOUT/~RDY pin. Sure, it is wired > to the SPI controller, but when you can't also receive interrupts on > that very same CPU pin an issue arises. So that pin is also wired to > another GPIO with interrupt support. You've lost me. It's a pin that has a state change when an error condition occurs. Why not an interrupt? Doesn't matter that the driver doesn't use this functionality. dt-bindings should be as comprehensive as possible. Given it's a multipurpose pin you'd also want to support it as a gpio to be complete alongside the other GPIOs. > > This is the same way that ad4130.yaml is written for example (with the > exception that ad4130 supports configuring where the interrupt is routed). > > In regards to SPI_READY _BITUL(7) /* slave pulls low to pause */: the > ad_sigma_delta framework (if it can be called that) is written to expect > a pin interrupt, not to use SPI_READY controller feature. SPI_READY is supported by only a couple of controllers. I'm not even that sure exactly how it is defined and whether that lines up with this usecase. >From some old asci art. https://lore.kernel.org/all/1456747459-8559-1-git-send-email-linux@xxxxxxxxxxxxxxxx/ Flow control: Ready Sequence Master CS |-----1\_______________________| Slave FC |--------2\____________________| DATA |-----------3\_________________| So you set master and then wait for a flow control pin (the ready signal) before you can actually talk to the device. Here we are indicating data is ready to be be read out. So I don't 'think' SPI_READY applies. Jonathan >
