targets:MIMXRT1050: Add QSPI flash support #12711
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@liugang-gavin, thank you for your changes. |
|
@liugang-gavin I think as a target update only this comes under patch not feature type? Could you please update. |
adbridge
previously requested changes
Mar 31, 2020
| @@ -271,11 +272,245 @@ status_t flexspi_nor_flash_page_program_ram(uint32_t address, const uint32_t *sr | |||
| return status; | |||
| } | |||
|
|
|||
| #else | |||
| AT_QUICKACCESS_SECTION_CODE(status_t flexspi_nor_enable_quad_mode_ram(void)); | |||
| status_t flexspi_nor_enable_quad_mode_ram(void) | |||
There was a problem hiding this comment.
Is this an internal only api or are you adding something exposed to the user ?
| flashXfer.dataSize = 1; | ||
|
|
||
| status = FLEXSPI_TransferBlocking(FLEXSPI, &flashXfer); | ||
| if (status != kStatus_Success) |
| /* Write enable */ | ||
| status = flexspi_nor_write_enable_ram(0); | ||
|
|
||
| if (status != kStatus_Success) |
| return status; | ||
| } | ||
|
|
||
| void flexspi_update_lut_ram(void) |
There was a problem hiding this comment.
Again , internal only api or externally exposed ?
| flashXfer.data = &readValue; | ||
| flashXfer.dataSize = 1; | ||
|
|
||
| do |
There was a problem hiding this comment.
This whole block is incorrectly formatted. Please check coding guidelines and / or run through astyle
|
|
||
| status = FLEXSPI_TransferBlocking(FLEXSPI, &flashXfer); | ||
|
|
||
| if (status != kStatus_Success) |
| flashXfer.seqIndex = NOR_CMD_LUT_SEQ_IDX_ERASESECTOR; | ||
| status = FLEXSPI_TransferBlocking(FLEXSPI, &flashXfer); | ||
|
|
||
| if (status != kStatus_Success) |
| flashXfer.deviceAddress = address + offset; | ||
| flashXfer.port = kFLEXSPI_PortA1; | ||
| flashXfer.cmdType = kFLEXSPI_Write; | ||
| flashXfer.SeqNumber = 1; |
There was a problem hiding this comment.
Indentation seems to have gone wrong here
| #else | ||
| const flexspi_nor_config_t qspiflash_config = { | ||
| .memConfig = | ||
| { |
| .serialClkFreq = kFlexSpiSerialClk_133MHz, | ||
| .lutCustomSeqEnable = 0u, | ||
| .sflashA1Size = 0x00800000u, /* 8MB/64Mbit */ | ||
| .lookupTable = |
|
thanks for your review, I'm working for the second version based on your comments. |
NXP MIMXRT1050 EVK can support Hyper Flash or QSPI Flash with small hardware reworks. Modify the XIP file to support boot from the two kinds of Flash device. The Hyper Flash should be the default device and defined in tartgets.json with the macro "HYPERFLASH_BOOT". To select the QSPI Flash, just remove the macro with the below line in any overriding json file. "target.macros_remove" : ["HYPERFLASH_BOOT"] Signed-off-by: Gavin Liu <[email protected]>
The flash access may fail when implementing flash initialization. So there is risk for interrupt handler which linked in flash space. Add the critical section to avoid the risk. Signed-off-by: Gavin Liu <[email protected]>
The memset function from c library will be linked in flash space, it's risk for FLASHIAP. So I wrote flexspi_memset to replace the memset for IMX FLASHIAP, and put the function into targets/.../TARGET_IMX/flash_api.c file. All IMX Soc platforms can declare it as extern and use in their Soc flexspi driver files. Signed-off-by: Gavin Liu <[email protected]>
Update the flash driver to support both Hyper Flash and QSPI Flash. In addition, the static function cannot be linked to SRAM even defined by AT_QUICKACCESS_SECTION_CODE macro. So remove all "static" modifier for the FLASHIAP functions. Signed-off-by: Gavin Liu <[email protected]>
For the application(firmware) booted by bootloader(OTA), the image doesn't need the "flash_config" and "ivt" header. So update the link file to support both kinds of application (firmware) booted by bootROM and bootloader. In default, the compilation will get the image with "flash_config" and "ivt" header, for example the bootloader compiling. When compiling the OTA application image, please add the line as below in the mbed_app.json file. "target.macros_add" : ["MBED_APP_COMPILE"] This will remove the "flash_config" and "ivt" header in the final image. Signed-off-by: Gavin Liu <[email protected]>
|
hi， adbridge and reviewer, I updated the PR with the second version patches, it includes new patches to fix some potential issues. And also I checked the code with astyle.
Thanks! |
0xc0170
approved these changes
Apr 3, 2020
|
HI @liugang-gavin we don't have anything to do with the Pelion example I'm afraid. |
Merged
|
CI restarted |
Test run: SUCCESSSummary: 6 of 6 test jobs passed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Impact of changes
Migration actions required
Documentation
None
Pull request type
Test results
Reviewers
[email protected]