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

Enable imxrt1010 #19335

Merged
merged 5 commits into from
Jan 9, 2020
Merged

Conversation

jhqian
Copy link
Collaborator

@jhqian jhqian commented Sep 24, 2019

Add board support for mimxrt1010_evk

  • Add board specific pinmux, doc and dts.
  • Tested with hello_world, synchronization, philosophers, basic/blinky and basic/button. Since no public support from Segger yet, ITCM is set for default code location.
  • Updated GPIO driver to fit RT1010 on which only 1 IRQ is available for GPIO2 and GPIO5

@zephyrbot
Copy link
Collaborator

zephyrbot commented Sep 24, 2019

All checks are passing now.

checkpatch (informational only, not a failure)

-:321: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "nxp,mimxrt1010" appears un-documented -- check ./dts/bindings/
#321: FILE: boards/arm/mimxrt1010_evk/mimxrt1010_evk.dts:13:
+	compatible = "nxp,mimxrt1010";

-:360: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "adesto,at25sf128a" appears un-documented -- check ./dts/bindings/
#360: FILE: boards/arm/mimxrt1010_evk/mimxrt1010_evk.dts:52:
+		compatible = "adesto,at25sf128a", "jedec,spi-nor";

-:360: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string vendor "adesto" appears un-documented -- check ./dts/bindings/vendor-prefixes.txt
#360: FILE: boards/arm/mimxrt1010_evk/mimxrt1010_evk.dts:52:
+		compatible = "adesto,at25sf128a", "jedec,spi-nor";

-:841: WARNING:LONG_LINE: line over 80 characters
#841: FILE: soc/arm/nxp_imx/rt/soc.c:52:
+#if defined(CONFIG_SOC_MIMXRT1021) || defined(CONFIG_SOC_MIMXRT1015) || defined(CONFIG_SOC_MIMXRT1011)

- total: 0 errors, 4 warnings, 758 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@jhqian jhqian force-pushed the enable_imxrt1010 branch 3 times, most recently from e9f3e54 to 2bea616 Compare September 24, 2019 01:45
Copy link
Member

@MaureenHelm MaureenHelm left a comment

Choose a reason for hiding this comment

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

Thanks @jhqian

Please update west.yml similar to dd783fe to pull the the changes from zephyrproject-rtos/hal_nxp#13. This will help fix the shippable failure.

@@ -263,6 +291,9 @@ config INIT_USB1_PLL
config INIT_VIDEO_PLL
bool "Initialize Video PLL"

config HAS_NO_ARM_DIV
Copy link
Member

Choose a reason for hiding this comment

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

Let's make this positive logic instead. Change to HAS_ARM_DIV, set default y here (since that is the typical case for most socs in this series), then override the default in Kconfig.defconfig.mimxrt1010 only

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, updated as suggested.

@@ -145,7 +146,9 @@ static ALWAYS_INLINE void clkInit(void)
CLOCK_InitVideoPll(&videoPllConfig);
#endif

#ifndef CONFIG_HAS_NO_ARM_DIV
Copy link
Member

Choose a reason for hiding this comment

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

#ifdef CONFIG_HAS_ARM_DIV

@@ -263,6 +290,10 @@ config INIT_USB1_PLL
config INIT_VIDEO_PLL
bool "Initialize Video PLL"

config HAS_ARM_DIV
bool "Has the divider for ARM"
default y
Copy link
Member

Choose a reason for hiding this comment

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

Replace the leading spaces with a tab

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will do.

- gnuarmemb
- xtools
ram: 32
flash: 16384
Copy link
Member

Choose a reason for hiding this comment

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

Make this 32. This should help resolve the shippable failures

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, thanks.

@jhqian jhqian force-pushed the enable_imxrt1010 branch 2 times, most recently from c272c5b to 019fcd6 Compare September 26, 2019 23:15
@jhqian jhqian force-pushed the enable_imxrt1010 branch 2 times, most recently from 289ce24 to e13bc71 Compare October 10, 2019 05:46
@MaureenHelm
Copy link
Member

Please address the shippable and documentation failures

@jhqian
Copy link
Collaborator Author

jhqian commented Nov 14, 2019

Although there's no Segger public support yet, switch to use external flash as code memory rather than tiny TCM.

@mnkp
Copy link
Member

mnkp commented Nov 14, 2019

The merge window for 2.1 is now closed so this PR should target topic-gpio branch where all the boards are converted to use the new API. In case of this PR the required modification is minimal. Only boards/arm/mimxrt1010_evk/mimxrt1010_evk.dts file needs to be updated to use the new GPIO_ACTIVE_LOW flag.

@pabigot
Copy link
Collaborator

pabigot commented Nov 14, 2019

The merge window for 2.1 is now closed so this PR should target topic-gpio branch where all the boards are converted to use the new API

I don't think that's what we agreed. New GPIO drivers must be developed to match the new GPIO API, based on an the expectation that the new API will be merged in the 2.2 window.

We should not be blocking the addition of boards until the new GPIO infrastructure is ready for master, just because they happen to touch a GPIO driver.

@jhqian jhqian force-pushed the enable_imxrt1010 branch 4 times, most recently from 9eaa8ee to e000ac1 Compare November 20, 2019 05:03
@jhqian
Copy link
Collaborator Author

jhqian commented Nov 20, 2019

Anybody can tell me how I can trigger another shippable check? I just updated the module for NXP hal.

Thanks in advance.

@microbuilder
Copy link
Member

It looks like this is passing now. I guess the only think preventing it being merged is the 2.1 release?

board_set_flasher_ifnset(jlink)
endif()

board_runner_args(jlink "--device=cortex-m7")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please check JLink V6.60

  • DLL: Added debug support for NXPx iMXRT1010 family (e.g. MIMXRT1011CAE4A)

@jhqian
Copy link
Collaborator Author

jhqian commented Jan 8, 2020

Is there anything blocking this patch being merged?

Comment on lines 352 to 355
FlexSPI configuration block consists of parameters regarding specific
flash devices including read command sequence, quad mode enablement
sequence (optional), etc. The boot ROM expectes FlexSPI configuration
parameter to be presented in serail 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.

Please adjust the alignment such that it is indented two spaces more than the help line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

revised.

#define DT_MCUX_IGPIO_5_IRQ_0 DT_NXP_IMX_GPIO_400C0000_IRQ_0
#define DT_MCUX_IGPIO_5_IRQ_0_PRI DT_NXP_IMX_GPIO_400C0000_IRQ_0_PRIORITY
#define DT_MCUX_IGPIO_5_IRQ_1 DT_NXP_IMX_GPIO_400C0000_IRQ_1
#define DT_MCUX_IGPIO_5_IRQ_1_PRI DT_NXP_IMX_GPIO_400C0000_IRQ_1_PRIORITY
Copy link
Member

Choose a reason for hiding this comment

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

Why are you adding IGPIO_5 again? These lines appear to be duplicates

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed

- Check presence of GPIO IRQ related macros before enabling IRQ

Signed-off-by: Ryan QIAN <jianghao.qian@nxp.com>
- Add device support for i.MXRT1010

Signed-off-by: Ryan QIAN <jianghao.qian@nxp.com>
Add board support files for mimxrt1010_evk, the development board for
i.MXRT1010 (CM7) SoC.

- Add pinmux, dts and doc.
- Tested samples: hello_world, philosophers, synchronization,
basic/blinky, basic/button.

Signed-off-by: Ryan QIAN <jianghao.qian@nxp.com>
- extend usb device support for mimxrt1010_evk

Signed-off-by: Ryan QIAN <jianghao.qian@nxp.com>
- to get the update hal_nxp from PR #13

Signed-off-by: Ryan QIAN <jianghao.qian@nxp.com>
@MaureenHelm MaureenHelm merged commit b4d774a into zephyrproject-rtos:master Jan 9, 2020
@MaureenHelm MaureenHelm deleted the enable_imxrt1010 branch January 9, 2020 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Boards area: Devicetree area: Kernel area: Modules area: Networking area: Samples Samples area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants