-
Notifications
You must be signed in to change notification settings - Fork 321
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.
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
Clean up dma-trace.h and dma-trace.c dependencies #9595
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).