rp: Allow SPI reset, slave mode

[bruwozniak]

This PR adds the ability to configure SPI as slave on the rp2040 as well as a function to reset the PL022 SPI device to a known state, re-applying the original configuration.
I guess something similar was asked for in #1169.

[Dirbaio]

Slave mdoe shouldn't be a bit in config, it should be a different driver with a differrent API. The reason is the API for master mode doesn't work well for slave mode. Also the embedded-hal traits are for master mode only, they shouldn't be implemented for an SPI in slave mode. See rust-embedded/embedded-hal#396

What's the use case for reset()? as long as the user doesn't touch the registers directly it should never be needed AFAICT.

[bruwozniak]

Hey @Dirbaio, I'll try to explain my rationale here. I am dealing with fairly weird implementation of SPI where the device I am implementing has to connect to master, but there's another line that's used to signal readiness to receive by the slave. When this ack pin is set high, the master starts pulsing the clock and transmission is made. When you exchanged the data, you are then supposed to set the ack low so the master stops the clock.
This works fine if I use slave mode with the current API, using the async DMA transfers. I setup the channels, set ack high, await for the symmetrical transmission, set ack low.
I understand your reservations about the confusing API design for typical slave mode, so I think I would be able to work around it in my app by writing to the register before setting up SPI, because the current setup code does not reset the device (maybe it should? the official C SDK is doing that). So I am happy to drop the slave flag, or follow any other suggestion.

Regarding the need for reset, after the transmission, because the master clock halt seems unpredictable, it may leave some bytes in the inbound FIFO, and even worse, I believe there are sometimes bits left over in the shift register. So when the clock resumes for next transmission, I get shifted bytes. The only way I found to deal with this is to reset the device, which is quite quick. However then the need arises to re-apply the original config. I think in this case, to do it outside of the API is very difficult because 1. reset functions are private, and 2. re-creating the struct is not possible because it holds ownership of the channels. So I'd have to deal with raw registers again, mirroring what the configure fn does.

Sorry for the long comment but I hope this sheds some light on my motivation. Would be curious to hear your suggestions here. Thanks for the awesome project by the way!

[Dirbaio]

This works fine if I use slave mode with the current API, using the async DMA transfers. I setup the channels, set ack high, await for the symmetrical transmission, set ack low.

Yes, the current API works for some slave scenarios (if you know the transfer lengths upfront). However, IMO we should have one SPI master API that works for all master use cases, and one slave API that works for all slave use cases.

also, the SPI master driver implements the embedded-hal SPI master traits. If you add a "slave mode" bit to it, a user could construct one in slave mode and pass it to a driver expecting master mode, breaking things. SPI slave should be a separate struct so it doesn't impl the master traits.

because the master clock halt seems unpredictable, it may leave some bytes in the inbound FIFO, and even worse, I believe there are sometimes bits left over in the shift register

IMO it feels like a workaround for using the SPI master driver in slave mode. If we had a slave-specific driver it could be handled automatically (like, reset everything when CS goes high)

[bruwozniak]

alright, I am convinced. I will close the PR, just one last question: what do you think of making at least the reset functions pub so they could be used from outside of the crate in a manual way? not as part of the master API, only separately