Re: [PATCH v3 5/5] iio: adc: ltc2378: Enable triggered buffer data capture

[Marcelo Schmitt]

On 06/17, David Lechner wrote:
> On 6/16/26 9:04 PM, Marcelo Schmitt wrote:
> > Enable users to run triggered data captures with LTC2378 and similar ADCs.
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@xxxxxxxxxx>
> > ---
...
> > @@ -27,6 +27,21 @@ int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *st
> >  		return dev_err_probe(dev, ret, "error on SPI offload setup\n");
> >  
> >  	return 0;
> > +
> > +trigger_buf_setup:
> > +	ret = __ltc2378_set_triggered_buf_ops(st);
> > +	if (ret == -EOPNOTSUPP)
> > +		return 0; /* Let device setup complete without buffer support */
> > +
> > +	if (!ret)
> > +		ret = st->ops->buffer_setup(indio_dev, st);
> > +
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "error on buffer setup\n");
> 
> This is repeating the code above. Seems like it would be better without
> the goto.
Ack

> 
> > +
> > +	st->chans[st->num_iio_chans++] = IIO_CHAN_SOFT_TIMESTAMP(1);
> 
> Adding a channel here seems messy. I still think static const channel
> data would be better. But at least would be better if this was moved
> to the same function as the other channel setup.
> 
Agree, will think of a better place to init the timestamp channel while
switching to static const channel.

> > +
> > +	return 0;
> >  }
...
> > +static irqreturn_t ltc2378_trigger_handler(int irq, void *p)
> > +{
> > +	struct iio_poll_func *pf = p;
> > +	struct iio_dev *indio_dev = pf->indio_dev;
> > +	struct ltc2378_state *st = iio_priv(indio_dev);
> > +	int ret;
> > +
> > +	ret = ltc2378_convert_and_acquire(st);
> > +	if (ret < 0)
> > +		goto err_out;
> 
> As mentioned elsewhere, SPI xfer size may be 2 or 4 bytes, but
> we are always pushing 4 bytes, so this only works on little-endian
> architecture.
> 
> I think best would be to have storagesize = 16 when appropriate to
> avoid having to manually handle this.
> 
Okay, it will probably be easier to do that with static const channels.
Am checking out other sashiko comments as well.

> > +
> > +	iio_push_to_buffers_with_ts(indio_dev, &st->scan, sizeof(st->scan),
> > +				    pf->timestamp);
> > +
> > +err_out:
> > +	iio_trigger_notify_done(indio_dev->trig);
> > +	return IRQ_HANDLED;
> > +}
> > +
> 
Thanks,
Marcelo