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), \ >> +} >> +