Repository navigation
VirtIO base #1992
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
VirtIO base #1992
Changes from all commits
907c012
b54dcc3
7de3c5e
2a0bdbd
dae9aee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,6 +77,7 @@ | |
| #define SOF_IPC_PM_CLK_REQ SOF_CMD_TYPE(0x006) | ||
| #define SOF_IPC_PM_CORE_ENABLE SOF_CMD_TYPE(0x007) | ||
| #define SOF_IPC_PM_GATE SOF_CMD_TYPE(0x008) | ||
| #define SOF_IPC_PM_VFE_POWER_STATUS SOF_CMD_TYPE(0x010) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to change the ABI, but the shared file with firmware needs to be updated.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @plbossart it is deliberate but we can discuss whether it's meaningful. I have no strong preference here. I tried to follow the pattern used in other IPC groups. Several of them jump to a new 0x10 as an indication of a new sub-group, and those jumps are never documented.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lyakh Can yo umake a FW PR for the addition. It's important to reserve the id, so it doesn't get used for something else.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lyakh we might as well start from BIT(8) to leave some space for 'regular' PM IPCs.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @plbossart as agreed with @lgirdwood in thesofproject/sof#2744 (comment) we'll drop this new IPC and re-use |
||
|
|
||
| /* component runtime config - multiple different types */ | ||
| #define SOF_IPC_COMP_SET_VALUE SOF_CMD_TYPE(0x001) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was reviewed on alsa-devel, so I'll just add this as a remark but multiple booleans in function arguments is pushing limits of readability. The newly added enum reads better to convey the requested action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kv2019i mmh, how are multiple booleans worse than multiple integers or anything else? :-) If you look inside
dpcm_process_paths()you'll see that those two booleans are handled absolutely independently, they really aren't related, don't think an enum would make sense here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lyakh don't have problems with booleans, but for some reason the name "new" bothers me :), so "new" what? also plain "new" brings me recollections from c++ etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juimonen oops, you're right, I keep forgetting about c++ :-) BTW, this PR is way outdated. We're back to #1501. But don't worry, the same comment applies there, so, your work wasn't in vain! But please continue there :-)