zephyr: macros: Avoid macros redefinition #6141
Conversation
ceolin
requested review from
dbaluta,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
|
Can one of the admins verify this patch?
|
|
Can one of the admins verify this patch? |
src/include/sof/bit.h
Outdated
There was a problem hiding this comment.
Found this via zephyrproject-rtos/zephyr#48963
This looks mildly dangerous to me. That resulting SOF code is going to get either the Zephyr or the SOF versions of these macros essentially arbitrarily, depending on header inclusion order.
Better to find the SOF definitions and put them under an #ifndef __ZEPHYR__, adding a #include <zephyr/sys/util.h> (or wherever these macros are) to the same file.
There was a problem hiding this comment.
The problem is that this would require Zephyr to have all these macros defined there and if we change some name it will break SoF. To make it deterministic I can include zephyr header in this file, and keep this conditionals, this way we ensure that the inclusion order doesn't matter.
There was a problem hiding this comment.
My preference here is to go full native (i.e. use the Zephyr macros directly), and only redefine locally when building for xtos (and this should be in a separate area).
There was a problem hiding this comment.
we don't have all these macros on Zephyr and will not have. For example Zephyr has a different implementation for MASK. It means that we will need a slightly bigger change in SoF. Let me look it.
|
v2: |
|
v3: |
Some macros defined in SoF are also being defined in Zephyr. Just avoiding redefinition including Zephyr's header when building with Zephyr. Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
|
v4: |
|
@ceolin I really like the way you add history of updates with v1, v2, ... v4. @lgirdwood we should keep in mind and use that for SOF more often. |
| #define SET_BITS(b_hi, b_lo, x) \ | ||
| (((x) & ((1ULL << ((b_hi) - (b_lo) + 1ULL)) - 1ULL)) << (b_lo)) | ||
| #define GET_BIT(b, x) \ | ||
| (((x) & (1ULL << (b))) >> (b)) |
There was a problem hiding this comment.
not for this PR - I see GET_BIT() and SET_BITS() defined in Zephyr in DMIC and DAI driver headers with the latter being defined in two headers, maybe they could be moved to a common shared header
|
The long term direction for SOF on Zephyr platforms is that IP drivers will migrate to Zephyr i.e. the need for bit setting macros will be reduced. We will still need an aligned bit setting macro for xtos users though. |
|
@ceolin can you check CI, looks like some build failures. |
I have avoided to duplicate these macros on Zephyr in every driver but I hit a road blocker. These macros need a proper namespace, moving them a common place started to conflict with other subsystems / hals. Regarding CI failing, that is the thing, this change requires the changes on Zephyr as well, it is the chicken and egg problem, Zephyr is also failing because SoF. One thing we can do to avoid break the code base is come back with that initial version that un-define the macros (to avoid re-definition), merge Zephyr changes, then we it again to this latest commit. |
|
No activity in a long time, converting as draft. |
9 participants
Some macros defined in SoF are also being defined in Zephyr.
Just avoiding redefinition un-defining these macros.
Signed-off-by: Flavio Ceolin flavio.ceolin@intel.com