Skip to content

mspm0: ti: cmake: migrate SoC selection from compiler flags to C macro#89

Open
ssekar15 wants to merge 1 commit into
zephyrproject-rtos:masterfrom
ssekar15:cflags
Open

mspm0: ti: cmake: migrate SoC selection from compiler flags to C macro#89
ssekar15 wants to merge 1 commit into
zephyrproject-rtos:masterfrom
ssekar15:cflags

Conversation

@ssekar15

Copy link
Copy Markdown
Collaborator

Avoid global namespace isolation pollution from zephyr_compile_definitions,
prevent HAL related cflags visible to app.

zephyrproject-rtos/zephyr#98647.

Comment thread mspm0/CMakeLists.txt Outdated
@@ -1,4 +1,16 @@
if(CONFIG_HAS_MSPM0_SDK)
if(CONFIG_SOC_FAMILY_TI_MSPM0)
string(TOUPPER ${CONFIG_SOC} SDK_SOC_SELECT)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lowercase names for local variables, also file is not indented right

Comment thread mspm0/CMakeLists.txt Outdated
if(CONFIG_SOC_FAMILY_TI_MSPM0)
string(TOUPPER ${CONFIG_SOC} SDK_SOC_SELECT)

file(WRITE "zephyr_device_selection.h"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be output to the correct location i.e. ${PROJECT_BINARY_DIR}/include/generated/zephyr/

Comment thread mspm0/source/ti/devices/DeviceFamily.h Outdated
#ifndef ti_devices_DeviceFamily__include
#define ti_devices_DeviceFamily__include

#include <zephyr_device_selection.h>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are missing the zephyr/ prefix

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compiles cleanly as compiler flag passed with -I zephyr/zephyr/build/zephyr/include/generated/zephyr

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because you have CONFIG_LEGACY_GENERATED_INCLUDE_PATH which is deprecated and really needs removing, so you need to add zephyr/

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Avoid global namespace isolation pollution from zephyr_compile_definitions,
prevent HAL related cflags visible to app.
zephyrproject-rtos/zephyr#98647.

Signed-off-by: Saravanan Sekar <saravanan@linumiz.com>
Comment thread mspm0/CMakeLists.txt
@@ -1,4 +1,16 @@
if(CONFIG_HAS_MSPM0_SDK)
if(CONFIG_SOC_FAMILY_TI_MSPM0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this check for CONFIG_SOC_FAMILY_TI_MSPM0, shouldn't CONFIG_HAS_MSPM0_SDK check above already handle this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as of now it is unclear whether unified HAL will be used for MSMP0 & MSPM33 etc. I would suggest to keep MSPM0 guard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants