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

Stm32 spi : use LL API to improve performances #4375

Merged
merged 1 commit into from Jun 23, 2017

Conversation

Projects
None yet
7 participants
@LMESTM
Contributor

LMESTM commented May 23, 2017

Description

This PR contains a change in SPI driver to improve performances by accessing to registers directly (using LL layer in STM32 SDK, which provides inline functions to access it.
See #4246

Resolves #3445

Status

READY
Waiting for 2 other PRS:

Related PRs

There were 2 pending dependencies of upgrading STM32 SDK for
F4 (#4299) => MERGED
F2 (#4402) => MERGED

This SDK updates will include the LL API that is used here.

I will rebase when this is done - but this PR can be used for test by anyone interested in it, and review can happen in parallel.

Test results

CI tests passed ok
+-------------------+---------------+---------------+--------+--------------------+-------------+
| target | platform_name | test suite | result | elapsed_time (sec) | copy_method |
+-------------------+---------------+---------------+--------+--------------------+-------------+
| NUCLEO_F091RC-ARM | NUCLEO_F091RC | tests-api-spi | OK | 60.5 | shell |
| NUCLEO_F103RB-ARM | NUCLEO_F103RB | tests-api-spi | OK | 61.56 | shell |
| NUCLEO_F303ZE-ARM | NUCLEO_F303ZE | tests-api-spi | OK | 61.3 | shell |
| NUCLEO_F446RE-ARM | NUCLEO_F446RE | tests-api-spi | OK | 59.17 | shell |
| NUCLEO_F767ZI-ARM | NUCLEO_F767ZI | tests-api-spi | OK | 58.7 | shell |
| NUCLEO_L073RZ-ARM | NUCLEO_L073RZ | tests-api-spi | OK | 64.15 | shell |
| NUCLEO_L152RE-ARM | NUCLEO_L152RE | tests-api-spi | OK | 61.34 | shell |
| NUCLEO_L476RG-ARM | NUCLEO_L476RG | tests-api-spi | OK | 60.87 | shell |
+-------------------+---------------+---------------+--------+--------------------+-------------+

Also MBED OS2 SPI tests verified OK.

@adbridge

This comment has been minimized.

Contributor

adbridge commented May 24, 2017

@LMESTM
Build failed with:

Executing: python tools/build.py -m NUCLEO_F207ZG -t GCC_ARM -j 4 -c --silent --dsp --usb

In file included from /home/travis/build/ARMmbed/mbed-os/targets/TARGET_STM/stm_spi_api.c:41:0:

/home/travis/build/ARMmbed/mbed-os/BUILD/mbed/TARGET_NUCLEO_F207ZG/TARGET_STM/TARGET_STM32F2/spi_device.h:33:30: fatal error: stm32f2xx_ll_spi.h: No such file or directory

 #include "stm32f2xx_ll_spi.h"

Could you investigate please

@0xc0170

This comment has been minimized.

Member

0xc0170 commented May 25, 2017

@adbridge I believe because of the dependency mentioned above, it's now ready to be merged, then this should be rebased and make it green.

@LMESTM

This comment has been minimized.

Contributor

LMESTM commented May 29, 2017

Hey
I still have another dependency to solve on my side:
=> F2 stm32 cube update (to be done)
this is under local test for now and I expect to release it today or tomorrow
br
Laurent

@LMESTM

This comment has been minimized.

Contributor

LMESTM commented May 29, 2017

@0xc0170 @adbridge I added #4402 to the dependency list and I will rebase when it is merged

@LMESTM LMESTM force-pushed the LMESTM:STM32_SPI_LL branch Jun 1, 2017

@LMESTM

This comment has been minimized.

Contributor

LMESTM commented Jun 1, 2017

@0xc0170 @adbridge Dependencies have been merged and rebase was done :-)

@LMESTM

This comment has been minimized.

Contributor

LMESTM commented Jun 1, 2017

#3445 is also being fixed as part of this PR

@sg-

This comment has been minimized.

Member

sg- commented Jun 5, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jun 6, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 456

All builds and test passed!

@0xc0170

LGTM, just that macro is not clear to me what it does and how to find documentation (changes HAL implementation).

targets/TARGET_STM/stm_spi_api.c Outdated
struct spi_s *spiobj = SPI_S(obj);
SPI_HandleTypeDef *handle = &(spiobj->handle);
size = (handle->Init.DataSize == SPI_DATASIZE_16BIT) ? 2 : 1;
#ifdef STM32_SPI_KEEP_HAL_USAGE

This comment has been minimized.

@0xc0170

0xc0170 Jun 12, 2017

Member

Where is this macro coming from ? IS it documented anywhere? This changes the implementation .

This comment has been minimized.

@0xc0170

0xc0170 Jun 12, 2017

Member

It differs from what was before (RX an error handling ? )

This comment has been minimized.

@LMESTM

LMESTM Jun 12, 2017

Contributor

The commit message describes it:

This commit implements a SPI mode which will offer better performance
thanks to usage of Lower Layer API which use fewer registers access,
at the cost of lower robustness (no error management at all).

The SPI driver can be configured to re-use the HAL and error management by
defining the STM32_SPI_KEEP_HAL_USAGE compilation switch

This comment has been minimized.

@sg-

sg- Jun 15, 2017

Member

Does this macro change the implementation? That wouldn't be good. We should choose one implementation and stick to it.

@LMESTM LMESTM force-pushed the LMESTM:STM32_SPI_LL branch Jun 15, 2017

@LMESTM

This comment has been minimized.

Contributor

LMESTM commented Jun 15, 2017

@sg- @0xc0170 Updated version of PR without the fallback switch mecanism

@LMESTM

This comment has been minimized.

Contributor

LMESTM commented Jun 16, 2017

CI Jenkins reports a build failure - can you share the error ?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 16, 2017

@LMESTM There is the old jenkins config, can you rebase from master to pick it up, that should resolve the error.

STM32 SPI specific mode for higher performance
This commit implements a SPI mode which will offer better performance
thanks to usage of Lower Layer API which use fewer registers access,
at the cost of lower robustness (no error management).

@LMESTM LMESTM force-pushed the LMESTM:STM32_SPI_LL branch to 20bd774 Jun 16, 2017

@LMESTM

This comment has been minimized.

Contributor

LMESTM commented Jun 16, 2017

@0xc0170 ok thx - rebased

@theotherjimmy theotherjimmy changed the title from Stm32 spi : use ll API to improve performances to Stm32 spi : use LL API to improve performances Jun 19, 2017

@studavekar

This comment has been minimized.

Collaborator

studavekar commented Jun 21, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jun 22, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 606

All builds and test passed!

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Jun 22, 2017

@adbridge adbridge merged commit 4f5d4f0 into ARMmbed:master Jun 23, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sg- sg- removed the ready for merge label Jun 23, 2017

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