Re: [PATCH 3/3] iio: dac: ad5710r: Add driver for AD5710R and AD5711R

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Potential NULL pointer dereference of `st->chip_info`.
- [Medium] The return value of reset_control_assert() is ignored, masking potential reset failures.
--

--- Patch [3]: [PATCH 3/3] iio: dac: ad5710r: Add driver for AD5710R and AD5711R ---
commit b76ab75f312950575e4dba88d63f7b40e599ffe4
Author: Kim Seer Paller <kimseer.paller@analog.com>

iio: dac: ad5710r: Add driver for AD5710R and AD5711R

This commit adds a new driver for the AD5710R and AD5711R DACs.
The devices are 8-channel, low-power, configurable current/voltage
output DACs.

> diff --git a/drivers/iio/dac/ad5710r.c b/drivers/iio/dac/ad5710r.c
> new file mode 100644
> index 0000000000000..05bdada0bb6eb
> --- /dev/null
> +++ b/drivers/iio/dac/ad5710r.c

[ ... ]

> +static int ad5710r_setup(struct ad5710r_state *st, int external_vref_uV, u8 *num_channels)
> +{

[ ... ]

> +	if (reset) {
> +		/* Hold RESET low; tRESET pulse width is 160 ns min (Table 4) */
> +		reset_control_assert(reset);

[Severity: Medium]
Could this reset assertion fail? The return value of reset_control_assert() is
ignored here, which might leave the device in an unknown state if the reset
controller (e.g., over I2C) returns an error.

> +		fsleep(1);
> +		ret = reset_control_deassert(reset);
> +		if (ret)
> +			return ret;
> +	} else {

[ ... ]

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

[ ... ]

> +	st->regmap = devm_regmap_init_spi(spi, &ad5710r_regmap_config);
> +	if (IS_ERR(st->regmap))
> +		return dev_err_probe(dev, PTR_ERR(st->regmap),
> +				     "Failed to init regmap\n");
> +
> +	st->chip_info = spi_get_device_match_data(spi);

[ ... ]

> +	ret = ad5710r_setup(st, external_vref_uV, &num_channels);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->name = st->chip_info->name;

[Severity: High]
Is it possible for st->chip_info to be NULL here? If the device is bound via
fallback mechanisms like writing to the driver_override sysfs attribute,
spi_get_device_match_data() might return NULL, leading to a NULL pointer
dereference when accessing st->chip_info->name.

> +	indio_dev->info = &ad5710r_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = st->iio_channels;
> +	indio_dev->num_channels = num_channels;
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-iio-ad5710r-upstream-v1-0-e848a4bb701f@analog.com?part=3