Skip to content

Conversation

@lyakh
Copy link
Collaborator

@lyakh lyakh commented Jan 12, 2026

Add a sys_sem test between kernel- and user-space. @dcpleung any idea why this might not be working? Just adding a sys_sem_take() in the kernel thread with a timeout hangs the DSP, instead of returning with a timeout as expected.

@lyakh lyakh changed the title [SKIP SOF-TEST][DNM] test: userspace: add a sys_sem test [DNM] test: userspace: add a sys_sem test Jan 14, 2026
Add a sys_sem test between kernel- and user-space.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@lyakh
Copy link
Collaborator Author

lyakh commented Jan 14, 2026

SOFCI TEST

@lyakh lyakh marked this pull request as ready for review January 14, 2026 12:40
Copilot AI review requested due to automatic review settings January 14, 2026 12:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Comment on lines +62 to +63
#include <zephyr/sys/sem.h>
#include <zephyr/app_memory/mem_domain.h>
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The include directives should be placed at the top of the file with the other includes, not in the middle of the file after a ZTEST function. This violates standard C coding conventions and can make the code harder to maintain.

Copilot uses AI. Check for mistakes.
sys_sem_init(&simple_sem.sem1, 0, 1);
sys_sem_init(&simple_sem.sem2, 0, 1);

k_thread_create(&user_thread, user_stack, USER_STACKSIZE,
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The global variable 'user_thread' is being reused from the earlier tests in this file. This can cause issues if multiple tests run in sequence, as the thread structure may contain state from previous test runs. Consider using a separate thread structure for this test, such as 'sys_sem_thread', to avoid potential conflicts.

Copilot uses AI. Check for mistakes.
};

ztest_test_pass();
k_mem_domain_init(&dp_mdom, 0, NULL);
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The memory domain is initialized but never cleaned up after the test completes. The memory domain should be destroyed with k_mem_domain_remove_thread or appropriate cleanup to avoid resource leaks, especially in a test suite where multiple tests may run.

Copilot uses AI. Check for mistakes.
@lyakh lyakh changed the title [DNM] test: userspace: add a sys_sem test test: userspace: add a sys_sem test Jan 15, 2026
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test code seems fine.

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyakh for record keeping its best we can resolve the AI reviews or answer where wrong. Otherwise this LGTM.

@lgirdwood lgirdwood merged commit 1e727ec into thesofproject:main Jan 16, 2026
42 of 50 checks passed
@lyakh lyakh deleted the syssem branch January 16, 2026 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants