Re: [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Hardware sequencing risk: external reference voltage is enabled before the analog power supply.
- [Low] Incompatible pointer type passed to `regmap_read()` causing a build failure.
- [Low] Missing `select REGMAP_SPI` in Kconfig leads to build or probe failures.
--

--- Patch [2]: [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163 ---
commit e7b20377f202f6c225fbb971eaeb3c40d160ef56
Author: Lukas Metz <lukas.metz@gmx.net>

iio: dac: dac8163: Add driver for DAC8163

The DAC756x, DAC816x, and DAC856x devices are low-power, voltage-output,
dual-channel, 12-, 14-, and 16-bit digital-to-analog converters (DACs),
respectively. These devices include a 2.5-V, 4-ppm/°C internal
reference, giving a full-scale output voltage range of 2.5 V or 5 V.

> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 657c68e75542e..257cb92f43fde 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -644,6 +644,21 @@ config TI_DAC7612
>  
>  	  If compiled as a module, it will be called ti-dac7612.
>  
> +config TI_DAC8163
> +	tristate "Texas Instruments 12/14/16-bit 2-channel DAC driver"
> +	depends on SPI_MASTER

[Severity: Low]
Should this Kconfig select REGMAP_SPI?  If CONFIG_TI_DAC8163=y is selected
without any other kernel configuration enabling REGMAP_SPI, the kernel build
might fail with a linker error for __devm_regmap_init_spi, or the driver might
fail to probe.

> +	help
> +	  Driver for the Texas Instruments digital-to-analog converter
> +	  family dacxx6x compatible with the following variants

[ ... ]

> diff --git a/drivers/iio/dac/ti-dac8163.c b/drivers/iio/dac/ti-dac8163.c
> new file mode 100644
> index 0000000000000..6be0aac2e8753
> --- /dev/null
> +++ b/drivers/iio/dac/ti-dac8163.c
> @@ -0,0 +1,373 @@

[ ... ]

> +static int dac8163_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int *val,
> +			    int *val2, long mask)
> +{
> +	struct dac8163_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = regmap_read(st->regmap,
> +				  CMD_SET(CMD_WRITE_UPDATE, chan->channel),
> +				  val);

[Severity: Low]
Can this cause a build failure on compilers that treat incompatible pointer
types as errors?  The dac8163_read_raw() function passes val (which is of
type int *) directly to regmap_read(), but regmap_read() strictly expects an
unsigned int *.

> +		if (ret)
> +			return ret;
> +		*val >>= chan->scan_type.shift;
> +		return IIO_VAL_INT;

[ ... ]

> +static int dac8163_probe(struct spi_device *spi)
> +{

[ ... ]

> +	if (device_property_present(&spi->dev, "vrefin-supply")) {
> +		ret = devm_regulator_get_enable_read_voltage(&spi->dev,
> +							     "vrefin");
> +		if (ret < 0)
> +			return ret;
> +
> +		st->vref_mV = ret / (MICRO / MILLI);
> +		internal_reference = false;
> +		st->gain = 1;
> +	} else {
> +		st->vref_mV = DAC8163_INTERNAL_REF_mV;
> +		internal_reference = true;
> +		st->gain = 2;
> +	}
> +
> +	ret = devm_regulator_get_enable(&spi->dev, "avdd");

[Severity: High]
Could this power sequencing damage the hardware?  During dac8163_probe(),
the external reference voltage (vrefin) is enabled before the primary analog
power supply (avdd).

Applying an external reference voltage before the analog supply is active might
forward-bias internal ESD protection diodes, which could lead to excessive
current draw or latch-up.

> +	if (ret < 0)
> +		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-dac8163-work-v2-0-3acd1bf20182@gmx.net?part=2