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

Add RT700 basic environment support #79376

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

Conversation

lucien-nxp
Copy link
Contributor

support gpio/uart function

@zephyrbot
Copy link
Collaborator

zephyrbot commented Oct 3, 2024

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
hal_nxp zephyrproject-rtos/hal_nxp@0ac8302 zephyrproject-rtos/hal_nxp#444 zephyrproject-rtos/hal_nxp#444/files

DNM label due to: 1 project with PR revision

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@zephyrbot zephyrbot added manifest manifest-hal_nxp DNM This PR should not be merged (Do Not Merge) labels Oct 3, 2024
@lucien-nxp lucien-nxp force-pushed the add_rt700_baseline_support branch from aebb39a to a8fc58d Compare October 3, 2024 16:42
soc/nxp/common/Kconfig.flexspi_xip 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/imxrt7xx/Kconfig Outdated Show resolved Hide resolved
soc/nxp/imxrt/imxrt7xx/Kconfig Outdated Show resolved Hide resolved
boards/nxp/mimxrt700_evk/doc/index.rst Outdated Show resolved Hide resolved
boards/nxp/mimxrt700_evk/mimxrt700_evk_mimxrt798s_cpu0.dts Outdated Show resolved Hide resolved
boards/nxp/mimxrt700_evk/doc/index.rst Outdated Show resolved Hide resolved
boards/nxp/mimxrt700_evk/Kconfig.defconfig Outdated Show resolved Hide resolved
boards/nxp/mimxrt700_evk/doc/mimxrt700_evk.webp Outdated Show resolved Hide resolved
boards/nxp/mimxrt700_evk/mimxrt700_evk-pinctrl.dtsi Outdated Show resolved Hide resolved
boards/nxp/mimxrt700_evk/mimxrt700_evk_mimxrt798s_cpu0.dts Outdated Show resolved Hide resolved
boards/nxp/mimxrt700_evk/mimxrt700_evk_mimxrt798s_cpu0.dts Outdated Show resolved Hide resolved
Comment on lines +75 to +86
config XSPI_CONFIG_BLOCK_OFFSET
hex "XSPI config block offset"
default 0x0
help
XSPI configuration block consists of parameters regarding specific
flash devices including read command sequence, quad mode enablement
sequence (optional), etc. The boot ROM expects XSPI configuration
parameter to be presented in serial nor flash.
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this not in dts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not addressed

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this make sense for DTS? It is a software setting for the offset of the XSPI configuration block, which will then be used by the bootrom to configure the XSPI to interface with an external flash device and execute from it

Copy link
Contributor Author

@lucien-nxp lucien-nxp Dec 12, 2024

Choose a reason for hiding this comment

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

There is no sense for dts. However, we need to use this kconfig to control config block offset in linker files to ensure the offset of the original block. https://github.com/zephyrproject-rtos/zephyr/pull/79376/files#diff-cbd91422112af1d51aabad377aa1a1a360111671c45cca2a795ea0c220e08cdb:~:text=.%20%3D%20CONFIG_XSPI_CONFIG_BLOCK_OFFSET%3B

Copy link
Member

Choose a reason for hiding this comment

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

@nordicjm Devicetree, as per spec, is meant to be consumed by client program. In this case you can think of the zephyr OS as client program. No software in the Zephyr OS will interact with or consume this config block. It is used by the chip boot ROM, and we only need this offset so we can link it to the proper place in the image in order for the chip to boot. I.e., this config block will be consumed even before the zephyr reset handler like z_arm_reset runs. I don't think it needs to be in DTS.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess it falls between the cracks. Will ask for @gmarull's opinion

soc/nxp/imxrt/imxrt7xx/Kconfig.soc Outdated Show resolved Hide resolved
@hakehuang
Copy link
Collaborator

@lucien-nxp , board testing on RT700 works fine now. and RTxxx hello_world works as well..

@lucien-nxp lucien-nxp force-pushed the add_rt700_baseline_support branch from c7552d8 to bdda482 Compare December 13, 2024 06:25
@lucien-nxp
Copy link
Contributor Author

Only rebase to the newest node to resolve the west.yml conflict.

1 similar comment
@lucien-nxp
Copy link
Contributor Author

Only rebase to the newest node to resolve the west.yml conflict.

@lucien-nxp lucien-nxp force-pushed the add_rt700_baseline_support branch from bdda482 to 91730dd Compare December 16, 2024 02:04
@lucien-nxp
Copy link
Contributor Author

Only rebase to the newest node to resolve the west.yml conflict.

Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

boards/nxp/mimxrt700_evk/board.yml Show resolved Hide resolved
boards/nxp/mimxrt700_evk/doc/index.rst Outdated Show resolved Hide resolved
boards/nxp/mimxrt700_evk/doc/index.rst Outdated Show resolved Hide resolved
@lucien-nxp lucien-nxp force-pushed the add_rt700_baseline_support branch from 91730dd to a4c09d5 Compare December 17, 2024 02:50
boards/nxp/mimxrt700_evk/doc/index.rst Outdated Show resolved Hide resolved
@lucien-nxp lucien-nxp force-pushed the add_rt700_baseline_support branch from a4c09d5 to f6e2758 Compare December 17, 2024 15:01
track rt700 sdk files update PR branch

Signed-off-by: Lucien Zhao <[email protected]>
add rt7xx files related to soc
support basic clock enablement
add common/Kconfig.xspi_xip file

Signed-off-by: Lucien Zhao <[email protected]>
add RT7xx dts files
add iocon/gpio/flexcomm/clock instances in dts

Signed-off-by: Lucien Zhao <[email protected]>
add nxp,xspi-device.yaml
add nxp,xspi-mx25um51345g.yaml
add nxp,xspi.yaml

Signed-off-by: Lucien Zhao <[email protected]>
add more flexcomm instances clock support to adapt
rt700 instances number

add xspi clock support

Signed-off-by: Lucien Zhao <[email protected]>
Define iocon array to store iocon base address
add index parameter support to support multi iocon instances

Signed-off-by: Lucien Zhao <[email protected]>
use clang-format to format gpio_mcux file

Signed-off-by: Lucien Zhao <[email protected]>
update gpio driver to adapt rt7xx gpio model:
1. There is no PORT_Type on RT7xx,so set PORT_Type as void
2. Add port_no parameter in gpio_mcux_config to adapt IOPCTL driver
3. Add gpio-port-offest parameter in blinding, it will help map the
   relation between index n and gpio port when some soc have domain
   access attribution.
3. Splite gpio_mcux_configure function into two functions(
   gpio_mcux_iopctl_configure and gpio_mcux_port_configure)
   according to CONFIG_PINCTRL_NXP_IOCON macro
4. Add code to adapt RT700 GPIO attribute configuration

Signed-off-by: Lucien Zhao <[email protected]>
add files related to mimxrt700_evk board
add gpio/uart function support on board

Signed-off-by: Lucien Zhao <[email protected]>
The number of mpu regions that can be configured is less than four cases.
Therefore, only remove this case on cm33 cores, failed log show below:
"num_parts of 4 exceeds maximum allowable partitions (3)"

Signed-off-by: Lucien Zhao <[email protected]>
@lucien-nxp lucien-nxp force-pushed the add_rt700_baseline_support branch from f6e2758 to be5c1bb Compare December 19, 2024 07:49
@lucien-nxp
Copy link
Contributor Author

Only rebase to the newest commit on main branch and resolve the west.yml conflicts.

Copy link
Member

Choose a reason for hiding this comment

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

This file is basically the same as the flexspi xip kconfig file except that the compatible we are working with is just different. I wonder if there could be a common file instead like Kconfig.spi_xip, which can just use the same code but set the relevant compatible differently ? then we don't need to make similar changes in two places. If you think this would be too complex and don't want to attempt this then it's fine but I think there might be a missed opportunity here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree your point. However, I think I should do this later than this PR merged, and then inform hake to test all the platforms affected by this change.

Copy link
Collaborator

@mmahadevan108 mmahadevan108 Jan 8, 2025

Choose a reason for hiding this comment

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

I agree with Declan. This extra file does not seem needed and we should be able to add the required changes to the current Kconfig file. We need to add a tracking JIRA issue so we don't miss deleting this file if this is not being addressed as part of this PR.

Comment on lines +75 to +86
config XSPI_CONFIG_BLOCK_OFFSET
hex "XSPI config block offset"
default 0x0
help
XSPI configuration block consists of parameters regarding specific
flash devices including read command sequence, quad mode enablement
sequence (optional), etc. The boot ROM expects XSPI configuration
parameter to be presented in serial nor flash.
Copy link
Member

Choose a reason for hiding this comment

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

@nordicjm Devicetree, as per spec, is meant to be consumed by client program. In this case you can think of the zephyr OS as client program. No software in the Zephyr OS will interact with or consume this config block. It is used by the chip boot ROM, and we only need this offset so we can link it to the proper place in the image in order for the chip to boot. I.e., this config block will be consumed even before the zephyr reset handler like z_arm_reset runs. I don't think it needs to be in DTS.

Comment on lines +46 to +48
- name: cm33_cpu1
- name: hifi1
- name: hifi4
Copy link
Member

Choose a reason for hiding this comment

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

@nordicjm @tejlmand if these clusters not yet supported should they be in the soc.yml ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would make sense to add them when support is added to keep things properly bisectable and minimal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I need to remove hifi1 and hifi4 core provisionally, right? and add back once hifi core supported?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, doesn't make sense to be able to select a core in the build system that is impossible to use but will be listed as supported for a given board

Copy link
Member

Choose a reason for hiding this comment

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

@lucien-nxp, please remove the cores that are not enabled yet on this platform.

@dleach02
Copy link
Member

dleach02 commented Jan 8, 2025

@nordicjm can you summarize what is needed to clear your change request? I'm having a hard time going through the comments in this PR to identify what @lucien-nxp needs to do to wrap this up.

@nordicjm
Copy link
Collaborator

nordicjm commented Jan 9, 2025

@nordicjm can you summarize what is needed to clear your change request? I'm having a hard time going through the comments in this PR to identify what @lucien-nxp needs to do to wrap this up.

This outstanding part here has been waiting for an update: #79376 (comment)

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

Successfully merging this pull request may close these issues.