Skip to content
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

Probe ipc4 info #9669

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Probe ipc4 info #9669

wants to merge 2 commits into from

Conversation

jsarha
Copy link
Contributor

@jsarha jsarha commented Nov 20, 2024

Implement probe IPC4 get_config() for IPC4_PROBE_MODULE_PROBE_POINTS_ADD and IPC4_PROBE_MODULE_AVAILABLE_PROBE_POINTS.

info->probe_point[j++] = _probe->probe_points[i];
}
info->num_elems = j;
comp_info(dev, "probe_get_config() %u probe points sent", j);
Copy link
Collaborator

Choose a reason for hiding this comment

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

With Zephyr logging the function name is included in the log message automatically. So including it also explicitly just wastes memory. Are you getting it duplicated in your logs or is it somehow just my configuration? If you do get it twice, let's remove it. Would be great to clean up all Zephyr-only sources from this... In theory probes can be used with XTOS too IIUC? But hardly important enough to keep these names.

if (offsetof(struct sof_ipc_probe_info_params, probe_point[index]) +
sizeof(pp) > max_size) {
info->num_elems = index;
return -1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

proper error code? I see that it isn't propagated (for now) but (1) it might get propagated in the future, and (2) -1 just doesn't make much sense - it looks like a magic constant. Something like -ENOMEM or -ENOSPC maybe

Implement IPC4 get_config() for getting currently active probe points,
by using IPC4_PROBE_MODULE_PROBE_POINTS_ADD config_id.

Signed-off-by: Jyri Sarha <[email protected]>
Add new param_id recognized by get_config() for getting all available
probe points.

Signed-off-by: Jyri Sarha <[email protected]>
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Btw, any API that returns IPC4 status to kernel should use the IPC4 return macros.

@jsarha
Copy link
Contributor Author

jsarha commented Nov 21, 2024

Btw, any API that returns IPC4 status to kernel should use the IPC4 return macros.

@lgirdwood , sorry I do not follow. Do you refer to module_interface get_configuration() method return values?

@lgirdwood
Copy link
Member

Btw, any API that returns IPC4 status to kernel should use the IPC4 return macros.

@lgirdwood , sorry I do not follow. Do you refer to module_interface get_configuration() method return values?

No I mean there is some IPC4 message codes that should be used instead or POSIX result values if any of these functions is returning a result to the host. Pls take a look at the ipc4 code and you should see IPC4_SUCCESS and similar macros that are used to transfer message state to host. This may not be the case here, but just need to check.

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