Use associated Word types

[GrantM11235]

No description provided.

[Rahix]

To me, it made a lot of sense to have the Word type be a generic parameter because there are e.g. SPI peripherals where you can write out bytes, half-words, or entire words at once. Making Word an associated type means it is no longer possible to provide all of those implementations for a single driver type. Instead, you'd have to make the word size a generic parameter on the driver, e.g. like

impl spi::Write for SpiDriver<u8> {
    type Word = u8;
}

impl spi::Write for SpiDriver<u16> {
    type Word = u16;
}

impl spi::Write for SpiDriver<u32> {
    type Word = u32;
}

To me, this would be a very odd API because most people will need to use SpiDriver<u8> with this, if there is even just one situation where a message size is not divisible by 2 (or 4).

At that point, I'm questioning whether the Word type is even needed at all. Can't we make the traits always take a byte-slice (i.e. Word = u8) and have the HAL drivers figure out which write mode to use? This would mean maximum compatibility because generic drivers don't have to support multiple word-sizes to work with every HAL driver either.

[GrantM11235]

My reasoning for making this change is that although there is plenty of hardware and HAL drivers that support multiple word sizes, I don't know of any that support multiple word sizes at the same time. Doing so would require either

• (A) Hardware that doesn't use a configuration register to set the word size, for example by differentiating between 8 bit and 16 bit access to the data register or by having separate 8 bit and 16 bit data registers.
• (B) A HAL driver that sets a configuration register to the appropriate word size every time a trait method is called. For nb::spi::FullDuplex this would be every word that is written and every word that is read. (Furthermore, I have no idea what would happen if you tried to write an 8 bit word and then read a 16 bit word.)

I believe that

• A is rare, perhaps nonexistent.
• B is currently nonexistent and also a bad idea for performance reasons.
• As a result that, generic device drivers do not and will not make use of interfaces that support multiple word sizes at the same time.

Because of this, I believe that it makes sense to use an associated type.

This change has very little effect on driver structs that are generic over word type, for example

impl<WORD> spi::Write<WORD> for SpiDriver<WORD> {
    /* ... */
}

simply becomes

impl<WORD> spi::Write for SpiDriver<WORD> {
    type Word = WORD;
    /* ... */
}

When I brought this up in the last meeting, @Dirbaio said:

To me it seems that we're losing the flexibility of impling Write<u8> and Write<u16> at the same time, but not gaining anything in exchange

To be honest, I'm not sure if I have a good answer for this. I would argue, however, that generic device drivers should not be written to require Write<u8> + Write<u16> because that would restrict them to only being usable with HALs that support it (currently none as far as I know) and even then it would probably reduce performance. If it is really necessary to change the word size at runtime in a generic device driver, perhaps we need a new trait to represent that.

[Rahix]

Just for reference, (A) does exist, take the MAX32666's SPI as an example (UG Section 14).

But as I wrote at the end of my previous comment, I am not sure whether the Word parameter shouldn't be dropped altogether (at least for SPI). Instead, I think it might be better to only provide essentially the equivalent of Write<u8>. Look at it this way: embedded-hal exists to make most hardware platforms compatible with most generic peripheral drivers. For doing this, the provided interfaces should the lowest common denominator (and specialized, hardware-specific interfaces can always be implemented in the HAL alongside them). For SPI (at least to my understanding) this would be byte-level IO (= Write<u8>). By not custom-casing the word-type, we guarantee that every generic driver expecing the Write trait will work with every HAL providing an implementation. Going with your examaple, there won't be any drivers which need Write<u16> and thus fail to work with certain HALs which only provide a mininmal u8 implementation. On the other hand, it also means HAL code can make its own decisions about which word-size to use for a transfer (or even switch it dynamically). Thus the hardware-specific decisions are made in the HAL and our generic drivers need not care about the actual word-size used.

Now for other communication interfaces I am less certain. Especially USART which can accomodate 9-bit characters certainly needs some way to represent those...

[therealprof]

Just for reference, (A) does exist, take the MAX32666's SPI as an example (UG Section 14).

Uhm, Section 14 talks about QSPI which is a completely different beast.

I'm not sure how to best address this, but supporting multiple word sizes simultaneously makes a ton of sense if you have a number of SPI devices connected which require different word sizes. Indeed this is often a mode of the SPI peripheral so I'd expect that if a MCU would support that it would (a) need to keep track of the current mode and (b) need to be able to switch to a different mode when such a transfer is requested. It seems to me that having a typestate on the impl (i.e. being generic over the wordsize) would make the most sense.

[adamgreig]

supporting multiple word sizes simultaneously makes a ton of sense if you have a number of SPI devices connected which require different word sizes

Since the current system can only support 8/16/32/64 bit words anyway, though, you can always use multiple 8-bit transfers to perform a larger SPI transfer, there's no difference from the SPI device's point of view. It's marginally less efficient for the microcontroller, but that's it. A 24-bit transfer can't be represented in the current system but can be done as three u8 transfers just fine. Plus, the current system doesn't give any way to control endianness, which can be different between different SPI devices.

But as I wrote at the end of my previous comment, I am not sure whether the Word parameter shouldn't be dropped altogether (at least for SPI). Instead, I think it might be better to only provide essentially the equivalent of Write.

I think I like this idea too now. It's simplest and as far as I can tell supports the exact same set of SPI devices (i.e. those that take a multiple of 8 bit word sizes).

---

That said, how some HALs implement this today is by making the SPI driver generic over a word size as well, and then the type of the driver is inferred from the type of the call to write, so a user has:

let data = [1u16, 2, 3, 4];
let driver = Spi::new();
driver.write(&data);

and driver gets type Spi<u16> by inference, and that only implements write<u16>, so you can't do a runtime swap; the configuration for word size is made once in new() and not on each write. So with the current trait a HAL doesn't have to support runtime reconfiguration, and assuming the HAL provides Spi and Spi and Spi then device drivers that want a specific size are probably compatible too.

[Rahix]

@therealprof: Uhm, Section 14 talks about QSPI which is a completely different beast.

Which (for this MCU) supports

• 3-wire and 4-wire SPI operation for single-bit communication
• Single, Dual and Quad I/O

---

@adamgreig: Since the current system can only support 8/16/32/64 bit words anyway, though, you can always use multiple 8-bit transfers to perform a larger SPI transfer, there's no difference from the SPI device's point of view. It's marginally less efficient for the microcontroller, but that's it. A 24-bit transfer can't be represented in the current system but can be done as three u8 transfers just fine.

This was exactly my line of thinking. We're making it harder on ourselves than we need to here; the embedded-hal trait should be as simple as possible to provide the widest range of intercompatibility. If you really care about performance, you're not going through this trait anyway - direct interaction with the HAL driver & DMA are the only sane options for that.

[Rahix]

... and thinking about user convenience, we could provide defaulted methods for other word-sizes:

pub trait Write {
    fn write(&mut self, buf: &[u8]);
    fn write_u16_be(&mut self, buf: &[u16]) { /* ... */ }
    fn write_u16_le(&mut self, buf: &[u16]) { /* ... */ }
    fn write_u32_be(&mut self, buf: &[u32]) { /* ... */ }
    fn write_u32_le(&mut self, buf: &[u32]) { /* ... */ }
}

maybe with num-traits/other trait magic it could even be possible to implement a

fn write_be<I: ...>(&mut self, buf: &[I]) { ... }
fn write_le<I: ...>(&mut self, buf: &[I]) { ... }

[GrantM11235]

Just for reference, (A) does exist, take the MAX32666's SPI as an example (UG Section 14).

See page 317, you must configure the number of bits per word in the QSPIn_CTRL2 register, which supports any number of bits between 1 and 16 (inclusive).

---

[...] supporting multiple word sizes simultaneously makes a ton of sense [...]
[...] having a typestate on the impl (i.e. being generic over the wordsize) would make the most sense.

If the HAL driver struct is generic over the word size, that means that any concrete instance of the struct can only support one word size

---

Since the current system can only support 8/16/32/64 bit words [...]

The current traits, as well as this PR, can in theory support any word size. It just requires HAL authors and generic device driver authors to agree on what types to use to support these other word sizes. They could use arbintrary for example.

---

Some more thoughts in no particular order:

I think there is some confusion about the word size of spi interfaces versus the data sent over an spi interface. For example, an ILI9341 LCD in 4-line spi mode uses an 8 bit interface. Many of its commands involve sending u16s, but in my mind the interface is still fundamentally 8 bit.

It is possible to emulate a 16 bit interface on an 8 bit interface by splitting each u16 into bytes and sending them in the order that matches the bit-endianness of the interface. If the bit-endianness matches the byte-endianness of the processor, this can be trivially done in-place with byte-slice-cast, but the most common case (most-significant-bit-first spi, little-endian cpu) will require byte-swapping each u16 (which cannot be done in-place in an immutable slice).

An ILI9341 LCD in 3-line spi mode uses a 9 bit interface where the first/most significant bit of every word indicates whether the remaining 8 bits are a command or data.

It is not possible to emulate a 9 bit interface on an 8 bit interface unless it is acceptable to add some padding words.

If a generic device driver communicates with two spi devices, it should take two different spi interface structs. If both devices are on the same bus, the user can use two shared_bus::SpiProxys (or something similar), but this also allows for the devices to be on different buses.

To further complicate things, I would like to point out that this PR also affects uart serial, where a 16 bit read or write is not equivalent to two 8 bit transfers.

[adamgreig]

It is possible to emulate a 16 bit interface on an 8 bit interface by splitting each u16 into bytes and sending them in the order that matches the bit-endianness of the interface. If the bit-endianness matches the byte-endianness of the processor, this can be trivially done in-place with byte-slice-cast, but the most common case (most-significant-bit-first spi, little-endian cpu) will require byte-swapping each u16 (which cannot be done in-place in an immutable slice).

Oh, yea, this is a really good point. Of course "native" 16-bit SPI doesn't have an endianness as such because it's thinking about entire words and uses whatever bit order (typically MSbit first as you said) the SPI interface is configured for. So it definitely makes life much harder for a driver that wants 16-bit words to use an 8-bit SPI interface.

The current traits, as well as this PR, can in theory support any word size. It just requires HAL authors and generic device driver authors to agree on what types to use to support these other word sizes. They could use arbintrary for example.

Yea, true. We could even define or re-export some suitable types in embedded-hal.

[Rahix]

See page 317, you must configure the number of bits per word in the QSPIn_CTRL2 register, which supports any number of bits between 1 and 16 (inclusive).

True. I think I got myself totally lost in transaction size and missed that just like USART we also have non-byte-aligned character widths. In that case, disregard my previous comments - we do need a way to represent this. Sorry for the noise.

The current traits, as well as this PR, can in theory support any word size. It just requires HAL authors and generic device driver authors to agree on what types to use to support these other word sizes. They could use arbintrary for example.

I think this is extremely imporant. embedded-hal should dictate which types to use for word-size so we can establish compatibility. If we don't, everyone will just roll their own and drivers won't work with HALs.

As @adamgreig said, I would also suggest that we either define our own wrapper types or re-export relevant ones from another crate like the arbintrary you mentioned (although we should be careful about adding new dependencies).

---

Now, back to the point of this PR, I'm still not convinced of having the character size associated with the entire peripheral. SPI is a bus with possibly many devices connected, which each need their own "configuration". This can mean different character sizes as well. So I would argue that the character size is a parameter of a transfer, not the entire peripheral. Specifically I do not agree with this:

If a generic device driver communicates with two spi devices, it should take two different spi interface structs.

We want a single electrical connection to the connected peripheral so from our side it is one SPI bus, not multiple. The difference is just that some transfers use different configuration. For me, the conclusion is that the generic driver gets one bus-handle (for example shared via shared-bus) which then implements different "transfers" where each one has different configuration. Going further, I think this should also include other settings like the spi-mode as well. Putting it into clear terms, I would like to see an interface like this:

pub trait Write<W> {
    fn write(&mut self, mode: spi::Mode, buf: &[W]);
}

and maybe even further:

pub trait Write<W, CS> {
    fn write(&mut self, mode: spi::Mode, cs_pin: Option<&mut CS>, buf: &[W]);
}

(which would solve race-conditions when the CS pin is managed externally on a shared SPI bus and also allow for hardware CS signal management)

---

In contrast, looking at USART, I specifically think the above does not apply. Here it is more sensible to do what your PR is suggesting as the most common case will certainly be one where the char size stays constant for all uses of the peripheral.

[GrantM11235]

Thinking about this has led me to open #299

[Sh3Rm4n]

It is not possible to emulate a 9 bit interface on an 8 bit interface unless it is acceptable to add some padding words.

I feel like the focus, that many (especially) serial devices support 9 bits instead of 8 and therefor we have to be generic over the word size, is trying to be a bit too generic.

9 bit for serial devices (edit: I mean UART in this example) needs to be supported by the hardware to be able to send a parity bit over the line without having to sacrifice the bandwidth of the data. I can't think of any serial interface where 9 bits are required and the receiving device does also support 9 data bits, but maybe I'm wrong on that part? This feels like a special case, which is not that important, when talking about generic interfaces (like this crate describes).

[GrantM11235]

9 bit for serial devices needs to be supported by the hardware to be able to send a parity bit over the line without having to sacrifice the bandwidth of the data. I can't think of any serial interface where 9 bits are required and the receiving device does also support 9 data bits, but maybe I'm wrong on that part?

Note that this is not a parity bit:

An ILI9341 LCD in 3-line spi mode uses a 9 bit interface where the first/most significant bit of every word indicates whether the remaining 8 bits are a command or data.

[Sh3Rm4n]

An ILI9341 LCD in 3-line spi mode uses a 9 bit interface where the first/most significant bit of every word indicates whether the remaining 8 bits are a command or data.

If it is the MSB, then this should be translatable to a 2 byte spi transaction, where the first byte represents the command or data bit and the second byte includes the actual data.

What my point is: We should really take the interface simple on the e-h because when relying on specific words sizes on the driver side does also mean, that the MCU hal implementation do also have to support the word size.

This is necessarily not true and sometimes makes the implementation to specific / complicated.

IMHO e-h should concentrate on u8-based implementations, because this is the data type on the wire. Every other special case should be constructable via functions which take &[u8] for example and not require another implementation of the trait.

[GrantM11235]

If it is the MSB, then this should be translatable to a 2 byte spi transaction, where the first byte represents the command or data bit and the second byte includes the actual data.

This would not work, the lcd would see this as sending one 9-bit word and then sending the first 7 bits of the next word

[Sh3Rm4n]

If it is the MSB, then this should be translatable to a 2 byte spi transaction, where the first byte represents the command or data bit and the second byte includes the actual data.

This would not work, the lcd would see this as sending one 9-bit word and then sending the first 7 bits of the next word

You mean the first significant byte would be interpreted as 8 bits and only the first bit of the second byte would be recognized? That's a good point.

An ILI9341 LCD in 3-line spi mode uses a 9 bit interface where the first/most significant bit of every word indicates whether the remaining 8 bits are a command or data.

Even though this does make sense on the hardware side, it makes it hard for the developer (for any implementation in- and outside the rust embedded ecosystem I guess) to work with that device, when any slight abstraction comes into play, because it is usually described with basic primitive types. I haven't looked into the datasheet of the ILI9341 LCD, but I hope this is not as a hard constraint, as you make it to be.

I mean, of course, it is desirable to have such a tool in place to write a driver for this device via e-h, but then the mcu hal has to implement 9 bit specifically. And 10bit, 11bit, ... .

But how would you even describe such an interface with primitive types? It doesn't matter if

pub trait Write<Word> {}

or

pub trait Write {
    /// Word type
    type Word;
}

is used, when Word usually only can be u8, u16, u32, ... No 9 bit type available. To support this, a custom 9bit type would be needed.

What I could imagine is support for a "variable bitwidth" type, where you can choose in runtime how many bits are actually sent on the wire with the SPI. Something like:

// This example has to definitely be more fleshed out. 
// I don't know if a generic `Word` would make sense in this context
// and if not, which base type to choose, as `u8` does not cover enough usecases at all (9bit does not work)
pub trait Write<Word> {
    /// ...

    fn write_with_bitwidth(&mut self, buffer: &[Word], bitwidth: u8) -> Result<(), Self::Error>;
}

Or introducing custom bitwidth types, but this seems like a lot of work for the implementation, as these are distinct types
and any type would require a new impl (this is to some extent solvable via macros, but feels ugly)

Either way, I think having an associated type for Word is the wrong way. Associated types usually means that there is only one type per trait implementation. But this is strictly not true. As you can see in my examples above, many types per implementations would be needed.

[Sh3Rm4n]

Source

If it is the MSB, then this should be translatable to a 2 byte spi transaction, where the first byte represents the command or data bit and the second byte includes the actual data.

The operation mode I described is supported by the chip with the 16-bit bus interface.
I would be really surprised if the chip had not such an interface method. Again, as 9 bit seem very special.

[Rahix]

A sidenote here: The WORD type also does not solve the problem entirely. If we provide a type for 9-bit characters, an array of this type is not guaranteed to have the correct layout the hardware expects e.g. for a DMA transfer...

[Sympatron]

Which endianness do drivers expect for Word != u8? It will probably be highly hardware dependent.
If this is not well defined, you can't really write hardware agnostic drivers and if it is well defined it could potentially not be implemented for certain MCUs because the hardware does not support it in this order.

So either way I don't really see the point of the Word size anyway. And 9-bit sizes are IMO so rare that they shouldn't make the interface for 99% of use casesmore complicated than it needs to be.

If you really need it you can still define a special SPIWriteU9 trait in the driver that users need to implement in order to use it. That's not as plug and play, but it's manageable. Common cases should be simple and rare cases can be harder IMO.

[ryankurte]

thanks for the PR! it seems to me that moving to associated types doesn't have substantial benefit to outweigh the inability to represent multiple word sizes or the increased complexity of genericising over associated types. it's also worth consideration i think that the existing approach (which may have room for improvement), does work for a bunch of folks / use cases, and a proposed change would need to demonstrably still achieve / improve on this.

as such, i propose we close this PR and open issues to address any specific points from the discussion here. any thoughts @rust-embedded/hal ?

[GrantM11235]

In conclusion, I still don't believe that there are any real drawbacks¹ to this change, but I can't find any real benefits either.

Footnotes

1. This is a complex topic, so to avoid restarting the discussion we can all just agree to disagree for now 😄 ↩