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

Fix STM32 SPI 3-wire (synchronous API) #14981

Merged
merged 4 commits into from
Sep 2, 2021

Conversation

vznncv
Copy link
Contributor

@vznncv vznncv commented Aug 5, 2021

Summary of changes

Fixes #14774

Problem

Almost all STM32 MCU families (except STM32H7) continuously generates clock signal, till SPI is disabled in 3-wire
receiving mode. It means that SPI should be disabled by software during last byte receiving.

It imposes the following limitations of the code that receive SPI data:

  • all interrupts (even systick timer) should be disabled to prevent any accidental delays.
  • the SPI receiving code should be "fast". Any extra error/state checks may cause that delay and SPI won't be disabled
    in time.

If SPI isn't be disabled in time, it starts generating next clock signal for next frame, and you get "dummy" read.

In some cases "dummy" reads don't hinder communication with devices/sensors, but in other cases it's crucial. For
example data reading from sensor FIFO. In this case any "dummy" reads cause data lost.

Current 3-Wire implementation uses STM HAL library functions HAL_SPI_Receive/HAL_SPI_Transmit that do many
error/state checks, but unfortunately they aren't fast enough to prevent dummy reads.

Fix

This pull request contains implements the following algorithm for SPI data receiving in 3-wire mode:

  1. Enter into critical section (disable all interrupts).
  2. Enable SPI to start clock generation.
  3. Wait at least one SPI clock cycle (STM32 SPI requirements for data receiving in 3-wire mode).
  4. Disable SPI.
  5. Exit from critical section.
  6. Wait till full SPI frame is received.
  7. Read data from SPI register.
  8. If more data is needed to receive, then go to step 1.

Notes:

  1. Unlike HAL_SPI_Receive, that disables SPI after byte receiving, this approach disables SPI during byte receiving,
    so next frame isn't generated and we don't get dummy read.

  2. Asynchronous SPI API still causes dummy reads. I tried to implement some approaches, but didn't get any success.

Tests

Currently, I have only tested this fix using demo project https://github.com/vznncv/mbed-os-stm32-spi-3-wire-demo with
BMX160 sensor and the following MCUs:

  1. STM32F103C8T6 (STM32 Blue Pill board; custom board port: https://github.com/vznncv/TARGET_BLUEPILL_F103C8)
  2. STM32F411CEU6 (STM32 Black Pill board https://github.com/WeActTC/MiniSTM32F4x1; custom board
    port: https://github.com/vznncv/TARGET_BLACKPILL_F411CE)
  3. STM32H743VIT6 (STM32 WeAct board https://github.com/WeActTC/MiniSTM32H7xx; custom board
    port: https://github.com/vznncv/TARGET_WEACT_H743VI)

The demo project simply reads and writes to BMX160 register (including burst reads/writes) to check that SPI works
correctly. Additionally, it counts SPI clock cycles using STM32 timer ETR input to detect "dummy" reads.

Test project results
  1. STM32F103C8T6

    Output logs:

    Results

    profile API type SPI frequency results data read/write operations are correct dummy reads absent
    debug synchronous 281250 Hz OK OK OK
    debug synchronous 562500 Hz OK OK OK
    debug synchronous 9000000 Hz ERROR ERROR ERROR
    debug asynchronous 281250 Hz ERROR OK ERROR
    debug asynchronous 562500 Hz ERROR OK ERROR
    debug asynchronous 9000000 Hz ERROR ERROR ERROR
    release synchronous 281250 Hz OK OK OK
    release synchronous 562500 Hz OK OK OK
    release synchronous 9000000 Hz OK OK OK
    release asynchronous 281250 Hz ERROR OK ERROR
    release asynchronous 562500 Hz ERROR OK ERROR
    release asynchronous 9000000 Hz ERROR ERROR ERROR
  2. STM32F411CEU6

    Output logs:

    Results

    profile API type SPI frequency results data read/write operations are correct dummy reads absent
    debug synchronous 390625 Hz OK OK OK
    debug synchronous 781250 Hz OK OK OK
    debug synchronous 12500000 Hz OK OK OK
    debug asynchronous 390625 Hz ERROR OK ERROR
    debug asynchronous 781250 Hz ERROR OK ERROR
    debug asynchronous 12500000 Hz ERROR ERROR ERROR
    release synchronous 390625 Hz OK OK OK
    release synchronous 781250 Hz OK OK OK
    release synchronous 12500000 Hz OK OK OK
    release asynchronous 390625 Hz ERROR OK ERROR
    release asynchronous 781250 Hz ERROR OK ERROR
    release asynchronous 12500000 Hz ERROR ERROR ERROR
  3. STM32H743VIT6

    Output logs:

    Results

    profile API type SPI frequency results data read/write operations are correct dummy reads absent
    debug synchronous 156250 Hz OK OK OK
    debug synchronous 625000 Hz OK OK OK
    debug synchronous 5000000 Hz OK OK OK
    debug asynchronous 156250 Hz OK OK OK
    debug asynchronous 625000 Hz OK OK OK
    debug asynchronous 5000000 Hz OK OK OK
    release synchronous 156250 Hz OK OK OK
    release synchronous 625000 Hz OK OK OK
    release synchronous 5000000 Hz OK OK OK
    release asynchronous 156250 Hz OK OK OK
    release asynchronous 625000 Hz OK OK OK
    release asynchronous 5000000 Hz OK OK OK

    Note: due current STM32 SPI/FDCAN clock configuration maximal SPI frequency is 5 MHz.

Logic analyzer data

Additionally, I have check communication with sigrock
and logical analyzer.

The configuration:

  • MCU: STM32F411CEU6
  • profile: release
  • frequency: 3125000 Hz

gives the following results:

According logical analyzer, the fix overhead adds small delay between frame receiving (about ~700 ns), but it's
acceptable.

Possible SPI test
  1. FPGA CI TEST shield (https://github.com/ARMmbed/mbed-os/tree/master/hal/tests/TESTS/mbed_hal_fpga_ci_test_shield).

    Probably it's good place for SPI 3-wire tests, and it seems that FPGA shield support half-duplex mode
    (https://github.com/ARMmbed/mbed-os/blob/master/features/frameworks/COMPONENT_FPGA_CI_TEST_SHIELD/include/fpga_ci_test_shield/SPIMasterTester.h#L73)
    , but:

    • it doesn't have a lot of documentation, and I don't understand how to transfer data from FPGA to MCU
    • I don't have any experience with FPGA
    • it's difficult to write test without a test FPGA shield
    • MbedOS doesn't provide any information about SPI 3-wire
      support (https://github.com/ARMmbed/mbed-os/blob/master/hal/include/hal/spi_api.h#L71), so any test with SPI
      3-wire mode will cause failure of devices that doesn't support it.
  2. CI TEST shield (https://github.com/ARMmbed/ci-test-shield)

    Probably loopback pins and STM32 ETR timer inputs can be used for some SPI tests (ETR inputs can be used to count
    clock cycles of SCK and to check MOSI data indirectly), but:

    • D2-D9 have no connection to timer ETR inputs on a Nucleo boards.
    • The tests will be STM32 specific.

So currently there is no possibility to implement 3-wire SPI test. At least with existed CI test shields.

Implementation notes:

I have implemented buffer logic, that is based on existed SPI 4-wire implementation:

int spi_master_block_write(spi_t *obj, const char *tx_buffer, int tx_length,
                           char *rx_buffer, int rx_length, char write_fill)
{
    struct spi_s *spiobj = SPI_S(obj);
    SPI_HandleTypeDef *handle = &(spiobj->handle);
    int total = (tx_length > rx_length) ? tx_length : rx_length;
    if (handle->Init.Direction == SPI_DIRECTION_2LINES) {
        for (int i = 0; i < total; i++) {
            char out = (i < tx_length) ? tx_buffer[i] : write_fill;
            char in = spi_master_write(obj, out);
            if (i < rx_length) {
                rx_buffer[i] = in;
            }
        }
    } else {
    ...

3-wire implementation:

static int spi_master_one_wire_write(spi_t *obj, const char *tx_buffer, int tx_length, char *rx_buffer, int rx_length)
{

...

        for (int i = 0; i < tx_length; i++) {
            msp_wait_writable(obj);
            msp_write_data(obj, tx_buffer[i], bitshift);
        }
        
...

        for (int i = 0; i < rx_length; i++) {
            core_util_critical_section_enter();
            LL_SPI_Enable(SPI_INST(obj));
            /* Wait single SPI clock cycle. */
            wait_ns(baudrate_period_ns);
            LL_SPI_Disable(SPI_INST(obj));
            core_util_critical_section_exit();

            msp_wait_readable(obj);
            rx_buffer[i] = msp_read_data(obj, bitshift);
        }

...
}

But I'm not sure that it's correct, as it seems that it doesn't handle data correctly if SPI frame size isn't 8 bits.

Impact of changes

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

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

Reviewers

@LMESTM @jeromecoutant


@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Aug 5, 2021
@ciarmcom
Copy link
Member

ciarmcom commented Aug 5, 2021

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

@@ -602,6 +606,24 @@ static const uint32_t baudrate_prescaler_table[] = {SPI_BAUDRATEPRESCALER_2,
SPI_BAUDRATEPRESCALER_256
};

static uint8_t spi_get_baudrate_prescaler_rank(uint32_t value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you aling { to a new line - style consistency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@ciarmcom ciarmcom added the stale Stale Pull Request label Aug 11, 2021
@ciarmcom
Copy link
Member

This pull request has automatically been marked as stale because it has had no recent activity. @jeromecoutant, @LMESTM, please complete review of the changes to move the PR forward. Thank you for your contributions.

@LMESTM
Copy link
Contributor

LMESTM commented Aug 12, 2021

@vznncv thanks for your great efforts in sharing this code.
The test results however do not look to be all OK, or am I misreading the test results table ?
I will have a look at the implementation proposal and review but this seems to bring in a lot of changes and I am a bit afraid of possible side effects :-\

Copy link
Contributor

@LMESTM LMESTM left a comment

Choose a reason for hiding this comment

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

Few questions or comments
I'd like to be able to review the change in a way that it impacts only the 3-wire mode. Maybe this would require to split into 2 commits, 1 for refactoring the code with inline functions. Another one for introducing the one_wire_write function

static uint8_t spi_get_baudrate_prescaler_rank(uint32_t value)
{
#if TARGET_STM32H7
return value >> 28 & 0x07;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to see HAL defines rather than hard coded values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced with switch...case statement.

#if TARGET_STM32H7
return value >> 28 & 0x07;
#else /* TARGET_STM32H7 */
return value >> 3 & 0x07;
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

SPI_HandleTypeDef *handle = &(spiobj->handle);

int freq = spi_get_clock_freq(obj);
uint8_t baudrate_rank = spi_get_baudrate_prescaler_rank(handle->Init.BaudRatePrescaler);
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't that possible to re-use LL macros like LL_SPI_GetBaudRatePrescaler or HAL equivalent ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LL_SPI_GetBaudRatePrescaler returns some prescaler constant, but not prescaler value itself. So extra code that converts this constant to prescaler rank itself is needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok !

/**
* Check if SPI is busy in master mode.
*/
static inline int msp_busy(spi_t *obj)
Copy link
Contributor

Choose a reason for hiding this comment

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

what does msp mean ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The are some slave SPI mode related functions with ssp_ prefix (ssp_readable, ssp_writable, etc.). Probably it's abbreviation of slave SPI peripheral (or something like that), so I add msp_ prefix (master SPI peripheral) for helper functions of master SPI mode.

LL_SPI_StartMasterTransfer(SPI_INST(obj));
#endif /* TARGET_STM32H7 */

/* Transmit data */
Copy link
Contributor

Choose a reason for hiding this comment

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

I would have preferred that only the 3-wires case is modified.
Or maybe create a first commit that introduce new inline functions, then introduce the 3-wires changes ... ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. The changes are splitted into multiple commits (inline functions, 3-wires changes for synchronous API, some improvements for 3-wire with asynchronous API)

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks

* @param rx_length number of bytes to read, may be zero
* @return number of transmitted and received bytes or negative code in case of error.
*/
static int spi_master_one_wire_write(spi_t *obj, const char *tx_buffer, int tx_length, char *rx_buffer, int rx_length)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this should actually be called "one_wire_transfer" ? as it does manage tx and rx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Yes, "_transfer" suffix is more meaningful.
note: I used "_write" suffix for consistency with mbed function spi_master_block_write function (it has _write suffix and manages rx and tx).

@0xc0170 0xc0170 removed the stale Stale Pull Request label Aug 12, 2021
@vznncv
Copy link
Contributor Author

vznncv commented Aug 15, 2021

The test results however do not look to be all OK, or am I misreading the test results table ?

Yes, my changes don't fix all 3-wire SPI issues:

  • high SPI frequency and debug build may cause problems (I have such issue with STM32F103 MCU)
  • I improved stability of asynchronous SPI API to prevent wrong data reading from registers, but it still have problems with dummy reads. The HAL function HAL_SPI_Receive_IT doesn't implement correct data receiving for 3-wire SPI (all STM32 series except H7) and causes dummy reads. Currently I don't have enough time to write correct/cross family replacement for HAL_SPI_Receive_IT.

@ciarmcom ciarmcom added the stale Stale Pull Request label Aug 18, 2021
@ciarmcom
Copy link
Member

This pull request has automatically been marked as stale because it has had no recent activity. @jeromecoutant, please complete review of the changes to move the PR forward. Thank you for your contributions.

Copy link
Contributor

@LMESTM LMESTM left a comment

Choose a reason for hiding this comment

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

thanks for this new version, this really helps review.
Previous comments have been covered, I have a few new ones now

SPI_HandleTypeDef *handle = &(spiobj->handle);

int freq = spi_get_clock_freq(obj);
uint8_t baudrate_rank = spi_get_baudrate_prescaler_rank(handle->Init.BaudRatePrescaler);
Copy link
Contributor

Choose a reason for hiding this comment

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

ok !

LL_SPI_StartMasterTransfer(SPI_INST(obj));
#endif /* TARGET_STM32H7 */

/* Transmit data */
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks

@@ -866,15 +866,6 @@ int spi_master_write(spi_t *obj, int value)
const int bitshift = datasize_to_transfer_bitshift(handle->Init.DataSize);
MBED_ASSERT(bitshift >= 0);

#if defined(LL_SPI_RX_FIFO_TH_HALF)
Copy link
Contributor

Choose a reason for hiding this comment

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

this change / commit here is an optimization and is not directly related to the SPI 3 wire change. I would prefer this to be a separate Pull Request and to have a full non regression testing before accepting such change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✔️ , the commit is removed from this Pull Request

@@ -1364,10 +1375,15 @@ void spi_abort_asynch(spi_t *obj)
NVIC_DisableIRQ(irq_n);

// clean-up
__HAL_SPI_DISABLE(handle);
LL_SPI_Disable(SPI_INST(obj));
Copy link
Contributor

Choose a reason for hiding this comment

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

How did you come up with this fix ? Is spi_abort_asynch being used in your tests ?
or did you plan to modify init_spi function ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is spi_abort_asynch being used in your tests ?

Yes, I have updated demo project to invoke SPI::abort_all_transfers between sensor API calls. It's invoked when no actual transfers ongoing, but it shouldn't have any side effects. Although my test shows that this call causes data corruption of the subsequent transmissions, so I found and fixed the error in the spi_abort_asynch. Demo project fragment:

https://github.com/vznncv/mbed-os-stm32-spi-3-wire-demo/blob/acadb2e2c213b2b986fec8ac6362e2dbd16749c3/src/main.cpp#L1002-L1008

            // call SPI:spi_abort_all_transfers to check that it has no side effects
            api->call_spi_abort_all_transfers();

            // transmit data to BMX160
            err_write = api->register_write(base_reg, out_buf, buf_len);
            // receive data from BMX160
            err_read = api->register_read(base_reg, in_buf, buf_len);

https://github.com/vznncv/mbed-os-stm32-spi-3-wire-demo/blob/acadb2e2c213b2b986fec8ac6362e2dbd16749c3/src/main.cpp#L778-L781

    void call_spi_abort_all_transfers()
    {
        return _spi.abort_all_transfers();
    }

did you plan to modify init_spi function ?

Unlike spi_abort_asynch the init_spi function already has correct code that doesn't enable SPI in 3 wire mode:

/* In case of standard 4 wires SPI,PI can be kept enabled all time
* and SCK will only be generated during the write operations. But in case
* of 3 wires, it should be only enabled during rd/wr unitary operations,
* which is handled inside STM32 HAL layer.
*/
if (handle->Init.Direction == SPI_DIRECTION_2LINES) {
__HAL_SPI_ENABLE(handle);
}

The only one thing that I have added to init_spi - explicit RX buffer/register cleanup:

    /* In some cases after SPI object re-creation SPI overrun flag may not
     * be cleared, so clear RX data explicitly to prevent any transmissions errors */
    spi_flush_rx(obj);

The necessity of these lines I also discovered by my test project (https://github.com/vznncv/mbed-os-stm32-spi-3-wire-demo/blob/40d1556b049a46e82fc2ed214fd4d5340e1e6814/src/main.cpp). The demo re-creates SPI object for each test run with different parameters (spi frequency, api type). So If previous test run fails, it may left some data in the RX buffer/register that causes failure of subsequent test runs.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok clear !

@ciarmcom ciarmcom removed the stale Stale Pull Request label Aug 19, 2021
@jeromecoutant
Copy link
Collaborator

Hi

My comment from a non-SPI expert :-)

  • "4-wires" SPI tests have been checked with all STM32 families => they are still OK
  • We don't have any 3-wire test setup, so it is not possible yet to check if this PR doesn't break setup with X_NUCLEO_IKS01A2_SPI3W...
  • About your custom boards (BLACKPILL_F411CE and WEACT_H743VI), I advice you to make it public in https://github.com/ARMmbed/stm32customtargets

@ciarmcom ciarmcom added stale Stale Pull Request and removed stale Stale Pull Request labels Aug 25, 2021
@vznncv
Copy link
Contributor Author

vznncv commented Aug 28, 2021

Hi

About your custom boards (BLACKPILL_F411CE and WEACT_H743VI), I advice you to make it public in https://github.com/ARMmbed/stm32customtargets

It looks interesting. I'll check it later.

Copy link
Contributor

@LMESTM LMESTM left a comment

Choose a reason for hiding this comment

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

LGTM - thanks @vznncv for the efforts in contributing !
@jeromecoutant please run the SPI test suite again as the PR has been updated

@jeromecoutant
Copy link
Collaborator

LGTM - thanks @vznncv for the efforts in contributing !
@jeromecoutant please run the SPI test suite again as the PR has been updated

OK, let's merge this one. I will then rebase #15028

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.

Basic SPI non regression tests OK with all STM32 families

0xc0170
0xc0170 previously approved these changes Aug 31, 2021
@vznncv
Copy link
Contributor Author

vznncv commented Sep 1, 2021

Hi @0xc0170

Please check CI/CD. It seems that scancode doesn't work in the current environment. Some investigation:

  1. According logs an installed [click] version is 7.1.2:
Requirement already satisfied: click!=7.0,>=6.7 in /usr/local/lib/python3.8/dist-packages (from scancode-toolkit) (7.1.2)
  1. The scandcode additionally pull a dependency commoncode:
Collecting commoncode>=21.7.23
  Downloading commoncode-21.8.27-py3-none-any.whl (81 kB)
  1. Since recent fix in the commoncode 21.8.27 (https://github.com/nexB/commoncode/pull/27/files) it implicitly requires click>=8 (nexB/commoncode@ef5f0a6), as it adds an update_min_steps option to progress_bar wrappers and passes it to click._termui_impl.ProgressBar. But the update_min_steps is added only since click 8.
  2. It cases the error:
  File "/usr/local/lib/python3.8/dist-packages/commoncode/cliutils.py", line 270, in progressmanager
    return progress_class(
TypeError: __init__() got an unexpected keyword argument 'update_min_steps'

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 1, 2021

@vznncv If you can rebase to the latest master, it should be all green and I'l start our CI

- move a code that waits readable SPI state from `spi_master_write`
  function to inline functions `msp_writable` and `msp_wait_writable`
- move a code that waits writeable SPI state from `spi_master_write`
  function to inline functions `msp_readable` and `msp_wait_readable`
- move a code that writes data to SPI from `spi_master_write`
  function to inline function `msp_write_data`
- move a code that reads data from SPI from `spi_master_write`
  function to inline function `msp_read_data`
All STM32 families except STM32H7 has the following 3-wire SPI peculiarity in master receive mode:
SPI continuously generates clock signal till it's disabled by a software. It causes that a software
must disable SPI in time. Otherwise, "dummy" reads will be generated.

Current STM32 synchronous SPI 3-wire implementation relies on HAL library functions HAL_SPI_Receive/HAL_SPI_Transmit.
It performs some SPI state checks to detect errors, but unfortunately it isn't fast enough to disable SPI in time.
Additionally, a multithreading environment or interrupt events may cause extra delays.

This commit contains the custom transmit/receive function for SPI 3-wire mode. It uses critical sections to
prevents accidental interrupt event delays, disables SPI after each frame receiving and disables SPI during
frame generation. It adds some delay between SPI frames (~700 ns), but gives reliable 3-wire SPI communications.
- add RX cleanup after SPI re-initialization,
  as it isn't implemented in the `HAL_SPI_Init`
- cancel SPI enabling for 3-wire mode
`HAL_SPI_Receive_IT` HAL function causes dummy reads in 3-wire mode,
that causes data corruption in RX FIFO/register. It isn't possible
to fix it without signification refactoring, but we may prevent data
corruption with the following fixes:

- RX buffer/register cleanup after asynchronous transfer in 3-wire mode
- Explicit RX buffer/register cleanup after SPI initialization
  (for cases if we re-create SPI object).
@mergify mergify bot dismissed 0xc0170’s stale review September 1, 2021 19:14

Pull request has been modified.

@vznncv
Copy link
Contributor Author

vznncv commented Sep 1, 2021

@0xc0170

@vznncv If you can rebase to the latest master, it should be all green and I'l start our CI

Done

@ciarmcom
Copy link
Member

ciarmcom commented Sep 1, 2021

This pull request has automatically been marked as stale because it has had no recent activity. @ARMmbed/mbed-os-maintainers, please start CI to get the PR merged.

@ciarmcom ciarmcom added the stale Stale Pull Request label Sep 1, 2021
@jeromecoutant
Copy link
Collaborator

Let's start CI ?

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 2, 2021

CI started

@0xc0170 0xc0170 removed the stale Stale Pull Request label Sep 2, 2021
@mbed-ci
Copy link

mbed-ci commented Sep 2, 2021

Jenkins CI Test : ✔️ SUCCESS

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

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-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 d87183a into ARMmbed:master Sep 2, 2021
@mergify mergify bot removed the ready for merge label Sep 2, 2021
@mbedmain mbedmain added release-version: 6.15.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Sep 16, 2021
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.

STM32F4 SPI 3-wire mode doesn't work correctly
7 participants