[SAADC] Zephyr's SAADC Driver (DMA Design choices analysis)

[Pawel(embeddedsolutions.pl)]

Setup:
NCS v2.8.0
nRF52840

Hi,

I would like to ask about the implementation of Zephyr's SAADC driver (`adc_nrfx_saadc.c`).

In the project I'm working on, I need to sample the ADC at 16 kHz.
However, I don't need to process every individual sample in the application. Instead, I would prefer to collect around 320 samples (equivalent to 20 ms of data) and then raise an interrupt. DMA seems like a perfect fit for this use case.

Initially, I thought that by using the `struct adc_sequence_options`—specifically the `extra_samplings` field—the SAADC driver would configure the `RESULT.MAXCNT` register to match the total number of samples (including `extra_samplings`), and thus trigger an interrupt only after all samples had been collected.

However, this does not appear to be the case. For each `adc_sequence` triggered by `adc_read()` or `adc_async_read()`, the driver calls `nrf_saadc_buffer_init()` with a buffer size (`RESULT.MAXCNT`) equal only to the number of channels selected for that sequence.
In my case, with a single channel, this results in `RESULT.MAXCNT = 1`, which causes the SAADC interrupt to fire after each individual sample—even if `extra_samplings` is set to a higher value.

This means the MCU is being interrupted every 62.5 μs, which is far too frequent for my application.

**My questions are:**

0. Am I correct in my understanding above?

1. Why was this design decision made in the SAADC driver implementation? What benefit does it provide?

2. What would be the best way to extend the SAADC driver to fully support single-channel continuous conversion mode, without needing to fork the driver?

3. What happens if an ADC sequence is triggered while another is still running? Will the previous sequence be aborted?

I look forward to your response.

Best regards,
Pawel

[Turbo J]

The ADC drivers are written with low power usage in mind and not to be compatible to audio frequency.

I would recommend not to fork the driver but to write a new one from scratch, possibly even without the NRFX hardware abstraction layer, as I have not check if this layer allows high sampling frequencies either.

The idea is to do something similar to the audio/I²S drivers, particularly how these handle lots of data with the k_mem_slab_ stuff.

[Pawel(embeddedsolutions.pl)]

Thanks, that explains much.

Would it be possible to use custom ADC driver together with Zephyr's one? 
Of course with proper synchronization. 

[Kazi Afroza Sultana]

Hello,

For implementing a high-frequency ADC solution you may consider using TIMER and PPI/DPPI to trigger ADC sampling at precise intervals without CPU involvement. This previous case (+) Using nRF52832 ADC for audio encoding - Nordic Q&A - Nordic DevZone - Nordic DevZone described about the implementation of TIMER and PPI.

For continuous sampling, you can implement double buffering also.

/* STEP 4.1 - Define the buffer size for the SAADC */
#define SAADC_BUFFER_SIZE 8000

/* STEP 4.2 - Declare the buffers for the SAADC */
static int16_t saadc_sample_buffer[2][SAADC_BUFFER_SIZE];
static uint32_t saadc_current_buffer = 0;

static void saadc_event_handler(nrfx_saadc_evt_t const * p_event)
{
    nrfx_err_t err;
    switch (p_event->type)
    {
        case NRFX_SAADC_EVT_READY:
            /* STEP 5.1 - Buffer is ready, timer (and sampling) can be started. */
            nrfx_timer_enable(&timer_instance);
            break;
            
        case NRFX_SAADC_EVT_BUF_REQ:
            /* STEP 5.2 - Set up the next available buffer. Alternate between buffer 0
             * and 1 */
            err = nrfx_saadc_buffer_set(
                saadc_sample_buffer[(saadc_current_buffer++) % 2], SAADC_BUFFER_SIZE);
            if (err != NRFX_SUCCESS) {
                LOG_ERR("nrfx_saadc_buffer_set error: %08x", err);
                return;
            }
            break;

        case NRFX_SAADC_EVT_DONE:
            /* STEP 5.3 - Buffer has been filled. Do something with the data */
            int64_t average = 0;
            int16_t max = INT16_MIN;
            int16_t min = INT16_MAX;
            int16_t current_value;
            
            for (int i = 0; i < p_event->data.done.size; i++) {
                current_value = ((int16_t *)(p_event->data.done.p_buffer))[i];
                average += current_value;
                if (current_value > max) {
                    max = current_value;
                }
                if (current_value < min) {
                    min = current_value;
                }
            }
            average = average / p_event->data.done.size;
            LOG_INF("SAADC buffer at 0x%x filled with %d samples",
                    (uint32_t)p_event->data.done.p_buffer, p_event->data.done.size);
            LOG_INF("AVG=%d, MIN=%d, MAX=%d", (int16_t)average, min, max);
            break;
            
        default:
            LOG_INF("Unhandled SAADC evt %d", p_event->type);
            break;
    }
}

How to set up double-buffering for continuous SAADC sampling, including the use of two buffers, the event handler, and the use of TIMER and (D)PPI for precise sampling intervals.

You can find the original source and detailed step-by-step instructions here: https://academy.nordicsemi.com/courses/nrf-connect-sdk-intermediate/lessons/lesson-6-analog-to-digital-converter-adc/topic/exercise-3-interfacing-with-adc-using-nrfx-drivers-and-timer-ppi/ 

[Pawel(embeddedsolutions.pl)]

Hi,

Thank you for bringing up this Nordic academy exercise.

As far as I understood the code there, I believe it is suitable for the scenarios where the SAADC peripheral is used only for single purpose.

What I encountered is that I need to sample the ADC channels for different purposes e.g Audio + Battery voltage and these two cases are far far away from being similar. 

What I have done so far is bringing up the HAL NRF API and use it together with Zephyr's ADC ASYNC API.
That make my code Nordic specific but I am totally fine with it. 

static int read_continuous_mode(const struct device *dev)
{
	const struct amic_config *config = dev->config;
	struct amic_data *data = dev->data;

	/*
	 * Continuous mode requires bringing up the underlying HAL SAADC NRF API.
	 * The Zephyr's SAADC driver is not able to handle the continuous mode.
	 */

	/* Disable interrupts to not be preempted before calling the `adc_read_async` */
	nrf_saadc_int_disable(NRF_SAADC, NRF_SAADC_INT_END | NRF_SAADC_INT_CALIBRATEDONE);

	/* Utilize the Zephyr's ADC async API to set the resolution and oversampling configuration
	 */
	const int ret = adc_read_async(config->port.dev, &data->sequence, NULL);
	if (ret) {
		LOG_ERR("Can't read adc: %d", ret);
		return ret;
	}

	/* Immediately STOP the SAADC sampling invoked by the `adc_read_async` */
	nrf_saadc_task_trigger(NRF_SAADC, NRF_SAADC_TASK_STOP);

	/* Disable SAADC sampling to modify the configuration safely */
	nrf_saadc_disable(NRF_SAADC);

	/* Prepare buffer for SAADC DMA Continuous mode*/
	nrf_saadc_buffer_init(NRF_SAADC, (nrf_saadc_value_t *)data->sequence.buffer,
			      config->samples_num);

	/* Enable continuous mode with calculated `data->interval_us` */
	nrf_saadc_continuous_mode_enable(NRF_SAADC,
					 (SAMPLERATE_FREQUENCY_RATIO * data->interval_us));

	/* Clear the `NRF_SAADC_EVENT_END` event to ensure clear state */
	nrf_saadc_event_clear(NRF_SAADC, NRF_SAADC_EVENT_END);
	/* Enable back the interrupts handled internally by the Zephyr's ADC driver. On
	 * `NRF_SAADC_EVENT_END` event the `adc_sequence_callback_handler` is called
	 */
	nrf_saadc_int_enable(NRF_SAADC, NRF_SAADC_INT_END | NRF_SAADC_INT_CALIBRATEDONE);

	/* Start sampling in continuous mode */
	nrf_saadc_enable(NRF_SAADC);
	nrf_saadc_task_trigger(NRF_SAADC, NRF_SAADC_TASK_START);
	nrf_saadc_task_trigger(NRF_SAADC, NRF_SAADC_TASK_SAMPLE);

	LOG_DBG("ADC continuous mode requested");
	return 0;
}

The next challenge I believe is to synchronize the calls of functions which use the SAADC so they won't interrupt with each other.
That is not because I brought up the NRF HAL API but because ADC_ASYNC API is in use.

Do you have any idea how this can be achievable? or maybe it is already handled by the ADC driver?