NRF5x: QSPI SFDP read, min read/write implementation #9291

j3hill · 2019-01-08T17:14:30Z

Description

In "targets\TARGET_NORDIC\TARGET_NRF5x\qspi_hal.c", add private helper function to the read QSPI SFDP data during initialization. Add support for Nordic QSPI custom instruction long frame mode in "targets\TARGET_NORDIC\TARGET_NRF5x\TARGET_SDK_14_2\drivers_nrf\qspi\nrf_drv_qspi.c/.h"
Add support for "QSPIF" component to MCU_NRF52840 in targets.json. Add support for "QSPI_MIN_READ_SIZE" and "QSPI_MIN_PROG_SIZE" for specific targets as defined in: "components\storage\blockdevice\COMPONENT_QSPIF\mbed-lib.json." Return these minimum read/write sizes for specific targets in QSPIFBlockDevice.

Pull request type

[X ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

j3hill · 2019-01-08T18:11:20Z

Results from mbed greentea "components-storage-blockdevice-component_qspif-tests-block_device-qspif":

mbedgt: greentea test automation tool ver. 1.4.0
mbedgt: test specification file '.\BUILD\tests\NRF52840_DK\GCC_ARM\test_spec.json' (specified with --test-spec option)
mbedgt: using '.\BUILD\tests\NRF52840_DK\GCC_ARM\test_spec.json' from current directory!
mbedgt: detecting connected mbed-enabled devices...
mbedgt: detected 1 device
mbedgt: processing target 'NRF52840_DK' toolchain 'GCC_ARM' compatible platforms... (note: switch set to --parallel 1)
mbedgt: test case filter (specified with -n option)
components-storage-blockdevice-component_qspif-tests-block_device-qspif
test filtered in 'components-storage-blockdevice-component_qspif-tests-block_device-qspif'
mbedgt: running 1 test for platform 'NRF52840_DK' and toolchain 'GCC_ARM'
mbedgt: mbed-host-test-runner: started
mbedgt: checking for GCOV data...
mbedgt: test on hardware with target id: 1102000044203120484d3943333038313430303197969903
mbedgt: test suite 'components-storage-blockdevice-component_qspif-tests-block_device-qspif' ......... OK in 43.52 sec
test case: 'Testing Multi Threads Erase Program Read' ........................................ OK in 5.95 sec
test case: 'Testing read write random blocks' ................................................ OK in 1.56 sec
test case: 'Testing unaligned erase blocks' .................................................. OK in 0.30 sec
mbedgt: test case summary: 3 passes, 0 failures
mbedgt: all tests finished!
mbedgt: shuffle seed: 0.6854612789
mbedgt: test suite report:
+---------------------+---------------+-------------------------------------------------------------------------+--------+--------------------+-------------+
| target | platform_name | test suite | result | elapsed_time (sec) | copy_method |
+---------------------+---------------+-------------------------------------------------------------------------+--------+--------------------+-------------+
| NRF52840_DK-GCC_ARM | NRF52840_DK | components-storage-blockdevice-component_qspif-tests-block_device-qspif | OK | 43.52 | default |
+---------------------+---------------+-------------------------------------------------------------------------+--------+--------------------+-------------+
mbedgt: test suite results: 1 OK
mbedgt: test case report:
+---------------------+---------------+-------------------------------------------------------------------------+------------------------------------------+--------+--------+--------+--------------------+
| target | platform_name | test suite | test case | passed | failed | result | elapsed_time (sec) |
+---------------------+---------------+-------------------------------------------------------------------------+------------------------------------------+--------+--------+--------+--------------------+
| NRF52840_DK-GCC_ARM | NRF52840_DK | components-storage-blockdevice-component_qspif-tests-block_device-qspif | Testing Multi Threads Erase Program Read | 1 | 0 | OK | 5.95 |
| NRF52840_DK-GCC_ARM | NRF52840_DK | components-storage-blockdevice-component_qspif-tests-block_device-qspif | Testing read write random blocks | 1 | 0 | OK | 1.56 |
| NRF52840_DK-GCC_ARM | NRF52840_DK | components-storage-blockdevice-component_qspif-tests-block_device-qspif | Testing unaligned erase blocks | 1 | 0 | OK | 0.3 |
+---------------------+---------------+-------------------------------------------------------------------------+------------------------------------------+--------+--------+--------+--------------------+
mbedgt: test case results: 3 OK
mbedgt: completed in 44.31 sec

targets/TARGET_NORDIC/TARGET_NRF5x/qspi_api.c

offirko · 2019-01-09T10:14:00Z

@j3hill - added a few remarks to the code.
In general, using configuration for program size seems like a good solution to me.
I seem to recall there was also an alignment issue for this target,
did you try testing it with data on non-4-bytes aligned address?

Adding: @dannybenor

j3hill · 2019-01-09T18:03:47Z

@maciejbocianski - Offir asked (above) if we tested with data on non-4-byte aligned addresses. Does the Greentea "components-storage-blockdevice-component_qspif-tests-block_device-qspif" test do this? It looks as though it generates random addresses for some of the tests, and I would think that some of them are non-4-byte aligned... but since they are random, we don't really know. Should we explicitly test non-aligned addresses, based on the minimum read/write size for a given device?

j3hill · 2019-01-09T20:35:08Z

Added a check for valid QSPI SFDP data. nRF52840-Preview-DK boards do not support QSPI SFDP reads, but production nRF52840 chipsets do support them with the workarounds implemented here.

offirko · 2019-01-10T07:16:13Z

@maciejbocianski - Offir asked (above) if we tested with data on non-4-byte aligned addresses. Does the Greentea "components-storage-blockdevice-component_qspif-tests-block_device-qspif" test do this? ...

Actually I wrote this test, it checks random addresses, but all are aligned to sector size (4096 in this case). I'll run a small test on my env with this PR and update..

maciejbocianski · 2019-01-10T08:54:10Z

@j3hill @offirko

Hal test for qspi tests-mbed_hal-qspi also uses random addresses (code) but only page number is random so each address is actually 256B aligned

offirko · 2019-01-10T10:11:56Z

Added a check for valid QSPI SFDP data. nRF52840-Preview-DK boards do not support QSPI SFDP reads, but production nRF52840 chipsets do support them with the workarounds implemented here.

@j3hill - Running the same test you ran on NRF52840_DK on remote RAAS fails on SFDP data validation, so I can not check unaligned read/write , you can do a small test that writes/reads 4 bytes to unaligned address.

Also, why did RAAS test failed, is it because it has nRF52840-Preview-DK boards? if so will CI not fail tests as well?

j3hill · 2019-01-10T16:44:21Z

@offirko I just sent an email to verify that we are using production nRF52840 DK boards for testing. Please also note that the bootloader must also use QSPIF and not SPIF. If the bootloader initializes/uses the same pins as SPIF, the application will not be able to initialize/use them as QSPIF. We are now trying to determine if this is a nRF52840 chipset limitation, or a SPI/QSPI software class constructor/destructor issue.

dlfryar-zz · 2019-01-16T18:12:36Z

Verified functionality on NRF52840_DK production board and a custom board with a production chip. Preview boards will get an error as expected.

@cmonr is there anything blocking a merge?

cmonr · 2019-01-17T00:21:37Z

@dlfryar Reviews by @offirko @donatieng @marcuschangarm and @maciejbocianski

offirko

Code LGTM
Should be ok to merge once @j3hill and @VeliMattiLahtela confirm
that "preview" boards in CI have been replaced

0xc0170 · 2019-01-17T10:15:34Z

Travis restarted (fetch package issues)

elm3 · 2019-01-18T20:20:46Z

Even though @marcuschangarm is listed as reviewer, he is not an approved merger. Is there someone in the reviewer list that can merge after approving?

0xc0170 · 2019-01-21T12:19:36Z

Should be ok to merge once @j3hill and @VeliMattiLahtela confirm
that "preview" boards in CI have been replaced

replaced? was it done? @offirko can you provide more info what we are waiting for?

@elm3 I'll review shortly.

0xc0170

Small request for better clarity in the bugfix: The commit for nrf5x but changing also QSPI block device, I would split it into two commits

fixing nrf51x qspi read
QSPI block device min read/prog size set via config

0xc0170 · 2019-01-21T15:42:03Z

@j3hill Can you split the commit as requested above?

j3hill · 2019-01-23T16:33:37Z

@0xc0170 The commits for this PR have been split as requested. The first commit updates the QSPIF min read/write implementation. The second commit implements SFDP reads for the nRF52840.

j3hill · 2019-01-23T16:56:29Z

@VeliMattiLahtela has confirmed that all of the "preview" boards have been replaced.

0xc0170 · 2019-01-23T19:15:06Z

CI started

mbed-ci · 2019-01-23T21:29:07Z

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 1
Build artifacts

j3hill requested review from offirko, marcuschangarm, maciejbocianski and donatieng January 8, 2019 17:14

cmonr added the needs: review label Jan 8, 2019

offirko reviewed Jan 9, 2019

View reviewed changes

targets/TARGET_NORDIC/TARGET_NRF5x/qspi_api.c Outdated Show resolved Hide resolved

targets/TARGET_NORDIC/TARGET_NRF5x/qspi_api.c Outdated Show resolved Hide resolved

j3hill force-pushed the QSPI_Align branch from cb9d7cb to dd20495 Compare January 9, 2019 17:52

j3hill force-pushed the QSPI_Align branch from dd20495 to 18fc1ef Compare January 9, 2019 20:29

j3hill mentioned this pull request Jan 16, 2019

nRF52840 QSPI - Truncated Read/Write Lengths #8447

Closed

offirko approved these changes Jan 17, 2019

View reviewed changes

0xc0170 approved these changes Jan 21, 2019

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Jan 21, 2019

j3hill added 2 commits January 23, 2019 09:55

NRF5x: min read/write implementation

e9f08cb

NRF5x: QSPI SFDP read implementation

2a72bb6

j3hill force-pushed the QSPI_Align branch from 18fc1ef to 2a72bb6 Compare January 23, 2019 16:27

0xc0170 added needs: CI and removed needs: work labels Jan 23, 2019

cmonr added release-version: 5.11.3 ready for merge and removed needs: CI labels Jan 24, 2019

cmonr merged commit 7c4668c into ARMmbed:master Jan 24, 2019

cmonr removed the ready for merge label Jan 24, 2019

j3hill deleted the QSPI_Align branch January 24, 2019 22:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NRF5x: QSPI SFDP read, min read/write implementation #9291

NRF5x: QSPI SFDP read, min read/write implementation #9291

j3hill commented Jan 8, 2019

j3hill commented Jan 8, 2019 •

edited

Loading

offirko commented Jan 9, 2019

j3hill commented Jan 9, 2019

j3hill commented Jan 9, 2019

offirko commented Jan 10, 2019

maciejbocianski commented Jan 10, 2019 •

edited

Loading

offirko commented Jan 10, 2019 •

edited

Loading

j3hill commented Jan 10, 2019 •

edited

Loading

dlfryar-zz commented Jan 16, 2019

cmonr commented Jan 17, 2019

offirko left a comment

0xc0170 commented Jan 17, 2019

elm3 commented Jan 18, 2019

0xc0170 commented Jan 21, 2019

0xc0170 left a comment

0xc0170 commented Jan 21, 2019

j3hill commented Jan 23, 2019

j3hill commented Jan 23, 2019

0xc0170 commented Jan 23, 2019

mbed-ci commented Jan 23, 2019

NRF5x: QSPI SFDP read, min read/write implementation #9291

NRF5x: QSPI SFDP read, min read/write implementation #9291

Conversation

j3hill commented Jan 8, 2019

Description

Pull request type

j3hill commented Jan 8, 2019 • edited Loading

offirko commented Jan 9, 2019

j3hill commented Jan 9, 2019

j3hill commented Jan 9, 2019

offirko commented Jan 10, 2019

maciejbocianski commented Jan 10, 2019 • edited Loading

offirko commented Jan 10, 2019 • edited Loading

j3hill commented Jan 10, 2019 • edited Loading

dlfryar-zz commented Jan 16, 2019

cmonr commented Jan 17, 2019

offirko left a comment

Choose a reason for hiding this comment

0xc0170 commented Jan 17, 2019

elm3 commented Jan 18, 2019

0xc0170 commented Jan 21, 2019

0xc0170 left a comment

Choose a reason for hiding this comment

0xc0170 commented Jan 21, 2019

j3hill commented Jan 23, 2019

j3hill commented Jan 23, 2019

0xc0170 commented Jan 23, 2019

mbed-ci commented Jan 23, 2019

Test run: SUCCESS

j3hill commented Jan 8, 2019 •

edited

Loading

maciejbocianski commented Jan 10, 2019 •

edited

Loading

offirko commented Jan 10, 2019 •

edited

Loading

j3hill commented Jan 10, 2019 •

edited

Loading