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

Expose all the CS signals (The SPI hardware can take multiple CS pins and turn them on/off for each transfer. Software CS shouldn't be the long term solution for shared buses) #2795

Open
Tracked by #2494
bjoernQ opened this issue Dec 13, 2024 · 3 comments
Labels
1.0 non-breaking Not needed for 1.0 and can be supported without breaking the driver. peripheral:spi SPI peripheral

Comments

@bjoernQ
Copy link
Contributor

bjoernQ commented Dec 13, 2024

No description provided.

@bugadani
Copy link
Contributor

We should probably expose a sort-of CS allocator. Devices should be able to request "CS tokens" (which essentially just stores the number of the CS signal) from the SPI driver, and the SPI driver should expose a setting to configure which CS token is in use, along with KEEP_CS_ACTIVE (#2797).

Something like:

let cs = Output::new(...);
let cs_token = unwrap!(spi.allocate_hw_chip_select(cs));

let device = SpiDevice::new(&spi, DeviceConfig::default().with_hardware_cs(cs_token)); 
// OR:
let device = SpiDevice::new(&spi, DeviceConfig::default().with_gpio_cs(cs));

This would mean a slight inconsistency in terms of how we set up a peripheral signal, but it also prevents routing a chip select to multiple outputs by accident. Since we don't clear existing peripheral output signal connections, that would be all too easy to do with just calling .with_cs0() multiple times.

@tom-borcin tom-borcin added the investigation Not needed for 1.0, but don't know if we can support without breaking the driver. label Dec 16, 2024
@MabezDev
Copy link
Member

Given that we're marking the current with_cs hardware cs method as unstable, and we're not planning on having our own hardware managed SpiDevice impl for 1.0, I'm dropping the label.

FWIW, I like @bugadani's suggestion and I think we'll likely take that route when it comes to implementing this.

@MabezDev MabezDev added 1.0 non-breaking Not needed for 1.0 and can be supported without breaking the driver. and removed 1.0-blocker investigation Not needed for 1.0, but don't know if we can support without breaking the driver. labels Jan 10, 2025
@Dominaezzz
Copy link
Collaborator

This would mean a slight inconsistency in terms of how we set up a peripheral signal, but it also prevents routing a chip select to multiple outputs by accident. Since we don't clear existing peripheral output signal connections, that would be all too easy to do with just calling .with_cs0() multiple times.

Now that #3012 has landed, we can just have a simple with_cs0, with_cs1, with_cs2, etc.
The Config could also store a bitmask of which CS pins to enable/disable, nice and simple.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0 non-breaking Not needed for 1.0 and can be supported without breaking the driver. peripheral:spi SPI peripheral
Projects
Status: Todo
Development

No branches or pull requests

5 participants