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.

Sign up for GitHub

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

Jump to bottom

Add rt1180 flexspi support #77656

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add rt1180 flexspi support #77656

wants to merge 4 commits into from
+193 −15

Conversation

lucien-nxp
Copy link
Contributor

Support flexspi to operate external nor flash.
FLASH_SHELL case have been tested on RT1180 CM33/CM7 cores.

decsny
decsny reviewed Aug 28, 2024
View reviewed changes
soc/nxp/imxrt/imxrt118x/CMakeLists.txt Outdated
if(CONFIG_MEMC_MCUX_FLEXSPI)
zephyr_sources(./../imxrt11xx/flexspi.c)
if(CONFIG_FLASH_MCUX_FLEXSPI_XIP)
zephyr_code_relocate(FILES ./../imxrt11xx/flexspi.c LOCATION ${CONFIG_FLASH_MCUX_FLEXSPI_XIP_MEM}_TEXT)
Copy link
Member

Choose a reason for hiding this comment

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

this dependency is not going to be obvious to someone only looking at rt1170 is my observation about this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have prepared standalone flexspi.c file for RT118x series.

soc/nxp/imxrt/imxrt118x/soc.c Outdated Show resolved Hide resolved
soc/nxp/imxrt/mpu_regions.c Outdated Show resolved Hide resolved
soc/nxp/imxrt/mpu_regions.c Outdated Show resolved Hide resolved
@lucien-nxp lucien-nxp requested a review from decsny August 29, 2024 10:25
@carlescufi carlescufi assigned decsny and unassigned nordic-krch Aug 29, 2024
@decsny decsny assigned danieldegrasse and unassigned decsny Aug 29, 2024
@hakehuang
Copy link
Collaborator

board testing pass for tests/drivers/flash/common

danieldegrasse
danieldegrasse reviewed Aug 30, 2024
View reviewed changes
soc/nxp/imxrt/imxrt118x/flexspi.c Outdated Show resolved Hide resolved
soc/nxp/imxrt/imxrt118x/soc.c Outdated Show resolved Hide resolved
soc/nxp/imxrt/imxrt118x/soc.h Outdated
@@ -20,6 +20,10 @@
extern "C" {
#endif

#ifdef CONFIG_MEMC
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#ifdef CONFIG_MEMC
#ifdef CONFIG_MEMC_MCUX_FLEXSPI

Let's use the same dependency between the CMake file where we pull in the C code for this function, and the function prototype

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

dts/arm/nxp/nxp_rt118x_cm33.dtsi Outdated Show resolved Hide resolved
dts/arm/nxp/nxp_rt118x_cm7.dtsi Outdated Show resolved Hide resolved
@lucien-nxp lucien-nxp force-pushed the add_rt1180_flexspi_support branch 3 times, most recently from 0df958b to 111e909 Compare September 4, 2024 14:57
danieldegrasse
danieldegrasse previously approved these changes Sep 19, 2024
View reviewed changes
@danieldegrasse
Copy link
Collaborator

Just a note here- as this board has a large flash, we will probably need a change similar to mcu-tools/mcuboot#1918 in order to get MCUboot support working

danieldegrasse
danieldegrasse reviewed Sep 20, 2024
View reviewed changes
soc/nxp/imxrt/imxrt118x/Kconfig
@@ -2,6 +2,7 @@
# SPDX-License-Identifier: Apache-2.0

config SOC_SERIES_IMXRT118X
select CPU_HAS_CUSTOM_FIXED_SOC_MPU_REGIONS if SOC_MIMXRT1189_CM7
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where do we pull in a custom MPU region definition for the RT1180 CM7 core?

Copy link
Contributor Author

@lucien-nxp lucien-nxp Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm sorry. I've been multi-threading lately, and I got this question wrong.
Here need to use common mpu_regions.c files here to re-configure mpu section for cm7 cores. Due to cm7 speculative prefetches feature, we need to forbid cm7 core to access some memory we don't configure. So I use common mpu_regions.c file under imxrt folder to re-config mpu regions.

@lucien-nxp lucien-nxp force-pushed the add_rt1180_flexspi_support branch 2 times, most recently from f825996 to ad514b8 Compare September 23, 2024 12:11
danieldegrasse
danieldegrasse previously approved these changes Sep 23, 2024
View reviewed changes
@lucien-nxp
soc: nxp: imxrt: imxrt118x: add flexspi support
cff817e 
add flexspi.c file to get flexspi clock rate.

Enable flexspi1 clock if don't boot from flash.

Use custom fixed mpu_regions.c file to config MPU for CM7

Signed-off-by: Lucien Zhao <[email protected]>
@lucien-nxp
dts: arm: nxp: rt118x: add flexspi instance support
7808247 
add flexspi2 and rename flexspi1 to flexspi to adapt
flexspi.c driver under soc/nxp/rt118x folder.

Signed-off-by: Lucien Zhao <[email protected]>
@lucien-nxp
drivers: clock_control:: Update ccm_rev2 clock driver for RT118X
41a327c 
flexspi_clock_set_freq can be applied for RT118X series.

Signed-off-by: Lucien Zhao <[email protected]>
@lucien-nxp
boards: nxp: mimxrt1180_evk: add flexspi1 support
87fca2a 
USE_HYPERRAM macro defined shouldn't be placed under
CONFIG_BOOT_FLEXSPI_NOR.

Add flexspi1 pinmux pinctrl and flash partitions

Add flash partitions and correct flash parameter.

This commit was tested with samples: flash_shell on cm33/cm7 cores

Signed-off-by: Lucien Zhao <[email protected]>
danieldegrasse
danieldegrasse reviewed Sep 24, 2024
View reviewed changes
boards/nxp/mimxrt1180_evk/mimxrt1180_evk.dtsi
pinctrl-names = "default";
};

&flexspi {
status = "okay";
ahb-prefetch;
ahb-read-addr-opt;
rx-clock-source = <1>;
w25q128jw: w25q128jw@0 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
w25q128jw: w25q128jw@0 {
w25q128jw: w25q128jw@2 {

Reg address should match here, otherwise we'll get a DTC warning

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

@danieldegrasse danieldegrasse danieldegrasse left review comments

@mmahadevan108 mmahadevan108 Awaiting requested review from mmahadevan108 mmahadevan108 is a code owner

@dleach02 dleach02 Awaiting requested review from dleach02 dleach02 is a code owner

@butok butok Awaiting requested review from butok

@dbaluta dbaluta Awaiting requested review from dbaluta

@DerekSnell DerekSnell Awaiting requested review from DerekSnell

@EmilioCBen EmilioCBen Awaiting requested review from EmilioCBen

@manuargue manuargue Awaiting requested review from manuargue

@MarkWangChinese MarkWangChinese Awaiting requested review from MarkWangChinese

@nordic-krch nordic-krch Awaiting requested review from nordic-krch

@yvanderv yvanderv Awaiting requested review from yvanderv

@decsny decsny Awaiting requested review from decsny

At least 2 approving reviews are required to merge this pull request.

Assignees

@danieldegrasse danieldegrasse

Labels
area: Clock Control platform: NXP Drivers NXP Semiconductors, drivers platform: NXP NXP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@lucien-nxp @hakehuang @danieldegrasse @decsny @nordic-krch @zephyrbot