On Fri, Sep 05, 2025 at 06:49:42AM -0300, 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.

The PGA gain is configured in run-time through the scale attribute,
if supported by the device. PGA is enabled and controlled by the GPIO
controller (GPIOs 0, 1 and 2) provided by the device with the SPI
interface.

The AAF gain is defined by hardware connections and should be specified
in the device tree.

...

+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]);

Is it usual patter in IIO? Otherwise it can be written as

	scan_type = iio_get_current_scan_type(dev, dev->channels);

+	if (scan_type->sign == 's')
+		val2 = scan_type->realbits - 1;
+	else
+		val2 = scan_type->realbits;

Wasn't smatch happy about this check?

+	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;
+		}

My question also was: Are these overflow u32? If not, the second patch is not
needed. Otherwise, how is the previous version supposed to work with unsigned
long type (if I am not mistaken) on 32-bit platforms?

+		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);
+		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_calc_pga_gain(struct ad7768_state *st, int gain_int,
+				int gain_fract, int precision)
+{
+	u64 gain_nano;
+	u32 tmp;
+
+	gain_nano = gain_int * NANO + gain_fract;
+	gain_nano = clamp(gain_nano, 0, ADAQ776X_GAIN_MAX_NANO);
+	tmp = DIV_ROUND_CLOSEST_ULL(gain_nano << precision, NANO);
+	gain_nano = DIV_ROUND_CLOSEST(st->vref_uv, tmp);
+	if (st->chip->has_variable_aaf)
+		gain_nano = DIV_ROUND_CLOSEST_ULL(gain_nano *  10000,

One space too many.

+						  ad7768_aaf_gains_bp[st->aaf_gain]);
+
+	return find_closest(gain_nano, st->chip->pga_gains,
+			    (int)st->chip->num_pga_modes);

Is casting needed?

+}

...

+	case IIO_CHAN_INFO_SCALE:  {

One space too many.

+		int gain_mode;
+
+		if (!st->chip->has_pga)
+			return -EOPNOTSUPP;
+
+		if (scan_type->sign == 's')
+			gain_mode = ad7768_calc_pga_gain(st, val, val2,
+							 scan_type->realbits - 1);
+		else
+			gain_mode = ad7768_calc_pga_gain(st, val, val2,
+							 scan_type->realbits);
+
+		return ad7768_set_pga_gain(st, gain_mode);
+	}

...

+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;

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

Hmm... It seems rather incorrect check. You should actually return an error
code in case it's provided but can't be read.

I would expect something like

	ret = device_property_read_u32(dev, "adi,aaf-gain-bp", &val);
	if (ret == -EINVAL)
		_gain_provided = false;
	else if (ret)
		return dev_err_probe(...);
	else
		_gain_provided = true;

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

We can refactor these to have one level indentation less for the switch-case.

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

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

+	if (aaf_gain_provided) {

So this one may be dropped.

+		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;
+}

-- 
With Best Regards,
Andy Shevchenko