Re: [PATCH 5/9] iio: adc: rzt2h: implement DMA buffer support

Implement buffered capture using a cyclic DMA transfer into a kfifo
buffer to support continuous high-rate sampling.

On buffer enable, switch the ADC to continuous conversion mode and start
a cyclic DMA transfer over the active channels.

Because the DMA controller does not support native scatter-gather, and
because of the cyclic DMA setup, transfers must be done in widths
covering all the enabled channels.

Since DMA transfer width must be a power of two and aligned to its size,
cover the smallest power-of-two-aligned group of channel registers
spanning the enabled channels.

Split the cyclic buffer into fixed-size periods. On each period
completion, bump a pending counter and wake a consumer kthread from the
DMA callback.

For every completed period, gather the enabled channels out of the DMA
layout into the scan layout the IIO core expects and push each scan
with iio_push_to_buffers().

Could you instead use the available_scan_masks infrastructure.  bit annoying
to specify the full list but isn't that long I think with 16 channels

16 x single
8 x double,
4 x quads
2 x octect
1 x all of them.

The the IIO demux in (sits behind the push_to_buffers path if
we have available_scan_masks set) will then deal with repacking
the data if necessary.

If the consumer kthread falls behind by a full buffer, drop the oldest
periods.

Because the DMA transfer must cover all channels between the first and
last enabled ones, skip disabled channels while compacting.

Also, the DMA controller transfers data in 32-bit words, but the ADC's
data registers are 16-bit wide, causing adjacent channel data to be
swapped. Swap consecutive channels while compacting to account for this.

Allocate the DMA buffer via dma_alloc_noncoherent() and synchronise it
per period to allow it to be cached by the CPU while compacting.

Disable the completion IRQ for the duration of the DMA transfer, as the
ICU does not mask this event from reaching the GIC even if it is being
used to drive the DMA capture.

Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>

I'm messing around with b4 review tui and sashiko integration. I've
left the Sashiko comments in here as I'm out of time today to look
at them in detial.

+}
+
+static void rzt2h_adc_advance_period_index(struct rzt2h_adc *adc, unsigned int i)
+{
+	adc->dma.period_index += i;
+	adc->dma.period_index %= RZT2H_ADC_DMA_PERIODS;
+}
+
+static void rzt2h_adc_dma_thread_loop(struct iio_dev *indio_dev)
+{
+	struct rzt2h_adc *adc = iio_priv(indio_dev);
+	int pending, drop;
+	dma_addr_t addr;
+	u16 *period;
+
+	pending = atomic_xchg(&adc->dma.pending_periods, 0);
+
+	if (pending >= RZT2H_ADC_DMA_PERIODS) {

Add a comment here to say what is being dropped and why.

+		drop = pending - RZT2H_ADC_DMA_PERIODS + 1;
+
+		rzt2h_adc_advance_period_index(adc, drop);
+		pending -= drop;
+	}
+
+	for (unsigned int i = 0; i < pending; i++) {
+		unsigned int backlog = atomic_read(&adc->dma.pending_periods) +
+				       pending - i;
+
+		/*
+		 * Bail if enough new periods have completed since reading the
+		 * pending_periods that the next period about to be read is at
+		 * risk of being overwritten.
+		 */
+		if (backlog >= RZT2H_ADC_DMA_PERIODS)
+			break;

+
+		period = adc->dma.buf + adc->dma.period_index *
+			 RZT2H_ADC_DMA_PERIOD_SAMPLES * adc->dma.sample_chans;
+		addr = adc->dma.addr + adc->dma.period_index *
+		       adc->dma.period_bytes;
+
+		rzt2h_adc_push_period(indio_dev, period, addr);
+		rzt2h_adc_advance_period_index(adc, 1);
+	}
+}
+
+static int rzt2h_adc_dma_thread(void *data)
+{
+	struct iio_dev *indio_dev = data;
+	struct rzt2h_adc *adc = iio_priv(indio_dev);
+
+	while (!kthread_should_stop()) {
+		wait_event_interruptible(adc->dma.wq,
+					 atomic_read(&adc->dma.pending_periods) ||
+					 kthread_should_stop());

I've not thought that much about the following but it seems plausible so please
take a look

+
+		if (kthread_should_stop())
+			break;
+
+		rzt2h_adc_dma_thread_loop(indio_dev);
+	}
+
+	return 0;
+}
+
+static void rzt2h_adc_dma_callback(void *data)
+{
+	struct iio_dev *indio_dev = data;
+	struct rzt2h_adc *adc = iio_priv(indio_dev);
+
+	atomic_inc(&adc->dma.pending_periods);
+	wake_up(&adc->dma.wq);
+}
+
+static void rzt2h_adc_dma_calc_layout(struct iio_dev *indio_dev)
+{
+	struct rzt2h_adc *adc = iio_priv(indio_dev);
+	unsigned int hi = 0, lo = RZT2H_ADC_MAX_CHANNELS - 1;
+	const struct iio_chan_spec *chan;
+	unsigned int sample_chans;
+	unsigned int first_chan;
+	unsigned int scan_index;
+	unsigned int swap;
+	unsigned int idx;

As mentioned at the top, I think you can probably avoid all this complexity.
Lots of devices have restrictions on combinations of channels that
are enabled together. For that we have available_scan_masks
and the demux stuff in the IIO core.

+
+	/* Find the lowest and highest enabled channel. */
+	iio_for_each_active_channel(indio_dev, scan_index) {
+		chan = &indio_dev->channels[scan_index];
+
+		lo = min_t(unsigned int, lo, chan->channel);
+		hi = max_t(unsigned int, hi, chan->channel);
+	}
+
+	/*
+	 * The DMA has no scatter-gather and transfers must have a power-of-two
+	 * width, so pick the smallest power-of-two-aligned block of channels
+	 * that covers all enabled channels.
+	 */
+	for (sample_chans = 1; sample_chans < RZT2H_ADC_MAX_CHANNELS; sample_chans <<= 1) {
+		first_chan = round_down(lo, sample_chans);
+
+		if (first_chan + sample_chans > hi)
+			break;
+	}
+
+	/*
+	 * Build a table to map each enabled channel to its position in the
+	 * transferred block, it will be used later to extract only the enabled
+	 * channels out of it.
+	 * The DMA moves data in 32-bit words, which swaps each pair of adjacent
+	 * 16-bit channels. Undo it.
+	 */
+	adc->dma.gather_len = 0;
+	swap = sample_chans > 1;
+	iio_for_each_active_channel(indio_dev, scan_index) {
+		chan = &indio_dev->channels[scan_index];
+		idx = chan->channel - first_chan;
+
+		adc->dma.gather[adc->dma.gather_len++] = idx ^ swap;
+	}
+
+	adc->dma.first_chan = first_chan;
+	adc->dma.sample_chans = sample_chans;
+	adc->dma.period_bytes = RZT2H_ADC_DMA_PERIOD_SAMPLES * sample_chans *
+				RZT2H_ADC_CHANNEL_BYTES;
+}
+
+static int rzt2h_adc_start_dma(struct iio_dev *indio_dev)
+{
+	struct rzt2h_adc *adc = iio_priv(indio_dev);
+	struct dma_async_tx_descriptor *desc;
+	struct dma_slave_config config = {};
+	unsigned int buffer_bytes;
+	dma_cookie_t cookie;
+	int ret;
+
+	rzt2h_adc_dma_calc_layout(indio_dev);
+
+	config.src_addr = adc->phys_base + RZT2H_ADDR_REG(adc->dma.first_chan);
+	config.src_addr_width = adc->dma.sample_chans * RZT2H_ADC_CHANNEL_BYTES;

	config = (struct dma_slave_config) {
		.src_addr = adc->phys_base + RZT2H_ADDR_REG(adc->dma.first_chan),
		.src_addr_width = adc->dma.sample_chans * RZT2H_ADC_CHANNEL_BYTES,
	};

	keeps all the filling in of info together and avoids need to do = { }
	above then overwrite some of the zeroed memory.

+
+	buffer_bytes = RZT2H_ADC_DMA_PERIODS * adc->dma.period_bytes;
+
+	ret = dmaengine_slave_config(adc->dma.chan, &config);
+	if (ret)
+		return ret;
+
+	desc = dmaengine_prep_dma_cyclic(adc->dma.chan, adc->dma.addr,
+					 buffer_bytes, adc->dma.period_bytes,
+					 DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
+	if (!desc)
+		return -EBUSY;
+
+	desc->callback = rzt2h_adc_dma_callback;
+	desc->callback_param = indio_dev;
+
+	cookie = dmaengine_submit(desc);
+	ret = dma_submit_error(cookie);
+	if (ret) {
+		dmaengine_terminate_sync(adc->dma.chan);
+		return ret;
+	}
+
+	adc->dma.thread = kthread_run(rzt2h_adc_dma_thread, indio_dev,
+				      "rzt2h-adc-dma");
+	if (IS_ERR(adc->dma.thread)) {
+		dmaengine_terminate_sync(adc->dma.chan);
+		return PTR_ERR(adc->dma.thread);
+	}
+
+	disable_irq(adc->irq);
+
+	dma_async_issue_pending(adc->dma.chan);
+
+	return 0;
+}
+

+static int rzt2h_adc_setup_dma(struct iio_dev *indio_dev)
+{
+	struct rzt2h_adc *adc = iio_priv(indio_dev);
+	struct device *dev = adc->dev;
+	int ret;
+
+	adc->dma.chan = devm_dma_request_chan(adc->dev, "rx");
+	if (IS_ERR(adc->dma.chan)) {
+		ret = PTR_ERR(adc->dma.chan);
+		if (ret != -ENODEV)
+			return dev_err_probe(adc->dev, ret, "DMA channel request failed\n");

You have dev that can be used here.

+
+		adc->dma.chan = NULL;
+		return 0;
+	}
+
+	adc->dma.buf = dma_alloc_noncoherent(adc->dev, RZT2H_ADC_DMA_BUFFER_SIZE,

Same here.

-- 
Jonathan Cameron [off-list ref]