Skip to content

audio: fix overriding const attribute #10118

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.

Sign up for GitHub

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

Merged
lgirdwood merged 3 commits into from
Jul 22, 2025
Merged

audio: fix overriding const attribute #10118

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
83506c8
audio: chain_dma: (cosmetic) remove function names from logs
lyakh Jul 15, 2025
cb8861a
audio: chain_dma: don't abuse driver list lock
lyakh Jul 15, 2025
200d822
audio: don't override the "const" attribute
lyakh Jul 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 6 additions & 18 deletions src/audio/chain_dma.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,17 +282,13 @@ static enum task_state chain_task_run(void *data)

static int chain_task_start(struct comp_dev *dev)
{
struct comp_driver_list *drivers = comp_drivers_get();
struct chain_dma_data *cd = comp_get_drvdata(dev);
k_spinlock_key_t key;
int ret;

comp_info(dev, "host_dma_id = 0x%08x", cd->host_connector_node_id.dw);

key = k_spin_lock(&drivers->lock);
switch (cd->chain_task.state) {
case SOF_TASK_STATE_QUEUED:
k_spin_unlock(&drivers->lock, key);
Copy link
Copy Markdown
Contributor

@jsarha jsarha Jul 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Even if the used lock is wrong, it does not automatically mean that no locking is needed. Even the wrong lock protects against interleaved function calls. I have no idea if those can happen, so this is just a worried comment.

Copy link
Copy Markdown
Member

@lgirdwood lgirdwood Jul 22, 2025

Choose a reason for hiding this comment

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

@lyakh I suppose you could followup with a comment declaring where the lock is held for the state check.

return 0;
case SOF_TASK_STATE_COMPLETED:
break;
Expand All @@ -302,27 +298,26 @@ static int chain_task_start(struct comp_dev *dev)
break;
default:
comp_err(dev, "bad state transition");
ret = -EINVAL;
goto error;
return -EINVAL;
}

if (cd->stream_direction == SOF_IPC_STREAM_PLAYBACK) {
ret = chain_host_start(dev);
if (ret)
goto error;
return ret;
ret = chain_link_start(dev);
if (ret) {
chain_host_stop(dev);
goto error;
return ret;
}
} else {
ret = chain_link_start(dev);
if (ret)
goto error;
return ret;
ret = chain_host_start(dev);
if (ret) {
chain_link_stop(dev);
goto error;
return ret;
}
}

Expand All @@ -342,29 +337,24 @@ static int chain_task_start(struct comp_dev *dev)
}

pm_policy_state_lock_get(PM_STATE_RUNTIME_IDLE, PM_ALL_SUBSTATES);
k_spin_unlock(&drivers->lock, key);

return 0;

error_task:
chain_host_stop(dev);
chain_link_stop(dev);
error:
k_spin_unlock(&drivers->lock, key);

return ret;
}

static int chain_task_pause(struct comp_dev *dev)
{
struct comp_driver_list *drivers = comp_drivers_get();
struct chain_dma_data *cd = comp_get_drvdata(dev);
k_spinlock_key_t key;
int ret, ret2;

if (cd->chain_task.state == SOF_TASK_STATE_FREE)
return 0;

key = k_spin_lock(&drivers->lock);
cd->first_data_received = false;
if (cd->stream_direction == SOF_IPC_STREAM_PLAYBACK) {
ret = chain_host_stop(dev);
Expand All @@ -376,8 +366,6 @@ static int chain_task_pause(struct comp_dev *dev)
if (!ret)
ret = ret2;

k_spin_unlock(&drivers->lock, key);

schedule_task_free(&cd->chain_task);
pm_policy_state_lock_put(PM_STATE_RUNTIME_IDLE, PM_ALL_SUBSTATES);

Expand Down