Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpmsg_virtio: add RPMSG_ASSERT to check the virtqueue add error #526

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

CV-Bowen
Copy link
Contributor

@CV-Bowen CV-Bowen commented Oct 18, 2023

Add RPMSG_ASSERT() to rpmsg_virtio_return_buffer() to check the possible virtqueue buffer add error.

Copy link
Collaborator

@arnopo arnopo left a comment

Choose a reason for hiding this comment

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

One minor comment for the code
please also update the commit message.
s/VQUEUE_DEBUG/RPMSG_DEBUG/

lib/rpmsg/rpmsg_virtio.c Outdated Show resolved Hide resolved
@arnopo
Copy link
Collaborator

arnopo commented Oct 26, 2023

"When enable the VQUEUE_DEBUG and RPMSG_DEBUG, RPMSG_ASSERT can
check the buffer add error."
I can not see how VQUEUE_DEBUG impact RPMSG_ASSERT,trace. I missed something?

@CV-Bowen
Copy link
Contributor Author

@arnopo I mean we can check the virtqueue_add_buffer() return error with RPMSG_ASSERT when enable VQUEUE_DEBUG.
Because virtqueue_add_buffer() can return error only when VQUEUE_DEBUG is enabled.

@arnopo
Copy link
Collaborator

arnopo commented Nov 2, 2023

@arnopo I mean we can check the virtqueue_add_buffer() return error with RPMSG_ASSERT when enable VQUEUE_DEBUG. Because virtqueue_add_buffer() can return error only when VQUEUE_DEBUG is enabled.

Got it, thanks!
Please remove the reference VQUEUE_DEBUG in the commit message , at RPMsg level we don't aware of virqueue configuration. we still test the the result ( even if always VQUEUE_SUCCESS)

@arnopo arnopo self-requested a review November 9, 2023 16:28
@arnopo
Copy link
Collaborator

arnopo commented Nov 9, 2023

@arnopo I mean we can check the virtqueue_add_buffer() return error with RPMSG_ASSERT when enable VQUEUE_DEBUG. Because virtqueue_add_buffer() can return error only when VQUEUE_DEBUG is enabled.

Got it, thanks! Please remove the reference VQUEUE_DEBUG in the commit message , at RPMsg level we don't aware of virqueue configuration. we still test the the result ( even if always VQUEUE_SUCCESS)

@CV-Bowen, please update the commit message, then ready to merge

Add RPMSG_ASSERT() to rpmsg_virtio_return_buffer() to check the
possible virtqueue buffer add error.

Signed-off-by: Bowen Wang <wangbowen6@xiaomi.com>
@CV-Bowen
Copy link
Contributor Author

CV-Bowen commented Dec 4, 2023

@arnopo Thanks, Done.

Copy link
Contributor

@edmooring edmooring left a comment

Choose a reason for hiding this comment

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

Looks good to go.

@arnopo arnopo merged commit 34f9a7c into OpenAMP:main Dec 5, 2023
3 checks passed
@arnopo arnopo added this to the Release V2024.04 milestone Jan 3, 2024
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.

5 participants