On 9/5/25 4:49 AM, Jonathan Santos wrote:

Add support for ADAQ7767/68/69-1 series, which includes PGIA and
Anti-aliasing filter (AAF) gains. Unlike the AD7768-1, they do not
provide a VCM regulator interface.

...

+static void ad7768_fill_scale_tbl(struct iio_dev *dev)
+{
+	struct ad7768_state *st = iio_priv(dev);
+	const struct iio_scan_type *scan_type;
+	int val, val2, tmp0, tmp1, i;
+	struct u64_fract fract;
+	unsigned long n, d;
+	u64 tmp2;
+
+	scan_type = iio_get_current_scan_type(dev, &dev->channels[0]);
+	if (scan_type->sign == 's')
+		val2 = scan_type->realbits - 1;
+	else
+		val2 = scan_type->realbits;
+
+	for (i = 0; i < st->chip->num_pga_modes; i++) {
+		/* Convert gain to a fraction format */
+		fract.numerator = st->chip->pga_gains[i];
+		fract.denominator = MILLI;
+		if (st->chip->has_variable_aaf) {
+			fract.numerator *= ad7768_aaf_gains_bp[st->aaf_gain];
+			fract.denominator *= 10000;
+		}
+
+		rational_best_approximation(fract.numerator, fract.denominator,
+					    INT_MAX, INT_MAX, &n, &d);
+
+		val = mult_frac(st->vref_uv, d, n);
+		/* Would multiply by NANO here, but value is already in milli */
+		tmp2 = shift_right((u64)val * MICRO, val2);

shift_right() is only needed for signed values. Can just use >> since this is
dealing with unsigned.

+		tmp0 = div_u64_rem(tmp2, NANO, &tmp1);
+		st->scale_tbl[i][0] = tmp0; /* Integer part */
+		st->scale_tbl[i][1] = abs(tmp1); /* Fractional part */
+	}
+}
+

...

+static int ad7768_set_pga_gain(struct ad7768_state *st,
+			       int gain_mode)
+{
+	int pgia_pins_value = abs(gain_mode - st->chip->pgia_mode2pin_offset);
+	int ret;
+
+	guard(mutex)(&st->pga_lock);
+
+	ret = regmap_write(st->regmap, AD7768_REG_GPIO_CONTROL, AD7768_GPIO_PGIA_EN);

Hiding multiple fields in a macro makes it harder to see what this is doing.

+	if (ret)
+		return ret;
+
+	/* Write the respective gain values to GPIOs 0, 1, 2 */
+	ret = regmap_write(st->regmap, AD7768_REG_GPIO_WRITE,
+			   AD7768_GPIO_WRITE(pgia_pins_value));

Hiding the FIELD_PREP() in a macro makes this a bit harder to understand.

Should we add a BASIS_POINTS macro to units.h since we are calling this a
standard unit?

+
+		*val = temp;
 		*val2 = scan_type->realbits;
 
 		return IIO_VAL_FRACTIONAL_LOG2;

...

+static int ad7768_parse_aaf_gain(struct device *dev, struct ad7768_state *st)
+{
+	bool aaf_gain_provided;
+	u32 val;
+	int ret;
+
+	st->aaf_gain = AD7768_AAF_IN1;

No sense in setting this if we return early on -EINVAL.

+	ret = device_property_read_u32(dev, "adi,aaf-gain-bp", &val);
+	aaf_gain_provided = (ret == 0);

Only -EINVAL means the property is not present. Other errors can be returned.
Or add a comment explaining why we don't care about other errors.

+	if (!aaf_gain_provided && st->chip->has_variable_aaf) {
+		dev_warn(dev, "AAF gain not specified, using default\n");

Using the default seems like a normal thing to do, so should not be a warning.

+		return 0;
+	}
+
+	if (aaf_gain_provided && !st->chip->has_variable_aaf) {
+		dev_warn(dev, "AAF gain not supported for %s\n", st->chip->name);

Returning error here would be senseible. Clearly there is something wrong with
the devicetree. But it could be something else, like the wrong compaible string.

+		return 0;
+	}
+
+	if (aaf_gain_provided) {
+		switch (val) {
+		case 10000:
+			st->aaf_gain = AD7768_AAF_IN1;
+			break;
+		case 3640:
+			st->aaf_gain = AD7768_AAF_IN2;
+			break;
+		case 1430:
+			st->aaf_gain = AD7768_AAF_IN3;
+			break;
+		default:
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid firmware provided AAF gain\n");
+		}
+	}
+
+	return 0;
+}
+

...

ad7768_register_regulators() now seems mis-named since it only handles VMC
supply.

+		if (ret)
+			return ret;
+	}
+
+	ret = ad7768_parse_aaf_gain(&spi->dev, st);
 	if (ret)
 		return ret;