ASoC: SOF: IPC: make sof_ipc_window monosized #2087
Conversation
ktrzcinx
requested review from
dbaluta,
juimonen,
keyonjie,
lyakh and
ranj063
as code owners
include/uapi/sound/sof/abi.h
Outdated
There was a problem hiding this comment.
how far are we with standardising our ABI changes? Haven't we yet agreed to first require a documented RFC for all changes?
There was a problem hiding this comment.
RFC as issue is good pre-work, we have work done here already so you can comment/ACK/NACK in PR
There was a problem hiding this comment.
@lyakh We agreed just that we need reviews from both sides, we don't have standarised flow for 'documenting', how would you like this to look? Author already wrote what he is bumping and why, however it may be not so 'detailed', cos he wrote that just backward compatibility is kept. Do you need it to be more detailed or maybe you need some other info?
There was a problem hiding this comment.
@jajanusz Having an implementation along with the ABI change is good IMHO. Without one, studying even an extended documentation might be difficult trying to understand whether the proposed change will really work. The change in this PR isn't particularly difficult either, so, I don't think it needs more documentation than is already provided. But the PR lacks both an ABI tag and an [RFC] keyword, so it could easily be missed.
sound/soc/sof/loader.c
Outdated
There was a problem hiding this comment.
mmh, correct me if I'm wrong. This calculation now makes w_size = sizeof(struct sof_ipc_window) + sizeof(w->window) * w->num_windows so now it includes SOF_IPC_MAX_ELEMS of struct sof_ipc_window_elem plus w->num_windows of them? Is this really what we need?
There was a problem hiding this comment.
Yes, I should use ext_hdr->hdr.size to get proper size.
ktrzcinx
changed the title
|
@ktrzcinx the ext_man series is not upstream yet. So can this be submitted as another fixup for the series instead of an incremental patch? Also could you please address Marc's other comment here #2095 (comment). We cannot afford to falter with this series again. |
|
@ranj063 Method of handling |
ktrzcinx
changed the title
plbossart
left a comment
plbossart
left a comment
There was a problem hiding this comment.
Not sold on the explanation below.
Also please fix typos in commit message
backwards
This step is needed to add possibility to pack sof_ipc_window inside another one in used FW build tools - for example in extended manifest. Structure reusability leads to easy parsing function reuse, so source code is shorter and easier to maintain. Using structures with constant size is less tricky and properly supported by each toolchain by contrast to variable size elements. This is minor ABI change - backward compatibility is kept. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
|
@plbossart fixed typo and little extended commit description. |
|
@kv2019i FW side now merged. |
7 participants
This step is needed to add possibility to pack sof_ipc_window inside
another one - for example for extended manifest.
Using structures with contant size is less tricky and properly supported by
each toolchain by contrast to variable size elements.
This is minor ABI change - bacward compatibility is kept.
Signed-off-by: Karol Trzcinski karolx.trzcinski@linux.intel.com
PR in FW:
thesofproject/sof#2906