nrfx_pwm_uninit fails to uninitialize PWM instance

[_Scott_]

In our project we have functions for outputting a single frequency via PWM for an indefinite amount of time until we call a function to turn it off. See attached code.

void speaker_output_freq(uint16_t freq) {
  ret_code_t err_code;
  const uint16_t counter_top = 1000000 / freq;
  static nrf_pwm_values_individual_t seq_values; // This array cannot be allocated on stack (hence "static") and it must be in RAM 
  seq_values.channel_0 = counter_top / 2; // always use 50% duty cycle for max volume

  
  nrfx_pwm_config_t const config0 = {
    .output_pins = {
      SPEAKER_PWM_PIN,		// channel 0
      NRFX_PWM_PIN_NOT_USED,	// channel 1
      NRFX_PWM_PIN_NOT_USED,	// channel 2
      NRFX_PWM_PIN_NOT_USED	// channel 3
    },
    .irq_priority = NRFX_PWM_DEFAULT_CONFIG_IRQ_PRIORITY,
    .base_clock   = NRF_PWM_CLK_1MHz,
    .count_mode   = NRF_PWM_MODE_UP,
    .top_value    = counter_top,
    .load_mode    = NRF_PWM_LOAD_INDIVIDUAL,
    .step_mode    = NRF_PWM_STEP_AUTO
  };

  nrf_pwm_sequence_t const seq = {
    .values.p_individual = &seq_values,
    .length              = NRF_PWM_VALUES_LENGTH(seq_values),
    .repeats             = 0,
    .end_delay           = 0
  };

  if (is_pwm_running == true) { 
    speaker_output_off();
  }

  err_code = nrfx_pwm_init(&pwm_instance, &config0, NULL);
  SFG_ERROR_CHECK(err_code, M_SPEAKER_OUTPUT);

  nrfx_pwm_simple_playback(&pwm_instance, &seq, 1, NRFX_PWM_FLAG_LOOP);
  is_pwm_running = true;
}

void speaker_output_off(void) {
  if (is_pwm_running == true) { // stop/uninit calls will cause reset if pwm is not initialized when called
    nrfx_pwm_stop(&pwm_instance, true); // waits until duty cycle is complete
    nrfx_pwm_uninit(&pwm_instance);
    is_pwm_running = false;
  }

  // Pin must be driven low so speaker FET doesn't get any signal. Must reconfigure as output each time since PWM module reconfigures pin
  nrf_gpio_cfg_output(SPEAKER_PWM_PIN);
  nrf_gpio_pin_clear(SPEAKER_PWM_PIN);
}

As you can see, we have a flag in place (is_pwm_running) to track if the PWM instance is running, and turn it off before attempting to start the next tone if so. Our SFG_ERROR_CHECK function serves to store errors in flash so we can retrieve and analyze them later.

What I am seeing is that occasionally nrfx_pwm_init() generates an error, NRFX_ERROR_INVALID_STATE, meaning the pwm instance was already initialized when we called nrfx_pwm_init(). Our logic ensures nrfx_pwm_uninit() was called prior the nrfx_pwm_init() call, so I am unsure how this error is being generated. I am unable to reproduce the error myself, but our error tracking indicates that it is indeed happening, albeit rarely.

So my questions are:

1. Am I misusing the PWM driver in some way I do not realize? Is my un-initialization procedure set up correctly?

2. If my driver usage is correct, are there any reasons why nrfx_pwm_uninit() would be failing? Is nrfx_pwm_uninit() non-blocking and I am simply calling nrfx_pwm_init too quickly for the driver to fully uninitialize? 

[Susheel Nuguru]

uninit does not seem to be your problem here, and the "init called too soon" theory does not hold up. uninit is synchronous and non-blocking, it puts the instance back to UNINITIALIZED before it returns, so there's nothing for the next init to race against.

init only returns INVALID_STATE when the instance wasn't UNINITIALIZED at the time you called it. uninit always clears that, so the error really means something got to init() without going through speaker_output_off() first. Your logic is fine for a single caller, so I'd bet on re-entrancy. You only set is_pwm_running = true after init and playback have finished. If speaker_output_freq() fires again from an ISR, a timer, or another thread in that gap, the second call sees the old false, skips the uninit, and inits an instance that's already initialized.

I would drop the is_pwm_running flag and just ask the driver, and put a lock around the check so it can't be interrupted:

uint32_t key = irq_lock();
if (nrfx_pwm_init_check(&pwm_instance)) {
    speaker_output_off();
}
err_code = nrfx_pwm_init(&pwm_instance, &config0, NULL);
nrfx_pwm_simple_playback(&pwm_instance, &seq, 1, NRFX_PWM_FLAG_LOOP);
irq_unlock(key);

nrfx_pwm_init_check() reads the actual driver state so it can't drift like your bool can. If all your speaker calls already come from one place the lock costs nothing, and if they don't, that is the window you most likely are seeing.

[_Scott_]

What version of the SDK was nrfx_pwm_init_check added? I am running 17.0.2 and do not see that function in nrfx_pwm.c/h. Is there an alternative method if checking if the pwm instance is initialized without that function (e.g. reading the register directly)

[Susheel Nuguru]

I could not have guessed that you were using older nrRF5 sdk when nrfx API is available in both older nRF5 SDK and newer nRf Connect SDK.

in nRF5SDKyou most likely need that flag.Try this

1. Set is_pwm_running = true before nrfx_pwm_init(), not after, so the flag is never stale while the driver is live.
2. Wrap the check-and-act in a critical section so it can't be interrupted partway:

CRITICAL_REGION_ENTER();
if (is_pwm_running) {
    speaker_output_off();
}
is_pwm_running = true;
err_code = nrfx_pwm_init(&pwm_instance, &config0, NULL);
nrfx_pwm_simple_playback(&pwm_instance, &seq, 1, NRFX_PWM_FLAG_LOOP);
CRITICAL_REGION_EXIT();