Re: [PATCH v4 4/4] iio: adc: update ad7779 to use IIO backend

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Wed, Aug 20, 2025 at 03:02:45PM +0300, Ioana Risteiu wrote:
> Add a new functionality to ad7779 driver that streams data through data
> output interface using IIO backend interface.

...

> +static int ad7779_set_data_lines(struct iio_dev *indio_dev,
> +				 unsigned int num_lanes)
> +{
> +	struct ad7779_state *st = iio_priv(indio_dev);

> +	int ret = -EINVAL;

In general the split assignment is easier to maintain and less prone to subtle
errors. In this case it's even worse as it's not needed...

> +	if (num_lanes != AD7779_1LINE &&
> +		num_lanes != AD7779_2LINES &&
> +		num_lanes != AD7779_4LINES)
> +		return ret;

...just return the error code directly here.

> +	ret = ad7779_set_sampling_frequency(st, num_lanes * AD7779_DEFAULT_SAMPLING_1LINE);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_backend_num_lanes_set(st->back, num_lanes);
> +	if (ret)
> +		return ret;
> +
> +	return ad7779_spi_write_mask(st, AD7779_REG_DOUT_FORMAT,
> +				    AD7779_DOUT_FORMAT_MSK,
> +				    FIELD_PREP(AD7779_DOUT_FORMAT_MSK, 2 - ilog2(num_lanes)));
> +}

...

> +static int ad7779_setup_channels(struct iio_dev *indio_dev, const struct ad7779_state *st)
> +{
> +	struct iio_chan_spec *channels;
> +	struct device *dev = &st->spi->dev;
> +
> +	channels = devm_kmemdup_array(dev, st->chip_info->channels,
> +					ARRAY_SIZE(ad7779_channels),
> +					sizeof(*channels), GFP_KERNEL);

Indentation...

> +	if (!channels)
> +		return -ENOMEM;
> +
> +	for (int i = 0; i < ARRAY_SIZE(ad7779_channels); i++)

Why signed iterator?

> +		channels[i].scan_type.endianness = IIO_CPU;
> +
> +	indio_dev->channels = channels;
> +
> +	return 0;
> +}

...

> +static int ad7779_setup_backend(struct ad7779_state *st, struct iio_dev *indio_dev)
> +{
> +	struct device *dev = &st->spi->dev;

> +	int ret = -EINVAL;

Why?!

> +	int num_lanes;

Can it be negatie?

> +	indio_dev->info = &ad7779_info_data;
> +
> +	ret = ad7779_setup_channels(indio_dev, st);
> +	if (ret)
> +		return ret;
> +
> +	st->back = devm_iio_backend_get(dev, NULL);
> +	if (IS_ERR(st->back))
> +		return dev_err_probe(dev, PTR_ERR(st->back),
> +				     "failed to get iio backend");
> +
> +	ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_backend_enable(dev, st->back);
> +	if (ret)
> +		return ret;
> +
> +	num_lanes = 4;
> +	ret = device_property_read_u32(dev, "adi,num-lanes", &num_lanes);
> +	if (ret && ret != -EINVAL)
> +		return ret;
> +
> +	return ad7779_set_data_lines(indio_dev, num_lanes);
> +}

-- 
With Best Regards,
Andy Shevchenko