From 2a70c3194535e90d871729e54035c96af9432fd2 Mon Sep 17 00:00:00 2001 From: Fabio Baltieri Date: Fri, 27 Oct 2023 09:27:48 +0000 Subject: [PATCH] scripts: check_init_priorities: drop the same priority check Since bb590b5b6e introduced ordinals in the priority sequence, the "same priority" case cannot happen anymore, furthermore the priority value in the script is now the position of the function in the init sequence, so if two devices have the same priority there's something real bad going on. Drop all the "same priority" handling code and tests, convert the case into ane exception instead. Drop the init stubs as well from the test, they are not required anymore. Signed-off-by: Fabio Baltieri --- CMakeLists.txt | 4 -- Kconfig.zephyr | 11 +----- scripts/build/check_init_priorities.py | 11 +----- scripts/build/check_init_priorities_test.py | 38 +++++++++++-------- .../boards/native_posix.overlay | 7 ---- tests/misc/check_init_priorities/src/main.c | 15 ++------ .../validate_check_init_priorities_output.py | 2 - 7 files changed, 30 insertions(+), 58 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3e3fd205a8a..be466a1ae81 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1797,14 +1797,10 @@ if(CONFIG_BUILD_OUTPUT_INFO_HEADER) endif() if(CONFIG_CHECK_INIT_PRIORITIES) - if(CONFIG_CHECK_INIT_PRIORITIES_FAIL_ON_WARNING) - set(fail_on_warning "--fail-on-warning") - endif() list(APPEND post_build_commands COMMAND ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/build/check_init_priorities.py --elf-file=${ZEPHYR_BINARY_DIR}/${KERNEL_ELF_NAME} - ${fail_on_warning} ) endif() diff --git a/Kconfig.zephyr b/Kconfig.zephyr index 0edb610d677..b532dbc4435 100644 --- a/Kconfig.zephyr +++ b/Kconfig.zephyr @@ -785,16 +785,7 @@ config CHECK_INIT_PRIORITIES derived from the devicetree definition. Fails the build on priority errors (dependent devices, inverted - priority), see CHECK_INIT_PRIORITIES_FAIL_ON_WARNING to fail on - warnings (dependent devices, same priority) as well. - -config CHECK_INIT_PRIORITIES_FAIL_ON_WARNING - bool "Fail the build on priority check warnings" - depends on CHECK_INIT_PRIORITIES - help - Fail the build if the dependency check script identifies any pair of - devices depending on each other but initialized with the same - priority. + priority). config EMIT_ALL_SYSCALLS bool "Emit all possible syscalls in the tree" diff --git a/scripts/build/check_init_priorities.py b/scripts/build/check_init_priorities.py index 88f7f310040..22a27f24ef9 100755 --- a/scripts/build/check_init_priorities.py +++ b/scripts/build/check_init_priorities.py @@ -244,7 +244,6 @@ class Validator(): self._obj = ZephyrInitLevels(elf_file_path) - self.warnings = 0 self.errors = 0 def _check_dep(self, dev_ord, dep_ord): @@ -275,9 +274,8 @@ class Validator(): return if dev_prio == dep_prio: - self.warnings += 1 - self.log.warning( - f"{dev_node.path} {dev_prio} == {dep_node.path} {dep_prio}") + raise ValueError(f"{dev_node.path} and {dep_node.path} have the " + f"same priority: {dev_prio}") elif dev_prio < dep_prio: self.errors += 1 self.log.error( @@ -311,8 +309,6 @@ def _parse_args(argv): parser.add_argument("-v", "--verbose", action="count", help=("enable verbose output, can be used multiple times " "to increase verbosity level")) - parser.add_argument("-w", "--fail-on-warning", action="store_true", - help="fail on both warnings and errors") parser.add_argument("--always-succeed", action="store_true", help="always exit with a return code of 0, used for testing") parser.add_argument("-o", "--output", @@ -363,9 +359,6 @@ def main(argv=None): if args.always_succeed: return 0 - if args.fail_on_warning and validator.warnings: - return 1 - if validator.errors: return 1 diff --git a/scripts/build/check_init_priorities_test.py b/scripts/build/check_init_priorities_test.py index 9783e5d828e..36d362a63cc 100755 --- a/scripts/build/check_init_priorities_test.py +++ b/scripts/build/check_init_priorities_test.py @@ -295,35 +295,46 @@ class testValidator(unittest.TestCase): validator = check_init_priorities.Validator("", "", None) validator.log = mock.Mock() validator._obj = mock.Mock() - validator.warnings = 0 validator.errors = 0 - validator._ord2node = {1: mock.Mock(), 2: mock.Mock(), 3: mock.Mock()} + validator._ord2node = {1: mock.Mock(), 2: mock.Mock()} validator._ord2node[1]._binding = None validator._ord2node[1].path = "/1" validator._ord2node[2]._binding = None validator._ord2node[2].path = "/2" - validator._ord2node[3]._binding = None - validator._ord2node[3].path = "/3" - validator._obj.devices = {1: 10, 2: 10, 3: 20} + validator._obj.devices = {1: 10, 2: 20} - validator._check_dep(3, 1) validator._check_dep(2, 1) - validator._check_dep(1, 3) + validator._check_dep(1, 2) - validator.log.info.assert_called_once_with("/3 20 > /1 10") - validator.log.warning.assert_called_once_with("/2 10 == /1 10") - validator.log.error.assert_called_once_with("/1 10 < /3 20") - self.assertEqual(validator.warnings, 1) + validator.log.info.assert_called_once_with("/2 20 > /1 10") + validator.log.error.assert_called_once_with("/1 10 < /2 20") self.assertEqual(validator.errors, 1) + @mock.patch("check_init_priorities.Validator.__init__", return_value=None) + def test_check_same_prio_assert(self, mock_vinit): + validator = check_init_priorities.Validator("", "", None) + validator.log = mock.Mock() + validator._obj = mock.Mock() + validator.errors = 0 + + validator._ord2node = {1: mock.Mock(), 2: mock.Mock()} + validator._ord2node[1]._binding = None + validator._ord2node[1].path = "/1" + validator._ord2node[2]._binding = None + validator._ord2node[2].path = "/2" + + validator._obj.devices = {1: 10, 2: 10} + + with self.assertRaises(ValueError): + validator._check_dep(1, 2) + @mock.patch("check_init_priorities.Validator.__init__", return_value=None) def test_check_swapped(self, mock_vinit): validator = check_init_priorities.Validator("", "", None) validator.log = mock.Mock() validator._obj = mock.Mock() - validator.warnings = 0 validator.errors = 0 save_inverted_priorities = check_init_priorities._INVERTED_PRIORITY_COMPATIBLES @@ -344,7 +355,6 @@ class testValidator(unittest.TestCase): mock.call("Swapped priority: compat-3, compat-1"), mock.call("/3 20 > /1 10"), ]) - self.assertEqual(validator.warnings, 0) self.assertEqual(validator.errors, 0) check_init_priorities._INVERTED_PRIORITY_COMPATIBLES = save_inverted_priorities @@ -354,7 +364,6 @@ class testValidator(unittest.TestCase): validator = check_init_priorities.Validator("", "", None) validator.log = mock.Mock() validator._obj = mock.Mock() - validator.warnings = 0 validator.errors = 0 save_ignore_compatibles = check_init_priorities._IGNORE_COMPATIBLES @@ -374,7 +383,6 @@ class testValidator(unittest.TestCase): self.assertListEqual(validator.log.info.call_args_list, [ mock.call("Ignoring priority: compat-3"), ]) - self.assertEqual(validator.warnings, 0) self.assertEqual(validator.errors, 0) check_init_priorities._IGNORE_COMPATIBLES = save_ignore_compatibles diff --git a/tests/misc/check_init_priorities/boards/native_posix.overlay b/tests/misc/check_init_priorities/boards/native_posix.overlay index d279a6d3f26..0f32c8121a8 100644 --- a/tests/misc/check_init_priorities/boards/native_posix.overlay +++ b/tests/misc/check_init_priorities/boards/native_posix.overlay @@ -34,12 +34,5 @@ reg = <0x11>; supply-gpios = <&test_gpio_0 2 0>; }; - - test_dev_c: test-i2c-dev@12 { - compatible = "vnd,i2c-device"; - status = "okay"; - reg = <0x12>; - supply-gpios = <&test_gpio_0 3 0>; - }; }; }; diff --git a/tests/misc/check_init_priorities/src/main.c b/tests/misc/check_init_priorities/src/main.c index a0d9e625a01..a9cf1c472cc 100644 --- a/tests/misc/check_init_priorities/src/main.c +++ b/tests/misc/check_init_priorities/src/main.c @@ -6,19 +6,12 @@ #include -static int device_init(const struct device *dev) -{ - return 0; -} - -DEVICE_DT_DEFINE(DT_INST(0, vnd_gpio_device), device_init, NULL, NULL, NULL, +DEVICE_DT_DEFINE(DT_INST(0, vnd_gpio_device), NULL, NULL, NULL, NULL, PRE_KERNEL_1, 50, NULL); -DEVICE_DT_DEFINE(DT_INST(0, vnd_i2c), device_init, NULL, NULL, NULL, +DEVICE_DT_DEFINE(DT_INST(0, vnd_i2c), NULL, NULL, NULL, NULL, PRE_KERNEL_1, 50, NULL); -DEVICE_DT_DEFINE(DT_INST(0, vnd_i2c_device), device_init, NULL, NULL, NULL, +DEVICE_DT_DEFINE(DT_INST(0, vnd_i2c_device), NULL, NULL, NULL, NULL, PRE_KERNEL_1, 49, NULL); -DEVICE_DT_DEFINE(DT_INST(1, vnd_i2c_device), device_init, NULL, NULL, NULL, +DEVICE_DT_DEFINE(DT_INST(1, vnd_i2c_device), NULL, NULL, NULL, NULL, PRE_KERNEL_1, 50, NULL); -DEVICE_DT_DEFINE(DT_INST(2, vnd_i2c_device), device_init, NULL, NULL, NULL, - PRE_KERNEL_1, 51, NULL); diff --git a/tests/misc/check_init_priorities/validate_check_init_priorities_output.py b/tests/misc/check_init_priorities/validate_check_init_priorities_output.py index 84d5335372f..eec705d251b 100755 --- a/tests/misc/check_init_priorities/validate_check_init_priorities_output.py +++ b/tests/misc/check_init_priorities/validate_check_init_priorities_output.py @@ -12,8 +12,6 @@ REFERENCE_OUTPUT = [ "ERROR: /i2c@11112222/test-i2c-dev@10 PRE_KERNEL_1 0 < /i2c@11112222 PRE_KERNEL_1 2", "INFO: /i2c@11112222/test-i2c-dev@11 PRE_KERNEL_1 3 > /gpio@ffff PRE_KERNEL_1 1", "INFO: /i2c@11112222/test-i2c-dev@11 PRE_KERNEL_1 3 > /i2c@11112222 PRE_KERNEL_1 2", - "INFO: /i2c@11112222/test-i2c-dev@12 PRE_KERNEL_1 4 > /gpio@ffff PRE_KERNEL_1 1", - "INFO: /i2c@11112222/test-i2c-dev@12 PRE_KERNEL_1 4 > /i2c@11112222 PRE_KERNEL_1 2", ] if len(sys.argv) != 2: