Re: [PATCH v8 1/2] dt-bindings: adc: add AD7173

[Ceclan Dumitru <mitrutzceclan@xxxxxxxxx>]

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.

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.