Skip to content
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

hwmv2: Port NXP RT Family #69264

Merged
merged 2 commits into from
Feb 28, 2024
Merged

hwmv2: Port NXP RT Family #69264

merged 2 commits into from
Feb 28, 2024

Conversation

decsny
Copy link
Member

@decsny decsny commented Feb 20, 2024

No description provided.

soc/nxp/imxrt/soc.h Outdated Show resolved Hide resolved
drivers/memc/memc_mcux_flexspi.c Outdated Show resolved Hide resolved
soc/nxp/imxrt/rt10xx/CMakeLists.txt Outdated Show resolved Hide resolved
soc/nxp/imxrt/rt10xx/Kconfig.defconfig Outdated Show resolved Hide resolved
soc/nxp/imxrt/rt10xx/Kconfig.soc Outdated Show resolved Hide resolved
soc/nxp/imxrt/rt11xx/Kconfig.defconfig Outdated Show resolved Hide resolved
boards/nxp/mimxrt1160_evk/doc/index.rst Outdated Show resolved Hide resolved
boards/nxp/mimxrt1160_evk/doc/index.rst Outdated Show resolved Hide resolved
boards/nxp/mimxrt1170_evk/Kconfig.defconfig Outdated Show resolved Hide resolved
boards/nxp/mimxrt1170_evk/Kconfig.mimxrt1170_evk Outdated Show resolved Hide resolved
boards/nxp/mimxrt1170_evk/board.cmake Outdated Show resolved Hide resolved
Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

Mostly minor nits.

Please check compliance.

As pointed out by @nordicjm then please consider if using board variant for "hyperflash" and "qspi" isn't more correct.

With hwmv2 we now support a more gerenal variant term.

soc/nxp/imxrt/Kconfig Outdated Show resolved Hide resolved
soc/nxp/imxrt/Kconfig Outdated Show resolved Hide resolved
soc/nxp/imxrt/Kconfig Outdated Show resolved Hide resolved
soc/nxp/imxrt/Kconfig.soc Outdated Show resolved Hide resolved
soc/nxp/imxrt/rt10xx/Kconfig Outdated Show resolved Hide resolved
@decsny
Copy link
Member Author

decsny commented Feb 21, 2024

@nordicjm @tejlmand

I did consider and try using variants but found revisions was easier to implement and in my opinion made more sense. The only issue is that there is no such thing as multidimensional revions. Such as, revision A and B, and revision hyperflash and qspi.

My response to Jamie above:

Not only are revisions the most convenient way to handle this in the board definition, but also, in my view, these really are different board revisions, because switching between hyperflash and qspi requires reworking the board hardware with a soldering iron. And hyperflash and qspi are not really variants of the SOC at all, they are still using the same interface to the SOC, they are just two different flash chips. The fact they are just on the same board is just because it's an EVK... these are really two different board versions that happen to exist on the same PCB.

Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

Minor proposal, not blocking for a 👍

boards/nxp/mimxrt1050_evk/revision.cmake Outdated Show resolved Hide resolved
boards/nxp/mimxrt1060_evk/revision.cmake Outdated Show resolved Hide resolved
@tejlmand
Copy link
Collaborator

My response to Jamie above:

Not only are revisions the most convenient way to handle this in the board definition, but also, in my view, these really are different board revisions, because switching between hyperflash and qspi requires reworking the board hardware with a soldering iron.

I can accept this as a valid reason for choosing board revision over variants.
A bit of advice to avoid future questions / wondering related to this, then I propose to add this reasoning in the revision.cmake.

But you still need to address comments like these:
#69264 (comment)
#69264 (comment)

I suggest you try it once more, cause the proposed pattern has worked for any other board.
Alternative push a fixup commit to this PR with the changes you did which was not working, perhaps @nordicjm or I can see what is wrong.

@zephyrbot zephyrbot added the DNM This PR should not be merged (Do Not Merge) label Feb 23, 2024
boards/madmachine/mm_swiftio/Kconfig.mm_swiftio Outdated Show resolved Hide resolved
boards/nxp/vmu_rt1170/vmu_rt1170.dts Outdated Show resolved Hide resolved
boards/nxp/vmu_rt1170/vmu_rt1170.yaml Outdated Show resolved Hide resolved
soc/nxp/imxrt/Kconfig Show resolved Hide resolved
soc/nxp/imxrt/rt11xx/Kconfig.soc Outdated Show resolved Hide resolved
soc/nxp/imxrt/Kconfig.soc Outdated Show resolved Hide resolved
soc/nxp/imxrt/rt10xx/Kconfig.soc Outdated Show resolved Hide resolved
soc/nxp/imxrt/rt11xx/Kconfig Outdated Show resolved Hide resolved
soc/nxp/imxrt/rt11xx/Kconfig.soc Outdated Show resolved Hide resolved
soc/nxp/imxrt/rt11xx/Kconfig.soc Outdated Show resolved Hide resolved
soc/nxp/imxrt/rt6xx/Kconfig.soc Outdated Show resolved Hide resolved
soc/nxp/imxrt/rt6xx/Kconfig.soc Outdated Show resolved Hide resolved
@zephyrbot zephyrbot removed manifest manifest-hal_nxp DNM This PR should not be merged (Do Not Merge) labels Feb 27, 2024
Copy link
Collaborator

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

Looks good, minor nit.

soc/nxp/imxrt/imxrt11xx/Kconfig.soc Outdated Show resolved Hide resolved
soc/nxp/imxrt/imxrt6xx/Kconfig.soc Outdated Show resolved Hide resolved
boards/nxp/mimxrt1160_evk/Kconfig.mimxrt1160_evk Outdated Show resolved Hide resolved
boards/nxp/mimxrt1170_evk/Kconfig.mimxrt1170_evk Outdated Show resolved Hide resolved
@nordicjm nordicjm added this to the v3.7.0 milestone Feb 27, 2024
@danieldegrasse
Copy link
Collaborator

@decsny it looks like some of the RT1170 EVKB board files were missed in the rename:

$ find tests -name "mimxrt1170_evkb_cm7*"
tests/drivers/i2c/i2c_target_api/boards/mimxrt1170_evkb_cm7.overlay
tests/drivers/dma/loop_transfer/boards/mimxrt1170_evkb_cm7.overlay
tests/drivers/dma/loop_transfer/boards/mimxrt1170_evkb_cm7.conf
tests/drivers/mbox/mbox_data/boards/mimxrt1170_evkb_cm7.overlay
tests/drivers/mbox/mbox_data/boards/mimxrt1170_evkb_cm7.conf
tests/drivers/adc/adc_api/boards/mimxrt1170_evkb_cm7.overlay

Copy link
Collaborator

Choose a reason for hiding this comment

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

This block


if FLASH_MCUX_FLEXSPI_XIP
# Avoid RWW hazards by defaulting logging to disabled
choice FLASH_LOG_LEVEL_CHOICE
	default FLASH_LOG_LEVEL_OFF
endchoice
choice MEMC_LOG_LEVEL_CHOICE
	default MEMC_LOG_LEVEL_OFF
endchoice
endif

Currently depends on SOC_SERIES_IMXRT10XX || SOC_SERIES_IMXRT11XX. It should only depend on SOC_FAMILY_NXP_IMXRT, this setting needs to be applied for all IMXRT SOCs

Port IMXRT family to HWMV2, including series:
- RT11XX
- RT10XX
- RT6XX

Not including RT5XX

Co-authored-by: Declan Snyder <[email protected]>
Co-authored-by: Daniel DeGrasse <[email protected]>
Co-authored-by: Mahesh Mahadevan <[email protected]>
Co-authored-by: David Leach <[email protected]>
Co-authored-by: Yves Vandervennet <[email protected]>
Co-authored-by: Emilio Benavente <[email protected]>

Signed-off-by: Declan Snyder <[email protected]>
Convert IMXRT boards except RT595

Co-authored-by: Declan Snyder <[email protected]>
Co-authored-by: Daniel DeGrasse <[email protected]>

Signed-off-by: Declan Snyder <[email protected]>
@nordicjm nordicjm merged commit dd7b98e into zephyrproject-rtos:collab-hwm Feb 28, 2024
28 checks passed
ubieda added a commit to ubieda/zephyr that referenced this pull request May 9, 2024
Instruction not getting compiled since zephyrproject-rtos#69264.

Signed-off-by: Luis Ubieda <[email protected]>
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.

7 participants