wake from GPIO using SENSE without a race condition

NCS2.3.0, NRF52833

I use a button to wake my product. The same button is used to turn off. The button is tied low with a 100K resistor and pulled high when pressed.

I found that very occasionally, when I turn the device off, it later fails to wake up when I press the button. I copied the code to EnterSystemOff() from one of the examples.

I read the product spec again and concluded that I must ensure the button is not pressed when I call pm_stat_force or the SENSE wont work. (please correct me if I'm wrong)

So I use this code  


void EnterSystemOff(void)
{
	LOG_INF("Entering system off; press BUTTON to restart");


// #ifdef thread0_id
	common.run_button_thread = false;
	// k_msleep(100);
	// k_thread_abort(thread0_id);
	// k_thread_abort(thread1_id);
// #endif

	LIS2DW12_power_down();

	ClearRGBLEDs();
	nrf_pwm_play_shutdown();
	
	nrf_pwm_stop();
	all_gpios_off();

	do
	{
		//We MUST not call pm_state_force if the button is pressed.
		// So lets wait for the button to be continuously released for at least half a second
		// This is still a bit risky because the button could be pressed again before we call pm_state_force
		
		uint32_t button_unpressed_counter = 0;
		while(button_unpressed_counter < 500) 
		{
			k_sleep(K_MSEC(1));

			if(nrf_gpio_pin_read(MY_BUTTON_PIN))
			{
				button_unpressed_counter = 0;
			}
			else
			{
				button_unpressed_counter++;
			}
		}

		pm_state_force(0u, &(struct pm_state_info){PM_STATE_SOFT_OFF, 0, 0});

		/* Now we need to go to sleep. This will let the idle thread run and
		* the pm subsystem will use the forced state. To confirm that the
		* forced state is used, lets set the same timeout used previously.
		*/
		k_sleep(K_SECONDS(2));

		LOG_INF("ERROR: System off failed - resetting");
		// We should never get here because the button should cause a reset which restarts the system.
		// Something serious has gone wrong. so lets try to reset the system. 
		sys_reboot(SYS_REBOOT_COLD);
		k_sleep(K_SECONDS(2));

		// If we're still here then the reset failed. Try to call pm_state_force again.
	} while (true);
}

It's hard to test if this fixes the issue because it was very rare to see the bug anyway.

As you can see there's a loop that waits for at least 500ms without a button press to *try* to ensure it doesn't go to sleep when the pin is high.

Obviously there's still a race condition which I don't like. I wanted to ask if there's a better way to do this. And to check if I've understood properly.

I need to ensure SENSE is always configured on the pin, so the first line of main() is 

nrf_gpio_cfg(
    MY_BUTTON_PIN, 
    NRF_GPIO_PIN_DIR_INPUT, 
    NRF_GPIO_PIN_INPUT_CONNECT, 
    NRF_GPIO_PIN_NOPULL, 
    NRF_GPIO_PIN_S0S1, 
    NRF_GPIO_PIN_SENSE_HIGH);

I can't see how to configure SENSE on the gpio in the zephyr .dts file, so I use the nrf_gpio functions instead and I don't configure any of my gpios in the .dts file.

Thanks for your help,
Kind regards,
-Jason
Parents
  • Hi,

    I read the product spec again and concluded that I must ensure the button is not pressed when I call pm_stat_force or the SENSE wont work. (please correct me if I'm wrong)

    Where did you read this? There is no such fundamental limit in the IC, but there the DK buttons and LEDs module disabled interrupt's while a button is pressed, as described in this post. I do not see any indication you are using that though?

    I can't see how to configure SENSE on the gpio in the zephyr .dts file, so I use the nrf_gpio functions instead and I don't configure any of my gpios in the .dts file.

    There is nothing wrong with that approach. Alternatively, you can use low-level function just to enable sense, as is done in the nRF5x System Off demo:

    	nrf_gpio_cfg_sense_set(NRF_DT_GPIOS_TO_PSEL(DT_ALIAS(sw0), gpios),
    			       NRF_GPIO_PIN_SENSE_LOW);

    This demonstrate wake-up from a button, and that approach should work regardless if you are pressing the button when entering system OFF mode or not.

  • Thank you for clearing that up for me.

    I misunderstood this section on page 142 of the nrf52833 product spec. 

    I've noticed this bug only recently but it's happened 3 times in 2 weeks. So when I reread the product spec section, I thought it might explain the failure to wake.I've seen it on several PCBs so it's not appeared because of a hardware issue. 

    Until today, my code for EnterSystemOff() did the same as the example you linked, it had a call to nrf_gpio_cfg_sense_set()  immediately before calling pm_state_force.

    My other guess was that the bug could be caused by zephyr initialising the button pin because of it's mention in the .dts file and removing the SENSE configuration and that somehow resulting in the bug. So today, I removed the mention of the button from the .dts file and changed my code to only interact with buttons using the nrf_gpio_ functions.

    My dts file had his section before

    	buttons {
    		compatible = "gpio-keys";
    		button0: button_0 {
    			gpios = <&gpio0 17 ( GPIO_ACTIVE_HIGH)>;
    			label = "Push button switch 0";
    		};
    	};

    I'm not sure what else to suspect so maybe I should look through all the changes I've made recently to see if there's any thing that might explain this.

    If you have any suggestions I'd greatly appreciate it.

  • So the bug happened again last night, after changing my code back. 

    void EnterSystemOff(void)
    {
    	LOG_INF("Entering system off; press BUTTON to restart");
    
    // #ifdef thread0_id
    	common.run_button_thread = false;
    	// k_msleep(100);
    	// k_thread_abort(thread0_id);
    	// k_thread_abort(thread1_id);
    // #endif
    
    	LIS2DW12_power_down();
    
    	ClearRGBLEDs();
    	nrf_pwm_play_shutdown();
    	
    	nrf_pwm_stop();
    	all_gpios_off();
    
    	do
    	{
    		nrf_gpio_cfg_sense_set(MY_BUTTON_PIN, NRF_GPIO_PIN_SENSE_HIGH);
    		pm_state_force(0u, &(struct pm_state_info){PM_STATE_SOFT_OFF, 0, 0});
    
    		/* Now we need to go to sleep. This will let the idle thread run and
    		* the pm subsystem will use the forced state. To confirm that the
    		* forced state is used, lets set the same timeout used previously.
    		*/
    		k_sleep(K_SECONDS(2));
    
    		LOG_INF("ERROR: System off failed - resetting");
    		// We should never get here because the button should cause a reset which restarts the system.
    		// Something serious has gone wrong. so lets try to reset the system. 
    		sys_reboot(SYS_REBOOT_COLD);
    		k_sleep(K_SECONDS(2));
    		
    		//keep trying 
    	} 
    	while (true);
    

    I had the power profiler attached so I could see it was ~700uA, the debugger was not hooked up (SWDIO and SWDCLK, Vdetect and GND). So it was not fully asleep, it should have been ~3uA. ~700uA normally means HFCLOCK was still running I think. And there was a slight increase when I pressed the button, which I think was the 100K resistor to GND.

    I wanted to attach the debugger but I don't know how to do it without NCS erasing and reflashing the firmware. Do you know how I can do this? 

    If I could attach the debugger when I see the problem then I might be able to figure out what's going on.

    Can you tell me if this NCS option is the same as pressing F5 or choosing the menu option Run->Start Debugger?

     

    I think I need to make a new configuration but I don't know how to do it. 

    I think it would be better if there was a checkbox to decide if it should erase and reflash when launching the debugger rather than always doing it.

    I've also noticed that after I've used the debugger I see strange crashes. I think it might be the breakpoints persisting outside the debug session. This is with a Jlink. And it still happens if I erase the board, then reflash, then reset the board. It still crashes. I have to run with the debugger again, clear the breakpoints and exit cleanly. 

    I am not sure if this is to be expected but I don't recall these problems with segger embedded Studio. 

  • I've added an "attach" configuration so I'm trying to trigger the bug to see if I can get it in the debugger.

  • I recently changed the function I use to stop PWM prior to shutdown. I think it could get stuck in the do-while loop waiting for EVENTS_STOPPED but I'm not sure if, or how, this could happen.
    I'm not sure if this is the correct way to write this function at all.
    Could you take a look please?
    void nrf_pwm_stop()
    {
        if (NRF_PWM0->ENABLE == (PWM_ENABLE_ENABLE_Enabled << PWM_ENABLE_ENABLE_Pos))
        {
            NRF_PWM0->TASKS_STOP = PWM_TASKS_STOP_TASKS_STOP_Trigger << PWM_TASKS_STOP_TASKS_STOP_Pos;
            NRF_PWM1->TASKS_STOP = PWM_TASKS_STOP_TASKS_STOP_Trigger << PWM_TASKS_STOP_TASKS_STOP_Pos;
            NRF_PWM2->TASKS_STOP = PWM_TASKS_STOP_TASKS_STOP_Trigger << PWM_TASKS_STOP_TASKS_STOP_Pos;
            NRF_PWM3->TASKS_STOP = PWM_TASKS_STOP_TASKS_STOP_Trigger << PWM_TASKS_STOP_TASKS_STOP_Pos;
    
            NRF_PWM0->EVENTS_STOPPED = 0;
            NRF_PWM1->EVENTS_STOPPED = 0;
            NRF_PWM2->EVENTS_STOPPED = 0;
            NRF_PWM3->EVENTS_STOPPED = 0;
    
            do
            {
    
            } 
            while (
                (0 == NRF_PWM0->EVENTS_STOPPED) ||
                (0 == NRF_PWM1->EVENTS_STOPPED) ||
                (0 == NRF_PWM2->EVENTS_STOPPED) ||
                (0 == NRF_PWM3->EVENTS_STOPPED));
            
            for (int ii = 0; ii < 4; ii++)
            {
                NRF_PWM0->PSEL.OUT[ii] = (PWM_PSEL_OUT_CONNECT_Disconnected << PWM_PSEL_OUT_CONNECT_Pos);
                NRF_PWM1->PSEL.OUT[ii] = (PWM_PSEL_OUT_CONNECT_Disconnected << PWM_PSEL_OUT_CONNECT_Pos);
                NRF_PWM2->PSEL.OUT[ii] = (PWM_PSEL_OUT_CONNECT_Disconnected << PWM_PSEL_OUT_CONNECT_Pos);
                NRF_PWM3->PSEL.OUT[ii] = (PWM_PSEL_OUT_CONNECT_Disconnected << PWM_PSEL_OUT_CONNECT_Pos);
            }
    
            NRF_PWM0->ENABLE = (PWM_ENABLE_ENABLE_Disabled << PWM_ENABLE_ENABLE_Pos);
            NRF_PWM1->ENABLE = (PWM_ENABLE_ENABLE_Disabled << PWM_ENABLE_ENABLE_Pos);
            NRF_PWM2->ENABLE = (PWM_ENABLE_ENABLE_Disabled << PWM_ENABLE_ENABLE_Pos);
            NRF_PWM3->ENABLE = (PWM_ENABLE_ENABLE_Disabled << PWM_ENABLE_ENABLE_Pos);
        }
    
    }
    I've since changed it to do each channel individually, *EDIT* and I noticed that I should clear EVENTS_STOPPED flag *before* doing TASKS_STOP. I think this might have been the race condition.
    The bug happens so rarely that I've not had it all day, so I'm trying to guess what code could be the culprit.
    void nrf_pwm_stop_x(NRF_PWM_Type * PWMx)
    {
        if (PWMx->ENABLE == (PWM_ENABLE_ENABLE_Enabled << PWM_ENABLE_ENABLE_Pos))
        {
            PWMx->EVENTS_STOPPED = 0;   // Need to clear this *before* TASKS_STOP!
            PWMx->TASKS_STOP = PWM_TASKS_STOP_TASKS_STOP_Trigger << PWM_TASKS_STOP_TASKS_STOP_Pos;
    
    
            while (0 == PWMx->EVENTS_STOPPED);
    
            for (int ii = 0; ii < 4; ii++)
            {
                PWMx->PSEL.OUT[ii] = (PWM_PSEL_OUT_CONNECT_Disconnected << PWM_PSEL_OUT_CONNECT_Pos);
            }
    
            PWMx->ENABLE = (PWM_ENABLE_ENABLE_Disabled << PWM_ENABLE_ENABLE_Pos);
        }
    }
    
    //--------------------------------------------------------------------------------------------------------------------------------------
    //--------------------------------------------------------------------------------------------------------------------------------------
    //--------------------------------------------------------------------------------------------------------------------------------------
    
    void nrf_pwm_stop()
    {
        nrf_pwm_stop_x(NRF_PWM0);
        nrf_pwm_stop_x(NRF_PWM1);
        nrf_pwm_stop_x(NRF_PWM2);
        nrf_pwm_stop_x(NRF_PWM3);
    }
  • I think my PWM code caused this issue, so I'm going to close this support request.

    THANK YOU!

Reply Children
No Data
Related