Clean up dma-trace.h and dma-trace.c dependencies #9595
Conversation
The dma_copy_new() implemented in dma-copy.c is only used by dma-trace.c, so if CONFIG_TRACE=n, the file can be omitted in builds. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
If the platform is build with native Zephyr drivers, dma-trace.h cannot be used as it relies on legacy XTOS DMA driver interface. Make include of dma-trace.h conditional. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
If the SOF is built with native Zephyr drivers, dma-trace.h cannot be used as it relies on legacy XTOS DMA driver interface. Make include of dma-trace.h conditional. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
requested review from
bardliao,
marcinszkudlinski,
pblaszko,
lgirdwood,
plbossart,
mmaka1,
lbetlej and
dbaluta
as code owners
| @@ -34,7 +34,9 @@ | |||
| #include <sof/platform.h> | |||
| #include <rtos/string.h> | |||
| #include <rtos/string_macro.h> | |||
| #if CONFIG_TRACE | |||
There was a problem hiding this comment.
why don't put those ifdefs in dma_trace.h file? It would have a global effect, also if somebody use dma_trace.h in future
There was a problem hiding this comment.
@marcinszkudlinski That's an option. We have a lot of ifdefs hidden like this. In this case, we have longterm goal to move to Zephry and stop using dma-trace (as it only covers parts of events when using Zephyr), so I actually want to make it explicit ont he user side where this is still needed. There are not many cases (= two) in common code, so this is a small amount of ifdefs and is a nice reminder of the dependencies. But yeah, we can go both ways.
There was a problem hiding this comment.
ok, so its to help track impact of where its used whilst we transition code to native Zephyr. I'm good with this now, but we should end up with a global fix as @marcinszkudlinski suggested once transition work is complete.
| @@ -34,7 +34,9 @@ | |||
| #include <sof/platform.h> | |||
| #include <rtos/string.h> | |||
| #include <rtos/string_macro.h> | |||
| #if CONFIG_TRACE | |||
There was a problem hiding this comment.
ok, so its to help track impact of where its used whilst we transition code to native Zephyr. I'm good with this now, but we should end up with a global fix as @marcinszkudlinski suggested once transition work is complete.
| @@ -25,7 +25,9 @@ | |||
| #include <sof/schedule/ll_schedule.h> | |||
| #include <sof/schedule/ll_schedule_domain.h> | |||
| #include <rtos/sof.h> | |||
| #if CONFIG_TRACE | |||
| #include <sof/trace/dma-trace.h> | |||
There was a problem hiding this comment.
Feel free to just drop the inclusion of dma-trace.h altogether if not too much of a hassle. 8ulp doesn't use trace anymore.
LE: nvm, we'll remove it later on as there's still some trace-related stuff there that needs to be removed.
There was a problem hiding this comment.
Ack @LaurentiuM1234 , works for me. I was testing all the upstream imx builds and this indeed was still required to be there.
As part of RTOS header cleanups, I've noted a few incorrect dependencies between common code and DMA trace. This series has a few small fixes to add proper ifdef guards to code only needed for DMA trace (like dma-copy).