Re: [PATCH v2 2/3] iio: dac: ad5706r: Add support for AD5706R DAC

[Andy Shevchenko]

On Wed, Mar 11, 2026 at 08:23:18AM +0800, Alexis Czezar Torreno wrote:
> Add support for the Analog Devices AD5706R, a 4-channel 16-bit
> current output digital-to-analog converter with SPI interface.
> 
> Features:
>   - 4 independent DAC channels
>   - Hardware and software LDAC trigger
>   - Configurable output range
>   - PWM-based LDAC control
>   - Dither and toggle modes
>   - Dynamically configurable SPI speed

...

> +#include <linux/device.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>

IWYU, please.
Yet another typical issue with ADI patches. Really, talk to your senior
colleagues who are experienced kernel developers before sending it to ML.

...

> +#define AD5706R_DAC_RESOLUTION		16
> +#define AD5706R_DAC_MAX_CODE		BIT(AD5706R_DAC_RESOLUTION)  /* 65536 */

Useless comment, use the same number explicitly instead

	BIT(16)

which is much more readable.

...

> +	st->tx_buf = cpu_to_be32((((u32)reg) << 16) |
> +			((u32)val << (16 - (num_bytes * 8))));

What the heck is this?

...

> +static const struct of_device_id ad5706r_of_match[] = {
> +	{ .compatible = "adi,ad5706r" },
> +	{ },

Even inside a single file this is inconsistent, besides being third of
101 IIO	patch issues.

> +};
> +MODULE_DEVICE_TABLE(of, ad5706r_of_match);
> +
> +static const struct spi_device_id ad5706r_id[] = {
> +	{ "ad5706r", 0 },

When there is no driver data, no need to explicitly initialise it.

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, ad5706r_id);

-- 
With Best Regards,
Andy Shevchenko