Buttons module stops working if button is pressed during power off

When a button is pressed during power off, the buttons module disables the wakeup interrupt but the button scan never happens, leaving the module in an invalid state and preventing any wakeup from system off.

I modified the buttons.c CAF module to log when it changes the interrupt configuration:

static int setup_pin_wakeup(void)
{
	uint32_t wakeup_cols = get_wakeup_mask(col, ARRAY_SIZE(col));
	uint32_t wakeup_rows = get_wakeup_mask(row, ARRAY_SIZE(row));

	LOG_DBG("Setting pin wakeup: 0x%08x 0x%08x", wakeup_cols, wakeup_rows);
	/* Disable callbacks (and cancel the work) to ensure it will not be scheduled by an
	 * invalid button in the idle state.
	 */
	int err = callback_ctrl(0);

	if (!err) {
		/* Setup callbacks and columns for the idle state. */
		err = set_cols(wakeup_cols);
		if (!err) {
			err = callback_ctrl(wakeup_rows);
		}
	}
	LOG_DBG("Completed setting pin wakeup: %d", err);

	return err;
}
...
static void button_pressed_isr(const struct device *gpio_dev,
			       struct gpio_callback *cb,
			       uint32_t pins)
{
	int err = 0;

	/* Scanning will be scheduled, switch off pins */
	if (set_cols(0)) {
		LOG_ERR("Cannot control pins");
		err = -EFAULT;
	}

	/* This is a workaround. Zephyr will set any pin triggering interrupt
	 * at the moment. Not only our pins.
	 */
	pins = pins & cb->pin_mask;

	/* Disable all interrupts synchronously requires holding a spinlock.
	 * The problem is that GPIO callback disable code takes time. If lock
	 * is kept during this operation BLE stack can fail in some cases.
	 * Instead we disable callbacks associated with the pins. This is to
	 * make sure CPU is available for threads. The remaining callbacks are
	 * disabled in the workqueue thread context. Work code also cancels
	 * itself to prevent double execution when interrupt for another
	 * pin was triggered in-between.
	 */
	for (uint32_t pin = 0; (pins != 0) && !err; pins >>= 1, pin++) {
		if ((pins & 1) != 0) {
			err = gpio_pin_interrupt_configure(gpio_dev, pin,
							   GPIO_INT_DISABLE);
		}
	}

	if (err) {
		LOG_ERR("Cannot disable callbacks");
		module_set_state(MODULE_STATE_ERROR);
	} else {
		err = k_work_reschedule(&button_pressed, K_NO_WAIT);
		LOG_DBG("Disabled callbacks: %d", err);
	}
}

Here are the logs when pressing the button during power off:

[00:00:00.309,814] <inf> app_event_manager: e: force_power_down_event
[00:00:00.309,814] <inf> power_manager: Force power down processing 
[00:00:00.309,906] <inf> app_event_manager: e: power_down_event
[00:00:00.310,058] <dbg> buttons: setup_pin_wakeup: Setting pin wakeup: 0xffffffff 0xffffffe4
[00:00:00.310,150] <dbg> buttons: setup_pin_wakeup: Completed setting pin wakeup: 0
[00:00:00.310,607] <inf> power_manager: Power down the board
[00:00:00.310,913] <inf> app_event_manager: e:module_state_event module:buttons state:STANDBY
[00:00:00.311,035] <inf> app_event_manager: e:module_state_event module:board state:OFF
[00:00:00.317,413] <wrn> power_manager: System turned off
[00:00:00.357,299] <dbg> buttons: button_pressed_isr: Disabled callbacks: 1

Although the k_work_reschedule call in button_pressed_isr succeeds, the work is never performed. It seems that the button module should somehow check if work can be scheduled, but I'm not sure what is preventing it from firing. Is the RTC stopped? Is the work cancelled on power down? Should k_work_reschedule even succeed in this situation?

Parents Reply Children
  • I have modified nRF Desktop example in version 2.9.0 of the SDK.

    I am using RTT output and when powering off it starts outputting really slowly, I guess this is because something changes when it is powered off. However, if the button is pressed at any time during this slow output, it happens.

    This should be reproducible with a simple example, possibly even with nRF Desktop, as I've not modified the buttons module otherwise.

  • I believe I've seen a few cases about similar things, but I think they should be solved in the newer NCS versions. Like the issue in this previous case. That was supposed to be fixed in 2.5.1 though.

    And sounds similar to the situation described here. As well as this one, which also comes with a solution that might work for you.

    When a button is pressed during power off, the buttons module disables the wakeup interrupt but the button scan never happens

    I am having trouble understanding this part. Did you mean something else than disable here?

    And could you confirm for me that it happens on the default nRF Desktop as well?

    Regards,

    Elfving

  • This issue is not the same as those linked. It is specific to the buttons module.

    It is reproducible in the nrf desktop sample without modification, however it saves time if you reduce power off timeout e.g. CONFIG_CAF_POWER_MANAGER_TIMEOUT=30 and enable timestamps CONFIG_LOG_BACKEND_FORMAT_TIMESTAMP=y. Using the nRF52840DK, flash the project and connect RTT. Wait for power down. Then wait longer until it starts outputting characters slowly (in below log, everything at 58s was output slowly). While this is happening, press a button. The button press will not be registered and will no longer work if pressed again.

    [00:00:29.331,085] <inf> power_manager: System power down
    [00:00:29.331,146] <inf> app_event_manager: e: power_down_event
    [00:00:29.331,542] <inf> ble_adv: Use slow advertising
    [00:00:29.333,038] <inf> app_event_manager: e:module_state_event module:buttons state:STANDBY
    [00:00:29.333,129] <inf> app_event_manager: e:module_state_event module:board state:OFF
    [00:00:29.333,221] <inf> app_event_manager: e:module_state_event module:ble_bond state:OFF
    [00:00:29.333,343] <inf> app_event_manager: e:module_state_event module:motion state:STANDBY
    [00:00:29.333,404] <inf> app_event_manager: e: power_down_event
    [00:00:29.333,465] <inf> app_event_manager: e: power_down_event
    [00:00:29.333,496] <inf> app_event_manager: e: power_down_event
    [00:00:29.333,587] <inf> app_event_manager: e: power_down_event
    [00:00:58.630,249] <inf> ble_adv: Advertising stopped
    
    [00:00:58.630,310] <inf> app_event_manager: e:module_state_event module:ble_adv state:OFF
    [00:00:58.630,371] <inf> app_event_manager: e:ble_peer_search_event inactive
    [00:00:58.630,462] <inf> app_event_manager: e: power_down_event
    
    [00:00:58.633,911] <inf> power_manager: Power down the board
    [00:00:58.633,941] <inf> app_event_manager: e:led_event led_id:1 effect:0x4e530
    [00:00:58.634,033] <inf> app_event_manager: e:module_state_event module:click_detector state:OFF
    [00:00:58.634,094] <inf> app_event_manager: e:module_state_event module:leds state:OFF
    [00:00:58.640,655] <wrn> power_manager: System turned off

    I have researched the issue further.

    • This issue occurs when a button GPIO interrupt is triggered after power down is initiated in power_manager, and the buttons module is suspended on power down event, but before system power off. This can occur during log_panic() processing (power_manager.c line 185) as this can take some time. To test this, RTT should be used, as it takes a few seconds to output the power off logs, giving plenty of time to press the button.
    • After log_panic is complete, the button interrupt is handled. This disables the interrupt and submits the button_pressed work to the queue immediately.
    • system_off_post_action is called which calls sys_poweroff. This powers off the CPU immediately
    • The button_pressed work is never performed due to system power off, so the interrupts are not re-enabled.

    Somehow, on interrupt, the buttons module should determine that power off is occurring, and not process the interrupt. This seems like the only solution to me, because after the buttons module suspends on power down event, there are no further notifications from the power_manager module that system off is about to happen, and the buttons module can't check hardware state because nothing has changed until power_manager calls sys_poweroff.

    So this can be achieved by adding a check for the buttons module state on interrupt. This seems to work fine, but I'm unsure if there could be problems I'm unaware of. In particular, it depends on the system resetting so that the buttons module starts up again. The buttons module cannot trigger a wake up event in button_pressed_fn with this modification. So there might be a more appropriate way to achieve this, like adding a new state.

    static void button_pressed_isr(const struct device *gpio_dev,
    			       struct gpio_callback *cb,
    			       uint32_t pins)
    {
    	int err = 0;
    	if (state == STATE_IDLE) {
    		// LOG_ERR("Button pressed in idle state");
    		return;
    	}

  • Hi,

    Just letting you know that we've reproduced it on our side and working on it.

    We should have this fixed soon. I'll keep you updated.

    Regards,

    Elfving

Related