boards: arm: add toradex verdin imx8m plus board #61713

GabrielHAFs · 2023-08-21T18:00:57Z

Add Verdin iMX8M Plus board with i.MX8MP SoC and ARM Cortex-M7 processor. Port and documentation are based on the NXP MIMX8MM EVK board. This code is intended to be used with the Cortex-M7.

github-actions

Hello @GabrielHAFs, and thank you very much for your first pull request to the Zephyr project!

A project maintainer just triggered our CI pipeline to run it against your PR and ensure it's compliant and doesn't cause any issues. You might want to take this opportunity to review the project's Contributor Expectations and make any updates to your pull request if necessary. 😊

danieldegrasse

Hi @GabrielHAFs, thank you for the contribution. Could you please address the compliance failures?

danieldegrasse · 2023-08-28T22:22:29Z

boards/arm/verdin_imx8mp_m7/verdin_imx8mp_m7_defconfig

+CONFIG_PINCTRL=y
+
+CONFIG_CODE_ITCM=y
+CONFIG_CODE_DDR=n


Is CONFIG_CODE_DDR=n needed here? This symbol is a Kconfig choice.

I'm not sure if this is the right approach, but I added both configs CONFIG_CODE_ITCM (enabled by default) and CONFIG_CODE_DDR (disabled by default) as options, as explained in the index.rst. If the user chooses to use the ITCM RAM, leave it as is. If it decides to use DDR RAM, enable the CONFIG_CODE_DDR=y and CONFIG_CODE_ITCM=n`.

Changed to two different targets, one for DDR and the other for TCM, as explained on #61713 (comment)

danieldegrasse · 2023-09-20T20:54:22Z

boards/arm/verdin_imx8mp_m7/verdin_imx8mp_m7.yaml

@@ -0,0 +1,25 @@
+#
+# Copyright (c) 2017, NXP


Copyright needs to be updated

Resolved on the last commit.

danieldegrasse · 2023-09-20T20:54:31Z

boards/arm/verdin_imx8mp_m7/verdin_imx8mp_m7.dts

@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2017,2019 NXP


Resolved on the last commit.

danieldegrasse · 2023-09-20T20:56:12Z

boards/arm/verdin_imx8mp_m7/verdin_imx8mp_m7.dts

+};
+
+&uart1 {
+	status = "disabled";


status = "disabled should not be needed here, the UART will be disabled at the SOC level

This was resolved. Thanks for the comment.

danieldegrasse · 2023-09-20T20:58:13Z

dts/arm/nxp/nxp_imx8ml_m7.dtsi

@@ -167,6 +167,14 @@
 			status = "disabled";
 		};

+		uart1: uart@30860000 {


Please split this change into another commit, and remove the comment in this file:

/* * For now only UART4 is supported and * tested with the serial driver */

This is resolved on the commit 0ffcb49.

danieldegrasse · 2023-09-20T21:02:51Z

boards/arm/verdin_imx8mp_m7/verdin_imx8mp_m7.dts

+		zephyr,flash = &itcm;
+		zephyr,sram = &dtcm;
+		/* DDR */
+		/* zephyr,flash = &ddr_code; */


Something you could consider to enable running from DDR or TCM is to create two different board targets, this is something that has been done for some of the IMX boards: https://github.com/zephyrproject-rtos/zephyr/tree/main/boards/arm/mimx8mp_evk

Thanks for the suggestion @danieldegrasse. I have changed the code to add two targets, such as NXP.

mmahadevan108 · 2023-09-21T16:16:52Z

cc @Mani-Sadhasivam

MaureenHelm · 2023-10-26T15:20:02Z

@GabrielHAFs can you please have a look at the review comments?

danieldegrasse · 2023-10-30T21:49:08Z

Thanks for the updates- a few things:

Please make sure the commit enabling the use of UART1 is the first commit in the series, before the one enabling the board itself
Please take a look at the CI failures, it looks like the board is failing to build a few tests.

Add support for UART1 usage by adding uart1 node and configuration to the i.MX 8ML devicetree include. Signed-off-by: Gabriel Freitas <gabriel.freitas@toradex.com>

GabrielHAFs · 2023-10-31T15:59:42Z

@danieldegrasse I have addressed the issues and changed the order of the commits, as requested.

danieldegrasse · 2023-10-31T16:01:28Z

boards/arm/verdin_imx8mp_m7/verdin_imx8mp_m7_ddr.yaml

+  - gnuarmemb
+  - xtools
+testing:
+  ignore_tags:


Question here- do we need ignore_tags set in order to pass CI on this board? Otherwise, I'd like to avoid defining it.

I tested locally and it seems to not be necessary. I'll remove the ignore_tags.

danieldegrasse · 2023-10-31T16:02:18Z

boards/arm/verdin_imx8mp_m7/verdin_imx8mp_m7_itcm_defconfig

+CONFIG_CLOCK_CONTROL=y
+CONFIG_UART_CONSOLE=y
+CONFIG_SERIAL=y
+CONFIG_UART_INTERRUPT_DRIVEN=y


Is CONFIG_UART_INTERRUPT_DRIVEN needed as part of the minimal set of Kconfigs to enable this platform? If not, I would like to drop it here

It's not necessary. I am dropping it.

danieldegrasse · 2023-10-31T16:02:59Z

boards/arm/verdin_imx8mp_m7/verdin_imx8mp_m7_ddr_defconfig

+CONFIG_CLOCK_CONTROL=y
+CONFIG_UART_CONSOLE=y
+CONFIG_SERIAL=y
+CONFIG_UART_INTERRUPT_DRIVEN=y


Same question here about CONFIG_UART_INTERRUPT_DRIVEN

Same as #61713 (comment)

Add Verdin iMX8M Plus board with i.MX8MP SoC and ARM Cortex-M7 processor. Add two targets (DDR and ITCM) for the iMX8M Plus board. Port and documentation are based on NXP MIMX8MM EVK board. This code is intented to be used with the Cortex-M7. Signed-off-by: Gabriel Freitas <gabriel.freitas@toradex.com>

github-actions · 2023-11-16T08:26:31Z

Hi @GabrielHAFs!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

kartben · 2023-11-17T17:46:45Z

boards/arm/verdin_imx8mp_m7/doc/index.rst

+.. _verdin_imx8mp_m7:
+
+Toradex Verdin iMX8M Plus SoM
+


FWIW, this improperly formatted heading is causing the page to not show up in the documentation. This, plus the fact that the "build hello world" instructions were basically broken (wrong board name) makes me think that the documentation for this page could have use a more thorough review (at a minimum testing the page properly shows up in the documentation ;-))

Just spent 30 minutes cleaning things up in #65420, please have a look so that we can get the table of contents for Arm boards back to a better state :)

Thanks for pointing this out, and sorry for missing it initially.

~~@GabrielHAFs can you take a look at this?~~ It looks like @kartben is already on it, thanks

I obviously linked to the wrong PR -- #65420

GabrielHAFs requested review from nashif, mmahadevan108, dleach02, MaureenHelm and galak as code owners August 21, 2023 18:00

zephyrbot added the platform: NXP NXP label Aug 21, 2023

zephyrbot requested review from danieldegrasse, dbaluta, decsny, DerekSnell, EmilioCBen, iuliana-prodan, manuargue and yvanderv August 21, 2023 18:01

github-actions bot reviewed Aug 21, 2023

View reviewed changes

danieldegrasse reviewed Aug 28, 2023

View reviewed changes

GabrielHAFs force-pushed the add_toradex_verdin_imx8mp_board branch 3 times, most recently from c365641 to 14f5303 Compare September 1, 2023 11:30

GabrielHAFs force-pushed the add_toradex_verdin_imx8mp_board branch from 14f5303 to 5484a02 Compare September 12, 2023 10:44

danieldegrasse reviewed Sep 20, 2023

View reviewed changes

mmahadevan108 requested a review from Mani-Sadhasivam September 21, 2023 16:17

GabrielHAFs force-pushed the add_toradex_verdin_imx8mp_board branch from 5484a02 to f627b6c Compare October 30, 2023 17:20

zephyrbot assigned dleach02 Oct 30, 2023

dts: add support for uart1 usage on imx8ml_m7 devicetree include file

22c6d02

Add support for UART1 usage by adding uart1 node and configuration to the i.MX 8ML devicetree include. Signed-off-by: Gabriel Freitas <gabriel.freitas@toradex.com>

GabrielHAFs force-pushed the add_toradex_verdin_imx8mp_board branch from f627b6c to 23df587 Compare October 31, 2023 13:55

danieldegrasse reviewed Oct 31, 2023

View reviewed changes

GabrielHAFs force-pushed the add_toradex_verdin_imx8mp_board branch from 23df587 to ea0b7cf Compare October 31, 2023 17:20

danieldegrasse approved these changes Oct 31, 2023

View reviewed changes

dleach02 approved these changes Nov 15, 2023

View reviewed changes

carlescufi merged commit eaec581 into zephyrproject-rtos:main Nov 16, 2023
20 checks passed

kartben reviewed Nov 17, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

boards: arm: add toradex verdin imx8m plus board #61713

boards: arm: add toradex verdin imx8m plus board #61713

GabrielHAFs commented Aug 21, 2023

github-actions bot left a comment

danieldegrasse left a comment

danieldegrasse Aug 28, 2023

GabrielHAFs Aug 31, 2023

GabrielHAFs Oct 30, 2023

danieldegrasse Sep 20, 2023

GabrielHAFs Oct 30, 2023

danieldegrasse Sep 20, 2023

GabrielHAFs Oct 30, 2023

danieldegrasse Sep 20, 2023

GabrielHAFs Oct 30, 2023

danieldegrasse Sep 20, 2023

GabrielHAFs Oct 30, 2023

danieldegrasse Sep 20, 2023

GabrielHAFs Oct 30, 2023

mmahadevan108 commented Sep 21, 2023

MaureenHelm commented Oct 26, 2023

danieldegrasse commented Oct 30, 2023

GabrielHAFs commented Oct 31, 2023

danieldegrasse Oct 31, 2023

GabrielHAFs Oct 31, 2023

danieldegrasse Oct 31, 2023

GabrielHAFs Oct 31, 2023

danieldegrasse Oct 31, 2023

GabrielHAFs Oct 31, 2023

github-actions bot commented Nov 16, 2023

kartben Nov 17, 2023 •

edited

danieldegrasse Nov 17, 2023 •

edited

kartben Nov 17, 2023

boards: arm: add toradex verdin imx8m plus board #61713

boards: arm: add toradex verdin imx8m plus board #61713

Conversation

GabrielHAFs commented Aug 21, 2023

github-actions bot left a comment

Choose a reason for hiding this comment

danieldegrasse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmahadevan108 commented Sep 21, 2023

MaureenHelm commented Oct 26, 2023

danieldegrasse commented Oct 30, 2023

GabrielHAFs commented Oct 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 16, 2023

kartben Nov 17, 2023 • edited

Choose a reason for hiding this comment

danieldegrasse Nov 17, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kartben Nov 17, 2023 •

edited

danieldegrasse Nov 17, 2023 •

edited