[RFC] treewide: zephyr: add sof_ prefix to dma_get/put() and struct dma#9728
[RFC] treewide: zephyr: add sof_ prefix to dma_get/put() and struct dma#9728kv2019i wants to merge 1 commit intothesofproject:mainfrom
Conversation
Add "sof_" namespace to dma_get()/put() in the Zephyr native drivers builds. The main "struct dma" is also renamed, so this touches quite many places in code. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Marking this as RFC. Not sure yet whether this is a net improvement given the level of code acrobatics needed to keep all targets compiling. But let me know what you think.
High-level logic remains the same. Target is to make code in src/audio/ is easier to read, even if at expense of uglier code in the RTOS adaption.
| #if CONFIG_ZEPHYR_NATIVE_DRIVERS | ||
| iipc->dh_buffer.dmac = sof_dma_get(SOF_DMA_DIR_HMEM_TO_LMEM, 0, SOF_DMA_DEV_HOST, | ||
| SOF_DMA_ACCESS_SHARED); | ||
| #else |
| #else | ||
| typedef struct dma sof_dma; | ||
| #endif | ||
|
|
There was a problem hiding this comment.
OK I admit this is starting to get (too?) ugly. This is really only needed for the few cases where SOF Zephyr is built with using XTOS drivers. One alternative is to convert the naming also in all XTOS code, but it is quite massive amount of change as this affects most drivers and most platform code.
There was a problem hiding this comment.
Its going to be short term, so will be removed at some point.
| cd->dma_link = sof_dma_get(dir, SOF_DMA_CAP_HDA, SOF_DMA_DEV_HDA, SOF_DMA_ACCESS_SHARED); | ||
| if (!cd->dma_link) { | ||
| dma_put(cd->dma_host); | ||
| sof_dma_put(cd->dma_host); |
There was a problem hiding this comment.
This is a good example of code where the namespacing helps readability. If you expand this function you can see calls to e.g. dma_get_attribute() (which is a Zephyr DMA call).
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
I'm assuming this fixes some naming collisions we are seeing now as we more tightly bind with Zephyr ?
| #else | ||
| typedef struct dma sof_dma; | ||
| #endif | ||
|
|
There was a problem hiding this comment.
Its going to be short term, so will be removed at some point.
|
@lgirdwood wrote:
I think it's a fair argument that the practical gains are not big. Neither Zephyr or SOF DMA interfaces see a lot of change (at this point at least), so actual risk of conflicts (that would show up as build errors), is small. So this is in the end about readability of code like: Having the prefix makes this immediately easier to follow and see which calls are calls to Zephyr and which are SOF side calls (sof_dma_get() is a simple helper on top of device-tree interfaces to find the correct DMA to use). But whether the readibility improvement is worth the cruft this PR brings in (like struct typedefs), that's another question. Other option is to wait until we can drop the XTOS support and clean at that time (which will be a lot easier as no need to keep various XTOS and Zephyr/XTOS hybrid build targets working anymore). |
|
Parts of this PR submitted as non-RFC as #9753 |
|
@kv2019i ping |
|
Will look at this now again. |
Btw - we should do likewise with Zephyr, e.g. |
|
See status update in #9561 (comment) . Closing for now. |
2 participants
Add "sof_" namespace to dma_get()/put() in the Zephyr native drivers builds. The main "struct dma" is also renamed, so this touches quite many places in code.
Link: #9561