Zephyr: add more compliant implementation for irq_(un)lock

This change replaces the stubbed irq_(un)lock static functions
defined in task.h with new functions that behave more like the
Zephyr implementation of irq_(un)lock functions. This should
make the migration from interrupt_(dis|en)able to Zephyr more
seamless.

BRANCH=none
BUG=b:172060699
TEST=Added unit tests, make runtests -j, and built for various
     boards: eve, volteer, arcada_ish, atlas, hatch, kohaku,
     nocturne, samus, and scarlet

Signed-off-by: Yuval Peress <peress@chromium.org>
Change-Id: Ia7ad2b8d7d411a11699353bf5d3cc36a261fad14
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2511720

common/build.mk

@@ -10,7 +10,7 @@
 _common_dir:=$(dir $(lastword $(MAKEFILE_LIST)))
 
 common-y=util.o
-common-y+=version.o printf.o queue.o queue_policies.o
+common-y+=version.o printf.o queue.o queue_policies.o irq_locking.o
 
 common-$(CONFIG_ACCELGYRO_BMI160)+=math_util.o
 common-$(CONFIG_ACCELGYRO_BMI260)+=math_util.o

common/irq_locking.c

@@ -0,0 +1,32 @@
+/* Copyright 2020 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/*
+ * This file is used for platform/ec implementations of irq_lock and irq_unlock
+ * which are defined by Zephyr.
+ */
+
+#include "task.h"
+#include "util.h"
+
+static uint32_t lock_count;
+
+uint32_t irq_lock(void)
+{
+	interrupt_disable();
+	return lock_count++;
+}
+
+void irq_unlock(uint32_t key)
+{
+	lock_count = key;
+
+	/*
+	 * Since we're allowing nesting locks, we only actually want to release
+	 * the lock if the lock count reached 0.
+	 */
+	if (lock_count == 0)
+		interrupt_enable();
+}

core/host/task.c

@@ -124,14 +124,14 @@ int in_interrupt_context(void)
 	return !!in_interrupt;
 }
 
-void interrupt_disable(void)
+test_mockable void interrupt_disable(void)
 {
 	pthread_mutex_lock(&interrupt_lock);
 	interrupt_disabled = 1;
 	pthread_mutex_unlock(&interrupt_lock);
 }
 
-void interrupt_enable(void)
+test_mockable void interrupt_enable(void)
 {
 	pthread_mutex_lock(&interrupt_lock);
 	interrupt_disabled = 0;

include/task.h

@@ -85,15 +85,29 @@ void interrupt_enable(void);
  * interrupt_disable() and interrupt_enable().
  */
 #ifndef CONFIG_ZEPHYR
-__maybe_unused static inline uint32_t irq_lock(void)
-{
-	interrupt_disable();
-	return 0;
-}
-__maybe_unused static inline void irq_unlock(uint32_t key)
-{
-	interrupt_enable();
-}
+/**
+ * Perform the same operation as interrupt_disable but allow nesting. The
+ * return value from this function should be used as the argument to
+ * irq_unlock. Do not attempt to parse the value, it is a representation
+ * of the state and not an indication of any form of count.
+ *
+ * For more information see:
+ * https://docs.zephyrproject.org/latest/reference/kernel/other/interrupts.html#c.irq_lock
+ *
+ * @return Lock key to use for restoring the state via irq_unlock.
+ */
+uint32_t irq_lock(void);
+
+/**
+ * Perform the same operation as interrupt_enable but allow nesting. The key
+ * should be the unchanged value returned by irq_lock.
+ *
+ * For more information see:
+ * https://docs.zephyrproject.org/latest/reference/kernel/other/interrupts.html#c.irq_unlock
+ *
+ * @param key The lock-out key used to restore the interrupt state.
+ */
+void irq_unlock(uint32_t key);
 #endif /* CONFIG_ZEPHYR */
 
 /**

test/build.mk

@@ -42,6 +42,7 @@ test-list-host += host_command
 test-list-host += i2c_bitbang
 test-list-host += inductive_charging
 test-list-host += interrupt
+test-list-host += irq_locking
 test-list-host += is_enabled
 test-list-host += is_enabled_error
 test-list-host += kasa
@@ -150,6 +151,7 @@ host_command-y=host_command.o
 i2c_bitbang-y=i2c_bitbang.o
 inductive_charging-y=inductive_charging.o
 interrupt-y=interrupt.o
+irq_locking-y=irq_locking.o
 is_enabled-y=is_enabled.o
 kb_8042-y=kb_8042.o
 kb_mkbp-y=kb_mkbp.o

test/irq_locking.c

@@ -0,0 +1,83 @@
+/* Copyright 2020 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "task.h"
+#include "test_util.h"
+
+static uint32_t interrupt_disable_count;
+static uint32_t interrupt_enable_count;
+
+/** Mock implementation of interrupt_disable. */
+void interrupt_disable(void)
+{
+	++interrupt_disable_count;
+}
+
+/** Mock implementation of interrupt_enable. */
+void interrupt_enable(void)
+{
+	++interrupt_enable_count;
+}
+
+static int test_simple_lock_unlock(void)
+{
+	uint32_t key = irq_lock();
+
+	irq_unlock(key);
+
+	TEST_EQ(interrupt_disable_count, 1, "%u");
+	TEST_EQ(interrupt_enable_count, 1, "%u");
+
+	return EC_SUCCESS;
+}
+
+static int test_unlock_when_all_keys_removed(void)
+{
+	uint32_t key0 = irq_lock();
+	uint32_t key1 = irq_lock();
+
+	TEST_EQ(interrupt_disable_count, 2, "%u");
+
+	irq_unlock(key1);
+
+	TEST_EQ(interrupt_enable_count, 0, "%u");
+
+	irq_unlock(key0);
+
+	TEST_EQ(interrupt_enable_count, 1, "%u");
+
+	return EC_SUCCESS;
+}
+
+static int test_unlock_from_root_key(void)
+{
+	uint32_t key0 = irq_lock();
+	uint32_t key1 = irq_lock();
+
+	TEST_NE(key0, key1, "%u");
+	TEST_EQ(interrupt_disable_count, 2, "%u");
+
+	irq_unlock(key0);
+	TEST_EQ(interrupt_enable_count, 1, "%u");
+
+	return EC_SUCCESS;
+}
+
+void before_test(void)
+{
+	interrupt_disable_count = 0;
+	interrupt_enable_count = 0;
+}
+
+void run_test(int argc, char **argv)
+{
+	test_reset();
+
+	RUN_TEST(test_simple_lock_unlock);
+	RUN_TEST(test_unlock_when_all_keys_removed);
+	RUN_TEST(test_unlock_from_root_key);
+
+	test_print_result();
+}

test/irq_locking.tasklist

@@ -0,0 +1,9 @@
+/* Copyright 2020 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/**
+ * See CONFIG_TASK_LIST in config.h for details.
+ */
+#define CONFIG_TEST_TASK_LIST /* No test task. */