Skip to content

Conversation

JordanYates
Copy link
Contributor

Add a counterpart to the spi_release function call that locks the context without needing to initiate a transaction. This is intended to be used when SPI bus access needs to be blocked before the device is accessed.

One example of this requirement is power cycling an SD card, where the bus should be kept idle after power up until starting the switch from SD mode to SPI mode. This requires locking the bus before power is applied.

Currently no tests in this PR, there doesn't appear to be any tests of spi_release in the tree, happy to take suggestions on what to do.

@JordanYates JordanYates changed the title spi: add spi_acquire call spi: add spi_acquire API Sep 6, 2025
@zephyrbot zephyrbot added platform: nRF Nordic nRFx platform: STM32 ST Micro STM32 platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) area: SPI SPI bus Release Notes To be mentioned in the release notes labels Sep 6, 2025
decsny
decsny previously requested changes Sep 6, 2025
Copy link
Member

@decsny decsny left a comment

Choose a reason for hiding this comment

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

what is the use case of this? and I didn't understand why the SD card requires locking/acquiring the spi bus

note: the block is over the error codes documentation, I am fine with adding a "symmetrical" API to spi_release

@JordanYates
Copy link
Contributor Author

what is the use case of this? and I didn't understand why the SD card requires locking/acquiring the spi bus

Because the SD card spec has a range of requirements around bus activity when powering up (see below). SPI bus activity to other devices on the bus during this time has a habit of breaking the initialization flow (Due to either activity on the MOSI line while CLK is toggling but CS is low, or any bus activity at all in the supply ramp up time). This is a problem seen on real cards.

image

Add a counterpart to the `spi_release` function call that locks the
context without needing to initiate a transaction. This is intended to
be used when SPI bus access needs to be blocked before the device is
accessed.

One example of this requirement is power cycling an SD card, where the
bus should be kept idle after power up until starting the switch from
SD mode to SPI mode. This requires locking the bus before power is
applied.

Signed-off-by: Jordan Yates <[email protected]>
Add `spi_acquire` implementations for a subset of SPI drivers.

Signed-off-by: Jordan Yates <[email protected]>
Document the new SPI API function, `spi_acquire`.

Signed-off-by: Jordan Yates <[email protected]>
Copy link

sonarqubecloud bot commented Sep 7, 2025

@decsny
Copy link
Member

decsny commented Sep 7, 2025

what is the use case of this? and I didn't understand why the SD card requires locking/acquiring the spi bus

Because the SD card spec has a range of requirements around bus activity when powering up (see below). SPI bus activity to other devices on the bus during this time has a habit of breaking the initialization flow (Due to either activity on the MOSI line while CLK is toggling but CS is low, or any bus activity at all in the supply ramp up time). This is a problem seen on real cards.
image

You said "while CS is low" it causes a problem, and the datasheet you screenshotted says to keep it high for 74 cycles. Isn't then the solution just to keep the CS high? I still don't understand what is the need to lock all other spi device from using the bus, since they have separate CS

@tbursztyka
Copy link
Contributor

I don't see a need for API change for such corner case.

A way to do what you want already I think, if you really need to lock a SPI controller to a user (whatever the reason is), will be to just have a temporary spi_config as a copy of your dts based spi_config but with SPI_LOCK_ON bit set, call spi_write with valid spi_buf_set but a spi_buf of length 0. It should be going through the config, setting the ownership and do nothing on the bus (at least not sending any data) and return now with the caller having the lock until calling spi_release with that same temporary config. No?

Copy link
Contributor

@tbursztyka tbursztyka left a comment

Choose a reason for hiding this comment

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

see my previous comment

@JordanYates
Copy link
Contributor Author

It should be going through the config, setting the ownership and do nothing on the bus (at least not sending any data) and return now with the caller having the lock until calling spi_release with that same temporary config. No?

Maybe in theory, but thats not the case even in the first controller I look at, spi_nrfx_spim.c. Even ignoring whatever transfer_next_chunk does if presented with a data length of 0, it will assert the CS line, which is exactly the opposite of what the SD card wants.

I agree that its a corner case, but supporting it this way is much lower overhead than looking at every driver and checking how it would have a transaction of length 0.

@JordanYates
Copy link
Contributor Author

You said "while CS is low" it causes a problem, and the datasheet you screenshotted says to keep it high for 74 cycles. Isn't then the solution just to keep the CS high? I still don't understand what is the need to lock all other spi device from using the bus, since they have separate CS

Sorry, my comment had it around the wrong way. Yes the CS needs to be high for 74 cycles. The problem is that on actual cards, the MOSI line toggling even while the CS line is high is enough to cause problems, presumably because the card is not yet in SPI mode and the toggling is being treated as part of some SDIO transaction.

@decsny decsny dismissed their stale review September 9, 2025 12:38

addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: SPI SPI bus platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) platform: nRF Nordic nRFx platform: STM32 ST Micro STM32 Release Notes To be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants