ASoC: SOF: pcm: Improve debug and error prints #5252
+163
−147
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Move the sof_pcm_stream_free() from sof-audio.c to pcm.c as static function and add wrapper to free all active stream, which is going to be used in ipc3/4 topology code (removes duplicated code). With this change most of the PCM stream related code is located in one source file for easier lookup and simplified flow. Signed-off-by: Peter Ujfalusi <[email protected]>
… open The platform specific pcm_open call via snd_sof_pcm_platform_open() can modify the initial buffer configuration via constraints. Move the prints as last step in the sof_pcm_open() function to reflect the final setup. Signed-off-by: Peter Ujfalusi <[email protected]>
…ev_err() Introduce spcm_dbg() and spcm_err() macros to provide consistent printing for debug and error messages which includes usable information in the print's prefix. Update the prints in pcm.c, ipc3-pcm.c and ipc4-pcm.c to take advantage of the features provided by the macros. The spcm_dbg() is prepared to handle the case when the print should not include direction (dir == -1). Signed-off-by: Peter Ujfalusi <[email protected]>
ujfalusi
requested review from
ranj063,
lgirdwood,
plbossart,
kv2019i,
dbaluta and
bardliao
as code owners
I'm going through the files and logs to see what can be improved. With a good log we can cut down on some back and forth to get the logs which have information and in case if it is a hard to reproduce issue, that can take a while... |
ranj063
reviewed
Nov 26, 2024
| * The spcm_dbg() is prepared to handle a call without valid direction (dir == -1) | ||
| */ | ||
| #define spcm_dbg(__spcm, __dir, __fmt, ...) { \ | ||
| if (__dir == -1) \ |
There was a problem hiding this comment.
@ujfalusi this is really used only at the time of creation. Do we really need to special handle it just for that? In all practical cases, I think we're going to have the dir defined.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
Our error printing alone leaves out information which can provide a needed context for the error itself.
For example in IPC3: "ipc tx error for 0x60010000 (msg/reply size: 108/20): -22"
This will not tell which PCM device, what direction this error had happened.
Similarly the debug prints are using ad-hoc formatting and they are not consistent, one needs to look for prior or later prints for context, which can be problematic with concurrent PCM operation.
At the heart this series will switch to a spcm_dbg() and spcm_err() wrapper to include useful information in prints where spcm is available.
As preparation, the PCM functions are relocated to pcm.c from sof-audio.c.