Support for different SPI configurations

[katyo]

This is awesome HAL in terms of usage safety than any other I seen before. But it isn't so yet flexible in several cases.

I would like to configure SPI for display driver, which uses unidirectional setup with only SCLK and MOSI lines.

As I can see, the Pins trait implementation for that case is missing.
Since MISO pin in target device already used as GPIO for other purpose, so I cannot use Pins<(sclk, miso, mosi)>.

Also there are some other SPI setups like a bi-directional half-duplex with single data line.

[therealprof]

Hey @katyo, thanks for this great suggestion. Indeed the implementation is rather barebones and lacking a lot of functionality. Unidirectional communication should be rather easy to implement by defining dummy pins and allowing their use like it has been done e.g. here: https://github.com/stm32-rs/stm32f4xx-hal/blob/master/src/spi.rs .

Maybe you'd like to give it a try?

[katyo]

Okay, it looks not so difficult.

[TheZoq2]

This is something I would find useful as well!

Let us know if you run into any issues if you decide to implement it!

[Windfisch]

Hi!
I made an attempt at implementing this, but it turned out more difficult than expected.
The issue is that we cannot just go the stm32f4xx-hal way by implementing traits PinMiso<SPI>, PinMosi<SPI> and PinSck<SPI>, all of which contain the NoMiso/etc. types in addition to the actual pins, and then requiring a tuple of these three traits.
This works for the STM32F4, but not with the remapping system of the F1...

My attempt ultimately fails with conflicting implementation for (_, _, _) in the second impl<SPI, PinMiso, PinMosi, PinSck> Pins<SPI> for (PinMiso, PinMosi, PinSck) clause :(

The only other way I found is to have eight impl pins<SPI1> for (..., ..., ...) { blah = true }, where the ... enumerate all possible combinations of pin and not-a-pin, and then have eight more for the blah=false case, and then eight more for SPI2 :/

Do you have a better idea on how to do this?

[Windfisch]

Okay, this attempt is a bit ugly, since it enumerates all seven possible combinations of "some pin" vs "NoPin", but it looks like it could work. (untested!)
What do you think?

[TheZoq2]

Hi, thanks for working on this!

It is indeed a tricky problem, as I also discovered when doing a similar thing to PWM in #120 which also ended up with some conflicting pin impls.

I think your latest attempt is perfectly fine, it doesn't add a lot of code and does exactly what we want. Additionally, it's not public API and unlikely to change, so even if it's not the neatest solution possible, it does what it should.

[Windfisch]

Thanks for your reply! I'd say I'm adding the same for UART and other peripherals and file it as a PR soon :).

[TheZoq2]

Sounds great, though I do foresee a potential problem if this is implemented.

Me and @therealprof discussed this in the matrix channel a while back, and I'm not sure he agrees, but I figure it might be worth pointing out anyway because I don't see why it wouldn't be a problem.

First, an assumption:

1. The configuration of the peripherals don't change depending on the remapping. I.e., even if NoMosi is passed, the perhipheral still acts as if it had a pin.
2. There is no mechanism to tell an AFIO pin which device to listen to

Look at the following code:

// Both SPI1 and 3 can use these pins
let pb3 = gpiob.into_alternate_push_pull();
let pb4 = gpiob.into_alternate_push_pull();
let pb5 = gpiob.into_alternate_push_pull();
// Set up SPI1 to only be an input. Since the configuration doesn't
// change, it will still be trying to output stuff to pb3 and pb5.
// Normally, this wouldn't be an issue, because the pins wouldn't be in alternate mode
let spi1 = Spi::spi1(..., (NoSck, NoMosi, pb4), ...);
// However, if another peripheral also uses the pins, they will
// need to be in AFIO mode, which means that we now have
// two peripherals that both try to control the 3 pins.
let spi3 = Spi::spi3(... (pb3, pb5, NoMiso), ...);

I'm not sure if there is a neat way to work around this. An easy option might be to just configure the SPI peripheral in RX-only mode if the Mosi pin is not used. However, I don't believe there is a way to not use the clock.

Another option might be to always take all the pins as input, and return new versions of the unused pins which can't be switched into AFIO mode.

[therealprof]

Ugh, I just read the RM0008 about the AFIO operation for the first time; I wasn't aware it's that bad until just now.

I guess the only sane approach here is to create special versions of the remappable pins and only allow those for peripheral use. To switch the pins into a certain mapping you have to turn in all generic pins changed by a mapping flag into a to-be-created-function which will consume all pins, activate the remapping and return the special pins which will prevent that a remapped pin can be used by a foreign peripheral. However to use them as GPIO we'd have to add another version of the pin which allows all GPIO functions but no AF functions.

[therealprof]

An easy option might be to just configure the SPI peripheral in RX-only mode if the Mosi pin is not used.

That's probably a good idea anyway but I don't see how that would prevent conflicts with other peripherals.

However, I don't believe there is a way to not use the clock.

I don't think promiscuous clock use is a big issue which needs solving at the moment.

[TheZoq2]

I guess the only sane approach here is to create special versions of the remappable pins and only allow those for peripheral use. To switch the pins into a certain mapping you have to turn in all generic pins changed by a mapping flag into a to-be-created-function which will consume all pins, activate the remapping and return the special pins which will prevent that a remapped pin can be used by a foreign peripheral. However to use them as GPIO we'd have to add another version of the pin which allows all GPIO functions but no AF functions.

Yep, this was the idea I had as well. It's ugly, and would take a bit of work to implement.

Also, this wouldn't allow the use of multiple peripherals which share a pin, but with that pin disabled by one of them.

That's probably a good idea anyway but I don't see how that would prevent conflicts with other peripherals.

Presumably, the peripheral wouldn't try to control the pins it doesn't use.

Though it does lead to another issue. Should .read be supported on a SPI device which doesn't have a MISO pin? If yes, we introduce UB, and if no, we need special cases for SPI and UART devices which don't have all pins assigned.

promiscuous clock use
I don't really understand what you mean by this. The clock I'm refering to is the SCK output of the SPI devices.