Re: [PATCH v1 01/15] dt-bindings: iio: adc: ad7768-1: add synchronization over SPI property

[David Lechner]

On 1/13/25 6:18 PM, Jonathan Santos wrote:
> On 01/12, Jonathan Cameron wrote:
>> On Sat, 11 Jan 2025 19:34:14 -0300
>> Jonathan Santos <jonath4nns@xxxxxxxxx> wrote:
>>
>>> On 01/07, David Lechner wrote:
>>>> On 1/7/25 9:24 AM, Jonathan Santos wrote:  
>>>>> Add adi,sync-in-spi property to enable synchronization over SPI.
>>>>> This should be used in the case when the GPIO cannot provide a
>>>>> pulse synchronous with the base MCLK signal.
>>>>>
>>>>> User can choose between SPI, GPIO synchronization or neither of them,
>>>>> but only if a external pulse can be provided, for example, by another
>>>>> device in a multidevice setup.
>>>>>   
>>>>
>>>> While we are fixing up these bindings, we could add some more trivial things,
>>>> like power supplies.
>>>>
>>>> Also, the interrupt property could use a description since the chip has multiple
>>>> output pins. I assume it means the /DRDY pin?
>>>>   
>>>
>>> Right! Yes, the interrupt pin refers to the /DRDY.
>>>
>>>>> Signed-off-by: Jonathan Santos <Jonathan.Santos@xxxxxxxxxx>
>>>>> ---
>>>>>  .../bindings/iio/adc/adi,ad7768-1.yaml        | 24 ++++++++++++++++++-
>>>>>  1 file changed, 23 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
>>>>> index 3ce59d4d065f..55cec27bfe60 100644
>>>>> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
>>>>> @@ -47,6 +47,15 @@ properties:
>>>>>        in any way, for example if the filter decimation rate changes.
>>>>>        As the line is active low, it should be marked GPIO_ACTIVE_LOW.
>>>>>  
>>>>> +  adi,sync-in-spi:  
>>>>
>>>> If this is saying that SYNC_OUT is connected to SYNC_IN, then I think the name
>>>> should be something like adi,sync-in-sync-out. SPI seems irrelevant here since
>>>> we should just be describing how things are wired up, not how it is being used.
>>>>
>>>> But if we also need to consider the case where SYNC_OUT of one chip is connected
>>>> to SYNC_IN of another chip, we might want to consider using trigger-source
>>>> bindings instead (recently standardized in dtschema).
>>>>   
>>>
>>> Do you mean the trigger-sources used for LEDs? I can try to see if it works, but would it
>>> handle the non-GPIO case? While testing a multidevice setup, I found it simpler to 
>>> have a single device to manage everything. It lets us toggle the GPIO or /SYNC_OUT
>>> without referencing another device and makes simultaneous buffered reads easier.
>>
>> Daisy-chain mode (figure 131)?  In that case we normally end up with a single presented device
>> with a 'lot' of channels. (See the electric car style battery charging chips, those can
>> be chained in very large numbers!)
>>
> 
> Actually, it is more like Figure 133 , but the premise is similar. We
> have here a Quad setup.
> 
>> Probably similar for figure 133 (which is a dual SPI setup) as the SPI clock must
>> be shared so we still see it over a single interface.
>>
> 
> We could view them as a single device with multiple channels, and since
> the goal is to read them simultaneously with buffered reads, some parameters
> such as sampling frequency should be equal to all devices.
> 
> However, there are some implications: If we do the above, we have
> limitations in the customization of the "channels", they would have
> the same filter, frequency modulator and scale (we plan to add support
> for ADAQ776x-1 series, which include PGA and AAF gain).
> 
> To customize them separetely, we would need to assert only the
> corresponding chip select, which is only possible with different
> instances, as far as I know.

FYI, I've been discussing with the HDL folks at ADI about how we could make a
multi-bus SPI controller, similar to controllers used for parallel SPI flash
memories that are used as a single logical device. So that is the solution I
am hoping for here. It would would allow a single IIO device instance for
multiple chips. But the SPI controller would allow addressing individual chips
for configuration and addressing all chips at the same time for reading sample
data.

> 
>> If those are the only two options then keeping this within the driver is fine.
>> For daisy chain there are examples in tree and it normally means we have to
>> have a DT parameter that says how long the chain is, though we maybe can
>> do that with per channel nodes as well if those make sense here.
>>
>> Jonathan
>>
> 
> Those are the options in the datasheet and in hardware so far. I was 
> considering other scenarios in case the user combine them differently.
> I believe keping within the driver covers the main cases. 
>