Skip to content

Conversation

@ranj063
Copy link
Collaborator

@ranj063 ranj063 commented Oct 5, 2021

@ujfalusi
Copy link
Collaborator

ujfalusi commented Oct 7, 2021

@ranj063, while at it, can you also update the ipc_log_header() to handle this new message?

Add a new SOF_IPC_TRACE_DMA_FREE IPC command to stop and free trace DMA
in the FW.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Parse all the trace DMA IPC commands in ipc_log_header().

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
Copy link
Collaborator Author

ranj063 commented Oct 15, 2021

@ranj063, while at it, can you also update the ipc_log_header() to handle this new message?

done. thanks @ujfalusi

@ranj063 ranj063 force-pushed the fix/trace_dma_free branch from a9f6186 to 1d50589 Compare October 15, 2021 04:52
@ranj063 ranj063 requested a review from dbaluta as a code owner October 15, 2021 04:52
@ranj063 ranj063 requested a review from ujfalusi October 15, 2021 04:56
Send the DMA_TRACE_FREE IPC during release to stop and free the trace
DMA in the DSP.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063 ranj063 force-pushed the fix/trace_dma_free branch from 1d50589 to d4de613 Compare October 15, 2021 05:09
Copy link
Collaborator

@ujfalusi ujfalusi left a comment

Choose a reason for hiding this comment

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

@ranj063, looks good, thanks. Only one nitpick if you want to fix it.

struct sof_ipc_fw_ready *ready = &sdev->fw_ready;
struct sof_ipc_fw_version *v = &ready->version;
struct sof_ipc_cmd_hdr hdr;
struct sof_ipc_reply ipc_reply;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: can you swap these?

or move them local under the if(ABI) ?

Copy link
Member

Choose a reason for hiding this comment

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

@ranj063 let's do this change suggested by @ujfalusi with a fixup

Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

nit-pick comments, ok to merge and deal with comments with a fixup

struct sof_ipc_fw_ready *ready = &sdev->fw_ready;
struct sof_ipc_fw_version *v = &ready->version;
struct sof_ipc_cmd_hdr hdr;
struct sof_ipc_reply ipc_reply;
Copy link
Member

Choose a reason for hiding this comment

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

@ranj063 let's do this change suggested by @ujfalusi with a fixup

ret = sof_ipc_tx_message(sdev->ipc, hdr.cmd, &hdr, hdr.size,
&ipc_reply, sizeof(ipc_reply));
if (ret < 0)
dev_err(sdev->dev, "DMA_TRACE_FREE failed with error: %d\n", ret);
Copy link
Member

Choose a reason for hiding this comment

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

return ret?

or add a comment that on why we don't return?

This can be done with a fixup PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the intention is to continue to the host side release, even if the IPC fails, in this case this should be dev_warn()?

@plbossart plbossart merged commit ead9a08 into thesofproject:topic/sof-dev Oct 21, 2021
@ranj063 ranj063 deleted the fix/trace_dma_free branch February 13, 2022 05:07
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