Re: [PATCH 5/5] iio: adc: Add ti-ads1263-adc2 driver

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 14 Jun 2026 15:43:49 -0500
"Kurt Borja" <kuurtb@gmail.com> wrote:

> On Sat Jun 13, 2026 at 9:10 AM -05, Jonathan Cameron wrote:
> > On Fri, 12 Jun 2026 17:46:23 -0500
> > Kurt Borja <kuurtb@gmail.com> wrote:
> >  
> >> The TI ADS1263 includes an auxiliary, 24-bit, delta-sigma ADC (ADC2).
> >> ADC2 operation is independent of ADC1, with independent selections of
> >> input channel, reference voltage, sample rate, and channel gain
> >> 
> >> Add support for this ADC as an independent IIO device, through the
> >> auxiliary bus API.  
> >
> > A few things inline.
> >  
> >> 
> >> Signed-off-by: Kurt Borja <kuurtb@gmail.com>  
> >  
> >> diff --git a/drivers/iio/adc/ti-ads1262.c b/drivers/iio/adc/ti-ads1262.c
> >> index b33505e7fdc7..1a4b2f934d43 100644
> >> --- a/drivers/iio/adc/ti-ads1262.c
> >> +++ b/drivers/iio/adc/ti-ads1262.c  
> >  
> >> +static int ads1262_aux_device_setup(struct ads1262 *st)
> >> +{
> >> +	struct device *dev = &st->spi->dev;
> >> +	struct ads1263_adc2_channel *chans;
> >> +	struct auxiliary_device *adev;
> >> +	struct ads1263_adc2_ctx *ctx;
> >> +	struct fwnode_handle *node;
> >> +	int id, ret;
> >> +
> >> +	node = device_get_named_child_node(dev, "adc");
> >> +	if (!node)
> >> +		return 0;
> >> +
> >> +	ctx = kzalloc_obj(*ctx);
> >> +	if (!ctx) {
> >> +		ret = -ENOMEM;
> >> +		goto out_node_put;
> >> +	}
> >> +
> >> +	id = ida_alloc(&ads1262_ida, GFP_KERNEL);
> >> +	if (id < 0) {
> >> +		ret = id;
> >> +		goto out_free_adc2;
> >> +	}
> >> +
> >> +	chans = kcalloc(st->num_channels, sizeof(*chans), GFP_KERNEL);
> >> +	if (!chans) {
> >> +		ret = -ENOMEM;
> >> +		goto out_free_id;
> >> +	}
> >> +
> >> +	for (unsigned int i = 0; i < st->num_channels; i++) {
> >> +		chans[i].negative_input = st->channels[i].negative_input;
> >> +		chans[i].positive_input = st->channels[i].positive_input;
> >> +	}
> >> +
> >> +	ctx->chip = st;
> >> +	ctx->num_channels = st->num_channels;
> >> +	ctx->channels = chans;
> >> +	ctx->enable = ads1263_adc2_enable;
> >> +	ctx->start = ads1263_adc2_start;
> >> +	ctx->stop = ads1263_adc2_stop;
> >> +	ctx->read = ads1263_adc2_read;
> >> +	mutex_init(&ctx->chan_lock);  
> > devm_mutex_init()  
> 
> I actually call mutex_destroy() on device .release.
> 
> I think it makes more sense that way, otherwise we would UAF?
It does indeed make more sense there.

Whether it ends up as a UAF will depend on how the mutex is used.
I 'think' you are fine either way because it is always in an
IIO callback which depending on call path is ether from sysfs
removed in the devm_iio_device_unregister() path or gated on
the iio_dev->info being set to NULL for in kernel users.

Handing over lifetime ownership to the device is absolutely fine
and a bit easier to reason about so ok to leave it like this.


> > 	},
> >  
> >> +	{ }
> >> +};
> >> +MODULE_DEVICE_TABLE(auxiliary, ads1263_adc2_auxiliary_match);  
> 
> Thanks for your feedback, Jonathan! Apologies if this version was a
> little rough... I'm a bit embarrased by the bugs found by Sashiko.
> 
Lol. We all have that problem - sometimes it is irritatingly good
at spotting silly mistakes ;)