Feature/enable zephyr integration #37

mcuxsusan · 2021-08-31T08:48:10Z

Prerequisites

I have checked latest main branch and the issue still exists.
I did not see it is stated as known-issue in release notes.
No similar GitHub issue is related to this change.
My code follows the commit guidelines of this project.
I have performed a self-review of my own code.
My changes generate no new warnings.
I have added tests that prove my fix is effective or that my feature works.

Describe the pull request

This pull request aims to enable zephyr hal nxp support using current mcux-sdk. Below integration has been done:

Cmake build system integration: reference existing CMakeLists.txt of hal-nxp, include driver component according to the Kconfig setting in zephyr.
Kconfig integration: For menuconfig item Modules->hal_nxp, menu MCUX SDK is listed when select to Use MCUX SDK, all available configuration for soc/drivers/components/middleware/utility are listed.
simple build test of hello_world has been executed for all RT1xxx boards, RT6xx boards, lpcxpresso54114 and lpcxpresso55s69, shows now build issue.

Fixes # (issue)

Type of change (please delete options that are not relevant):

Bug fix (non-breaking change which fixes an issue)
[*] New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Tests

Test configuration (please complete the following information):
- Hardware setting:
- Toolchain:
- Test Tool preparation:
- Any other dependencies:
Test executed
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
- [*] Build Test
- Run Test

github-actions · 2021-08-31T08:48:27Z

We do not accept contributions to files under CMSIS, these files are sourced from https://github.com/ARM-software/CMSIS_5, please contribute to the upstream directly.

hakehuang · 2021-08-31T09:40:00Z

this is a huge change, do you have any documnets on those changes? @mcuxsusan

hakehuang · 2021-08-31T09:44:24Z

drivers/lpsci/fsl_lpsci.c

@@ -0,0 +1,1074 @@
+/*
+ * Copyright (c) 2015-2016, Freescale Semiconductor, Inc.


license correct?

I reused the lpsci driver in current hal_nxp module because the KL25 device which has lpsci driver, has been dropped from SDK release for a long time, so I simply used the verified lpsci driver in hal_nxp.

hakehuang

There are many Zephyr unrealted changes, can you split the PR into serval meanful ones, and let zephyr team review only zephyr related?

hakehuang · 2021-08-31T09:56:19Z

CMSIS/kconfig.CMSIS

@@ -0,0 +1,89 @@
+# Copyright 2021 NXP


zephyr does not use this CMSIS. so this shall be in another PR

In the Kconfig framework for MCUXpresso SDK, kconfig.xx is prepared in each category to describe the components in this directory only. Currently all the components are included in the configurable items of zephyr module hal_nxp according to the zephyr/Kconfig description, it's OK to remove some configuration items which are not used in zephyr. Currently the below relative kconfig.xx source are included in the kconfig file, please suggest which are not needed.

rsource "./devices/kconfig.socs"

rsource "./drivers/kconfig.drivers"

rsource "./CMSIS/kconfig.CMSIS"

rsource "./components/kconfig.components"

rsource "./devices/kconfig.devices"

rsource "./utilities/kconfig.utilities"

rsource "./middleware/kconfig.middleware"

above list are all needed. but not all middleware is included

since we are not quite clear about the scenario for providing kconfig item up to zephyr, I will hold this kconfig commit and not put this into zephyr integration for now.

I'll note that the case for the name 'kconfig' doesn't follow what I believe to be the standard practice to use 'Kconfig'

hakehuang · 2021-08-31T10:00:17Z

devices/LPC54628/all_lib_device_LPC54628.cmake

-#    include(driver_flexcomm_usart_freertos)
-#    include(component_wm8904_adapter)
-#    include(driver_reset)
+mcux_include_as_kconfig(driver_ctimer)


put header file for each driver in a dedicated folder will make the the compiler seaching each folder again and again, which is not a good practise.

mcuxsusan · 2021-08-31T11:38:11Z

There are many Zephyr unrealted changes, can you split the PR into serval meanful ones, and let zephyr team review only zephyr related?

Sorry for the inconvenience, I will split the changes into separate commits.

hakehuang · 2021-09-08T07:32:45Z

zephyr/Kconfig

+	  This option enables the use of NXP MCUXpresso SDK
+
+if USE_MCUX_SDK
+rsource "../Kconfig"


does Zephyr need use this? if not used by zephyr, need protect with another maco

Thanks for highlighting this, I will hold this kconfig change and not put this into zephyr integration for now.

hakehuang · 2021-09-08T07:34:55Z

zephyr/hal_nxp.cmake

+    ${CMAKE_CURRENT_LIST_DIR}/../devices/${MCUX_DEVICE}
+    ${CMAKE_CURRENT_LIST_DIR}/../devices/${MCUX_DEVICE}/drivers
+    ${CMAKE_CURRENT_LIST_DIR}/../drivers/common
+    ${CMAKE_CURRENT_LIST_DIR}/../CMSIS/Core/Include


remove this line

hakehuang · 2021-09-08T07:43:00Z

zephyr/hal_nxp.cmake

+endif()
+
+#include shared drivers
+include_driver_ifdef(CONFIG_ADC_MCUX_LPADC		lpadc	driver_lpadc)


each driver in one folder, why do this way?

@hakehuang, I suspect this is because of how we maintain the drivers in the internal repository. We flattened this in the Zephyr NXP HAL but it is probably easier for keeping the external and internal repositories synchronized.

mmahadevan108

Could the below commit go into mcux-sdk repo as a separate PR. This way we can have this in the repo without waiting for the Zephyr integration to complete.

zephyr: Fill the device and driver support gap in mcux-sdk
 - Add missing device support for K8x, KE18F, KL25Z, KW2x, KW3x and
   KW4x. Reuse files in hal_nxp.
 - Add lpsci driver which is needed for KL25Z.

Signed-off-by: Susan Su <susan.su@nxp.com>

mcuxsusan · 2021-10-09T08:25:43Z

Could the below commit go into mcux-sdk repo as a separate PR. This way we can have this in the repo without waiting for the Zephyr integration to complete.
zephyr: Fill the device and driver support gap in mcux-sdk
 - Add missing device support for K8x, KE18F, KL25Z, KW2x, KW3x and
   KW4x. Reuse files in hal_nxp.
 - Add lpsci driver which is needed for KL25Z.

Signed-off-by: Susan Su <susan.su@nxp.com>

OK, will create another PR for this.

dleach02 · 2021-10-11T04:05:07Z

components/kconfig.components

@@ -0,0 +1,457 @@
+# Copyright 2021 NXP
+# SPDX-License-Identifier: BSD-3-Clause


'Kconfig.components'

Hi David, thanks for the review comments, I have updated this PR to only include only the change needed for zephyr build. For the module kconfig framework, I created another PR in #42, and in this zephyr integration PR currently I am not going to add kconfig items for this module. We need to discuss more on the requirement for the kconfig items for module hal_nxp.

- For device, board support and mcux drivers, change to use that in mcux-sdk offerring. - For middleware, imx drivers and the component drivers, keep using existing enablement in hal-nxp. Signed-off-by: Susan Su <susan.su@nxp.com>

mcuxsusan · 2021-10-11T09:30:10Z

Hi @dleach02, @hakehuang, @mmahadevan108,

Hope you had a nice break.
It’s very exciting to me that you have started reviewing my PR in mcux-sdk repo to support zephyr integration. This comment is to inform you that I have force pushed the feature branch for below update:

Focus the review on cmake build support for zephyr integration, remove the commit change for kconfig framework and missing device support. The kconfig framework is now in separate review Kconfig: Create Kconfig framework for mcux-sdk #42 and PR for adding device please see zephyr: Fill the device and driver support gap in mcux-sdk #41.
For the cmake build support:
- Use mcux-sdk board/device support and drivers to replace previous boards, devices, drivers enablement in hal_nxp/mcux folder.
- Copy the existing enablement for imx drivers in hal_nxp/imx, middleware and the component drivers in hal_nxp/mcux/components, hal_nxp/mcux/middleware to the mcux-sdk/zephyr folder, reuse these support.
- Exclude the dts folder according to feedback from @mmahadevan108 .

hakehuang · 2021-10-19T06:08:35Z

zephyr/components/mcr20a/MCR20Overwrites.h

+};
+
+
+/* begin of deprecated versions


why we keep deprcated version?

This is sourced from the hal_nxp module. https://github.com/zephyrproject-rtos/hal_nxp/blob/master/mcux/components/mcr20a/MCR20Overwrites.h, I agree with your suggestion to put this file to zephyr project.

What is the latest version of this driver called?

Build the status reset controller driver if hwinfo is enabled

add rtadc zephyr driver compiling Signed-off-by: Andrei Ovchinnikov<ovchinnikov@strim-tech.com>

Enabled GPT driver in CMakeLists.txt when the GPT driver is used as a timer within Zephyr Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

mcuxsusan · 2021-10-19T07:59:35Z

I also cherry-picked the commits in hal_nxp master for adding the needed driver/component to cmake build system.
For other change of commits not included, listed them below:

RT685: Update CTimer Clock API
Adds MK66FN2 part device tree description
mcux: devices: mke1xf: fix CAN_Type number of mailboxes

For these bugs will be firstly fixed in our internal develop repo, then deploy to the mcux-sdk, this way ensure the bug is fixed in the source. For the dts change, in current integration we did not include that part so the commit will be dropped.

dleach02 · 2021-10-19T20:58:33Z

I also cherry-picked the commits in hal_nxp master for adding the needed driver/component to cmake build system. For other change of commits not included, listed them below:

RT685: Update CTimer Clock API

Adds MK66FN2 part device tree description

mcux: devices: mke1xf: fix CAN_Type number of mailboxes

For these bugs will be firstly fixed in our internal develop repo, then deploy to the mcux-sdk, this way ensure the bug is fixed in the source. For the dts change, in current integration we did not include that part so the commit will be dropped.

for the fix to the MKE1xF CAN_TYPE number of mailboxes, I have an internal jira ticket for this... since this file is generated, it seems like we should understand how this is wrong.

mcuxsusan · 2021-12-14T05:55:32Z

Closed the PR because we have aligned to keep using hal_nxp to provide NXP HAL support for zephyr. In the approach mcux-sdk is integrated to the existing hal_nxp as subtree to leverage the MCUXpresso SDK driver and device support. The PR to switch Zephyr HAL to use mcux-sdk in hal_nxp has been merged.

mcuxsusan requested review from hakehuang, MaureenHelm and dleach02 August 31, 2021 08:48

mcuxsusan self-assigned this Aug 31, 2021

github-actions bot closed this Aug 31, 2021

mcuxsusan reopened this Aug 31, 2021

hakehuang reviewed Aug 31, 2021

View reviewed changes

hakehuang suggested changes Aug 31, 2021

View reviewed changes

mcuxsusan force-pushed the feature/enable-zephyr-integration branch from b7e0ad4 to c5ac7e5 Compare August 31, 2021 13:30

hakehuang reviewed Sep 8, 2021

View reviewed changes

mmahadevan108 suggested changes Oct 8, 2021

View reviewed changes

dleach02 reviewed Oct 11, 2021

View reviewed changes

zephyr: Enable NXP HAL support in mcux-sdk

cdea0b5

- For device, board support and mcux drivers, change to use that in mcux-sdk offerring. - For middleware, imx drivers and the component drivers, keep using existing enablement in hal-nxp. Signed-off-by: Susan Su <susan.su@nxp.com>

mcuxsusan force-pushed the feature/enable-zephyr-integration branch from c5ac7e5 to cdea0b5 Compare October 11, 2021 08:56

mcuxsusan mentioned this pull request Oct 19, 2021

zephyr: Fill the device and driver support gap in mcux-sdk #41

Merged

12 tasks

hakehuang reviewed Oct 19, 2021

View reviewed changes

pdgendt and others added 3 commits October 19, 2021 15:35

mcux: Build SRC driver

195ba7f

Build the status reset controller driver if hwinfo is enabled

mcux: add imx adc driver compiling

a3a7be5

add rtadc zephyr driver compiling Signed-off-by: Andrei Ovchinnikov<ovchinnikov@strim-tech.com>

modules: mcux: Enabled GPT driver when GPT Timer KConfig is selected

7b98fcc

Enabled GPT driver in CMakeLists.txt when the GPT driver is used as a timer within Zephyr Signed-off-by: Daniel DeGrasse <daniel.degrasse@nxp.com>

mcuxsusan closed this Dec 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/enable zephyr integration #37

Feature/enable zephyr integration #37

mcuxsusan commented Aug 31, 2021

github-actions bot commented Aug 31, 2021

hakehuang commented Aug 31, 2021

hakehuang Aug 31, 2021

mcuxsusan Aug 31, 2021

hakehuang left a comment

hakehuang Aug 31, 2021

mcuxsusan Aug 31, 2021

hakehuang Sep 1, 2021

mcuxsusan Oct 9, 2021

dleach02 Oct 11, 2021

hakehuang Aug 31, 2021

mcuxsusan commented Aug 31, 2021

hakehuang Sep 8, 2021 •

edited

Loading

mcuxsusan Oct 9, 2021

hakehuang Sep 8, 2021

hakehuang Sep 8, 2021

dleach02 Oct 19, 2021

mmahadevan108 left a comment

mcuxsusan commented Oct 9, 2021

dleach02 Oct 11, 2021

mcuxsusan Oct 11, 2021

mcuxsusan commented Oct 11, 2021

hakehuang Oct 19, 2021

mcuxsusan Oct 19, 2021

mmahadevan108 Oct 19, 2021

mcuxsusan commented Oct 19, 2021 •

edited

Loading

dleach02 commented Oct 19, 2021

mcuxsusan commented Dec 14, 2021

		@@ -0,0 +1,1074 @@
		/*
		* Copyright (c) 2015-2016, Freescale Semiconductor, Inc.

		@@ -0,0 +1,457 @@
		# Copyright 2021 NXP
		# SPDX-License-Identifier: BSD-3-Clause

Feature/enable zephyr integration #37

Feature/enable zephyr integration #37

Conversation

mcuxsusan commented Aug 31, 2021

github-actions bot commented Aug 31, 2021

hakehuang commented Aug 31, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hakehuang 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

mcuxsusan commented Aug 31, 2021

hakehuang Sep 8, 2021 • 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

mmahadevan108 left a comment

Choose a reason for hiding this comment

mcuxsusan commented Oct 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcuxsusan commented Oct 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcuxsusan commented Oct 19, 2021 • edited Loading

dleach02 commented Oct 19, 2021

mcuxsusan commented Dec 14, 2021

hakehuang Sep 8, 2021 •

edited

Loading

mcuxsusan commented Oct 19, 2021 •

edited

Loading