On Tue, 15 Oct 2024 11:09:08 +0200 Julien Stephan <jstephan@xxxxxxxxxxxx> wrote: > adaq4370-4 (2MSPS) and adaq4380-4 (4MSPS) are quad-channel precision data > acquisition signal chain μModule solutions compatible with the ad738x > family, with the following differences: > > - configurable gain in front of each 4 adc > - internal reference is 3V derived from refin-supply (5V) > - additional supplies > > This implies that IIO_CHAN_INFO_SCALE can not be shared by type. > > Signed-off-by: Julien Stephan <jstephan@xxxxxxxxxxxx> Not that much code, so I'll give it a quick review even though I know you posted it for that one specific dt binding question. Main thing here is I'd drop the scale from dt bindings and make it writable via write_raw. You'll also need to compute the appropriate _available stuff so userspace knows what scales it can use. Jonathan > @@ -876,8 +933,15 @@ static int ad7380_read_raw(struct iio_dev *indio_dev, > * * (2 × VREF) / 2^N, for differential chips > * * VREF / 2^N, for pseudo-differential chips > * where N is the ADC resolution (i.e realbits) > + * > + * The gain is stored as a fraction of 1000 and, as we need to > + * divide vref_mv by the gain, we invert the gain/1000 fraction. > */ > - *val = st->vref_mv; > + if (st->chip_info->has_hardware_gain) > + *val = mult_frac(st->vref_mv, MILLI, > + st->gain_milli[chan->scan_index]); > + else > + *val = st->vref_mv; > *val2 = scan_type->realbits - chan->differential; > > return IIO_VAL_FRACTIONAL_LOG2; > @@ -1058,7 +1122,19 @@ static int ad7380_probe(struct spi_device *spi) > "Failed to enable power supplies\n"); > msleep(T_POWERUP_MS); > > - if (st->chip_info->external_ref_only) { > + if (st->chip_info->adaq_internal_ref_only) { > + /* > + * ADAQ chips use fixed internal reference but still > + * require an external reference supply to power it. I'd just go with * require a specific external supply to power it. Reference kind of implies it is a quality supply. This is 4.5-5.5V so not so much ;) > + * "refin" is already enabled with other power supplies > + * in bulk_get_enable(). > + */ > + > + st->vref_mv = ADAQ4380_INTERNAL_REF_MV; > + > + /* these chips don't have a register bit for this */ > + external_ref_en = false; > + } else if (st->chip_info->external_ref_only) { > ret = devm_regulator_get_enable_read_voltage(&spi->dev, > "refin"); > if (ret < 0) > @@ -1104,6 +1180,34 @@ static int ad7380_probe(struct spi_device *spi) > st->vcm_mv[i] = ret / 1000; > } > > + for (i = 0; i < MAX_NUM_CHANNELS; i++) > + st->gain_milli[i] = AD7380_DEFAULT_GAIN_MILLI; > + > + if (st->chip_info->has_hardware_gain) { Why is this a DT thing rather than exposed to userspace? Mostly we only control ranges in DT for output devices (where there is a chance of burning things if we let them be controlled from userspace.). For ADC amplifiers we tend to just expose them as controllable _SCALE. > + device_for_each_child_node_scoped(&spi->dev, node) { > + unsigned int channel, gain; > + > + ret = fwnode_property_read_u32(node, "reg", &channel); > + if (ret) > + return dev_err_probe(&spi->dev, ret, > + "Failed to read reg property\n"); > + > + if (channel >= st->chip_info->num_channels - 1) > + return dev_err_probe(&spi->dev, -EINVAL, > + "Invalid channel number %i\n", > + channel); > + > + ret = fwnode_property_read_u32(node, "adi,gain-milli", > + &gain); > + if (ret && ret != -EINVAL) > + return dev_err_probe(&spi->dev, ret, > + "Failed to read gain for channel %i\n", > + channel); > + if (ret != -EINVAL) > + st->gain_milli[channel] = gain; > + } > + }