Allow creating a Spi Peripheral #2085
Conversation
|
I think that this split makes sense. It seems notable that despite the SAM4L having an implementation for Also, it seems that tock-on-titan had to make changes compared to our capsule to support spi peripheral functionality (https://github.com/google/tock-on-titan/pull/162/files). I support changing the names, though I am not sure if that should be part of this PR or would be better off as an independent PR. I also think that we should consider that the OpenTitan project uses the terms |
There was a problem hiding this comment.
Agreed that this looks good. I think it's always easier to manage smaller changesets. This changes interface/arch design a bit, so let's leave it at that, merge this, and then rename next.
I've seen CIPO/COPI a few other places for SPI as well at this point; it'd be nice to align with the greater community here if we can. Device/Host is a bit overloaded in other contexts I think, but I don't feel terribly strongly.
|
Until specs catch up we are going to have some naming pain. SPI is a little different as there isn't a spec. CIPO/COPI seems to be the way people are going (or the original MISO/MOSI) which is why I picked it. |
|
+1 on splitting SPI device / peripheral and SPI host / controller. I found that having both in the same file makes working with the capsule, HIL and driver code a bit awkward, since you easily end up looking at the "wrong" code. |
|
We talked about naming on the call today, and there was consensus that Controller/Peripheral is a reasonable approach to take for now, and that we can always change again in the future if industry converges on some other terminology. |
There was a problem hiding this comment.
I think it makes sense to leave them both in the capules/spi.rs file, since they are both part of the SPI protocol and we include both peripheral and controller in the hil/spi.rs. I don't think it really makes sense to have the same syscall interface do both roles (unlike in I2C where the protocol does support a single device being both roles), so including them both in the same file is just a convenient grouping around the SPI protocol.
That being said, I don't really care either way and if it makes it cleaner to have two files that's fine. I would like to see the files mirror each other (spi_controller.rs and spi_peripheral.rs), the comment in what is currently spi.rs updated, and the text in capsules/readme.md updated.
Create a SPI Peripheral (SPI Slave) driver number. Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Split out the SpiPeripheral from the SPI capsule, while touching the code let's also rename SpiSlave to SpiPeripheral. Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Rename the SPI controller file to match the spi_peripherla.rs file, no other chagnes to the file have been made. Also update the README. Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
alistair23
force-pushed
the
alistair/spi-perph
branch
from
65408f2
to
9e40b9f
Compare
|
I pushed an update renaming the |
alistair23
force-pushed
the
alistair/spi-perph
branch
from
9e40b9f
to
5c8cc42
Compare
|
bors r+ |
Pull Request Overview
In the context of this pull request the SPI Slave is refereed to as SPI Peripheral and the SPI Master is the SPI Controller. See https://hackaday.com/2020/06/29/updating-the-language-of-spi-pin-labels-to-remove-casual-references-to-slavery/ for details on why I picked these names.
Previously the SPI peripheral was tightly coupled to the SPI controller capsule in Tock. I'm assuming the idea was that a SPI device would support both controller and peripheral functionality. In practice most hardware seems to have a separate module for controller and peripheral (see Apollo3 documentation).
As well as this SPI generally only supports a single controller, so it's unlikely that a single hardware device will swap between controller and peripheral modes. It also doesn't appear like Tock supports that. In theory a virtual SPI capsule could allow one app to send data on the bus as a controller while another app receives data as a peripheral, but that doesn't seem like a workable hardware design.
In light on this let's split out the SPI peripheral to be independent. This makes it possible for OpenTitan (which only supports peripheral mode) and Apollo3 (which has separate controller and peripheral devices) to use the SPI peripheral.
This will allow boards to attach a SPI peripheral and allow apps to access the SPI peripheral.
While touching the code let's also rename SPISlave to SPIPeripheral. I haven't renamed everything, just bits that I'm touching.
Otherwise no changes have been made to the current implementation. That is left for future patches.
Testing Strategy
I have some OpenTitan patches on top of this that create a SPI peripheral device that I have used for testing. I can now get userspace apps to call the SPI peripheral device functions (which aren't implemented yet).
TODO or Help Wanted
Documentation Updated
/docs, or no updates are required.Formatting
make prepush.