QSPIF: Enable QPI mode as a second option, if available #12304

michalpasztamobica · 2020-01-23T08:20:20Z

Summary of changes

Fixes #11994.

Prefer 1-4-4 option over QPI, but allow QPI to be used as a second choice (which was effectively impossible after #9057).

Impact of changes

QPI will be used wherever possible.

Migration actions required

None

Documentation

Not required.

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@SeppoTakalo
@AnttiKauppila
@jeromecoutant
@kyle-cypress
@VeijoPesonen

jeromecoutant · 2020-01-23T09:29:46Z

Mmm, not easy to test on my side now...
I found that QSPIF tests are already failed since 5.14.2 on DISCO-F769NI :-(

ciarmcom · 2020-01-23T10:00:54Z

@michalpasztamobica, thank you for your changes.
@AnttiKauppila @kyle-cypress @VeijoPesonen @jeromecoutant @SeppoTakalo @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

michalpasztamobica · 2020-01-23T10:03:23Z

@jeromecoutant . I am new to the QSPIF module. Would you please point me to the tests you were running? I haven't found any QSPI-related tests in mbed-os...

jeromecoutant · 2020-01-23T10:04:51Z

features-storage-tests-blockdevice-general_block_device

VeijoPesonen · 2020-01-23T11:31:52Z

@jeromecoutant . I am new to the QSPIF module. Would you please point me to the tests you were running? I haven't found any QSPI-related tests in mbed-os...

@michalpasztamobica Please take a look into PR #12270 to see how the tests can be executed.

michalpasztamobica · 2020-01-23T11:34:47Z

I've run the tests on K64F locally and they passed fine:

| target     | platform_name | test suite                                              | test case                                         | passed | failed | result | elapsed_time (sec) |
|------------|---------------|---------------------------------------------------------|---------------------------------------------------|--------|--------|--------|--------------------|
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | DEFAULT Testing get type functionality            | 1      | 0      | OK     | 0.1                |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing BlockDevice erase functionality  | 1      | 0      | OK     | 0.52               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing Deinit block device              | 1      | 0      | OK     | 0.1                |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing Init block device                | 1      | 0      | OK     | 0.1                |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing contiguous erase, write and read | 1      | 0      | OK     | 0.94               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing multi threads erase program read | 1      | 0      | OK     | 7.36               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing program read small data sizes    | 1      | 0      | OK     | 0.18               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing read write random blocks         | 1      | 0      | OK     | 1.66               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing unaligned erase blocks           | 1      | 0      | OK     | 0.12               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | SD Testing BlockDevice erase functionality        | 1      | 0      | OK     | 0.19               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | SD Testing Deinit block device                    | 1      | 0      | OK     | 0.09               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | SD Testing Init block device                      | 1      | 0      | OK     | 10.2               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | SD Testing contiguous erase, write and read       | 1      | 0      | OK     | 0.38               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | SD Testing multi threads erase program read       | 1      | 0      | OK     | 0.82               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | SD Testing program read small data sizes          | 1      | 0      | OK     | 0.24               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | SD Testing read write random blocks               | 1      | 0      | OK     | 0.37               |
| K64F-ARMC6 | K64F          | features-storage-tests-blockdevice-general_block_device | SD Testing unaligned erase blocks                 | 1      | 0      | OK     | 0.15               |
mbedgt: test case results: 17 OK
mbedgt: completed in 43.64 sec

But I don't have DISCO-F769NI available and when trying to run on raas I keep getting some errors, most likely due to incorrect python environment setup (I've seen this before some time ago).
So the issue with test is perhaps device-specific?

jeromecoutant · 2020-01-23T12:02:58Z

@michalpasztamobica K64F doesn't support QSPIF :-)
See test cases, you have checked FLASHIAP and SD...

VeijoPesonen · 2020-01-23T12:53:04Z

mbed test --compile -t GCC_ARM -m NRF52840_DK -n features-storage-*,components-storage-* --app-config tools/test_configs/QSPIFBlockDeviceAndHeapBlockDevice.json

To be exact.

michalpasztamobica · 2020-01-23T13:02:51Z

I managed to get my tools in order and I got this on DISCO_F769NI:

| target             | platform_name | test suite                                              | test case                                      | passed | failed | result  | elapsed_time (sec) |
|--------------------|---------------|---------------------------------------------------------|------------------------------------------------|--------|--------|---------|--------------------|
| DISCO_F769NI-ARMC6 | DISCO_F769NI  | features-storage-tests-blockdevice-general_block_device | DEFAULT Testing get type functionality         | 0      | 0      | SKIPPED | 0.0                |
| DISCO_F769NI-ARMC6 | DISCO_F769NI  | features-storage-tests-blockdevice-general_block_device | QSPIF Testing BlockDevice erase functionality  | 0      | 0      | ERROR   | 0.0                |
| DISCO_F769NI-ARMC6 | DISCO_F769NI  | features-storage-tests-blockdevice-general_block_device | QSPIF Testing Deinit block device              | 0      | 0      | SKIPPED | 0.0                |
| DISCO_F769NI-ARMC6 | DISCO_F769NI  | features-storage-tests-blockdevice-general_block_device | QSPIF Testing Init block device                | 0      | 1      | FAIL    | 0.3                |
| DISCO_F769NI-ARMC6 | DISCO_F769NI  | features-storage-tests-blockdevice-general_block_device | QSPIF Testing contiguous erase, write and read | 0      | 1      | FAIL    | 0.3                |
| DISCO_F769NI-ARMC6 | DISCO_F769NI  | features-storage-tests-blockdevice-general_block_device | QSPIF Testing multi threads erase program read | 1      | 0      | OK      | 0.19               |
| DISCO_F769NI-ARMC6 | DISCO_F769NI  | features-storage-tests-blockdevice-general_block_device | QSPIF Testing program read small data sizes    | 0      | 0      | SKIPPED | 0.0                |
| DISCO_F769NI-ARMC6 | DISCO_F769NI  | features-storage-tests-blockdevice-general_block_device | QSPIF Testing read write random blocks         | 1      | 0      | OK      | 0.16               |
| DISCO_F769NI-ARMC6 | DISCO_F769NI  | features-storage-tests-blockdevice-general_block_device | QSPIF Testing unaligned erase blocks           | 0      | 0      | SKIPPED | 0.0                |
mbedgt: test case results: 4 SKIPPED / 1 ERROR / 2 FAIL / 2 OK

So yes - tests are failing and I guess it's worth debugging them before.
@jeromecoutant , is this also what you are seeing?

jeromecoutant · 2020-01-23T13:05:46Z

@jeromecoutant , is this also what you are seeing?

Yes,
5.14.1 : OK
5.14.2 : FAILED
5.15.0 : FAILED

kyle-cypress

The changes look fine to me. Confirmed block device and storage tests all still pass on CY8CPROTO_062_4343W

michalpasztamobica · 2020-01-24T07:45:29Z

If @kyle-cypress confirms the tests passed for him on at least one platform, then I suggest not to withhold this PR and get it merged. I will put the tests fixes in a separate PR, anyway and I can see some more github issues relate to it (for example #11845), so I am sure we won't forget it.
@jeromecoutant , @SeppoTakalo , does this make sense to you?

michalpasztamobica · 2020-01-24T09:52:49Z

I also checked NRF52840_DK and it also passes the QSPIF tests (both on master and with this PR):

| target            | platform_name | test suite                                              | test case                                         | passed | failed | result | elapsed_time (sec) |
|-------------------|---------------|---------------------------------------------------------|---------------------------------------------------|--------|--------|--------|--------------------|
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | DEFAULT Testing get type functionality            | 1      | 0      | OK     | 0.1                |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing BlockDevice erase functionality  | 1      | 0      | OK     | 0.6                |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing Deinit block device              | 1      | 0      | OK     | 0.11               |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing Init block device                | 1      | 0      | OK     | 0.11               |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing contiguous erase, write and read | 1      | 0      | OK     | 0.91               |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing multi threads erase program read | 1      | 0      | OK     | 7.32               |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing program read small data sizes    | 1      | 0      | OK     | 0.8                |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing read write random blocks         | 1      | 0      | OK     | 1.71               |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | FLASHIAP Testing unaligned erase blocks           | 1      | 0      | OK     | 0.2                |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | QSPIF Testing BlockDevice erase functionality     | 1      | 0      | OK     | 0.5                |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | QSPIF Testing Deinit block device                 | 1      | 0      | OK     | 0.06               |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | QSPIF Testing Init block device                   | 1      | 0      | OK     | 0.13               |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | QSPIF Testing contiguous erase, write and read    | 1      | 0      | OK     | 1.64               |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | QSPIF Testing multi threads erase program read    | 1      | 0      | OK     | 5.53               |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | QSPIF Testing program read small data sizes       | 1      | 0      | OK     | 0.5                |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | QSPIF Testing read write random blocks            | 1      | 0      | OK     | 1.21               |
| NRF52840_DK-ARMC6 | NRF52840_DK   | features-storage-tests-blockdevice-general_block_device | QSPIF Testing unaligned erase blocks              | 1      | 0      | OK     | 0.1                |
mbedgt: test case results: 17 OK

mbed-ci · 2020-01-29T13:32:18Z

Test run: SUCCESS

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

mergify · 2020-01-30T08:22:16Z

This PR does not contain release version label after merging.

0xc0170 · 2020-02-04T14:46:26Z

I've fixed the version: Set to 6.0.0-alpha-2

QSPIF: Enable QPI mode as a second option, if available

bf11bbd

ciarmcom requested review from AnttiKauppila, jeromecoutant, kyle-cypress, SeppoTakalo, VeijoPesonen and a team January 23, 2020 10:00

ciarmcom added the needs: review label Jan 23, 2020

kyle-cypress approved these changes Jan 23, 2020

View reviewed changes

SeppoTakalo approved these changes Jan 29, 2020

View reviewed changes

SeppoTakalo added needs: CI and removed needs: review labels Jan 29, 2020

mergify bot added ready for merge and removed needs: CI labels Jan 29, 2020

kjbracey merged commit e8eed6e into ARMmbed:master Jan 30, 2020

mergify bot removed the ready for merge label Jan 30, 2020

mergify bot added the Release review required label Jan 30, 2020

0xc0170 added release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0 and removed Release review required labels Feb 4, 2020

0xc0170 mentioned this pull request Feb 24, 2020

QSPIF: 4-byte addressing support causes failure on NRF52840_DK #11845

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QSPIF: Enable QPI mode as a second option, if available #12304

QSPIF: Enable QPI mode as a second option, if available #12304

michalpasztamobica commented Jan 23, 2020

jeromecoutant commented Jan 23, 2020

ciarmcom commented Jan 23, 2020

michalpasztamobica commented Jan 23, 2020

jeromecoutant commented Jan 23, 2020

VeijoPesonen commented Jan 23, 2020

michalpasztamobica commented Jan 23, 2020

jeromecoutant commented Jan 23, 2020

VeijoPesonen commented Jan 23, 2020

michalpasztamobica commented Jan 23, 2020

jeromecoutant commented Jan 23, 2020 •

edited

Loading

kyle-cypress left a comment

michalpasztamobica commented Jan 24, 2020

michalpasztamobica commented Jan 24, 2020 •

edited

Loading

mbed-ci commented Jan 29, 2020

mergify bot commented Jan 30, 2020

0xc0170 commented Feb 4, 2020

QSPIF: Enable QPI mode as a second option, if available #12304

QSPIF: Enable QPI mode as a second option, if available #12304

Conversation

michalpasztamobica commented Jan 23, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

jeromecoutant commented Jan 23, 2020

ciarmcom commented Jan 23, 2020

michalpasztamobica commented Jan 23, 2020

jeromecoutant commented Jan 23, 2020

VeijoPesonen commented Jan 23, 2020

michalpasztamobica commented Jan 23, 2020

jeromecoutant commented Jan 23, 2020

VeijoPesonen commented Jan 23, 2020

michalpasztamobica commented Jan 23, 2020

jeromecoutant commented Jan 23, 2020 • edited Loading

kyle-cypress left a comment

Choose a reason for hiding this comment

michalpasztamobica commented Jan 24, 2020

michalpasztamobica commented Jan 24, 2020 • edited Loading

mbed-ci commented Jan 29, 2020

Test run: SUCCESS

mergify bot commented Jan 30, 2020

0xc0170 commented Feb 4, 2020

jeromecoutant commented Jan 23, 2020 •

edited

Loading

michalpasztamobica commented Jan 24, 2020 •

edited

Loading