arch/arm/soc/nxp_imx: added support to imx7d m4 core #5330

uLipe · 2017-12-10T21:44:16Z

board/colibri_imx7d_m4: added board support to toradex colibri imx7d single board computer

This change basically adds support of a popular IMX7D processors from NXP, which has a ARM Cortex A7 Core and a Cortex M4 Core, this change is targeted to the second one and now is possible to select it as a supported SoC, Toradex has some attractive single board computers with industrial grade using this processor and this change also the basic support of the board and the NXP supplied HAL.

This support is intended to increase the options of heterogeneous processors in Zephyr SoC portfolio, adds the SoC and Board template is the first step only, so I divided the change in two steps, first provide a functional SoC support to perform build and image generation to be flashed to TCMRAM (imx7d m4 core does not has built in flash), the next step is to provide the serial drivers and the IPM driver to communicate m4 core with a7 core, this approach was chosen to keep the changes not too large and hard to verify.

This changed was validated as follows:

built the hello world sample under build folder with: $cmake -DBOARD=colibri_imx7d_m4 .. and make.
the generated image can be loaded through a Jlink probe or with A7 running Linux + uBoot using the fatload command

Signed-off-by: Felipe Neves [email protected]

board/colibri_imx7d_m4: added board support to toradex colibri imx7d single board computer This change basically adds support of a popular IMX7D processors from NXP, which has a ARM Cortex A7 Core and a Cortex M4 Core, this change is targeted to the second one and now is possible to select it as a supported SoC, Toradex has some attractive single board computers with industrial grade using this processor and this change also the basic support of the board and the NXP supplied HAL. This support is intended to increase the options of heterogeneous processors in Zephyr SoC portfolio, adds the SoC and Board template is the first step only, so I divided the change in two steps, first provide a functional SoC support to perform build and image generation to be flashed to TCMRAM (imx7d m4 core does not has built in flash), the next step is to provide the serial drivers and the IPM driver to communicate m4 core with a7 core, this approach was chosen to keep the changes not too large and hard to verify. This changed was validated as follows: - built the hello world sample under build folder with: $cmake -DBOARD=colibri_imx7d_m4 .. and make. - the generated image can be loaded through a Jlink probe or with A7 running Linux + uBoot using the fatload command Signed-off-by: Felipe Neves <[email protected]>

galak

Can you split this into at least 2 patches, one for the soc code and one for the board.

uLipe · 2017-12-11T19:05:58Z

Hi, yes I can.

In this case I will modify this patch and the next one include board support.

Thanks

pfalcon · 2017-12-11T19:16:50Z

@uLipe : Note that you can just make this pull request contain 2 commits (no need to create 2 pull requests for that).

uLipe · 2017-12-11T19:20:20Z

@pfalcon, thanks for the tip, I will submit the just one PR containing the soc and board commits.

Best :)

MaureenHelm

Hi @uLipe , thanks for the PR! This is something we have wanted to do for some time.

As others have already said, please split this up into multiple commits as it will help with the review process and keep the git history clean. It's ok to force push your branch to update the PR. I think you should have three commits, rather than two:

Import mcux hal for imx7d. Please reuse the existing ext/hal/nxp/mcux folder and configs rather than creating a new folder and configs.
Add imx7d SoC support
Add colibri board support. Please also add a board doc to this commit, refer to boards/arm/mimxrt1050_evk/doc/mimxrt1050_evk.rst as an example.

cc: @MarekNovakNXP

MaureenHelm · 2017-12-11T20:44:28Z

boards/arm/colibri_imx7d_m4/Kconfig.defconfig

+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.


Replace with SPDX header as in other files

MaureenHelm · 2017-12-11T20:53:44Z

boards/arm/colibri_imx7d_m4/board.c

+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.


In general, we need to keep MCUXpresso SDK files in ext/ only. For this particular case, I think it makes more sense to reimplement the board pinmuxing; see boards/mimxrt1050_evk/pinmux.c for an example.

MaureenHelm · 2017-12-11T20:55:04Z

boards/arm/colibri_imx7d_m4/board.h

+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.


Replace with SPDX Apache header

MaureenHelm · 2017-12-11T20:58:51Z

boards/arm/colibri_imx7d_m4/colibri_imx7d_m4_defconfig

+CONFIG_CONSOLE=y
+CONFIG_CONSOLE_HAS_DRIVER=y
+CONFIG_UART_CONSOLE=y
+CONFIG_UART_CONSOLE_ON_DEV_NAME="UART_2"


Remove this line, the uart dev comes from the board dts

MaureenHelm · 2017-12-11T21:01:38Z

boards/arm/colibri_imx7d_m4/colibri_imx7d_m4_defconfig

+CONFIG_SOC_MCIMX7_M4=y
+CONFIG_BOARD_COLIBRI_IMX7D_M4=y
+CONFIG_CORTEX_M_SYSTICK=y
+CONFIG_UART_NXP_IMX=y


Remove this line, we can select the appropriate uart driver in the SoC defconfig. See arch/arm/soc/nxp_imx/rt/Kconfig.defconfig.mimxrt1052 for an example.

MaureenHelm · 2017-12-11T21:08:10Z

dts/arm/nxp/nxp_imx.dtsi

+
+	soc {
+
+		tcram: tcram@1fff8000 {


Let's call this tcml to match the hardware reference manual, and add tcmu.

MaureenHelm · 2017-12-11T21:10:27Z

dts/arm/nxp/nxp_imx.dtsi

+		ddram: ddram@10000000 {
+			compatible = "nxp,imx-ddram";
+			reg = <0x10000000 0x10000000>;
+			label = "CCM";


MaureenHelm · 2017-12-11T21:14:48Z

dts/arm/nxp/nxp_imx.dtsi

+
+		uart2: uart@30890000 {
+			compatible = "nxp,imx-uart";
+			reg = <0x30890000 0x4000>;


Base address should be 0x30870000

MaureenHelm · 2017-12-11T21:17:23Z

ext/hal/Kconfig

@@ -20,6 +20,8 @@ source "ext/hal/nordic/Kconfig"

 source "ext/hal/nxp/mcux/Kconfig"

+source "ext/hal/nxp/imx/Kconfig"


Merge contents into existing ext/hal/nxp/mcux

uLipe · 2017-12-12T01:08:15Z

@MaureenHelm , thanks for the tips I started to work in these changes, let me ask a question about the mcux hal and imx hal, I noted the nxp sdk for the IMX7 microcontroller side seems to not compatible with the new mcuxpresso drivers (the files are quite different and the IPs too), more than that currently the MCUX dashboard does not support the imx7d processors, the duplication of the hal was based on this incompatibility, but I agree with you to keep only the mcux hal.

Given this scenario the question is: Can I move the drivers source files of IMX7 directly to the mcuexpresso drivers/ directory? or should I create a second directory called for example: drivers_imx7d ?

MaureenHelm · 2017-12-12T14:38:47Z

I noted the nxp sdk for the IMX7 microcontroller side seems to not compatible with the new mcuxpresso drivers (the files are quite different and the IPs too), more than that currently the MCUX dashboard does not support the imx7d processors, the duplication of the hal was based on this incompatibility, but I agree with you to keep only the mcux hal.

My mistake, I thought we had imx7d m4 support in mcux 2.x but it turns out the drivers and headers are based on ksdk 1.x. Let me check with the internal development team to see if there are any plans to update it, and then we can decide how to proceed.

galak · 2017-12-13T17:01:19Z

@MaureenHelm curious if the imx7d and imx6 code would be similar (for those imx6 devices with cortex-m as well).

MaureenHelm · 2017-12-13T22:04:30Z

@MaureenHelm curious if the imx7d and imx6 code would be similar (for those imx6 devices with cortex-m as well).

For now, yes. Existing imx6 and imx7 devices have similar ksdk 1.x based m4 drivers, but future devices in these families may have mcux 2.x.

My mistake, I thought we had imx7d m4 support in mcux 2.x but it turns out the drivers and headers are based on ksdk 1.x. Let me check with the internal development team to see if there are any plans to update it, and then we can decide how to proceed.

There are no plans to update this device to mcux 2.x, so I now agree with your original decision to create a new hal folder. I'm not sure I like "ext/hal/nxp/imx" and CONFIG_HAS_IMX_HAL though, as I think that will create confusion for imx devices that do have mcux. NXP releases the code in a "FreeRTOS BSP", which is based on ksdk 1.x, so some possible alternatives are:
ext/hal/nxp/freertos_bsp
ext/hal/nxp/ksdk

Thoughts?

galak · 2017-12-13T22:50:14Z

.gitignore

@@ -7,6 +7,7 @@
 *.swp
 *.swo
 *~
+*.DS_Store


I'm not really sure we should be adding this, and if it should be in a separate PR/commit.

galak

Remove the .bin.inc files that are checked in.

galak · 2017-12-13T22:51:43Z

samples/net/sockets/dumb_http_server/src/response_big.html.bin.inc

@@ -0,0 +1,273 @@
+0x48, 0x54, 0x54, 0x50, 0x2f, 0x31, 0x2e, 0x30,


This file should not be check in.

uLipe · 2017-12-13T22:52:57Z

Hi,

@MaureenHelm, thanks for the support, I agree the imx named folder in hal could cause confusion, ksdk seems confusing too since it refers to the old kinetis sdk.

freertos_bsp is also too generic but since it will contained in nxp folder, this can be a solution.

Should i proceed with hal/nxp/freertos_sdk(or bsp)?

Thanks again :)

galak · 2017-12-13T23:01:39Z

There are no plans to update this device to mcux 2.x, so I now agree with your original decision to create a new hal folder. I'm not sure I like "ext/hal/nxp/imx" and CONFIG_HAS_IMX_HAL though, as I think that will create confusion for imx devices that do have mcux. NXP releases the code in a "FreeRTOS BSP", which is based on ksdk 1.x, so some possible alternatives are:
ext/hal/nxp/freertos_bsp
ext/hal/nxp/ksdk

Thoughts?

Do you expect that something like i.mx8 would be supported by mcux?

I wondering if we should just have native drivers. Wondering how many peripheral drivers would actually be used/supported? @uLipe which devices do you see using from the Cortex-M4 on i.mx7d?

MaureenHelm · 2017-12-14T14:16:18Z

Do you expect that something like i.mx8 would be supported by mcux?

Yes

I wondering if we should just have native drivers.

Why? We still need the device headers, so doing native drivers doesn't eliminate the problem of how to import the nxp freertos bsp.

Should i proceed with hal/nxp/freertos_sdk(or bsp)

I think we should go with freertos_bsp. It's a little weird, but it's closest to the package name from nxp. @galak, do you agree?

galak · 2017-12-14T16:12:41Z

I think we should go with freertos_bsp. It's a little weird, but it's closest to the package name from nxp. @galak, do you agree?

Uugh, why'd you guys have to use freertos in the bsp name ;). Could we do something like ext/hal/nxp/imx_legacy/ or 'imx_6_7' or something like that to convey the SoCs we are supporting?

MaureenHelm · 2017-12-15T22:13:49Z

Uugh, why'd you guys have to use freertos in the bsp name ;). Could we do something like ext/hal/nxp/imx_legacy/ or 'imx_6_7' or something like that to convey the SoCs we are supporting?

No, there may be new SoCs in these families that have mcux. I don't like having freertos in the name either, but I still think keeping the folder name close to the nxp package name makes the origin clear.

galak · 2018-01-10T16:30:26Z

@uLipe - any feedback to my question?

uLipe · 2018-01-11T17:24:31Z

Hello all, sorry for long delayed response (I was travelling with very limited internet access).

Regarding the question, @galak , I don't think the mention of freertos is the more correct way, but based on sdk delivered by NXP and if the goal is to make the external resources on hal folder compliant with the chip manufacturer, the freertos_bsp is the better way to naming,using imx_legacy is not good at all too because IMX6 and 7 families as well their sdk are not obsolete, discontinued or even legacy, is just not compatible with the current mcux sdk.

I used the name as just "imx" because it sounds a mid-term but at cost of some confusion risk by the user, @MaureenHelm can I keep the naming of the original PR? I think, because mcux incompatibility by the IMX7 SoCs, the naming solution chosen will aways have a drawback.

I will resume the PR task at this weekend.

Thanks again by the support.

Felipe

galak · 2018-01-31T15:07:56Z

@uLipe any chance you'll get back to this?

uLipe · 2018-02-02T03:20:57Z

Hello @galak , I will suffering some busy times here and due this I will not be able to continue it, my colleague @diegosueiro will take this task from ahead.

Thanks for the support

MaureenHelm · 2018-02-02T16:34:19Z

I used the name as just "imx" because it sounds a mid-term but at cost of some confusion risk by the user, @MaureenHelm can I keep the naming of the original PR? I think, because mcux incompatibility by the IMX7 SoCs, the naming solution chosen will aways have a drawback.

Yes

I will suffering some busy times here and due this I will not be able to continue it, my colleague @diegosueiro will take this task from ahead.

Understand. Thanks @diegosueiro for picking this up.

diegosueiro · 2018-02-03T07:22:18Z

@MaureenHelm,

It will be a pleasure to continue this PR.
Actually, I'm the original author of this porting and @uLipe helped me to adapt it to use cmake (when I started Zephyr was based on KBuild), among other things and created the PR.

I'm just wondering if it is better to create a new PR applying the changes suggested by you and @galak.

I'm just confused regarding the naming the hal directory. What should I use? "ext/hal/nxp/imx"?

MaureenHelm · 2018-02-05T17:41:50Z

I'm just wondering if it is better to create a new PR applying the changes suggested by you and @galak.

Either way is fine. We usually do force pushes to update a PR, but in this case you would have to force push to @uLipe 's repo. If you do create a new PR, please add a reference to this one in the description.

I'm just confused regarding the naming the hal directory. What should I use? "ext/hal/nxp/imx"?

Yes, sorry for the confusion. I don't really like this name or any of the others we kicked around, but I don't have any better suggestions. Please also have a look at http://docs.zephyrproject.org/contribute/contribute_non-apache.html

diegosueiro · 2018-02-06T07:31:11Z

@MaureenHelm,

Either way is fine. We usually do force pushes to update a PR, but in this case you would have to force push to @uLipe 's repo. If you do create a new PR, please add a reference to this one in the description.

Got it. I'll create a new PR.

Please also have a look at http://docs.zephyrproject.org/contribute/contribute_non-apache.html

The iMX7D drivers and utilities were taken from NXP FreeRTOS BSP and according to the file "SW-Content-Register-FreeRTOS-BSP-1.0.1-i.MX7D.txt" they are released under BSD-3-Clause.

In this case, should I need to make the submission for TSC review?

MaureenHelm · 2018-02-06T21:57:39Z

In this case, should I need to make the submission for TSC review?

Please add a README to your PR, and I'll submit it to the TSC/gov board for review.

diegosueiro · 2018-02-25T18:34:31Z

@MaureenHelm,

Just to let you know that I just submitted the PR #6367

MaureenHelm · 2018-02-26T16:57:22Z

Just to let you know that I just submitted the PR #6367

Thanks!

uLipe requested review from nashif and SebastianBoe as code owners December 10, 2017 21:44

SebastianBoe removed their request for review December 11, 2017 09:21

galak requested review from MaureenHelm and galak December 11, 2017 15:09

galak added area: ARM ARM (32-bit) Architecture area: Boards platform: NXP NXP labels Dec 11, 2017

galak requested changes Dec 11, 2017

View reviewed changes

MaureenHelm requested changes Dec 11, 2017

View reviewed changes

galak self-assigned this Dec 12, 2017

galak reviewed Dec 13, 2017

View reviewed changes

galak requested changes Dec 13, 2017

View reviewed changes

diegosueiro mentioned this pull request Feb 25, 2018

arch: Add support to imx7d m4 core and colibri_imx7d_m4 board #6367

Merged

MaureenHelm closed this Feb 26, 2018

		@@ -20,6 +20,8 @@ source "ext/hal/nordic/Kconfig"

		source "ext/hal/nxp/mcux/Kconfig"

		source "ext/hal/nxp/imx/Kconfig"

		@@ -0,0 +1,273 @@
		0x48, 0x54, 0x54, 0x50, 0x2f, 0x31, 0x2e, 0x30,

arch/arm/soc/nxp_imx: added support to imx7d m4 core #5330

arch/arm/soc/nxp_imx: added support to imx7d m4 core #5330

Conversation

uLipe commented Dec 10, 2017

galak left a comment

Choose a reason for hiding this comment

uLipe commented Dec 11, 2017

pfalcon commented Dec 11, 2017

uLipe commented Dec 11, 2017

MaureenHelm left a comment • edited Loading

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

uLipe commented Dec 12, 2017

MaureenHelm commented Dec 12, 2017

galak commented Dec 13, 2017

MaureenHelm commented Dec 13, 2017

Choose a reason for hiding this comment

galak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uLipe commented Dec 13, 2017

galak commented Dec 13, 2017

MaureenHelm commented Dec 14, 2017

galak commented Dec 14, 2017 • edited Loading

MaureenHelm commented Dec 15, 2017

galak commented Jan 10, 2018

uLipe commented Jan 11, 2018

galak commented Jan 31, 2018

uLipe commented Feb 2, 2018

MaureenHelm commented Feb 2, 2018

diegosueiro commented Feb 3, 2018

MaureenHelm commented Feb 5, 2018

diegosueiro commented Feb 6, 2018

MaureenHelm commented Feb 6, 2018

diegosueiro commented Feb 25, 2018

MaureenHelm commented Feb 26, 2018

MaureenHelm left a comment •

edited

Loading

galak commented Dec 14, 2017 •

edited

Loading