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

High-ish SPI clock speed breaks STM async SPI #13940

Closed
multiplemonomials opened this issue Nov 23, 2020 · 9 comments
Closed

High-ish SPI clock speed breaks STM async SPI #13940

multiplemonomials opened this issue Nov 23, 2020 · 9 comments

Comments

@multiplemonomials
Copy link
Contributor

Description of defect

In my testing, I found that setting a high-ish SPI clock speed (at least 1.5MHz) on my NUCLEO_F429ZI board causes all SPI transactions to fail with SPI_EVENT_ERROR. However, if I set the clock speed to 1.4MHz or lower, my driver works as expected.

After some investigation from the debugger, I found that it's entering this error block here in the HAL's SPI interrupt function.

I believe I understand why this error is happening -- it comes from the way that STMicro has implemented async SPI using interrupts. Their setup relies on the processor being interrupted by the SPI peripheral each time a byte has been transmitted. The interrupt handler (HAL_SPI_IRQHandler) enqueues the next byte in software. However, it seems that once you go above a certain (not actually that high) SPI frequency, the interrupt simply can't keep up anymore. This then causes the overflow error to be generated by the hardware, since the first byte of the message doesn't get transferred by the time the second byte is read.

As far as what can be done about this, I recognize that it's to some extent a design limitation. However, at minimum, it would be very nice if it were documented as an issue somewhere, such as the docstring for SPI::transfer(). Maybe just a note that, "Some processors have a limit on the maximum bus clock speed that can be reached when using asynchronous SPI, especially if other interrupts are triggered during the transfer. If the SPI transfer fails, try reducing the clock speed." Even better would be to add more detailed error codes including something like SPI_EVENT_OVERRUN that indicate to the user what has happened.

There also ways you could try to address the issue directly. Maybe there's some way to optimize the SPI IRQ handler so it requires less instructions to copy the byte and get out? Currently the SPI IRQ seems to take about 100 instructions based on the max achievable speed, which seems like more than would strictly be needed. Even better, however, would be to enable the use of DMA for high speed SPI transactions, which wouldn't tax the CPU at all. I've heard that it's supposed to be on the roadmap soon, right? So fingers crossed.

Target(s) affected by this defect ?

Tested on STM32F429ZI. I assume it affects all STMicro chips, though the max usable frequency depends on the processor clock.

Toolchain(s) (name and version) displaying this defect ?

Tested with GCC_ARM, using the standard debug profile

What version of Mbed-os are you using (tag or sha) ?

mbed-os-6.1.0

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

I'm using mbed-cmake 1.4.1 as my build system, but this should be identical to mbed-cli.

How is this defect reproduced ?

  • Create an SPI object and set its clock speed to a value of 1500000 Hz or more.
  • Start an asynchronous transfer to read bytes from any sensor or other peripheral
  • Observe SPI_EVENT_ERROR being passed to the callback
@ciarmcom
Copy link
Member

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/IOTOSM-2852

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 23, 2020

@ARMmbed/team-st-mcd please review. Any experience with event error?

Their setup relies on the processor being interrupted by the SPI peripheral each time a byte has been transmitted.

Byte transaction instead of transaction ? This does not sound right.

@multiplemonomials
Copy link
Contributor Author

Yeah, it's not what I was initially expecting when I heard "async SPI"... The other side effect of per-byte interrupts is that using async SPI is very CPU intensive and doesn't really let other tasks do very much. My team is having a bit of "buyer's remorse" at the moment since we just chose an ST processor partially because of async SPI support to optimize performance. We're probably going to try and update the old SPI DMA patch I found for Nucleo H7.

@multiplemonomials
Copy link
Contributor Author

Also, that issue I linked seems to indicate that the patch wasn't considered because DMA support hadn't been officially included anywhere in Mbed OS yet. However, in master branch it seems several other targets currently implement async SPI using DMA. So, is it possible that that concern isn't valid anymore and a patch like that could be considered?

@jeromecoutant
Copy link
Collaborator

Hi

We're probably going to try and update the old SPI DMA patch I found for Nucleo H7.

Yes, check #10057 (comment)

@MubeenHCLite
Copy link
Contributor

@multiplemonomials
The behavior is different on different platforms based on the depth of FIFOs present and MPU operating frequency. But unfortunately there is no formula where we can mention the maximum possible frequency for async SPI for a particular platform.
And currently DMA is not supported by ARM Mbed. Hence there is no scope for efficiency cannot improvement for now.
We are not left with any other solution.

Request you to close the issue.

Thanks

@multiplemonomials
Copy link
Contributor Author

multiplemonomials commented May 3, 2021

I think that the bare minimum to close this would be adding a note to the docs of SPI::transfer() as I mentioned. I can work on a PR to do that.

@0xc0170
Copy link
Contributor

0xc0170 commented May 4, 2021

+1 for stating the limitations in the docs

@MubeenHCLite
Copy link
Contributor

Raised a PR in official repo.
Added information on limitation of async SPI and a pointer to an example of DMA, which can be implemented if required.
#14689

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

No branches or pull requests

6 participants