Race condition in adc_read on nrf52832/zephyr

[Poorp]

Hi

I'm trying to use the battery sample to create some code that reads the battery level at boot. However I'm experiencing what I believe to be a race condition

If I run the sample below, without k_sleep, it seems that adc_read hangs forever. If I run the same code through a debugger with breakpoints, then everything works fine as far as I can tell.
If I run the code snippet, with k_sleep uncommented, everything works fine.

I have lightly modified the sample code because I couldn't seem to get it to find a device called ADC_0, so I have hardcoded it to use the saadc. I have attached the exact files I use, with some names changed to ensure anonymity. If it doesn't compile, that's why.

void powermgr_init(void)
{
	battery_measure_enable(true);
	//k_sleep(K_MSEC(1));
	LOG_INF("after sleep");
	int batt_mV = battery_sample();

	if (batt_mV < 0) {
		LOG_ERR("Failed to read battery voltage: %d\n",
			batt_mV);
	}
	LOG_INF("battery mv %d", batt_mV);
	//lpcomp_init_buckle();

}

log output

*** Booting Zephyr OS build v2.4.0-ncs1-1710-g5a7b4eb71047  ***
[00:00:00.000,457] <inf> BATTERY: inside else
[00:00:00.000,488] <inf> BATTERY: Setup AIN0 got 0
[00:00:00.000,488] <inf> BATTERY: Battery setup: 0 1
[00:00:01.704,620] <inf> powermgr_buckle: after sleep
[00:00:01.704,650] <inf> BATTERY: inside sample
[00:00:01.704,650] <inf> BATTERY: before read

Fullscreen battery.c Download

/*
 * Copyright (c) 2018-2019 Peter Bigot Consulting, LLC
 * Copyright (c) 2019-2020 Nordic Semiconductor ASA
 *
 * SPDX-License-Identifier: Apache-2.0
 */

#include <math.h>
#include <stdio.h>
#include <stdlib.h>

#include <zephyr.h>
#include <init.h>
#include <drivers/gpio.h>
#include <drivers/adc.h>
#include <drivers/sensor.h>
#include <logging/log.h>

#include "battery"

LOG_MODULE_REGISTER(BATTERY, CONFIG_ADC_LOG_LEVEL);

#define VBATT DT_PATH(vbatt)

#ifdef CONFIG_BOARD_THINGY52_NRF52832
/* This board uses a divider that reduces max voltage to
 * reference voltage (600 mV).
 */
#define BATTERY_ADC_GAIN ADC_GAIN_1
#else
/* Other boards may use dividers that only reduce battery voltage to
 * the maximum supported by the hardware (3.6 V)
 */
#define BATTERY_ADC_GAIN ADC_GAIN_1_6
#endif

struct io_channel_config {
	const char *label;
	uint8_t channel;
};

struct gpio_channel_config {
	const char *label;
	uint8_t pin;
	uint8_t flags;
};

struct divider_config {
	struct io_channel_config io_channel;
	struct gpio_channel_config power_gpios;
	/* output_ohm is used as a flag value: if it is nonzero then
	 * the battery is measured through a voltage divider;
	 * otherwise it is assumed to be directly connected to Vdd.
	 */
	uint32_t output_ohm;
	uint32_t full_ohm;
};

static const struct divider_config divider_config = {
#if DT_NODE_HAS_STATUS(VBATT, okay)
	.io_channel = {
		DT_IO_CHANNELS_LABEL(VBATT),
		DT_IO_CHANNELS_INPUT(VBATT),
	},
#if DT_NODE_HAS_PROP(VBATT, power_gpios)
	.power_gpios = {
		DT_GPIO_LABEL(VBATT, power_gpios),
		DT_GPIO_PIN(VBATT, power_gpios),
		DT_GPIO_FLAGS(VBATT, power_gpios),
	},
#endif
	.output_ohm = DT_PROP(VBATT, output_ohms),
	.full_ohm = DT_PROP(VBATT, full_ohms),
#else /* /vbatt exists */
	.io_channel = {
		//DT_LABEL(DT_ALIAS(batterysense)),
		DT_LABEL(DT_INST(0, nordic_nrf_saadc)),
	},
#endif /* /vbatt exists */
};

struct divider_data {
	const struct device *adc;
	const struct device *gpio;
	struct adc_channel_cfg adc_cfg;
	struct adc_sequence adc_seq;
	int16_t raw;
};
static struct divider_data divider_data;

static int divider_setup(void)
{
	const struct divider_config *cfg = &divider_config;
	const struct io_channel_config *iocp = &cfg->io_channel;
	const struct gpio_channel_config *gcp = &cfg->power_gpios;
	struct divider_data *ddp = &divider_data;
	struct adc_sequence *asp = &ddp->adc_seq;
	struct adc_channel_cfg *accp = &ddp->adc_cfg;
	int rc;

	if (iocp->label == NULL) {
		return -ENOTSUP;
	}

	ddp->adc = device_get_binding(iocp->label);
	if (ddp->adc == NULL) {
		LOG_ERR("Failed to get ADC %s", iocp->label);
		return -ENOENT;
	}

	if (gcp->label) {
		ddp->gpio = device_get_binding(gcp->label);
		if (ddp->gpio == NULL) {
			LOG_ERR("Failed to get GPIO %s", gcp->label);
			return -ENOENT;
		}
		rc = gpio_pin_configure(ddp->gpio, gcp->pin,
					GPIO_OUTPUT_INACTIVE | gcp->flags);
		if (rc != 0) {
			LOG_ERR("Failed to control feed %s.%u: %d",
				gcp->label, gcp->pin, rc);
			return rc;
		}
	}

	*asp = (struct adc_sequence){
		.channels = BIT(0),
		.buffer = &ddp->raw,
		.buffer_size = sizeof(ddp->raw),
		.oversampling = 4,
		.calibrate = true,
	};

#ifdef CONFIG_ADC_NRFX_SAADC
	*accp = (struct adc_channel_cfg){
		.gain = BATTERY_ADC_GAIN,
		.reference = ADC_REF_INTERNAL,
		.acquisition_time = ADC_ACQ_TIME(ADC_ACQ_TIME_MICROSECONDS, 40),
	};

	if (cfg->output_ohm != 0) {
		accp->input_positive = SAADC_CH_PSELP_PSELP_AnalogInput7
			+ iocp->channel;
		LOG_INF("inside if");
	} else {
		accp->input_positive = SAADC_CH_PSELP_PSELP_VDD;
		LOG_INF("inside else");
	}

	asp->resolution = 14;
#else /* CONFIG_ADC_var */
#error Unsupported ADC
#endif /* CONFIG_ADC_var */

	rc = adc_channel_setup(ddp->adc, accp);
	LOG_INF("Setup AIN%u got %d", iocp->channel, rc);

	return rc;
}

static bool battery_ok;

static int battery_setup(const struct device *arg)
{
	int rc = divider_setup();

	battery_ok = (rc == 0);
	LOG_INF("Battery setup: %d %d", rc, battery_ok);
	return rc;
}

SYS_INIT(battery_setup, APPLICATION, CONFIG_APPLICATION_INIT_PRIORITY);

int battery_measure_enable(bool enable)
{
	int rc = -ENOENT;

	if (battery_ok) {
		const struct divider_data *ddp = &divider_data;
		const struct gpio_channel_config *gcp = &divider_config.power_gpios;

		rc = 0;
		if (ddp->gpio) {
			rc = gpio_pin_set(ddp->gpio, gcp->pin, enable);
		}
	}
	return rc;
}

int battery_sample(void)
{
	int rc = -ENOENT;
	LOG_INF("inside sample");
	if (battery_ok) {
		struct divider_data *ddp = &divider_data;
		const struct divider_config *dcp = &divider_config;
		struct adc_sequence *sp = &ddp->adc_seq;
		LOG_INF("before read");
		rc = adc_read(ddp->adc, sp);
		LOG_INF("after read");
		sp->calibrate = false;
		if (rc == 0) {
			LOG_INF("inside sample if");
			int32_t val = ddp->raw;

			adc_raw_to_millivolts(adc_ref_internal(ddp->adc),
					      ddp->adc_cfg.gain,
					      sp->resolution,
					      &val);

			if (dcp->output_ohm != 0) {
				rc = val * (uint64_t)dcp->full_ohm
					/ dcp->output_ohm;
				LOG_INF("raw %u ~ %u mV => %d mV\n",
					ddp->raw, val, rc);
			} else {
				rc = val;
				LOG_INF("raw %u ~ %u mV\n", ddp->raw, val);
			}
		}
	}

	return rc;
}

unsigned int battery_level_pptt(unsigned int batt_mV,
				const struct battery_level_point *curve)
{
	const struct battery_level_point *pb = curve;

	if (batt_mV >= pb->lvl_mV) {
		/* Measured voltage above highest point, cap at maximum. */
		return pb->lvl_pptt;
	}
	/* Go down to the last point at or below the measured voltage. */
	while ((pb->lvl_pptt > 0)
	       && (batt_mV < pb->lvl_mV)) {
		++pb;
	}
	if (batt_mV < pb->lvl_mV) {
		/* Below lowest point, cap at minimum */
		return pb->lvl_pptt;
	}

	/* Linear interpolation between below and above points. */
	const struct battery_level_point *pa = pb - 1;

	return pb->lvl_pptt
	       + ((pa->lvl_pptt - pb->lvl_pptt)
		  * (batt_mV - pb->lvl_mV)
		  / (pa->lvl_mV - pb->lvl_mV));
}

battery.h

[Jørgen Holmefjord]

Hi,

Which NCS version are you using? Can you post your prj.conf and dts/overlay files?

I tried to reproduce your issue by adding the battery.c/h files to Bluetooth peripheral_uart example in NCS v1.5, but it seems to work fine without the delay:

00> [00:00:00.006,561] <inf> BATTERY: inside else
00> [00:00:00.006,591] <inf> BATTERY: Setup AIN0 got 0
00> [00:00:00.006,591] <inf> BATTERY: Battery setup: 0 1
00> [00:00:00.006,927] <inf> peripheral_uart: after sleep
00> [00:00:00.006,927] <inf> BATTERY: inside sample
00> [00:00:00.006,927] <inf> BATTERY: before read
00> [00:00:00.011,566] <inf> BATTERY: after read
00> [00:00:00.011,596] <inf> BATTERY: inside sample if
00> [00:00:00.011,596] <inf> BATTERY: raw 13175 ~ 2894 mV

I added the following configs to prj.conf:

# ADC
CONFIG_ADC=y
CONFIG_ADC_ASYNC=y
CONFIG_ADC_LOG_LEVEL_DBG=y

Best regards,
Jørgen

[Jørgen Holmefjord]

Hi,

Just wanted to let you know that I was able to reproduce the issue you are describing with your provided application. I'm still not able to reproduce with my own application through, so I will need some more time to determine the differences.

Best regards,
Jørgen

[Poorp]

Hi

Do you have any update on a fix for this issue?