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

Add OSPI driver to support the Octa mode of Macronix octaflash MX25LM51245G #12619

Closed
wants to merge 3 commits into from

Conversation

rogeryou
Copy link
Contributor

@rogeryou rogeryou commented Mar 12, 2020

Summary of changes

Add OSPI HAL driver and OSPIF block device driver for using Macronix octaflash MX25LM51245G.

ST Octo-SPI was ported on QSPI MBED API, but we can only use in SPI mode and the Octo mode is not supported. So, we add OSPI driver to enable Octo mode to improve read and write performance.


Impact of changes

Enable the octa mode of MX25LM51245G.


Migration actions required

N/A


Documentation

N/A

Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[x] 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

OSPI HAL test was executed.

target platform_name test suite test case passed failed result elapsed_time (sec)
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi frequency setting test 1 0 OK 0.12
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi init/free test 1 0 OK 0.07
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi memory id test 1 0 OK 0.11
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi write(1-1-1)/x1 read(1-1-1)/x1 repeat/x1 test 1 0 OK 0.1
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi write(1-1-1)/x1 read(1-1-1)/x1 repeat/x4 test 1 0 OK 0.19
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi write(1-1-1)/x1 read(1-1-1)/x4 repeat/x1 test 1 0 OK 0.1
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi write(1-1-1)/x4 read(1-1-1)/x1 repeat/x1 test 1 0 OK 0.13
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi write(8D_8D_8D)/x1 read(8D_8D_8D)/x1 repeat/x1 test 1 0 OK 0.12
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi write(8D_8D_8D)/x1 read(8D_8D_8D)/x1 repeat/x4 test 1 0 OK 0.18
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi write(8D_8D_8D)/x1 read(8D_8D_8D)/x4 repeat/x1 test 1 0 OK 0.12
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi write(8D_8D_8D)/x4 read(8D_8D_8D)/x1 repeat/x1 test 1 0 OK 0.12
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi write(8_8_8)/x1 read(8_8_8)/x1 repeat/x1 test 1 0 OK 0.11
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi write(8_8_8)/x1 read(8_8_8)/x1 repeat/x4 test 1 0 OK 0.18
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi write(8_8_8)/x1 read(8_8_8)/x4 repeat/x1 test 1 0 OK 0.11
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-ospi ospi write(8_8_8)/x4 read(8_8_8)/x1 repeat/x1 test 1 0 OK 0.1

OSPIF driver test was executed.

target platform_name test suite test case passed failed result elapsed_time (sec)
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-general_block_device OSPIF Testing BlockDevice erase functionality 1 0 OK 0.16
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-general_block_device OSPIF Testing Deinit block device 1 0 OK 0.13
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-general_block_device OSPIF Testing Init block device 1 0 OK 0.12
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-general_block_device OSPIF Testing contiguous erase, write and read 1 0 OK 0.16
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-general_block_device OSPIF Testing multi threads erase program read 1 0 OK 0.16
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-general_block_device OSPIF Testing program read small data sizes 1 0 OK 0.17
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-general_block_device OSPIF Testing read write random blocks 1 0 OK 0.15
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-general_block_device OSPIF Testing unaligned erase blocks 1 0 OK 0.13
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-general_block_device OSPIF Testing write -> deinit -> init -> read 1 0 OK 0.01

Reviewers


@mergify mergify bot added the needs: work label Mar 12, 2020
@ciarmcom ciarmcom requested review from a team March 12, 2020 06:00
@ciarmcom
Copy link
Member

@rogeryou, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-wan @ARMmbed/mbed-os-pan @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-hal @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-connectivity @ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-test @ARMmbed/mbed-os-crypto @ARMmbed/mbed-os-tls please review.

@rogeryou rogeryou changed the title Macronix ospi Add OSPI driver to support the Octa mode of Macronix Octa flash MX25LM51245G Mar 12, 2020
@rogeryou rogeryou marked this pull request as ready for review March 12, 2020 06:19
@rogeryou rogeryou changed the title Add OSPI driver to support the Octa mode of Macronix Octa flash MX25LM51245G Add OSPI driver to support the Octa mode of Macronix octaflash MX25LM51245G Mar 12, 2020
@mergify mergify bot removed the needs: review label Mar 12, 2020
Copy link
Collaborator

@jeromecoutant jeromecoutant left a comment

Choose a reason for hiding this comment

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

Great work!
I am going to execute tests asap!

@ARMmbed/team-st-mcd

@@ -12525,7 +12525,7 @@
}
},
"components_add": [
"FLASHIAP"
"OSPIF"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why FLASHIAP is removed ?

Copy link
Contributor Author

@rogeryou rogeryou Mar 13, 2020

Choose a reason for hiding this comment

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

Why FLASHIAP is removed ?

@jeromecoutant

Before I add the OSPI driver, I have executed the test "features-storage-tests-blockdevice-general_block_device" of FLASHIAP, but it failed.

Have you tested FLASHIAP on DISCO_L4R9I before?

You can see the error information below.

mbedgt: processing target 'DISCO_L4R9I' toolchain 'GCC_ARM' compatible platforms... (note: switch set to --parallel 1)
mbedgt: test case filter (specified with -n option)
features-storage-tests-blockdevice-general_block_device
test filtered in 'features-storage-tests-blockdevice-general_block_device'
mbedgt: running 1 test for platform 'DISCO_L4R9I' and toolchain 'GCC_ARM'
mbedgt: mbed-host-test-runner: started
mbedgt: :741::FAIL: Expected -1 Was 0
mbedgt: :741::FAIL: Expected -1 Was 0
mbedgt: :747::FAIL: Expected 0 Was 1
mbedgt: :741::FAIL: Expected -1 Was 0
mbedgt: :747::FAIL: Expected 0 Was 2
mbedgt: :258::FAIL: Expected 0 Was -1
mbedgt: :274::FAIL: Expected 92 Was 23
mbedgt: :274::FAIL: Expected 163 Was 0
mbedgt: :274::FAIL: Expected 105 Was 0
mbedgt: :274::FAIL: Expected 177 Was 0
mbedgt: :274::FAIL: Expected 133 Was 0
mbedgt: :274::FAIL: Expected 106 Was 56

Copy link
Collaborator

Choose a reason for hiding this comment

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

Working in mbed-os master branch:

target platform_name test suite result elapsed_time (sec) copy_method
DISCO_L4R9I-ARMC6 DISCO_L4R9I features-storage-tests-blockdevice-buffered_block_device OK 15.64 default
DISCO_L4R9I-ARMC6 DISCO_L4R9I features-storage-tests-blockdevice-flashsim_block_device OK 15.32 default
DISCO_L4R9I-ARMC6 DISCO_L4R9I features-storage-tests-blockdevice-general_block_device OK 26.57 default
DISCO_L4R9I-ARMC6 DISCO_L4R9I features-storage-tests-blockdevice-heap_block_device OK 17.56 default
DISCO_L4R9I-ARMC6 DISCO_L4R9I features-storage-tests-blockdevice-mbr_block_device OK 17.6 default
DISCO_L4R9I-ARMC6 DISCO_L4R9I features-storage-tests-blockdevice-util_block_device OK 16.34 default
DISCO_L4R9I-ARMC6 DISCO_L4R9I tests-mbed_hal-qspi OK 38.15 default
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-buffered_block_device OK 15.58 default
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-flashsim_block_device OK 15.26 default
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-general_block_device OK 26.53 default
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-heap_block_device OK 17.63 default
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-mbr_block_device OK 16.44 default
DISCO_L4R9I-GCC_ARM DISCO_L4R9I features-storage-tests-blockdevice-util_block_device OK 16.07 default
DISCO_L4R9I-GCC_ARM DISCO_L4R9I tests-mbed_hal-qspi OK 37.88 default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi,
I execute the command below, and I got failed result.
sudo mbed test -m DISCO_L4R9I -t GCC_ARM -n features-storage-tests-blockdevice-general_block_device

Could you tell me the command you executed to test many test suite in one time?
Thanks a lot!

@adbridge
Copy link
Contributor

@rogeryou thanks for the PR but it looks like you have modified the PR header template to remove some fields (or are using a very old template). Could you please update to the correct template and fill in the missing information? Thanks.

@jeromecoutant
Copy link
Collaborator

Hi

I have try this branch:

  • Could you squash some commits and rebase on top of mbed-os master ?
  • I got compilation error:
[Error] @0,0: L6218E: Undefined symbol ospi_init_direct (referred from BUILD/tests/DISCO_L4R9I/ARM/drivers/source/OSPI.o).
  • in drivers/source/SFDP.cpp
    #if (DEVICE_SPI || DEVICE_QSPI)
    You should add DEVICE_OSPI ?

@rogeryou
Copy link
Contributor Author

rogeryou commented Mar 16, 2020

@rogeryou thanks for the PR but it looks like you have modified the PR header template to remove some fields (or are using a very old template). Could you please update to the correct template and fill in the missing information? Thanks.

Hi,
I have filled in some missing information.Please review.@adbridge
Thanks.

@jeromecoutant
Copy link
Collaborator

  • I got compilation error:
[Error] @0,0: L6218E: Undefined symbol ospi_init_direct (referred from BUILD/tests/DISCO_L4R9I/ARM/drivers/source/OSPI.o).

OSPI code is missing in static_pinmap.cpp/h files

@rogeryou
Copy link
Contributor Author

Hi

I have try this branch:

* Could you squash some commits and rebase on top of mbed-os master ?

* I got compilation error:
[Error] @0,0: L6218E: Undefined symbol ospi_init_direct (referred from BUILD/tests/DISCO_L4R9I/ARM/drivers/source/OSPI.o).
* in drivers/source/SFDP.cpp
  #if (DEVICE_SPI || DEVICE_QSPI)
  You should add DEVICE_OSPI ?

Hi,
I have squashed some commits and add DEVICE_OSPI in SFDP.cpp. @jeromecoutant
Thanks!

@rogeryou
Copy link
Contributor Author

  • I got compilation error:
[Error] @0,0: L6218E: Undefined symbol ospi_init_direct (referred from BUILD/tests/DISCO_L4R9I/ARM/drivers/source/OSPI.o).

OSPI code is missing in static_pinmap.cpp/h files

Hi,

There is no compilation error here without OSPI code in static_pinmap.cpp/h.
But, I have added OSPI code in static_pinmap.cpp/h. Please review.
Thanks a lot!@jeromecoutant

@0xc0170 0xc0170 removed request for a team March 16, 2020 14:41
@0xc0170 0xc0170 removed request for a team March 16, 2020 14:41
@0xc0170
Copy link
Contributor

0xc0170 commented Mar 16, 2020

@rogeryou Hi, can you please rebase your branch to remove merge commits ? To have clean linear git history, just adding new files (or any fixes to these).

@jeromecoutant
Copy link
Collaborator

There is no compilation error here without OSPI code in static_pinmap.cpp/h.
But, I have added OSPI code in static_pinmap.cpp/h. Please review.

Still compilation error with ARM.
And static_pinmap files...

@mergify
Copy link

mergify bot commented Mar 17, 2020

This PR cannot be merged due to conflicts. Please rebase to resolve them.

@jeromecoutant
Copy link
Collaborator

@rogeryou Hi, can you please rebase your branch to remove merge commits ? To have clean linear git history, just adding new files (or any fixes to these).

I agree... I currently can't work with that branch...
Starts from mbed-os master and just add your files (no merged branches)

Modify the astyle

Add DEVICE_OSPI in SFDP.cpp
@rogeryou rogeryou closed this Mar 18, 2020
@rogeryou rogeryou deleted the macronix_ospi branch March 18, 2020 06:49
@rogeryou rogeryou restored the macronix_ospi branch March 18, 2020 06:52
@rogeryou rogeryou reopened this Mar 18, 2020
@rogeryou rogeryou closed this Mar 18, 2020
@rogeryou rogeryou deleted the macronix_ospi branch March 18, 2020 07:18
@rogeryou
Copy link
Contributor Author

@rogeryou Hi, can you please rebase your branch to remove merge commits ? To have clean linear git history, just adding new files (or any fixes to these).

I agree... I currently can't work with that branch...
Starts from mbed-os master and just add your files (no merged branches)

@jeromecoutant
I have created a new macronix_ospi branch from mbed-os master and a new pull request PR #12644 .
Please review.

@mergify
Copy link

mergify bot commented Mar 20, 2020

This PR cannot be merged due to conflicts. Please rebase to resolve them.

@mergify mergify bot added the needs: work label Mar 20, 2020
@VeijoPesonen
Copy link
Contributor

@adbridge the bot doesn't seem to realize that this PR has been closed.

@adbridge
Copy link
Contributor

@VeijoPesonen yeah it's a known issue, it is on the list of investigations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants