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

STM32: fix SPI 16 bit mode #15206

Merged
merged 2 commits into from
Jan 17, 2022
Merged

Conversation

vznncv
Copy link
Contributor

@vznncv vznncv commented Jan 15, 2022

Summary of changes

The current low-level STM32 SPI API implementation (file stm_spi_api.c) doesn't processes SPI frames in 16 bit mode
correctly. If we need to sent 2 bytes 0x11 0x22 in 16 bit mode, SPI device sends 4 bytes (extra 0x00 byte is added
before each one) 0x00 0x11 0x00 0x22. This pull request fixes this behaviour to send correct data: 0x22 0x11

issue: #15113
related pull request: #15115

Impact of changes

Fix data frame format of SPI in 16 bit mode for all STM32 devices.

Migration actions required

Documentation

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

SPI monitoring with logic analyzer

This demo program simply writes bytes to SPI device with loopback (MISO is connected to MOSI). It allows checking SPI
frames with logic analyzer.

Results of const uint16_t tx_data_16[2] = {0x1122, 0x3344}; data transmission without fixes:

  • 3-wire mode, synchronous transmission

    no_fix_3wire_sync

  • 3-wire mode, asynchronous transmission:

    no_fix_3wire_async

  • 4-wire mode, synchronous transmission:

    no_fix_4wire_sync

  • 4-wire mode, asynchronous transmission:

    no_fix_4wire_async

  • full results. Console logs: no_fix_serial_logs.txt, logical analyzer data (sigrok format): no_fix_data.zip

Although we transmit the same data, synchronous and asynchronous SPI API give different results (asynchronous API
transmits data correctly).

Results of const uint16_t tx_data_16[2] = {0x1122, 0x3344}; data transmission after fixes:

  • 3-wire mode, synchronous transmission:

    with_fix_3wire_sync

  • 3-wire mode, asynchronous transmission:

    with_fix_3wire_async

  • 4-wire mode, synchronous transmission:

with_fix_4wire_sync

All SPI APIs give the same result after fix.

SPI test with sensor BMX160 (3 wire SPI mode)

This demo writes data to BMX160 sensor, reads them back and compares sent and received bytes. Additionally, it controls
number of SPI clock cycles with STM32 timer ETR feature.

After data sending/receiving with different SPI modes (8/16 bit) and frequencies it prints summary table with test
results.

Results:

  • STM32F103C8
api case name target freq (Hz) actual freq (Hz) init result data clock
1 sync single_w8_r8 1000000 562500 success success success success
2 sync burst_w8_r8 1000000 562500 success success success success
3 sync single_w16_r8 1000000 562500 success success success success
4 sync burst_w16_r8 1000000 562500 success success success success
5 sync single_w8_r16 1000000 562500 success success success success
6 sync burst_w8_r16 1000000 562500 success success success success
7 sync single_w8_r8 200000 281250 success success success success
8 sync burst_w8_r8 200000 281250 success success success success
9 sync single_w16_r8 200000 281250 success success success success
10 sync burst_w16_r8 200000 281250 success success success success
11 sync single_w8_r16 200000 281250 success success success success
12 sync burst_w8_r16 200000 281250 success success success success
13 sync single_w8_r8 10000000 9000000 success success success success
14 sync burst_w8_r8 10000000 9000000 success success success success
15 sync single_w16_r8 10000000 9000000 success success success success
16 sync burst_w16_r8 10000000 9000000 success success success success
17 sync single_w8_r16 10000000 9000000 success success success success
18 sync burst_w8_r16 10000000 9000000 success success success success
19 async (+ abort_all_transfers after) single_w8_r8 1000000 562500 success error success error
20 async burst_w8_r8 1000000 562500 success error success error
21 async single_w16_r8 1000000 562500 success error success error
22 async burst_w16_r8 1000000 562500 success error success error
23 async single_w8_r16 1000000 562500 success error success error
24 async burst_w8_r16 1000000 562500 success error success error
25 async (+ abort_all_transfers after) single_w8_r8 200000 281250 success error success error
26 async burst_w8_r8 200000 281250 success error success error
27 async single_w16_r8 200000 281250 success error success error
28 async burst_w16_r8 200000 281250 success error success error
29 async single_w8_r16 200000 281250 success error success error
30 async burst_w8_r16 200000 281250 success error success error
31 async single_w8_r8 10000000 9000000 success error error error
32 async burst_w8_r8 10000000 9000000 success error error error
33 async single_w16_r8 10000000 9000000 success error error error
34 async burst_w16_r8 10000000 9000000 success error error error
35 async single_w8_r16 10000000 9000000 success error success error
36 async burst_w8_r16 10000000 9000000 success error error error

full logs: main_3wire_stm32f103c8_logs.txt

  • STM32F411CE
api case name target freq (Hz) actual freq (Hz) init result data clock
1 sync single_w8_r8 1000000 781250 success success success success
2 sync burst_w8_r8 1000000 781250 success success success success
3 sync single_w16_r8 1000000 781250 success success success success
4 sync burst_w16_r8 1000000 781250 success success success success
5 sync single_w8_r16 1000000 781250 success success success success
6 sync burst_w8_r16 1000000 781250 success success success success
7 sync single_w8_r8 200000 390625 success success success success
8 sync burst_w8_r8 200000 390625 success success success success
9 sync single_w16_r8 200000 390625 success success success success
10 sync burst_w16_r8 200000 390625 success success success success
11 sync single_w8_r16 200000 390625 success success success success
12 sync burst_w8_r16 200000 390625 success success success success
13 sync single_w8_r8 10000000 6250000 success success success success
14 sync burst_w8_r8 10000000 6250000 success success success success
15 sync single_w16_r8 10000000 6250000 success success success success
16 sync burst_w16_r8 10000000 6250000 success success success success
17 sync single_w8_r16 10000000 6250000 success success success success
18 sync burst_w8_r16 10000000 6250000 success success success success
19 async (+ abort_all_transfers after) single_w8_r8 1000000 781250 success error success error
20 async burst_w8_r8 1000000 781250 success error success error
21 async single_w16_r8 1000000 781250 success error success error
22 async burst_w16_r8 1000000 781250 success error success error
23 async single_w8_r16 1000000 781250 success error success error
24 async burst_w8_r16 1000000 781250 success error success error
25 async (+ abort_all_transfers after) single_w8_r8 200000 390625 success error success error
26 async burst_w8_r8 200000 390625 success error success error
27 async single_w16_r8 200000 390625 success error success error
28 async burst_w16_r8 200000 390625 success error success error
29 async single_w8_r16 200000 390625 success error success error
30 async burst_w8_r16 200000 390625 success error success error
31 async single_w8_r8 10000000 6250000 success error success error
32 async burst_w8_r8 10000000 6250000 success error error error
33 async single_w16_r8 10000000 6250000 success error error error
34 async burst_w16_r8 10000000 6250000 success error error error
35 async single_w8_r16 10000000 6250000 success error success error
36 async burst_w8_r16 10000000 6250000 success error success error

full logs: main_3wire_stm32f411ce_logs.txt

  • STM32H743VI
api case name target freq (Hz) actual freq (Hz) init result data clock
1 sync single_w8_r8 1000000 625000 success success success success
2 sync burst_w8_r8 1000000 625000 success success success success
3 sync single_w16_r8 1000000 625000 success success success success
4 sync burst_w16_r8 1000000 625000 success success success success
5 sync single_w8_r16 1000000 625000 success success success success
6 sync burst_w8_r16 1000000 625000 success success success success
7 sync single_w8_r8 200000 156250 success success success success
8 sync burst_w8_r8 200000 156250 success success success success
9 sync single_w16_r8 200000 156250 success success success success
10 sync burst_w16_r8 200000 156250 success success success success
11 sync single_w8_r16 200000 156250 success success success success
12 sync burst_w8_r16 200000 156250 success success success success
13 sync single_w8_r8 10000000 5000000 success success success success
14 sync burst_w8_r8 10000000 5000000 success success success success
15 sync single_w16_r8 10000000 5000000 success success success success
16 sync burst_w16_r8 10000000 5000000 success success success success
17 sync single_w8_r16 10000000 5000000 success success success success
18 sync burst_w8_r16 10000000 5000000 success success success success
19 async (+ abort_all_transfers after) single_w8_r8 1000000 625000 success success success success
20 async burst_w8_r8 1000000 625000 success success success success
21 async single_w16_r8 1000000 625000 success success success success
22 async burst_w16_r8 1000000 625000 success success success success
23 async single_w8_r16 1000000 625000 success success success success
24 async burst_w8_r16 1000000 625000 success success success success
25 async (+ abort_all_transfers after) single_w8_r8 200000 156250 success success success success
26 async burst_w8_r8 200000 156250 success success success success
27 async single_w16_r8 200000 156250 success success success success
28 async burst_w16_r8 200000 156250 success success success success
29 async single_w8_r16 200000 156250 success success success success
30 async burst_w8_r16 200000 156250 success success success success
31 async single_w8_r8 10000000 5000000 success success success success
32 async burst_w8_r8 10000000 5000000 success success success success
33 async single_w16_r8 10000000 5000000 success success success success
34 async burst_w16_r8 10000000 5000000 success success success success
35 async single_w8_r16 10000000 5000000 success success success success
36 async burst_w8_r16 10000000 5000000 success success success success

full logs: main_3wire_stm32h743vi_logs.txt

Result notes:

  1. Result tables have the "case name" column with value <single|burst>_w<8|16>_r<8|16> that means:
    • single - test sends single SPI data frame to BMX160 and then receives single SPI data frame
    • burst - test sends multiple SPI data frames to BMX160 and then receives multiple SPI data frames
    • w<8|16> - test sends SPI data frames in 8/16 bit mode
    • r<8|16> - test receives SPI data frames in 8/16 bit mode
  2. Asynchronous API doesn't work correctly in SPI IP version 1 (STM32F103C8 and STM32F411CE) due HAL library usage that
    doesn't implement correct data reading with interrupts (it doesn't disable SPI in time, that causes dummy reads).

SPI loopback test (4 wire SPI mode)

This demo sends and receives data with SPI loopback. After data transmission it checks that sent and received data is
the same. Additionally, it controls number of SPI clock cycles with STM32 timer ETR feature.

After data sending/receiving with different SPI modes (8/16 bit) and frequencies it prints summary table with test
results.

Results:

  • STM32F103C8
case name target freq (Hz) actual freq (Hz) result data clock
1 single word write/read 1000000 562500 success success success
2 multiple word write/read 1000000 562500 success success success
3 multiple word write/read async 1000000 562500 success success success
4 multiple word write/read with default fill 1000000 562500 success success success
5 single word write/read 200000 281250 success success success
6 multiple word write/read 200000 281250 success success success
7 multiple word write/read async 200000 281250 success success success
8 multiple word write/read with default fill 200000 281250 success success success
9 single word write/read 10000000 9000000 success success success
10 multiple word write/read 10000000 9000000 success success success
11 multiple word write/read async 10000000 9000000 success success success
12 multiple word write/read with default fill 10000000 9000000 success success success

full logs: main_4wire_stm32f103c8_logs.txt

  • STM32F411CE
case name target freq (Hz) actual freq (Hz) result data clock
1 single word write/read 1000000 781250 success success success
2 multiple word write/read 1000000 781250 success success success
3 multiple word write/read async 1000000 781250 success success success
4 multiple word write/read with default fill 1000000 781250 success success success
5 single word write/read 200000 390625 success success success
6 multiple word write/read 200000 390625 success success success
7 multiple word write/read async 200000 390625 success success success
8 multiple word write/read with default fill 200000 390625 success success success
9 single word write/read 10000000 6250000 success success success
10 multiple word write/read 10000000 6250000 success success success
11 multiple word write/read async 10000000 6250000 success success success
12 multiple word write/read with default fill 10000000 6250000 success success success

full logs: main_4wire_stm32f411ce_logs.txt

  • STM32H743VI
case name target freq (Hz) actual freq (Hz) result data clock
1 single word write/read 1000000 625000 success success success
2 multiple word write/read 1000000 625000 success success success
3 multiple word write/read asprojectync 1000000 625000 success success success
4 multiple word write/read with default fill 1000000 625000 success success success
5 single word write/read 200000 156250 success success success
6 multiple word write/read 200000 156250 success success success
7 multiple word write/read async 200000 156250 success success success
8 multiple word write/read with default fill 200000 156250 success success success
9 single word write/read 10000000 5000000 success success success
10 multiple word write/read 10000000 5000000 success success success
11 multiple word write/read async 10000000 5000000 success success success
12 multiple word write/read with default fill 10000000 5000000 success success success

full logs: main_4wire_stm32h743vi_logs.txt


Reviewers


Update SPI logic to process 16 bit words in the same way by sync/async,
3/4 wires modes:
- fix 3-wire synchronous transmission to move 2 or more bytes between buffer and
  SPI register per word tarnsmission
- fix 4-wire synchronous transmission to move 2 or more bytes between buffer and
  SPI register per word tarnsmission
By default, HAL functions (HAL_SPI_TransmitReceive_IT/HAL_SPI_Transmit_IT/HAL_SPI_Receive_IT) assume that SPI is disabled between function invocation.
It's needed to set transfer size (CR2 register), that can be modified only if SPI disabled. But `stm32_spi_api.c` keeps SPI enabled after initialization.

This commit adds helper code for STM32H7 (SPI_IP_VERSION_V2) that disables SPI before HAL_SPI_TransmitReceive_IT/HAL_SPI_Transmit_IT/HAL_SPI_Receive_IT
and after end of transaction for HAL API compatibility.
@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Jan 15, 2022
@ciarmcom ciarmcom requested a review from a team January 15, 2022 20:30
@ciarmcom
Copy link
Member

@vznncv, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 17, 2022

related pull request: #15115

We can close #15115 if this is accepted?

cc @JojoS62

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 17, 2022

Asynchronous API doesn't work correctly in SPI IP version 1 (STM32F103C8 and STM32F411CE) due HAL library usage that
doesn't implement correct data reading with interrupts (it doesn't disable SPI in time, that causes dummy reads).

Is this known issue or should be reported?

@mergify mergify bot added needs: CI and removed needs: review labels Jan 17, 2022
@jeromecoutant
Copy link
Collaborator

Asynchronous API doesn't work correctly in SPI IP version 1 (STM32F103C8 and STM32F411CE) due HAL library usage that
doesn't implement correct data reading with interrupts (it doesn't disable SPI in time, that causes dummy reads).

Is this known issue or should be reported?

https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/README.md#asynchronous-spi-limitation

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.

Impressive work!
Thx

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 17, 2022

Impressive work!

Tested impressively 💯

@mbed-ci
Copy link

mbed-ci commented Jan 17, 2022

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

@0xc0170 0xc0170 merged commit 1443257 into ARMmbed:master Jan 17, 2022
@mergify mergify bot removed the ready for merge label Jan 17, 2022
@vznncv
Copy link
Contributor Author

vznncv commented Jan 17, 2022

Asynchronous API doesn't work correctly in SPI IP version 1 (STM32F103C8 and STM32F411CE) due HAL library usage that
doesn't implement correct data reading with interrupts (it doesn't disable SPI in time, that causes dummy reads).

Is this known issue or should be reported?

https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/README.md#asynchronous-spi-limitation

Not exactly. With 4-wire or 3-wire (write only) mode you can use high frequency SPI clock signal, but due interrupt routine overhead there will be delays between frames. It significantly reduces average SPI speed and increases CPU usage, but it is still correct SPI communication and you don't lose any data.

But the mentioned issue is related only with 3-wire mode and data reading: in this case STM SPI IP v1 generates SPI clock signal and receives frames continuously until SPI is disabled programmatically. It means that after receiving of last SPI frame, SPI device should be disabled immediately before next SPI clock cycle. Unfortunately HAL library IRQ implementation isn't fast enough (even with "release" build), so SPI starts receiving next frame (in rare cases I got up to several dozens of extra frames). Usually it doesn't hinder many SPI sensors/modules, but if you read data from sensor/module buffer, you will lose some data. It may occurs even with low SPI speed (400 kHz).

Is this known issue or should be reported?

I don't see any github issue about it. If it's needed I can create a new one.

related pull request: #15115

We can close #15115 if this is accepted?

cc @JojoS62

I'd prefer to wait @JojoS62. This PR contains only fixes without SPI state logic changes, whereas @JojoS62 wanted to implement some SPI enabling/disabling optimizations.

@JojoS62
Copy link
Contributor

JojoS62 commented Jan 24, 2022

thanks, I had no time to work on this further. I think the fix is ok, for optimization the transfer function should be used.

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.

7 participants