Skip to content

audio: host-zephyr: rework calls to DMA driver, remove channel pointer #10721

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

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

audio: host-zephyr: rework calls to DMA driver, remove channel pointer #10721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
589dbeb
audio: host-zephyr: rework calls to DMA driver, remove channel pointer
kv2019i Feb 16, 2026
b6ca080
audio: host-zephyr: rework calls to DMA driver, remove channel pointer
kv2019i Apr 27, 2026
9805a75
audio: host-zephyr: clean DMA if channel set at free
kv2019i Apr 27, 2026
746f49e
audio: host-zephyr: fix log print formatting
kv2019i Apr 27, 2026
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
3 changes: 2 additions & 1 deletion src/audio/copier/host_copier.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,11 @@ struct host_data {
/* local DMA config */
#if CONFIG_ZEPHYR_NATIVE_DRIVERS
struct sof_dma *dma;
int chan_index;
#else
struct dma *dma;
#endif
struct dma_chan_data *chan;
#endif
struct dma_sg_config config;
#ifdef __ZEPHYR__
struct dma_config z_config;
Expand Down
71 changes: 36 additions & 35 deletions src/audio/host-zephyr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ static int host_dma_set_config_and_copy(struct host_data *hd, struct comp_dev *d
local_elem->size = bytes;

/* reconfigure transfer */
ret = sof_dma_config(hd->chan->dma, hd->chan->index, &hd->z_config);
ret = sof_dma_config(hd->dma, hd->chan_index, &hd->z_config);
if (ret < 0) {
comp_err(dev, "dma_config() failed, ret = %d",
ret);
Expand All @@ -93,9 +93,9 @@ static int host_dma_set_config_and_copy(struct host_data *hd, struct comp_dev *d

cb(dev, bytes);

ret = sof_dma_reload(hd->chan->dma, hd->chan->index, bytes);
ret = sof_dma_reload(hd->dma, hd->chan_index, bytes);
if (ret < 0) {
comp_err(dev, "dma_copy() failed, ret = %d",
comp_err(dev, "dma_reload() failed, ret = %d",
ret);
return ret;
}
Expand Down Expand Up @@ -223,17 +223,17 @@ static int host_copy_one_shot(struct host_data *hd, struct comp_dev *dev, copy_c
hd->z_config.head_block->block_size = local_elem->size;

/* reconfigure transfer */
ret = sof_dma_config(hd->chan->dma, hd->chan->index, &hd->z_config);
ret = sof_dma_config(hd->dma, hd->chan_index, &hd->z_config);
if (ret < 0) {
comp_err(dev, "dma_config() failed, ret = %u", ret);
comp_err(dev, "dma_config() failed, ret = %d", ret);
return ret;
Comment on lines +226 to 229
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

ret is an int and can be negative here, but the log uses "%u". This will print negative error codes as large unsigned values; use a signed format (e.g., %d) for ret in this error path.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

@kv2019i kv2019i Apr 24, 2026

Choose a reason for hiding this comment

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

The PR does not change this code.

}

cb(dev, copy_bytes);

ret = sof_dma_reload(hd->chan->dma, hd->chan->index, copy_bytes);
ret = sof_dma_reload(hd->dma, hd->chan_index, copy_bytes);
if (ret < 0)
comp_err(dev, "dma_copy() failed, ret = %u", ret);
comp_err(dev, "dma_reload() failed, ret = %d", ret);

Comment on lines +234 to 237
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

The error log says "dma_copy() failed" but the failing call is sof_dma_reload(), and it also prints ret with "%u" even though ret can be negative. Adjust the message to refer to dma_reload()/sof_dma_reload and use a signed format for ret.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

@kv2019i kv2019i Apr 24, 2026

Choose a reason for hiding this comment

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

The PR does not change this code.

return ret;
}
Expand Down Expand Up @@ -369,7 +369,7 @@ static void host_dma_cb(struct comp_dev *dev, size_t bytes)
/* get status from dma and check for xrun */
static int host_get_status(struct comp_dev *dev, struct host_data *hd, struct dma_status *stat)
{
int ret = sof_dma_get_status(hd->chan->dma, hd->chan->index, stat);
int ret = sof_dma_get_status(hd->dma, hd->chan_index, stat);
#if CONFIG_XRUN_NOTIFICATIONS_ENABLE
if (ret == -EPIPE && !hd->xrun_notification_sent) {
hd->xrun_notification_sent = send_copier_gateway_xrun_notif_msg
Expand Down Expand Up @@ -428,7 +428,7 @@ static uint32_t host_get_copy_bytes_normal(struct host_data *hd, struct comp_dev
/* get data sizes from DMA */
ret = host_get_status(dev, hd, &dma_stat);
if (ret < 0) {
comp_err(dev, "dma_get_status() failed, ret = %u",
comp_err(dev, "dma_get_status() failed, ret = %d",
ret);
/* return 0 copy_bytes in case of error to skip DMA copy */
return 0;
Expand Down Expand Up @@ -556,10 +556,10 @@ static int host_copy_normal(struct host_data *hd, struct comp_dev *dev, copy_cal
if (!copy_bytes) {
if (hd->partial_size != 0) {
if (stream_sync(hd, dev)) {
ret = sof_dma_reload(hd->chan->dma, hd->chan->index,
ret = sof_dma_reload(hd->dma, hd->chan_index,
hd->partial_size);
if (ret < 0)
comp_err(dev, "dma_reload() failed, ret = %u", ret);
comp_err(dev, "dma_reload() failed, ret = %d", ret);

hd->partial_size = 0;
}
Expand All @@ -583,10 +583,10 @@ static int host_copy_normal(struct host_data *hd, struct comp_dev *dev, copy_cal
hd->dma_buffer_size - hd->partial_size <=
(2 + threshold) * hd->period_bytes) {
if (stream_sync(hd, dev)) {
ret = sof_dma_reload(hd->chan->dma, hd->chan->index,
ret = sof_dma_reload(hd->dma, hd->chan_index,
hd->partial_size);
if (ret < 0)
comp_err(dev, "dma_reload() failed, ret = %u", ret);
comp_err(dev, "dma_reload() failed, ret = %d", ret);

hd->partial_size = 0;
}
Expand Down Expand Up @@ -651,22 +651,22 @@ int host_common_trigger(struct host_data *hd, struct comp_dev *dev, int cmd)
if (cmd != COMP_TRIGGER_START && hd->copy_type == COMP_COPY_ONE_SHOT)
return ret;

if (!hd->chan) {
if (hd->chan_index == -EINVAL) {
comp_err(dev, "no dma channel configured");
return -EINVAL;
}

switch (cmd) {
case COMP_TRIGGER_START:
hd->partial_size = 0;
ret = sof_dma_start(hd->chan->dma, hd->chan->index);
ret = sof_dma_start(hd->dma, hd->chan_index);
if (ret < 0)
comp_err(dev, "dma_start() failed, ret = %u",
comp_err(dev, "dma_start() failed, ret = %d",
ret);
break;
case COMP_TRIGGER_STOP:
case COMP_TRIGGER_XRUN:
ret = sof_dma_stop(hd->chan->dma, hd->chan->index);
ret = sof_dma_stop(hd->dma, hd->chan_index);
if (ret < 0)
comp_err(dev, "dma stop failed: %d",
ret);
Expand Down Expand Up @@ -726,7 +726,7 @@ __cold int host_common_new(struct host_data *hd, struct comp_dev *dev,
sof_dma_put(hd->dma);
return -ENOMEM;
}
hd->chan = NULL;
hd->chan_index = -EINVAL;
hd->copy_type = COMP_COPY_NORMAL;

return 0;
Expand Down Expand Up @@ -784,6 +784,13 @@ __cold void host_common_free(struct host_data *hd)
}
#endif

/* release DMA channel if not already done by reset */
if (hd->chan_index != -EINVAL) {
sof_dma_stop(hd->dma, hd->chan_index);
sof_dma_release_channel(hd->dma, hd->chan_index);
hd->chan_index = -EINVAL;
}

sof_dma_put(hd->dma);

ipc_msg_free(hd->msg);
Expand Down Expand Up @@ -865,7 +872,7 @@ int host_common_params(struct host_data *hd, struct comp_dev *dev,
uint32_t buffer_size_preferred;
uint32_t addr_align;
uint32_t align;
int i, channel, err;
int i, err;
bool is_scheduling_source = dev == dev->pipeline->sched_comp;
uint32_t round_up_size;

Expand Down Expand Up @@ -1001,22 +1008,16 @@ int host_common_params(struct host_data *hd, struct comp_dev *dev,
/* get DMA channel from DMAC
* note: stream_tag is ignored by dw-dma
*/
channel = sof_dma_request_channel(hd->dma, hda_chan);
if (channel < 0) {
hd->chan_index = sof_dma_request_channel(hd->dma, hda_chan);
if (hd->chan_index < 0) {
comp_err(dev, "requested channel %d is busy", hda_chan);
return -ENODEV;
}
hd->chan = &hd->dma->chan[channel];
Copy link
Copy Markdown
Contributor

@wjablon1 wjablon1 Apr 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I did not fully analyze this code but at first glance I see this risk:

The previous invalid value (hd->chan == NULL) was also the default value (zero memset) whereas the current invalid value (hd->chan_index == -1) is not the default. Did you make sure the code is secured in that regard?

Also instead of "-1" you could use some predefined error code like -ENXIO


uint32_t buffer_addr = 0;
uint32_t buffer_bytes = 0;
uint32_t addr;

hd->chan->direction = config->direction;
hd->chan->desc_count = config->elem_array.count;
hd->chan->is_scheduling_source = config->is_scheduling_source;
hd->chan->period = config->period;
Copy link
Copy Markdown
Collaborator

@lyakh lyakh Apr 27, 2026

Choose a reason for hiding this comment

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

if CONFIG_ZEPHYR_NATIVE_DRIVERS isn't set, .chan is kept, but this is still not needed?

Copy link
Copy Markdown
Collaborator Author

@kv2019i kv2019i Apr 27, 2026

Choose a reason for hiding this comment

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

@lyakh CONFIG_ZEPHYR_NATIVE_DRIVERS is used to select whether to compile host-zephyr.c or host-host.c, so when not set, this file should not be built.


memset(dma_cfg, 0, sizeof(*dma_cfg));

dma_block_cfg = rzalloc(SOF_MEM_FLAG_USER,
Expand Down Expand Up @@ -1063,7 +1064,7 @@ int host_common_params(struct host_data *hd, struct comp_dev *dev,
break;
}

err = sof_dma_config(hd->chan->dma, hd->chan->index, dma_cfg);
err = sof_dma_config(hd->dma, hd->chan_index, dma_cfg);
if (err < 0) {
comp_err(dev, "dma_config() failed");
goto err_free_block_cfg;
Expand Down Expand Up @@ -1095,7 +1096,7 @@ int host_common_params(struct host_data *hd, struct comp_dev *dev,
*/
struct io_perf_data_item init_data = {
IO_PERF_HDA_ID,
hd->chan->index,
hd->chan_index,
params->direction,
IO_PERF_POWERED_UP_ENABLED,
IO_PERF_D0IX_POWER_MODE,
Expand All @@ -1111,8 +1112,8 @@ int host_common_params(struct host_data *hd, struct comp_dev *dev,
dma_cfg->head_block = NULL;
rfree(dma_block_cfg);
err_release_channel:
sof_dma_release_channel(hd->dma, hd->chan->index);
hd->chan = NULL;
sof_dma_release_channel(hd->dma, hd->chan_index);
hd->chan_index = -EINVAL;

return err;
}
Expand Down Expand Up @@ -1170,10 +1171,10 @@ static int host_position(struct comp_dev *dev,

void host_common_reset(struct host_data *hd, uint16_t state)
{
if (hd->chan) {
sof_dma_stop(hd->chan->dma, hd->chan->index);
sof_dma_release_channel(hd->dma, hd->chan->index);
hd->chan = NULL;
if (hd->chan_index != -EINVAL) {
sof_dma_stop(hd->dma, hd->chan_index);
sof_dma_release_channel(hd->dma, hd->chan_index);
hd->chan_index = -EINVAL;
}

/* free all DMA elements */
Expand Down
Loading