On 7/20/19 2:45 PM, Cezary Rojewski wrote:
Existing IPC framework omits crucial part of the entire communication: reply header. Some IPCs cannot function at all without the access to said header. Update the sst-ipc with new sst_ipc_message structure to represent both request and reply allowing reply-processing handlers to save received responses.
Despite the range of changes required for model to be updated, no functional changes are made for core hanswell, baytrail and skylake message handlers. Reply-processing handlers now save received response header yet no usage is added by default.
To allow for future changes, righful kings of IPC kingdom need to be put back on the throne. This update addresses one of them: LARGE_CONFIG_GET.
Cezary Rojewski (5): ASoC: Intel: Update request-reply IPC model
I had a doubt on the structure of this patchset since you first change the structure definitions/prototypes and then use them in follow-up patches, and sure enough if I apply the first patch only the compilation is broken, see below.
The rule is that we can't break git bisect. And if you squash the patches together, then you have a really complicated patch to review/test, so like I said earlier such invasive changes in shared prototypes are really painful.
ASoC: Intel: Haswell: Align with updated request-reply model ASoC: Intel: Baytrail: Align with updated request-reply model ASoC: Intel: Skylake: Align with updated request-reply model ASoC: Intel: Skylake: large_config_get overhaul
sound/soc/intel/baytrail/sst-baytrail-ipc.c | 65 ++++---- sound/soc/intel/common/sst-ipc.c | 68 ++++---- sound/soc/intel/common/sst-ipc.h | 27 ++-- sound/soc/intel/haswell/sst-haswell-ipc.c | 164 +++++++++++--------- sound/soc/intel/skylake/cnl-sst.c | 6 +- sound/soc/intel/skylake/skl-messages.c | 3 +- sound/soc/intel/skylake/skl-sst-ipc.c | 152 ++++++++++-------- sound/soc/intel/skylake/skl-sst-ipc.h | 3 +- 8 files changed, 259 insertions(+), 229 deletions(-)
CC [M] sound/soc/intel/baytrail/sst-baytrail-ipc.o /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c: In function ‘sst_byt_stream_update’: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:214:18: error: ‘struct ipc_message’ has no member named ‘header’ u64 header = msg->header; ^~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c: In function ‘sst_byt_process_reply’: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:244:6: error: ‘struct ipc_message’ has no member named ‘rx_size’ msg->rx_size = sst_byt_header_data(header); ^~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:245:35: error: ‘struct ipc_message’ has no member named ‘rx_data’ sst_dsp_inbox_read(byt->dsp, msg->rx_data, msg->rx_size); ^~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:245:49: error: ‘struct ipc_message’ has no member named ‘rx_size’ sst_dsp_inbox_read(byt->dsp, msg->rx_data, msg->rx_size); ^~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c: In function ‘sst_byt_stream_commit’: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:418:43: error: incompatible type for argument 2 of ‘sst_ipc_tx_message_wait’ ret = sst_ipc_tx_message_wait(&byt->ipc, header, str_req, ^~~~~~ In file included from /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:25: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/../common/sst-ipc.h:70:25: note: expected ‘struct sst_ipc_message’ but argument is of type ‘u64’ {aka ‘long long unsigned int’} struct sst_ipc_message request, struct sst_ipc_message *reply); ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:418:51: error: passing argument 3 of ‘sst_ipc_tx_message_wait’ from incompatible pointer type [-Werror=incompatible-pointer-types] ret = sst_ipc_tx_message_wait(&byt->ipc, header, str_req, ^~~~~~~ In file included from /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:25: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/../common/sst-ipc.h:70:58: note: expected ‘struct sst_ipc_message *’ but argument is of type ‘struct sst_byt_alloc_params *’ struct sst_ipc_message request, struct sst_ipc_message *reply); ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:418:8: error: too many arguments to function ‘sst_ipc_tx_message_wait’ ret = sst_ipc_tx_message_wait(&byt->ipc, header, str_req, ^~~~~~~~~~~~~~~~~~~~~~~ In file included from /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:25: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/../common/sst-ipc.h:69:5: note: declared here int sst_ipc_tx_message_wait(struct sst_generic_ipc *ipc, ^~~~~~~~~~~~~~~~~~~~~~~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c: In function ‘sst_byt_stream_free’: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:442:43: error: incompatible type for argument 2 of ‘sst_ipc_tx_message_wait’ ret = sst_ipc_tx_message_wait(&byt->ipc, header, NULL, 0, NULL, 0); ^~~~~~ In file included from /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:25: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/../common/sst-ipc.h:70:25: note: expected ‘struct sst_ipc_message’ but argument is of type ‘u64’ {aka ‘long long unsigned int’} struct sst_ipc_message request, struct sst_ipc_message *reply); ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:442:8: error: too many arguments to function ‘sst_ipc_tx_message_wait’ ret = sst_ipc_tx_message_wait(&byt->ipc, header, NULL, 0, NULL, 0); ^~~~~~~~~~~~~~~~~~~~~~~ In file included from /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:25: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/../common/sst-ipc.h:69:5: note: declared here int sst_ipc_tx_message_wait(struct sst_generic_ipc *ipc, ^~~~~~~~~~~~~~~~~~~~~~~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c: In function ‘sst_byt_stream_operations’: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:466:45: error: incompatible type for argument 2 of ‘sst_ipc_tx_message_wait’ return sst_ipc_tx_message_wait(&byt->ipc, header, NULL, ^~~~~~ In file included from /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:25: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/../common/sst-ipc.h:70:25: note: expected ‘struct sst_ipc_message’ but argument is of type ‘u64’ {aka ‘long long unsigned int’} struct sst_ipc_message request, struct sst_ipc_message *reply); ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:466:10: error: too many arguments to function ‘sst_ipc_tx_message_wait’ return sst_ipc_tx_message_wait(&byt->ipc, header, NULL, ^~~~~~~~~~~~~~~~~~~~~~~ In file included from /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:25: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/../common/sst-ipc.h:69:5: note: declared here int sst_ipc_tx_message_wait(struct sst_generic_ipc *ipc, ^~~~~~~~~~~~~~~~~~~~~~~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:469:47: error: incompatible type for argument 2 of ‘sst_ipc_tx_message_nowait’ return sst_ipc_tx_message_nowait(&byt->ipc, header, ^~~~~~ In file included from /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:25: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/../common/sst-ipc.h:73:25: note: expected ‘struct sst_ipc_message’ but argument is of type ‘u64’ {aka ‘long long unsigned int’} struct sst_ipc_message request); ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:469:10: error: too many arguments to function ‘sst_ipc_tx_message_nowait’ return sst_ipc_tx_message_nowait(&byt->ipc, header, ^~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:25: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/../common/sst-ipc.h:72:5: note: declared here int sst_ipc_tx_message_nowait(struct sst_generic_ipc *ipc, ^~~~~~~~~~~~~~~~~~~~~~~~~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c: In function ‘sst_byt_stream_start’: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:490:45: error: incompatible type for argument 2 of ‘sst_ipc_tx_message_nowait’ ret = sst_ipc_tx_message_nowait(&byt->ipc, header, tx_msg, size); ^~~~~~ In file included from /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:25: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/../common/sst-ipc.h:73:25: note: expected ‘struct sst_ipc_message’ but argument is of type ‘u64’ {aka ‘long long unsigned int’} struct sst_ipc_message request); ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:490:8: error: too many arguments to function ‘sst_ipc_tx_message_nowait’ ret = sst_ipc_tx_message_nowait(&byt->ipc, header, tx_msg, size); ^~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:25: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/../common/sst-ipc.h:72:5: note: declared here int sst_ipc_tx_message_nowait(struct sst_generic_ipc *ipc, ^~~~~~~~~~~~~~~~~~~~~~~~~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c: In function ‘byt_tx_msg’: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:626:9: error: ‘struct ipc_message’ has no member named ‘header’ if (msg->header & IPC_HEADER_LARGE(true)) ^~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:627:37: error: ‘struct ipc_message’ has no member named ‘tx_data’ sst_dsp_outbox_write(ipc->dsp, msg->tx_data, msg->tx_size); ^~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:627:51: error: ‘struct ipc_message’ has no member named ‘tx_size’ sst_dsp_outbox_write(ipc->dsp, msg->tx_data, msg->tx_size); ^~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:629:55: error: ‘struct ipc_message’ has no member named ‘header’ sst_dsp_shim_write64_unlocked(ipc->dsp, SST_IPCX, msg->header); ^~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c: In function ‘byt_tx_data_copy’: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:651:13: error: ‘struct ipc_message’ has no member named ‘tx_data’ *(u32 *)msg->tx_data = (u32)(msg->header & (u32)-1); ^~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:651:34: error: ‘struct ipc_message’ has no member named ‘header’ *(u32 *)msg->tx_data = (u32)(msg->header & (u32)-1); ^~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:652:12: error: ‘struct ipc_message’ has no member named ‘tx_data’ memcpy(msg->tx_data + sizeof(u32), tx_data, tx_size); ^~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:653:5: error: ‘struct ipc_message’ has no member named ‘tx_size’ msg->tx_size += sizeof(u32); ^~ /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c: In function ‘sst_byt_stream_operations’: /data/pbossart/ktest/broonie-next/sound/soc/intel/baytrail/sst-baytrail-ipc.c:471:1: warning: control reaches end of non-void function [-Wreturn-type] } ^ cc1: some warnings being treated as errors