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

openamp/virtio.h: update vdev->features and make final_features optional #610

Merged
merged 3 commits into from
Oct 9, 2024

Conversation

wyr-7
Copy link
Contributor

@wyr-7 wyr-7 commented Sep 18, 2024

add helpful judge for final_features is NULL and update vdev->features in virtio_negotiate_features

@wyr-7 wyr-7 force-pushed the virtio_final_feature_null branch from e09a641 to 412f254 Compare September 18, 2024 08:55
lib/include/openamp/virtio.h Show resolved Hide resolved
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.

not approved but request changes...

@wyr-7 wyr-7 force-pushed the virtio_final_feature_null branch from 412f254 to f472fe6 Compare September 23, 2024 07:36
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.

This is a good change, but it should be in its own pull request. It also should have a better explanation in the commit message, and a comment explaining what the function actually returns.

Suggestion for commit message:

Make rproc_virtio_negotiate_features() return the mask of features successfully negotiated.

The comment for rproc_virtio_negotiate_features () could be worded similarly.

@wyr-7 wyr-7 force-pushed the virtio_final_feature_null branch from f472fe6 to 1f14efb Compare October 8, 2024 02:47
@wyr-7 wyr-7 changed the title openamp/virtio.h: negotiate also can be call when final_features is NULL openamp/virtio.h: update vdev->features and make final_features optional Oct 8, 2024
@wyr-7 wyr-7 force-pushed the virtio_final_feature_null branch from 1f14efb to 41dbe49 Compare October 8, 2024 03:34
wyr-7 added 3 commits October 8, 2024 12:16
Make rproc_virtio_negotiate_features() return the mask of features
successfully negotiated.

Signed-off-by: Yongrong Wang <wangyongrong@xiaomi.com>
set vdev->features in virtio_negotiate_features

Signed-off-by: Yongrong Wang <wangyongrong@xiaomi.com>
negotiate also can be call when final_features is NULL

Signed-off-by: Yongrong Wang <wangyongrong@xiaomi.com>
@wyr-7 wyr-7 force-pushed the virtio_final_feature_null branch from 41dbe49 to 8c2929e Compare October 8, 2024 04:17
@wyr-7 wyr-7 closed this Oct 8, 2024
@wyr-7 wyr-7 reopened this Oct 8, 2024
@wyr-7
Copy link
Contributor Author

wyr-7 commented Oct 8, 2024

This is a good change, but it should be in its own pull request. It also should have a better explanation in the commit message, and a comment explaining what the function actually returns.

Suggestion for commit message:

Make rproc_virtio_negotiate_features() return the mask of features successfully negotiated.

The comment for rproc_virtio_negotiate_features () could be worded similarly.

Ok, done.

@arnopo arnopo requested a review from edmooring October 8, 2024 08:03
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 added this to the Release V2024.10 milestone Oct 9, 2024
@arnopo arnopo merged commit 81ac4d3 into OpenAMP:main Oct 9, 2024
5 checks passed
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