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

SPI Rev A - SPI Req. from ISR #974

Open
BrentK-ADI opened this issue Apr 2, 2024 · 4 comments
Open

SPI Rev A - SPI Req. from ISR #974

BrentK-ADI opened this issue Apr 2, 2024 · 4 comments

Comments

@BrentK-ADI
Copy link
Contributor

A recent porting project highlighted a potential deficiency in the SPI Rev A driver. The baseline project is heavily interrupt driven and utilized the SPI bus. At points in time, the SPI callback function attempts to issue another SPI transaction from within the interrupt context. This caused their state machine to break.

Looking through, it appears the user's callback is not the last action of the ISR, so the new transaction was corrupting the state of the previous and/or the remainder of the ISR corrupted the new transaction. One example of this is here with operations occuring after TransHandler is called, but there may be others.

A work around was created to check if within the context of an interrupt, save off the SPI request, and send it again when user space was activated. This worked well, and is a more typical solution anyways.

This ticket is intended to start a discussion if this should be looked into closer, or categorize it as an unusual and non-recommended application design choice, and leave as is.

@Jake-Carter
Copy link
Contributor

This is a relatively common scenario we should be able to support. It's an issue that's been fixed in the "V2" version of the SPI drivers, but most of the parts are still on "V1"...

The main SPI effort now is in porting the rest of the parts to the V2 version (we just did the 32690 in #964). It was these types of problems and performance issues that motivated the V2 rewrite to begin with.

I think we should take a look at your workaround to compare against what we've done for V2. If it's straightforward to pull into V1 as well I'm open to it. Otherwise we may want to prioritize V2 a little higher for the rest of the parts.

The MAX32572, MAX32690, and MAX78002 now support V2. What's the part in question here?

@BrentK-ADI
Copy link
Contributor Author

@Jake-Carter, must just have been good timing between this issue and PR#964. This is the MAX32690. I'll try out the commit in #964 to see if it resolves the issue.

FWIW, The work around I have is in application space, not the MSDK, so not sure it adds the value you want.

@Jake-Carter
Copy link
Contributor

@BrentK-ADI any success with the V2 drivers?

@BrentK-ADI
Copy link
Contributor Author

Sorry @Jake-Carter, testing this got lost in the pile. Yes this looks like it resolved the issues I was seeing the V1 SPI with respect to chaining SPI transactions within the interrupt context.

Late to answer the questions:
I think we should take a look at your workaround to compare against what we've done for V2.
My work around is to do things in application space. So detect the intention to do another SPI transaction while handling the previous one by looking at the SCB_ICSR_VECTACTIVE bit to determine our context. And 'defer' it to be run in the next iteration of the app's super loop if within an interrupt. One of the constraints for this project is to use a stock MSDK install so, application work around made the most sense.

The MAX32572, MAX32690, and MAX78002 now support V2. What's the part in question here?
This is the platform layer to support one of our ethernet device's EvKit. So it's target agnostic code (with a couple target specific #ifdefs) to run on the 32650, 32655, 32666, 32672, 32690 and 78000.

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

No branches or pull requests

2 participants