i2c-sam0 sleeps waiting for interrupt

[sslupsky]

Describe the bug
This is likely more a question than a bug report. However, at this point I have not been able to determine what the correct course of action is to address this observation.

When CONFIG_SYS_POWER_MANAGEMENT and CONFIG_SYS_POWER_SLEEP_STATES are enabled, the i2c-sam0 driver can suffer corruption when the device enters a sleep state. This happens because the i2c-sam0 driver waits for a semaphore. During the wait, a sleep state can be entered where the i2c clock is turned off.

Here is where this issue manifests itself in the i2c-sam0 driver:

zephyr/drivers/i2c/i2c_sam0.c

Line 459 in 42c5b0a

k_sem_take(&data->sem, K_FOREVER);

I believe this issue is related to issue #10524. In that issue @nashif indicates:

i2c and other drivers might require multithreading. this is documented now in the single thread configuration option.

and subsequently closed the issue. From the comment it is not clear what is referred to. I reviewed the CONFIG_MULTITHREADING documentation but there is no mention of how to address the pm issue with i2c.

To Reproduce
I have been testing this with my own SAMD21 board with an i2c peripheral. I believe any SAMD21 board (Adafruit M0 proto board) will reproduce the problem if the config options are set as described above and the following sleep states are defined:

void sys_set_power_state(enum power_states state)
{
	LOG_DBG("SoC entering power state %d", state);

	/* FIXME: When this function is entered the Kernel has disabled
	 * interrupts using BASEPRI register. This is incorrect as it prevents
	 * waking up from any interrupt which priority is not 0. Work around the
	 * issue and disable interrupts using PRIMASK register as recommended
	 * by ARM.
	 */

	/* Set PRIMASK */
	__disable_irq();
	/* Set BASEPRI to 0 */
	irq_unlock(0);

	switch (state) {
#ifdef CONFIG_SYS_POWER_SLEEP_STATES
#ifdef CONFIG_HAS_SYS_POWER_STATE_SLEEP_1
	case SYS_POWER_STATE_SLEEP_1:
        SCB->SCR &= ~SCB_SCR_SLEEPDEEP_Msk;
        PM->SLEEP.reg = 2;
        __DSB();
        __WFI();
		break;
#endif /* CONFIG_HAS_SYS_POWER_STATE_SLEEP_1 */
#ifdef CONFIG_HAS_SYS_POWER_STATE_SLEEP_2
	case SYS_POWER_STATE_SLEEP_2:
        // Disable systick interrupt:  See https://www.avrfreaks.net/forum/samd21-samd21e16b-sporadically-locks-and-does-not-wake-standby-sleep-mode
        // If SysTick is not enabled then no need to mask the interrupt
#ifdef CONFIG_CORTEX_M_SYSTICK
        SysTick->CTRL &= ~SysTick_CTRL_TICKINT_Msk;
#endif
        SCB->SCR |= SCB_SCR_SLEEPDEEP_Msk;
        __DSB();
        __WFI();
        // Re-enable systick interrupt if SysTick is enabled
#ifdef CONFIG_CORTEX_M_SYSTICK
        SysTick->CTRL |= SysTick_CTRL_TICKINT_Msk;	
#endif
		break;
#endif /* CONFIG_HAS_SYS_POWER_STATE_SLEEP_2 */
#ifdef CONFIG_HAS_SYS_POWER_STATE_SLEEP_3
	case SYS_POWER_STATE_SLEEP_3:
		break;

Expected behavior
System power sleep states that affect the i2c peripheral clock (sleep state 2 in my example above) should be avoided when an i2c master is waiting for an i2c bus event or transaction to complete.

Note, if a driver is configured to act as an i2c slave, it is possible to wake the cpu on an i2c address match without an internal clock gated on to the peripheral. So, in this instance, the peripheral clock could be gated off while waiting for the device to be addressed.

Impact
I2C is non functional during sleep.

[sslupsky]

I have another observation regarding the sam0 i2c driver. It appears that the i2c bus can be left in a non idle state. This is a problem because the SCL and/or SDA lines can be held low by the sam0 during and after sleep resulting in significant current (>500uA) due to the pull up resistors on these lines.

I have not isolated what is causing this issue but I have confirmed using an oscilloscope that the SCL and/or SDA lines remain low after an i2c transaction.

It is possible this issue is not related to the topic of this issue and should be a recorded as a separate issue.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[carlescufi]

@sslupsky is this still an issue? Otherwise please close it.

[sslupsky]

@carlescufi I submitted a PR quite a while ago that addresses this. The PR hasn't been accepted yet so I left this open.

I'll close it.