[PATCH v2 0/9] iio: adc: rzt2h: add DMA buffer support

[PATCH v2 0/9] iio: adc: rzt2h: add DMA buffer support

[Cosmin Tanislav]

Add DMA-based buffered capture to the Renesas RZ/T2H and RZ/N2H ADC
driver and the necessary device-tree properties. Also add a per-channel
sampling-frequency control.

Capture uses a cyclic DMA transfer into a non-coherent buffer. The DMA
controller has no per-descriptor scatter-gather and each transfer needs
to be of power-of-two width, so the transfer covers the smallest
power-of-two-aligned group of channel registers spanning the enabled
channels, and a consumer kthread compacts each completed period down to
the enabled channels before handing it to the IIO core.

V2:
 * advance period index on backlog overflow
 * add a comment describing the logic for dropping already overwritten
   periods
 * build dma_slave_config as a compound literal
 * use the local dev instead of adc->dev in rzt2h_adc_setup_dma()
 * sync the DMA buffer to device immediately after allocation to handle
   the zero-fill done by the CPU
 * extend the single read completion timeout to maximum possible
   conversion time + 1 jiffy
 * pick up Conor's Acked-by

Cosmin Tanislav (9):
  iio: adc: rzt2h: remove unused struct rzt2h_adc::max_channels
  iio: adc: rzt2h: store IRQ in private state
  iio: adc: rzt2h: store the physical address in private state
  iio: adc: rzt2h: claim direct mode on single reads
  iio: adc: rzt2h: implement DMA buffer support
  iio: adc: rzt2h: expose sampling frequency
  dt-bindings: iio: adc: renesas,r9a09g077-adc: document DMA support
  arm64: dts: renesas: r9a09g077: Wire up DMA support for ADC
  arm64: dts: renesas: r9a09g087: Wire up DMA support for ADC

 .../iio/adc/renesas,r9a09g077-adc.yaml        |  12 +
 arch/arm64/boot/dts/renesas/r9a09g077.dtsi    |   6 +
 arch/arm64/boot/dts/renesas/r9a09g087.dtsi    |   6 +
 drivers/iio/adc/Kconfig                       |   2 +
 drivers/iio/adc/rzt2h_adc.c                   | 499 +++++++++++++++++-
 5 files changed, 509 insertions(+), 16 deletions(-)

-- 
2.55.0

[PATCH v2 1/9] iio: adc: rzt2h: remove unused struct rzt2h_adc::max_channels

[Cosmin Tanislav]

struct rzt2h_adc::max_channels is unused, and is probably a leftover
of the devm_iio_adc_device_alloc_chaninfo_se() conversion done after the
initial submission. Remove it and the logic used to set it.

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

V2:
 * no changes

 drivers/iio/adc/rzt2h_adc.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
index 4e0eb02d3d14..42ff6bf2ab08 100644
--- a/drivers/iio/adc/rzt2h_adc.c
+++ b/drivers/iio/adc/rzt2h_adc.c
@@ -42,7 +42,6 @@ struct rzt2h_adc {
 
 	const struct iio_chan_spec *channels;
 	unsigned int num_channels;
-	unsigned int max_channels;
 };
 
 static void rzt2h_adc_start(struct rzt2h_adc *adc, unsigned int conversion_type)
@@ -190,7 +189,6 @@ static const struct iio_chan_spec rzt2h_adc_chan_template = {
 static int rzt2h_adc_parse_properties(struct rzt2h_adc *adc)
 {
 	struct iio_chan_spec *chan_array;
-	unsigned int i;
 	int ret;
 
 	ret = devm_iio_adc_device_alloc_chaninfo_se(adc->dev,
@@ -203,10 +201,6 @@ static int rzt2h_adc_parse_properties(struct rzt2h_adc *adc)
 	adc->num_channels = ret;
 	adc->channels = chan_array;
 
-	for (i = 0; i < adc->num_channels; i++)
-		if (chan_array[i].channel + 1 > adc->max_channels)
-			adc->max_channels = chan_array[i].channel + 1;
-
 	return 0;
 }
 
-- 
2.55.0

[PATCH v2 2/9] iio: adc: rzt2h: store IRQ in private state

[Cosmin Tanislav]

To prepare for adding support for DMA transfers, store the IRQ in
private state to disable it while DMA is in progress. The RZ/T2H ICU
does not mask the interrupts itself when they are being used for DMA
transfers.

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

V2:
 * no changes

 drivers/iio/adc/rzt2h_adc.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
index 42ff6bf2ab08..470b16b5b9c8 100644
--- a/drivers/iio/adc/rzt2h_adc.c
+++ b/drivers/iio/adc/rzt2h_adc.c
@@ -42,6 +42,8 @@ struct rzt2h_adc {
 
 	const struct iio_chan_spec *channels;
 	unsigned int num_channels;
+
+	int irq;
 };
 
 static void rzt2h_adc_start(struct rzt2h_adc *adc, unsigned int conversion_type)
@@ -209,7 +211,7 @@ static int rzt2h_adc_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct iio_dev *indio_dev;
 	struct rzt2h_adc *adc;
-	int ret, irq;
+	int ret;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*adc));
 	if (!indio_dev)
@@ -239,11 +241,11 @@ static int rzt2h_adc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	irq = platform_get_irq_byname(pdev, "adi");
-	if (irq < 0)
-		return irq;
+	adc->irq = platform_get_irq_byname(pdev, "adi");
+	if (adc->irq < 0)
+		return adc->irq;
 
-	ret = devm_request_irq(dev, irq, rzt2h_adc_isr, 0, dev_name(dev), adc);
+	ret = devm_request_irq(dev, adc->irq, rzt2h_adc_isr, 0, dev_name(dev), adc);
 	if (ret)
 		return ret;
 
-- 
2.55.0

[PATCH v2 3/9] iio: adc: rzt2h: store the physical address in private state

[Cosmin Tanislav]

To prepare for adding support for DMA-based transfers, store the
physical address of the device in struct rzt2h_adc::phys_base.

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

V2:
 * no changes

 drivers/iio/adc/rzt2h_adc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
index 470b16b5b9c8..995308a419f1 100644
--- a/drivers/iio/adc/rzt2h_adc.c
+++ b/drivers/iio/adc/rzt2h_adc.c
@@ -36,6 +36,7 @@ struct rzt2h_adc {
 	void __iomem *base;
 	struct device *dev;
 
+	phys_addr_t phys_base;
 	struct completion completion;
 	/* lock to protect against multiple access to the device */
 	struct mutex lock;
@@ -211,6 +212,7 @@ static int rzt2h_adc_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct iio_dev *indio_dev;
 	struct rzt2h_adc *adc;
+	struct resource *res;
 	int ret;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*adc));
@@ -231,10 +233,12 @@ static int rzt2h_adc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	adc->base = devm_platform_ioremap_resource(pdev, 0);
+	adc->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
 	if (IS_ERR(adc->base))
 		return PTR_ERR(adc->base);
 
+	adc->phys_base = res->start;
+
 	pm_runtime_set_autosuspend_delay(dev, 300);
 	pm_runtime_use_autosuspend(dev);
 	ret = devm_pm_runtime_enable(dev);
-- 
2.55.0

[PATCH v2 4/9] iio: adc: rzt2h: claim direct mode on single reads

[Cosmin Tanislav]

Claim direct mode around the single read to prevent it from conflicting
with an ongoing buffered read.

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

V2:
 * no changes

 drivers/iio/adc/rzt2h_adc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
index 995308a419f1..95bcebdc02cb 100644
--- a/drivers/iio/adc/rzt2h_adc.c
+++ b/drivers/iio/adc/rzt2h_adc.c
@@ -158,8 +158,13 @@ static int rzt2h_adc_read_raw(struct iio_dev *indio_dev,
 	struct rzt2h_adc *adc = iio_priv(indio_dev);
 
 	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_RAW: {
+		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+		if (IIO_DEV_ACQUIRE_FAILED(claim))
+			return -EBUSY;
+
 		return rzt2h_adc_read_single(adc, chan->channel, val);
+	}
 	case IIO_CHAN_INFO_SCALE:
 		*val = 1800;
 		*val2 = 12;
-- 
2.55.0

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

[Cosmin Tanislav]

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().

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>
---

V2:
 * advance period index on backlog overflow
 * add a comment describing the logic for dropping already overwritten
   periods
 * build dma_slave_config as a compound literal
 * use the local dev instead of adc->dev in rzt2h_adc_setup_dma()
 * sync the DMA buffer to device immediately after allocation to handle
   the zero-fill done by the CPU

 drivers/iio/adc/Kconfig     |   2 +
 drivers/iio/adc/rzt2h_adc.c | 352 ++++++++++++++++++++++++++++++++++++
 2 files changed, 354 insertions(+)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 7a60ffd0ddd8..0aca02962947 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1575,6 +1575,8 @@ config RZT2H_ADC
 	tristate "Renesas RZ/T2H / RZ/N2H ADC driver"
 	depends on ARCH_RENESAS || COMPILE_TEST
 	select IIO_ADC_HELPER
+	select IIO_BUFFER
+	select IIO_KFIFO_BUF
 	help
 	  Say yes here to build support for the ADC found in Renesas
 	  RZ/T2H / RZ/N2H SoCs.
diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
index 95bcebdc02cb..0460dffe189e 100644
--- a/drivers/iio/adc/rzt2h_adc.c
+++ b/drivers/iio/adc/rzt2h_adc.c
@@ -4,11 +4,16 @@
 #include <linux/cleanup.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
 #include <linux/iio/adc-helpers.h>
+#include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/kfifo_buf.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
+#include <linux/kthread.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -18,6 +23,7 @@
 #define RZT2H_ADCSR_ADIE_MASK		BIT(12)
 #define RZT2H_ADCSR_ADCS_MASK		GENMASK(14, 13)
 #define RZT2H_ADCSR_ADCS_SINGLE		0b00
+#define RZT2H_ADCSR_ADCS_CONTINUOUS	0b10
 #define RZT2H_ADCSR_ADST_MASK		BIT(15)
 
 #define RZT2H_ADANSA0_REG		0x04
@@ -31,18 +37,47 @@
 #define RZT2H_ADCALCTL_CAL_ERR_MASK	BIT(2)
 
 #define RZT2H_ADC_MAX_CHANNELS		16
+#define RZT2H_ADC_CHANNEL_BYTES		sizeof(u16)
+#define RZT2H_ADC_DMA_PERIOD_SAMPLES	128
+#define RZT2H_ADC_DMA_PERIODS		64
+#define RZT2H_ADC_DMA_BUFFER_SAMPLES	(RZT2H_ADC_DMA_PERIODS * \
+					 RZT2H_ADC_DMA_PERIOD_SAMPLES)
+#define RZT2H_ADC_DMA_BUFFER_SIZE	(RZT2H_ADC_DMA_BUFFER_SAMPLES * \
+					 RZT2H_ADC_MAX_CHANNELS * \
+					 RZT2H_ADC_CHANNEL_BYTES)
+
+struct rzt2h_adc_dma {
+	struct dma_chan *chan;
+	u16 *buf;
+	dma_addr_t addr;
+
+	unsigned int period_index;
+	unsigned int period_bytes;
+	unsigned int first_chan;
+	unsigned int sample_chans;
+
+	u8 gather[RZT2H_ADC_MAX_CHANNELS];
+	unsigned int gather_len;
+
+	atomic_t pending_periods;
+
+	wait_queue_head_t wq;
+	struct task_struct *thread;
+};
 
 struct rzt2h_adc {
 	void __iomem *base;
 	struct device *dev;
 
 	phys_addr_t phys_base;
+	struct rzt2h_adc_dma dma;
 	struct completion completion;
 	/* lock to protect against multiple access to the device */
 	struct mutex lock;
 
 	const struct iio_chan_spec *channels;
 	unsigned int num_channels;
+	u16 buf[RZT2H_ADC_MAX_CHANNELS];
 
 	int irq;
 };
@@ -151,6 +186,263 @@ static int rzt2h_adc_calibrate(struct rzt2h_adc *adc)
 	return 0;
 }
 
+static void rzt2h_adc_push_period(struct iio_dev *indio_dev, u16 *period,
+				  dma_addr_t addr)
+{
+	struct rzt2h_adc *adc = iio_priv(indio_dev);
+	u16 *dst = adc->buf;
+	u16 *src = period;
+
+	dma_sync_single_for_cpu(adc->dev, addr, adc->dma.period_bytes,
+				DMA_FROM_DEVICE);
+
+	for (unsigned int sample = 0; sample < RZT2H_ADC_DMA_PERIOD_SAMPLES; sample++) {
+		for (unsigned int i = 0; i < adc->dma.gather_len; i++)
+			dst[i] = src[adc->dma.gather[i]];
+
+		src += adc->dma.sample_chans;
+
+		iio_push_to_buffers(indio_dev, adc->buf);
+	}
+}
+
+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) {
+		/*
+		 * The consumer fell a full buffer behind and the oldest periods
+		 * have already been overwritten by the cyclic DMA transfer.
+		 * Drop them and jump straight to the oldest period that has
+		 * not been overwritten.
+		 */
+		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) {
+			rzt2h_adc_advance_period_index(adc, pending - i);
+			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());
+
+		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;
+
+	/* 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 = (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,
+	};
+
+	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_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct rzt2h_adc *adc = iio_priv(indio_dev);
+	const struct iio_chan_spec *chan;
+	struct device *dev = adc->dev;
+	unsigned int scan_index;
+	u16 val = 0;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return ret;
+
+	iio_for_each_active_channel(indio_dev, scan_index) {
+		chan = &indio_dev->channels[scan_index];
+		val |= RZT2H_ADANSA0_CH_MASK(chan->channel);
+	}
+
+	writew(val, adc->base + RZT2H_ADANSA0_REG);
+
+	adc->dma.period_index = 0;
+	atomic_set(&adc->dma.pending_periods, 0);
+
+	ret = rzt2h_adc_start_dma(indio_dev);
+	if (ret) {
+		pm_runtime_put_autosuspend(dev);
+		return ret;
+	}
+
+	rzt2h_adc_start(adc, RZT2H_ADCSR_ADCS_CONTINUOUS);
+
+	return 0;
+}
+
+static int rzt2h_adc_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct rzt2h_adc *adc = iio_priv(indio_dev);
+	struct device *dev = adc->dev;
+
+	rzt2h_adc_stop(adc);
+
+	dmaengine_terminate_sync(adc->dma.chan);
+
+	kthread_stop(adc->dma.thread);
+
+	enable_irq(adc->irq);
+
+	pm_runtime_put_autosuspend(dev);
+
+	return 0;
+}
+
 static int rzt2h_adc_read_raw(struct iio_dev *indio_dev,
 			      struct iio_chan_spec const *chan,
 			      int *val, int *val2, long mask)
@@ -174,6 +466,11 @@ static int rzt2h_adc_read_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static const struct iio_buffer_setup_ops rzt2h_adc_buffer_setup_ops = {
+	.postenable = rzt2h_adc_buffer_postenable,
+	.predisable = rzt2h_adc_buffer_predisable,
+};
+
 static const struct iio_info rzt2h_adc_iio_info = {
 	.read_raw = rzt2h_adc_read_raw,
 };
@@ -192,6 +489,12 @@ static const struct iio_chan_spec rzt2h_adc_chan_template = {
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
 			      BIT(IIO_CHAN_INFO_SCALE),
 	.type = IIO_VOLTAGE,
+	.scan_type = {
+		.sign = 'u',
+		.realbits = 12,
+		.storagebits = 16,
+		.endianness = IIO_CPU,
+	},
 };
 
 static int rzt2h_adc_parse_properties(struct rzt2h_adc *adc)
@@ -209,9 +512,53 @@ static int rzt2h_adc_parse_properties(struct rzt2h_adc *adc)
 	adc->num_channels = ret;
 	adc->channels = chan_array;
 
+	for (unsigned int i = 0; i < adc->num_channels; i++)
+		chan_array[i].scan_index = i;
+
 	return 0;
 }
 
+static void rzt2h_adc_free_dma_buf(void *p)
+{
+	struct rzt2h_adc *adc = p;
+
+	dma_free_noncoherent(adc->dev, RZT2H_ADC_DMA_BUFFER_SIZE,
+			     adc->dma.buf, adc->dma.addr, DMA_FROM_DEVICE);
+}
+
+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(dev, "rx");
+	if (IS_ERR(adc->dma.chan)) {
+		ret = PTR_ERR(adc->dma.chan);
+		if (ret != -ENODEV)
+			return dev_err_probe(dev, ret, "DMA channel request failed\n");
+
+		adc->dma.chan = NULL;
+		return 0;
+	}
+
+	adc->dma.buf = dma_alloc_noncoherent(dev, RZT2H_ADC_DMA_BUFFER_SIZE,
+					     &adc->dma.addr, DMA_FROM_DEVICE,
+					     GFP_KERNEL);
+	if (!adc->dma.buf)
+		return -ENOMEM;
+
+	dma_sync_single_for_device(dev, adc->dma.addr, RZT2H_ADC_DMA_BUFFER_SIZE,
+				   DMA_FROM_DEVICE);
+
+	ret = devm_add_action_or_reset(dev, rzt2h_adc_free_dma_buf, adc);
+	if (ret)
+		return ret;
+
+	return devm_iio_kfifo_buffer_setup_ext(dev, indio_dev,
+					       &rzt2h_adc_buffer_setup_ops, NULL);
+}
+
 static int rzt2h_adc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -227,6 +574,7 @@ static int rzt2h_adc_probe(struct platform_device *pdev)
 	adc = iio_priv(indio_dev);
 	adc->dev = dev;
 	init_completion(&adc->completion);
+	init_waitqueue_head(&adc->dma.wq);
 
 	ret = devm_mutex_init(dev, &adc->lock);
 	if (ret)
@@ -264,6 +612,10 @@ static int rzt2h_adc_probe(struct platform_device *pdev)
 	indio_dev->channels = adc->channels;
 	indio_dev->num_channels = adc->num_channels;
 
+	ret = rzt2h_adc_setup_dma(indio_dev);
+	if (ret)
+		return ret;
+
 	return devm_iio_device_register(dev, indio_dev);
 }
 
-- 
2.55.0

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

[Jonathan Cameron]

> 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().
> 
> 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>


Hi Cosmin.

A couple of comments inline but so far nothing to stop me applying
this series and one of them is random musings :)

Jonathan


> diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
> index 95bcebdc02cb..0460dffe189e 100644
> --- a/drivers/iio/adc/rzt2h_adc.c

> +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;
> +
> +	/* 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);

I'd normally be fussy about use of max_t etc but here it is
highlighting something that came up in another review - why is
channel an signed?  We use negative in the scan index but
I don't think we ever do for channel.  Anyhow, unrelated to this
series other than we might be able to tidy this up shortly if
I get round to checking if we can just make chan->channel unsigned.

> +	}
> +
> +	/*
> +	 * 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 = (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,
> +	};
> +
> +	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);

It is fairly common to combine these
	ret = dma_submit_error(dmaengine_submit(desc))

If you happen to respin for other reasons consider doing this

-- 
Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

[PATCH v2 6/9] iio: adc: rzt2h: expose sampling frequency

[Cosmin Tanislav]

Expose the sampling frequency as a per-channel IIO_CHAN_INFO_SAMP_FREQ
to let userspace control conversion time.

Each channel conversion takes a fixed 13 ADCLK cycles plus the sample
time programmed in ADSSTRn, giving a rate of ADCLK / (13 + ADSSTRn).

Read the ADCLK rate from the "adclk" clock to derive the frequency.

Claim direct mode while writing so the rate cannot change during a
capture.

Program the sample time into ADSSTRn for each enabled channel on single
reads and on buffer enable.

Wait for the maximum amount of time a conversion can take + 1 jiffy for
the completion event to come after triggering a single read.

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

V2:
 * extend the single read completion timeout to maximum possible
   conversion time + 1 jiffy

 drivers/iio/adc/rzt2h_adc.c | 120 ++++++++++++++++++++++++++++++++++--
 1 file changed, 115 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
index 0460dffe189e..8ba7b0fde8a1 100644
--- a/drivers/iio/adc/rzt2h_adc.c
+++ b/drivers/iio/adc/rzt2h_adc.c
@@ -2,6 +2,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/cleanup.h>
+#include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
@@ -31,6 +32,18 @@
 
 #define RZT2H_ADDR_REG(x)		(0x20 + 0x2 * (x))
 
+#define RZT2H_ADSSTRn(n)		(0xe0 + 0x1 * (n))
+
+/*
+ * Each A/D channel conversion takes a fixed base of 13 ADCLK cycles plus the
+ * sample time programmed in ADSSTRn, giving a conversion rate of
+ * ADCLK / (13 + ADSSTRn).
+ */
+#define RZT2H_ADC_CONV_CYCLES_BASE	13
+#define RZT2H_ADC_SST_MIN		0x7
+#define RZT2H_ADC_SST_MAX		0xff
+#define RZT2H_ADC_SST_DEFAULT		0xb
+
 #define RZT2H_ADCALCTL_REG		0x1f0
 #define RZT2H_ADCALCTL_CAL_MASK		BIT(0)
 #define RZT2H_ADCALCTL_CAL_RDY_MASK	BIT(1)
@@ -79,6 +92,10 @@ struct rzt2h_adc {
 	unsigned int num_channels;
 	u16 buf[RZT2H_ADC_MAX_CHANNELS];
 
+	unsigned long adclk_rate;
+	int samp_freq_avail[3];
+	u8 sst[RZT2H_ADC_MAX_CHANNELS];
+
 	int irq;
 };
 
@@ -109,6 +126,11 @@ static void rzt2h_adc_stop(struct rzt2h_adc *adc)
 	writew(reg, adc->base + RZT2H_ADCSR_REG);
 }
 
+static void rzt2h_adc_set_sst(struct rzt2h_adc *adc, unsigned int ch, u8 sst)
+{
+	writeb(sst, adc->base + RZT2H_ADSSTRn(ch));
+}
+
 static int rzt2h_adc_read_single(struct rzt2h_adc *adc, unsigned int ch, int *val)
 {
 	int ret;
@@ -124,13 +146,18 @@ static int rzt2h_adc_read_single(struct rzt2h_adc *adc, unsigned int ch, int *va
 	/* Enable a single channel */
 	writew(RZT2H_ADANSA0_CH_MASK(ch), adc->base + RZT2H_ADANSA0_REG);
 
+	rzt2h_adc_set_sst(adc, ch, adc->sst[ch]);
+
 	rzt2h_adc_start(adc, RZT2H_ADCSR_ADCS_SINGLE);
 
 	/*
-	 * Datasheet Page 2770, Table 41.1:
-	 * 0.32us per channel when sample-and-hold circuits are not in use.
+	 * Conversion can take up to ~4.3us at the maximum configurable ADSSTRn
+	 * (ADSSTRn + 13 cycles at 62.5 MHz), which rounds up to 1 jiffy. A bare
+	 * 1-jiffy timeout can expire almost immediately if it's armed right
+	 * before a tick, so add one more jiffy to guarantee the conversion time
+	 * actually elapses.
 	 */
-	ret = wait_for_completion_timeout(&adc->completion, usecs_to_jiffies(1));
+	ret = wait_for_completion_timeout(&adc->completion, usecs_to_jiffies(5) + 1);
 	if (!ret) {
 		ret = -ETIMEDOUT;
 		goto disable;
@@ -407,6 +434,7 @@ static int rzt2h_adc_buffer_postenable(struct iio_dev *indio_dev)
 	iio_for_each_active_channel(indio_dev, scan_index) {
 		chan = &indio_dev->channels[scan_index];
 		val |= RZT2H_ADANSA0_CH_MASK(chan->channel);
+		rzt2h_adc_set_sst(adc, chan->channel, adc->sst[chan->channel]);
 	}
 
 	writew(val, adc->base + RZT2H_ADANSA0_REG);
@@ -443,6 +471,23 @@ static int rzt2h_adc_buffer_predisable(struct iio_dev *indio_dev)
 	return 0;
 }
 
+static int rzt2h_adc_sst_to_freq(struct rzt2h_adc *adc, u8 sst)
+{
+	return adc->adclk_rate / (RZT2H_ADC_CONV_CYCLES_BASE + sst);
+}
+
+static u8 rzt2h_adc_freq_to_sst(struct rzt2h_adc *adc, int freq)
+{
+	unsigned int cycles;
+
+	cycles = DIV_ROUND_CLOSEST(adc->adclk_rate, freq);
+	cycles = clamp_val(cycles,
+			   RZT2H_ADC_CONV_CYCLES_BASE + RZT2H_ADC_SST_MIN,
+			   RZT2H_ADC_CONV_CYCLES_BASE + RZT2H_ADC_SST_MAX);
+
+	return cycles - RZT2H_ADC_CONV_CYCLES_BASE;
+}
+
 static int rzt2h_adc_read_raw(struct iio_dev *indio_dev,
 			      struct iio_chan_spec const *chan,
 			      int *val, int *val2, long mask)
@@ -461,6 +506,51 @@ static int rzt2h_adc_read_raw(struct iio_dev *indio_dev,
 		*val = 1800;
 		*val2 = 12;
 		return IIO_VAL_FRACTIONAL_LOG2;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = rzt2h_adc_sst_to_freq(adc, adc->sst[chan->channel]);
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int rzt2h_adc_write_raw(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int val, int val2, long mask)
+{
+	struct rzt2h_adc *adc = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ: {
+		if (val <= 0)
+			return -EINVAL;
+
+		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+		if (IIO_DEV_ACQUIRE_FAILED(claim))
+			return -EBUSY;
+
+		adc->sst[chan->channel] = rzt2h_adc_freq_to_sst(adc, val);
+
+		return 0;
+	}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int rzt2h_adc_read_avail(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				const int **vals, int *type, int *length,
+				long mask)
+{
+	struct rzt2h_adc *adc = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = adc->samp_freq_avail;
+		*type = IIO_VAL_INT;
+		*length = ARRAY_SIZE(adc->samp_freq_avail);
+		return IIO_AVAIL_RANGE;
 	default:
 		return -EINVAL;
 	}
@@ -473,6 +563,8 @@ static const struct iio_buffer_setup_ops rzt2h_adc_buffer_setup_ops = {
 
 static const struct iio_info rzt2h_adc_iio_info = {
 	.read_raw = rzt2h_adc_read_raw,
+	.write_raw = rzt2h_adc_write_raw,
+	.read_avail = rzt2h_adc_read_avail,
 };
 
 static irqreturn_t rzt2h_adc_isr(int irq, void *private)
@@ -487,7 +579,9 @@ static irqreturn_t rzt2h_adc_isr(int irq, void *private)
 static const struct iio_chan_spec rzt2h_adc_chan_template = {
 	.indexed = 1,
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-			      BIT(IIO_CHAN_INFO_SCALE),
+			      BIT(IIO_CHAN_INFO_SCALE) |
+			      BIT(IIO_CHAN_INFO_SAMP_FREQ),
+	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 	.type = IIO_VOLTAGE,
 	.scan_type = {
 		.sign = 'u',
@@ -512,8 +606,11 @@ static int rzt2h_adc_parse_properties(struct rzt2h_adc *adc)
 	adc->num_channels = ret;
 	adc->channels = chan_array;
 
-	for (unsigned int i = 0; i < adc->num_channels; i++)
+	for (unsigned int i = 0; i < adc->num_channels; i++) {
+		adc->sst[chan_array[i].channel] = RZT2H_ADC_SST_DEFAULT;
+
 		chan_array[i].scan_index = i;
+	}
 
 	return 0;
 }
@@ -565,6 +662,7 @@ static int rzt2h_adc_probe(struct platform_device *pdev)
 	struct iio_dev *indio_dev;
 	struct rzt2h_adc *adc;
 	struct resource *res;
+	struct clk *adclk;
 	int ret;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*adc));
@@ -592,6 +690,18 @@ static int rzt2h_adc_probe(struct platform_device *pdev)
 
 	adc->phys_base = res->start;
 
+	adclk = devm_clk_get(dev, "adclk");
+	if (IS_ERR(adclk))
+		return dev_err_probe(dev, PTR_ERR(adclk), "failed to get adclk\n");
+
+	adc->adclk_rate = clk_get_rate(adclk);
+	if (!adc->adclk_rate)
+		return dev_err_probe(dev, -EINVAL, "invalid adclk rate\n");
+
+	adc->samp_freq_avail[0] = rzt2h_adc_sst_to_freq(adc, RZT2H_ADC_SST_MAX);
+	adc->samp_freq_avail[1] = 1;
+	adc->samp_freq_avail[2] = rzt2h_adc_sst_to_freq(adc, RZT2H_ADC_SST_MIN);
+
 	pm_runtime_set_autosuspend_delay(dev, 300);
 	pm_runtime_use_autosuspend(dev);
 	ret = devm_pm_runtime_enable(dev);
-- 
2.55.0

Re: [PATCH v2 6/9] iio: adc: rzt2h: expose sampling frequency

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Spurious interrupt / stale completion race in the timeout path can persistently corrupt subsequent conversions.
--

commit a15fac37325034f14ef0f8f0fcfbb6e8dc9066ff
Author: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>

iio: adc: rzt2h: expose sampling frequency

This commit adds support for exposing the sampling frequency attribute to let
userspace control the conversion time. It reads the adclk rate to derive
the frequency and updates the single read path to use a timeout calculated
from the maximum possible conversion time.

> diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
> index 0460dffe189e2..8ba7b0fde8a19 100644
> --- a/drivers/iio/adc/rzt2h_adc.c
> +++ b/drivers/iio/adc/rzt2h_adc.c

[ ... ]

> @@ -124,13 +146,18 @@ static int rzt2h_adc_read_single(struct rzt2h_adc *adc, unsigned int ch, int *va
>  	/* Enable a single channel */
>  	writew(RZT2H_ADANSA0_CH_MASK(ch), adc->base + RZT2H_ADANSA0_REG);
>  
> +	rzt2h_adc_set_sst(adc, ch, adc->sst[ch]);
> +
>  	rzt2h_adc_start(adc, RZT2H_ADCSR_ADCS_SINGLE);
>  
>  	/*
> -	 * Datasheet Page 2770, Table 41.1:
> -	 * 0.32us per channel when sample-and-hold circuits are not in use.
> +	 * Conversion can take up to ~4.3us at the maximum configurable ADSSTRn
> +	 * (ADSSTRn + 13 cycles at 62.5 MHz), which rounds up to 1 jiffy. A bare
> +	 * 1-jiffy timeout can expire almost immediately if it's armed right
> +	 * before a tick, so add one more jiffy to guarantee the conversion time
> +	 * actually elapses.
>  	 */
> -	ret = wait_for_completion_timeout(&adc->completion, usecs_to_jiffies(1));
> +	ret = wait_for_completion_timeout(&adc->completion, usecs_to_jiffies(5) + 1);
>  	if (!ret) {
>  		ret = -ETIMEDOUT;
>  		goto disable;

[Severity: High]
This is a pre-existing issue, but does branching to the disable path on a 
timeout leave the driver vulnerable to a race with the interrupt handler?

If the timeout expires and the hardware interrupt fires immediately after,
the ISR can execute locklessly and call complete(&adc->completion).

Since the disable path calls rzt2h_adc_stop(), which only masks the interrupt
at the device level without waiting for executing handlers via 
synchronize_irq(), could the delayed interrupt complete the structure after
the next read operation calls reinit_completion() in rzt2h_adc_read_single()?

If this sequence occurs, the next read could return immediately with stale
data, and its own triggered conversion could eventually interrupt and corrupt
the subsequent read, leading to a persistent desynchronization.

>  	}
>  
>  	*val = readw(adc->base + RZT2H_ADDR_REG(ch));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910085430.3173166-1-cosmin-gabriel.tanislav.xa@renesas.com?part=6

Re: [PATCH v2 6/9] iio: adc: rzt2h: expose sampling frequency

[Jonathan Cameron]

> Expose the sampling frequency as a per-channel IIO_CHAN_INFO_SAMP_FREQ
> to let userspace control conversion time.
> 
> Each channel conversion takes a fixed 13 ADCLK cycles plus the sample
> time programmed in ADSSTRn, giving a rate of ADCLK / (13 + ADSSTRn).
> 
> Read the ADCLK rate from the "adclk" clock to derive the frequency.
> 
> Claim direct mode while writing so the rate cannot change during a
> capture.
> 
> Program the sample time into ADSSTRn for each enabled channel on single
> reads and on buffer enable.
> 
> Wait for the maximum amount of time a conversion can take + 1 jiffy for
> the completion event to come after triggering a single read.
> 
> Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>

There is a sashiko comment on this one (See below) but I think it's
in the category of things that we don't are about if a timeout
has occured.  Please sanity check but I'll not hold series for that
one.

>
> diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
> index 0460dffe189e..8ba7b0fde8a1 100644
> --- a/drivers/iio/adc/rzt2h_adc.c
> +++ b/drivers/iio/adc/rzt2h_adc.c
> @@ -2,6 +2,7 @@
>  
>  #include <linux/bitfield.h>
>  #include <linux/cleanup.h>
> +#include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/delay.h>
>  #include <linux/dma-mapping.h>

..

> -	ret = wait_for_completion_timeout(&adc->completion, usecs_to_jiffies(1));
> +	ret = wait_for_completion_timeout(&adc->completion, usecs_to_jiffies(5) + 1);
>  	if (!ret) {
>  		ret = -ETIMEDOUT;
>  		goto disable;

Sashiko:
[Severity: High]
This is a pre-existing issue, but does branching to the disable path on a 
timeout leave the driver vulnerable to a race with the interrupt handler?

If the timeout expires and the hardware interrupt fires immediately after,
the ISR can execute locklessly and call complete(&adc->completion).

Since the disable path calls rzt2h_adc_stop(), which only masks the interrupt
at the device level without waiting for executing handlers via 
synchronize_irq(), could the delayed interrupt complete the structure after
the next read operation calls reinit_completion() in rzt2h_adc_read_single()?

If this sequence occurs, the next read could return immediately with stale
data, and its own triggered conversion could eventually interrupt and corrupt
the subsequent read, leading to a persistent desynchronization.
-
With a timeout we never expect to see I don't think it makes sense
to care too much about stale data.

Jonathan

-- 
Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

RE: [PATCH v2 6/9] iio: adc: rzt2h: expose sampling frequency

[Cosmin-Gabriel Tanislav]

> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Monday, September 14, 2026 12:13 AM
> 
> > Expose the sampling frequency as a per-channel IIO_CHAN_INFO_SAMP_FREQ
> > to let userspace control conversion time.
> >
> > Each channel conversion takes a fixed 13 ADCLK cycles plus the sample
> > time programmed in ADSSTRn, giving a rate of ADCLK / (13 + ADSSTRn).
> >
> > Read the ADCLK rate from the "adclk" clock to derive the frequency.
> >
> > Claim direct mode while writing so the rate cannot change during a
> > capture.
> >
> > Program the sample time into ADSSTRn for each enabled channel on single
> > reads and on buffer enable.
> >
> > Wait for the maximum amount of time a conversion can take + 1 jiffy for
> > the completion event to come after triggering a single read.
> >
> > Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> 
> There is a sashiko comment on this one (See below) but I think it's
> in the category of things that we don't are about if a timeout
> has occured.  Please sanity check but I'll not hold series for that
> one.
> 
> >
> > diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
> > index 0460dffe189e..8ba7b0fde8a1 100644
> > --- a/drivers/iio/adc/rzt2h_adc.c
> > +++ b/drivers/iio/adc/rzt2h_adc.c
> > @@ -2,6 +2,7 @@
> >
> >  #include <linux/bitfield.h>
> >  #include <linux/cleanup.h>
> > +#include <linux/clk.h>
> >  #include <linux/completion.h>
> >  #include <linux/delay.h>
> >  #include <linux/dma-mapping.h>
> 
> ..
> 
> > -	ret = wait_for_completion_timeout(&adc->completion, usecs_to_jiffies(1));
> > +	ret = wait_for_completion_timeout(&adc->completion, usecs_to_jiffies(5) + 1);
> >  	if (!ret) {
> >  		ret = -ETIMEDOUT;
> >  		goto disable;
> 
> Sashiko:
> [Severity: High]
> This is a pre-existing issue, but does branching to the disable path on a
> timeout leave the driver vulnerable to a race with the interrupt handler?
> 
> If the timeout expires and the hardware interrupt fires immediately after,
> the ISR can execute locklessly and call complete(&adc->completion).
> 
> Since the disable path calls rzt2h_adc_stop(), which only masks the interrupt
> at the device level without waiting for executing handlers via
> synchronize_irq(), could the delayed interrupt complete the structure after
> the next read operation calls reinit_completion() in rzt2h_adc_read_single()?
> 
> If this sequence occurs, the next read could return immediately with stale
> data, and its own triggered conversion could eventually interrupt and corrupt
> the subsequent read, leading to a persistent desynchronization.
> -
> With a timeout we never expect to see I don't think it makes sense
> to care too much about stale data.
> 

If a timeout occurs with 1-2 jiffies of leeway at standard CONFIG_HZ
values (plus however much time it takes for the user to read the next
value), something is terribly wrong. I don't think we need to handle
this situation.

[PATCH v2 7/9] dt-bindings: iio: adc: renesas,r9a09g077-adc: document DMA support

[Cosmin Tanislav]

The ADC can transfer conversion results to memory over DMA.

Allow up to three DMA specifiers, as the ADC conversion DMA request can
be routed to any of the SoC's three DMA controllers.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
---

V2:
 * pick up Conor's Acked-by

 .../bindings/iio/adc/renesas,r9a09g077-adc.yaml      | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,r9a09g077-adc.yaml b/Documentation/devicetree/bindings/iio/adc/renesas,r9a09g077-adc.yaml
index dc0206b28231..4b5934ac296b 100644
--- a/Documentation/devicetree/bindings/iio/adc/renesas,r9a09g077-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/renesas,r9a09g077-adc.yaml
@@ -57,6 +57,18 @@ properties:
       - const: adclk
       - const: pclk
 
+  dmas:
+    minItems: 1
+    maxItems: 3
+    description:
+      Must contain a list of references to DMA RX specifiers.
+
+  dma-names:
+    minItems: 1
+    maxItems: 3
+    items:
+      const: rx
+
   power-domains:
     maxItems: 1
 
-- 
2.55.0

Re: [PATCH v2 7/9] dt-bindings: iio: adc: renesas,r9a09g077-adc: document DMA support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The IIO channel specifies `IIO_CPU` endianness for the DMA buffer, but the driver copies the raw hardware samples without converting them to native CPU endianness.
--

commit 7eb4eed24fc9164ecef6de1efee2ea6e90c109dc
Author: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>

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.

> diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
> index 95bcebdc02cb3..0460dffe189e2 100644
> --- a/drivers/iio/adc/rzt2h_adc.c
> +++ b/drivers/iio/adc/rzt2h_adc.c

[ ... ]

> @@ -151,6 +186,263 @@ static int rzt2h_adc_calibrate(struct rzt2h_adc *adc)
>  	return 0;
>  }
>  
> +static void rzt2h_adc_push_period(struct iio_dev *indio_dev, u16 *period,
> +				  dma_addr_t addr)
> +{
> +	struct rzt2h_adc *adc = iio_priv(indio_dev);
> +	u16 *dst = adc->buf;
> +	u16 *src = period;
> +
> +	dma_sync_single_for_cpu(adc->dev, addr, adc->dma.period_bytes,
> +				DMA_FROM_DEVICE);
> +
> +	for (unsigned int sample = 0; sample < RZT2H_ADC_DMA_PERIOD_SAMPLES; sample++) {
> +		for (unsigned int i = 0; i < adc->dma.gather_len; i++)
> +			dst[i] = src[adc->dma.gather[i]];

[Severity: High]
Does this code copy the raw hardware samples into the IIO buffer without
converting them to native CPU endianness?

If a user-space application enables the IIO buffer and reads samples from the
ADC on a big-endian architecture, the hardware writes little-endian 16-bit
words to DMA memory. This function appears to copy those raw little-endian
bytes verbatim.

[ ... ]

> @@ -192,6 +489,12 @@ static const struct iio_chan_spec rzt2h_adc_chan_template = {
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>  			      BIT(IIO_CHAN_INFO_SCALE),
>  	.type = IIO_VOLTAGE,
> +	.scan_type = {
> +		.sign = 'u',
> +		.realbits = 12,
> +		.storagebits = 16,
> +		.endianness = IIO_CPU,
> +	},
>  };

[Severity: High]
Does setting endianness to IIO_CPU incorrectly inform the IIO core and
userspace that the buffered data is in native CPU endianness?

Because the channel is marked as IIO_CPU here, the IIO core and userspace
assume the data is natively ordered. Combined with the verbatim copy in
rzt2h_adc_push_period(), they might interpret the little-endian bytes
incorrectly on big-endian CPUs, potentially leading to data corruption where
applications read swapped bytes.

Should the little-endian data be converted to CPU endianness before being
pushed to the buffer, or should the scan type reflect the actual endianness
of the hardware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910085430.3173166-1-cosmin-gabriel.tanislav.xa@renesas.com?part=7

[PATCH v2 8/9] arm64: dts: renesas: r9a09g077: Wire up DMA support for ADC

[Cosmin Tanislav]

RZ/T2H (R9A09G077) has three DMA controllers that can be used by
peripherals like the ADC to offload data transfers from the CPU.

Wire up the DMA channels for the ADC peripherals.

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

V2:
 * no changes

 arch/arm64/boot/dts/renesas/r9a09g077.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g077.dtsi b/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
index 42883338a6fd..5ea10361e300 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g077.dtsi
@@ -1275,6 +1275,8 @@ adc0: adc@90014000 {
 			clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKL>,
 				 <&cpg CPG_MOD 206>;
 			clock-names = "adclk", "pclk";
+			dmas = <&dmac0 0x26ba>, <&dmac1 0x26ba>, <&dmac2 0x26ba>;
+			dma-names = "rx", "rx", "rx";
 			power-domains = <&cpg>;
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -1297,6 +1299,8 @@ adc1: adc@90014400 {
 			clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKL>,
 				 <&cpg CPG_MOD 207>;
 			clock-names = "adclk", "pclk";
+			dmas = <&dmac0 0x26bf>, <&dmac1 0x26bf>, <&dmac2 0x26bf>;
+			dma-names = "rx", "rx", "rx";
 			power-domains = <&cpg>;
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -1319,6 +1323,8 @@ adc2: adc@80008000 {
 			clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKL>,
 				 <&cpg CPG_MOD 225>;
 			clock-names = "adclk", "pclk";
+			dmas = <&dmac0 0x26c4>, <&dmac1 0x26c4>, <&dmac2 0x26c4>;
+			dma-names = "rx", "rx", "rx";
 			power-domains = <&cpg>;
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
2.55.0

[PATCH v2 9/9] arm64: dts: renesas: r9a09g087: Wire up DMA support for ADC

[Cosmin Tanislav]

RZ/N2H (R9A09G087) has three DMA controllers that can be used by
peripherals like the ADC to offload data transfers from the CPU.

Wire up the DMA channels for the ADC peripherals.

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

V2:
 * no changes

 arch/arm64/boot/dts/renesas/r9a09g087.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g087.dtsi b/arch/arm64/boot/dts/renesas/r9a09g087.dtsi
index 437f45649ecd..c9acf37db051 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g087.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g087.dtsi
@@ -1278,6 +1278,8 @@ adc0: adc@90014000 {
 			clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKL>,
 				 <&cpg CPG_MOD 206>;
 			clock-names = "adclk", "pclk";
+			dmas = <&dmac0 0x26ba>, <&dmac1 0x26ba>, <&dmac2 0x26ba>;
+			dma-names = "rx", "rx", "rx";
 			power-domains = <&cpg>;
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -1300,6 +1302,8 @@ adc1: adc@90014400 {
 			clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKL>,
 				 <&cpg CPG_MOD 207>;
 			clock-names = "adclk", "pclk";
+			dmas = <&dmac0 0x26bf>, <&dmac1 0x26bf>, <&dmac2 0x26bf>;
+			dma-names = "rx", "rx", "rx";
 			power-domains = <&cpg>;
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -1322,6 +1326,8 @@ adc2: adc@80008000 {
 			clocks = <&cpg CPG_CORE R9A09G087_CLK_PCLKL>,
 				 <&cpg CPG_MOD 225>;
 			clock-names = "adclk", "pclk";
+			dmas = <&dmac0 0x26c4>, <&dmac1 0x26c4>, <&dmac2 0x26c4>;
+			dma-names = "rx", "rx", "rx";
 			power-domains = <&cpg>;
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
2.55.0

Re: [PATCH v2 0/9] iio: adc: rzt2h: add DMA buffer support

[Jonathan Cameron]

> Add DMA-based buffered capture to the Renesas RZ/T2H and RZ/N2H ADC
> driver and the necessary device-tree properties. Also add a per-channel
> sampling-frequency control.
> 
> Capture uses a cyclic DMA transfer into a non-coherent buffer. The DMA
> controller has no per-descriptor scatter-gather and each transfer needs
> to be of power-of-two width, so the transfer covers the smallest
> power-of-two-aligned group of channel registers spanning the enabled
> channels, and a consumer kthread compacts each completed period down to
> the enabled channels before handing it to the IIO core.

Looks good to me. I did reply to a couple of patchs, but nothing that
requires a v3.  Patches 1-7 applied to the testing branch of iio.git.
I'll send that out as togreg once 0-day has had a look.

Nice little series. Thanks!

Jonathan

-- 
Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>