Re: [PATCH] iio: ad7380: add support for SPI offload
From: David Lechner
Date: Fri Mar 14 2025 - 16:10:55 EST
On 2/22/25 11:31 AM, David Lechner wrote:
> On 2/20/25 12:03 PM, Angelo Dureghello wrote:
>> From: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
>>
>> Add support for SPI offload to the ad7380 driver. SPI offload allows
>> sampling data at the max sample rate (2MSPS with one SDO line).
>>
>> This is developed and tested against the ADI example FPGA design for
>> this family of ADCs [1].
>>
>> [1]: http://analogdevicesinc.github.io/hdl/projects/ad738x_fmc/index.html
>>
>> Signed-off-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
>> ---
>
...
>> +#define _AD7380_OFFLOAD_CHANNEL(index, bits, diff, sign, gain) { \
>> + .type = IIO_VOLTAGE, \
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>> + ((gain) ? BIT(IIO_CHAN_INFO_SCALE) : 0) | \
>> + ((diff) ? 0 : BIT(IIO_CHAN_INFO_OFFSET)), \
>> + .info_mask_shared_by_type = ((gain) ? 0 : BIT(IIO_CHAN_INFO_SCALE)) | \
>> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) | \
>> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
>
> Not sure if this is worth troubling with, but it might make more sense to make
> IIO_CHAN_INFO_SAMP_FREQ info_mask_separate instead of info_mask_shared_by_type,
> at least in the case of the single-ended chips.
>
> This family of chips does simultaneous conversions so shared_by_type (or shared_by_all)
> would typically be the right thing to do here. However, the single-ended versions
> of these chips also have a multiplexer, so there are 2 banks of simultaneously
> sampled inputs. So the effective sample rate as far as IIO is concerned would
> actually be 1/2 of the sampling_frequency attribute value.
>
> Since we have a channel mask restriction where we force all channels in a bank
> to be enabled at once, I think it would work to make IIO_CHAN_INFO_SAMP_FREQ
> info_mask_separate where the reported sampling frequency is the conversion rate
> divided by the number of channels in a bank.
>
Hi Jonathan,
You might have missed this since v2 was sent before you had a chance to review
v1. I am testing the chip with the mux now, so I was curious if you had any
wisdom to add here. The way we implemented it feels a little odd to me with
sampling_frequency as info_mask_shared_by_type instead of info_mask_separate
or info_mask_shared_by_all like on most other chips I've worked with so far.
I found a bug in this series that I need to send a fix for, so if we decide
there is a better way here, now would be a good time to do it.
>> + .info_mask_shared_by_type_available = \
>> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) | \
>> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
>> + .indexed = 1, \
>> + .differential = (diff), \
>> + .channel = (diff) ? (2 * (index)) : (index), \
>> + .channel2 = (diff) ? (2 * (index) + 1) : 0, \
>> + .scan_index = (index), \
>> + .has_ext_scan_type = 1, \
>> + .ext_scan_type = ad7380_scan_type_##bits##_##sign##_offload, \
>> + .num_ext_scan_type = \
>> + ARRAY_SIZE(ad7380_scan_type_##bits##_##sign##_offload), \
>> + .event_spec = ad7380_events, \
>> + .num_event_specs = ARRAY_SIZE(ad7380_events), \
>> +}
>> +