audio: host-zephyr: rework calls to DMA driver, remove channel pointer#10721
audio: host-zephyr: rework calls to DMA driver, remove channel pointer#10721kv2019i wants to merge 3 commits intothesofproject:mainfrom
Conversation
kv2019i
requested review from
abonislawski,
dbaluta,
lbetlej,
lgirdwood,
mmaka1,
pblaszko and
plbossart
as code owners
|
Tested as part of bigger #10558 |
There was a problem hiding this comment.
Pull request overview
This PR simplifies host-zephyr DMA channel handling by removing the stored DMA channel pointer and tracking only the allocated channel index, using that index for all DMA driver calls.
Changes:
- Replace
hd->chan->dma/hd->chan->indexusage withhd->dma/hd->chan_indexin host-zephyr DMA operations. - Store/initialize/reset DMA channel state via
chan_index(using-1as “not configured”). - Update
struct host_data(Zephyr native drivers path) to includechan_indexinstead of a channel pointer.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/audio/host-zephyr.c |
Switch all DMA config/reload/start/stop/status calls to use hd->chan_index and update lifecycle checks/reset paths accordingly. |
src/audio/copier/host_copier.h |
Adjust struct host_data layout for Zephyr native drivers to store chan_index. |
|
The quickbuild fails seems to be network infra related " error: RPC failed; HTTP 500 curl 22 The requested URL returned error: 500". @lrudyX can you check? |
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
The copilot comments are valid, but they are not related to code changes in this PR.
| comp_err(dev, "requested channel %d is busy", hda_chan); | ||
| return -ENODEV; | ||
| } | ||
| hd->chan = &hd->dma->chan[channel]; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Ack, should be resolved. The value is now initialized and -EINVAL is used aligned to Zephyr dma.h. Does this look better now @wjablon1 ?
| return -ENOMEM; | ||
| } | ||
| hd->chan = NULL; | ||
| hd->chan_index = -1; |
There was a problem hiding this comment.
Zephyr DMA API uses -EINVAL for an invalid channel, so we could follow the same convention.
kv2019i
force-pushed
the
202604-host-zephyr-cleanup
branch
from
3d8330c to
746f49e
Compare
|
V2 pushed:
|
|
Two first commits not squashed properly, will fix in V3 soon... |
For historical reasons, host-zephyr has somewhat complicated code to manage the DMA channel instance information. When a DMA channel is allocated, a pointer to the system DMA channel table is acquired and some additional information is stored per channel. This is however redundant as the only piece of information actually needed is the channel index. Simplify the code by not storing the channel pointer anymore, but rather just store the channel index and use that in all calls to the DMA driver. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add code to check for a valid DMA channel in host_common_free() and if a DMA channel is set, stop the channel and free DMA resources. Issue found in code review. This should not be hit in normal use-cases, but this makes the free implementation more robust in case of errors. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Multiple comp_err() statements refer to incorrect DMA function call names and/or use "%u" to print integer value. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
force-pushed
the
202604-host-zephyr-cleanup
branch
from
746f49e to
aa1838d
Compare
|
V3 pushed:
|
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
LGTM, just one open to align.
| comp_err(dev, "requested channel %d is busy", hda_chan); | ||
| return -ENODEV; | ||
| } | ||
| hd->chan = &hd->dma->chan[channel]; |
For historical reasons, host-zephyr has somewhat complicated code to manage the DMA channel instance information. When a DMA channel is allocated, a pointer to the system DMA channel table is acquired and some additional information is stored per channel. This is however redundant as the only piece of information actually needed is the channel index.
Simplify the code by not storing the channel pointer anymore, but rather just store the channel index and use that in all calls to the DMA driver.