Upgrading to nRF Connect SDK v2.6.0 breaks Zephyr regulator driver startup-delay-us support

[BeekBoy2002]

In our project we use a SHT4x sensor connected to I2C bus of nRF5340.
To lower power consumption during idle/sleep we've a power line switch to disconnect the SHT4x from the power supply line (2V8).

In our device tree we've included the following to achieve this:

/* This regulator controls PSW+2V8_ON onboard supply */
pwr_switch_2v8: pwr-switch-2v8 {
    compatible = "regulator-fixed";
    enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
    regulator-name = "pwr_switch_2v8";
    regulator-boot-on;
    startup-delay-us = <5000>;
    off-on-delay-us = <5000>;
};

After upgrading to NCS v2.6.0 the SHT4x init after reset fails due to the fact the startup delay isn't applied anymore.
In Zephyr v3.5.xx the regulator driver common code is updated to support startup-delay-us and off-on-delay-us for all regulator types (not only fixed and GPIO as it was in Zephyr v3.4.xx).
However this refactoring of the code results in the startup-delay-us not being applied in all cases.

We had to modify int regulator_common_init(const struct device *dev, bool is_enabled) to get the same behavior as in nRF Connect SDK v2.5.2.
Hopefully you can verify this and get the needed change pushed in Zephyr project code base.

Changed:

  if (is_enabled) {
    data->refcnt++;
  } else if ((config->flags & REGULATOR_INIT_ENABLED) != 0U) {
    ret = api->enable(dev);
    if (ret < 0) {
      return ret;
    }

    regulator_delay(config->startup_delay_us);
    data->refcnt++;
  }

  return 0;

to:

  ret = 0;

  /* enable not supported (always on) */
  if (api->enable == NULL) {
    return 0;
  }

  if (is_enabled || ((config->flags & REGULATOR_INIT_ENABLED) != 0U)) {
    data->refcnt++;
  }

  if (data->refcnt == 1) {
    ret = api->enable(dev);
    if (ret < 0) {
      data->refcnt--;
    } else {
      regulator_delay(config->startup_delay_us);
    }
  }

  return ret;

See also:

Fullscreen 2112.regulator_common.c Download

/*
 * Copyright 2022 Nordic Semiconductor ASA
 * SPDX-License-Identifier: Apache-2.0
 */

#include <zephyr/kernel.h>
#include <zephyr/drivers/regulator.h>

static void regulator_delay(uint32_t delay_us)
{
	if (delay_us > 0U) {
#ifdef CONFIG_MULTITHREADING
		k_sleep(K_USEC(delay_us));
#else
		k_busy_wait(delay_us);
#endif
	}
}

void regulator_common_data_init(const struct device *dev)
{
	struct regulator_common_data *data = dev->data;

#ifdef CONFIG_REGULATOR_THREAD_SAFE_REFCNT
	(void)k_mutex_init(&data->lock);
#endif
	data->refcnt = 0;
}

int regulator_common_init(const struct device *dev, bool is_enabled)
{
	const struct regulator_driver_api *api = dev->api;
	const struct regulator_common_config *config = dev->config;
	struct regulator_common_data *data = dev->data;
	int32_t current_uv;
	int ret = 0;

	if (config->initial_mode != REGULATOR_INITIAL_MODE_UNKNOWN) {
		ret = regulator_set_mode(dev, config->initial_mode);
		if (ret < 0) {
			return ret;
		}
	}

	if (config->init_uv > INT32_MIN) {
		ret = regulator_set_voltage(dev, config->init_uv, config->init_uv);
		if (ret < 0) {
			return ret;
		}
	}

	/* If we have valid range values, we try to match them before enabling */
	if ((config->min_uv > INT32_MIN) || (config->max_uv < INT32_MAX)) {

		ret = regulator_get_voltage(dev, &current_uv);
		if (ret < 0) {
			return ret;
		}

		/* Snap to closest interval value if out of range */
		if (current_uv < config->min_uv) {
			ret = regulator_set_voltage(dev, config->min_uv, config->min_uv);
			if (ret < 0) {
				return ret;
			}
		} else if (current_uv > config->max_uv) {
			ret = regulator_set_voltage(dev, config->max_uv, config->max_uv);
			if (ret < 0) {
				return ret;
			}
		}
	}

	/* enable not supported (always on) */
	if (api->enable == NULL) {
		return 0;
	}

	if (is_enabled || ((config->flags & REGULATOR_INIT_ENABLED) != 0U)) {
		data->refcnt++;
	}

	if (data->refcnt == 1) {
		ret = api->enable(dev);
		if (ret < 0) {
			data->refcnt--;
		} else {
			regulator_delay(config->startup_delay_us);
		}
	}

	return ret;
}

int regulator_enable(const struct device *dev)
{
	const struct regulator_driver_api *api = dev->api;
	const struct regulator_common_config *config = dev->config;
	struct regulator_common_data *data = dev->data;
	int ret = 0;

	/* enable not supported (always on) */
	if (api->enable == NULL) {
		return 0;
	}

	/* regulator must stay always on */
	if  ((config->flags & REGULATOR_ALWAYS_ON) != 0U) {
		return 0;
	}

#ifdef CONFIG_REGULATOR_THREAD_SAFE_REFCNT
	(void)k_mutex_lock(&data->lock, K_FOREVER);
#endif

	data->refcnt++;

	if (data->refcnt == 1) {
		ret = api->enable(dev);
		if (ret < 0) {
			data->refcnt--;
		} else {
			regulator_delay(config->off_on_delay_us);
		}
	}

#ifdef CONFIG_REGULATOR_THREAD_SAFE_REFCNT
	k_mutex_unlock(&data->lock);
#endif

	return ret;
}

bool regulator_is_enabled(const struct device *dev)
{
	const struct regulator_common_config *config = dev->config;
	struct regulator_common_data *data = dev->data;
	bool enabled;

	if ((config->flags & REGULATOR_ALWAYS_ON) != 0U) {
		enabled = true;
	} else {
#ifdef CONFIG_REGULATOR_THREAD_SAFE_REFCNT
		(void)k_mutex_lock(&data->lock, K_FOREVER);
#endif
		enabled = data->refcnt != 0;
#ifdef CONFIG_REGULATOR_THREAD_SAFE_REFCNT
		k_mutex_unlock(&data->lock);
#endif
	}

	return enabled;
}

int regulator_disable(const struct device *dev)
{
	const struct regulator_driver_api *api = dev->api;
	const struct regulator_common_config *config = dev->config;
	struct regulator_common_data *data = dev->data;
	int ret = 0;

	/* disable not supported (always on) */
	if (api->disable == NULL) {
		return 0;
	}

	/* regulator must stay always on */
	if  ((config->flags & REGULATOR_ALWAYS_ON) != 0U) {
		return 0;
	}

#ifdef CONFIG_REGULATOR_THREAD_SAFE_REFCNT
	(void)k_mutex_lock(&data->lock, K_FOREVER);
#endif

	data->refcnt--;

	if (data->refcnt == 0) {
		ret = api->disable(dev);
		if (ret < 0) {
			data->refcnt++;
		}
	}

#ifdef CONFIG_REGULATOR_THREAD_SAFE_REFCNT
	k_mutex_unlock(&data->lock);
#endif

	return ret;
}

bool regulator_is_supported_voltage(const struct device *dev, int32_t min_uv,
				    int32_t max_uv)
{
	const struct regulator_common_config *config = dev->config;
	unsigned int volt_cnt;

	/* voltage may not be allowed, even if supported */
	if ((min_uv > config->max_uv) || (max_uv < config->min_uv)) {
		return false;
	}

	volt_cnt = regulator_count_voltages(dev);

	for (unsigned int idx = 0U; idx < volt_cnt; idx++) {
		int32_t volt_uv;

		(void)regulator_list_voltage(dev, idx, &volt_uv);

		if ((volt_uv >= min_uv) && (volt_uv <= max_uv)) {
			return true;
		}
	}

	return false;
}

int regulator_set_voltage(const struct device *dev, int32_t min_uv,
			  int32_t max_uv)
{
	const struct regulator_common_config *config = dev->config;
	const struct regulator_driver_api *api = dev->api;

	if (api->set_voltage == NULL) {
		return -ENOSYS;
	}

	/* voltage may not be allowed, even if supported */
	if ((min_uv > config->max_uv) || (max_uv < config->min_uv)) {
		return -EINVAL;
	}

	return api->set_voltage(dev, min_uv, max_uv);
}

int regulator_set_current_limit(const struct device *dev, int32_t min_ua,
				int32_t max_ua)
{
	const struct regulator_common_config *config = dev->config;
	const struct regulator_driver_api *api = dev->api;

	if (api->set_current_limit == NULL) {
		return -ENOSYS;
	}

	/* current limit may not be allowed, even if supported */
	if ((min_ua > config->max_ua) || (max_ua < config->min_ua)) {
		return -EINVAL;
	}

	return api->set_current_limit(dev, min_ua, max_ua);
}

int regulator_set_mode(const struct device *dev, regulator_mode_t mode)
{
	const struct regulator_common_config *config = dev->config;
	const struct regulator_driver_api *api = dev->api;

	if (api->set_mode == NULL) {
		return -ENOSYS;
	}

	/* no mode restrictions */
	if (config->allowed_modes_cnt == 0U) {
		return api->set_mode(dev, mode);
	}

	/* check if mode is allowed, apply if it is */
	for (uint8_t i = 0U; i < config->allowed_modes_cnt; i++) {
		if (mode == config->allowed_modes[i]) {
			return api->set_mode(dev, mode);
		}
	}

	return -ENOTSUP;
}

[BeekBoy2002]

Please note the original code snippet was reflecting the current Zephyr v3.6.xx code. This is not yet inline with our change due to the fact the check on REGULATOR_BOOT_OFF has been added (this macro is not yet available in Zephyr v3.5.xx).

The Zephyr v3.5.xx code being changed is:

  if (is_enabled) {
    data->refcnt++;
  } else if ((config->flags & REGULATOR_INIT_ENABLED) != 0U) {
    ret = api->enable(dev);
    if (ret < 0) {
      return ret;
    }

    regulator_delay(config->startup_delay_us);
    data->refcnt++;
  }

  return 0;

[Priyanka]

Hi Robert,

Tank you for informing us. I have reported this internally and we will we looking into this.

Regards,

Priyanka

[BeekBoy2002]

Did the fix for this issue make it into nRF Connect SDK v2.7.0?

[Priyanka]

Hi Robert,

Please take a look at this fix: https://github.com/zephyrproject-rtos/zephyr/pull/65878 

-Priyanka

[BeekBoy2002]

I've tested it with a bare v2.7.0 release and there it works again as expected (using sysbuild and updated devicetree for our custom board to HW model v2).