Add miso pullup to spi configuration

[joelsa]

Closes #2798 by adding the MISO-pull-up as an option in the Config struct.

The only interesting bit is that in order for get_current_config to still work, I added a pull() function to the SealedPin trait:

    /// Get the pull-up configuration.
    #[inline]
    fn pull(&self) -> Pull {
        critical_section::with(|_| {
            let r = self.block();
            let n = self._pin() as usize;
            #[cfg(gpio_v1)]
            {
                let crlh = if n < 8 { 0 } else { 1 };
                match r.cr(crlh).read().mode(n % 8) {
                    vals::Mode::INPUT => match r.cr(crlh).read().cnf_in(n % 8) {
                        vals::CnfIn::PULL => match r.odr().read().odr(n % 8) {
                            vals::Odr::LOW => Pull::Down,
                            vals::Odr::HIGH => Pull::Up,
                        },
                        _ => Pull::None,
                    },
                    _ => Pull::None,
                }
            }
            #[cfg(gpio_v2)]
            {
                match r.pupdr().read().pupdr(n % 8) {
                    vals::Pupdr::FLOATING => Pull::None,
                    vals::Pupdr::PULLDOWN => Pull::Down,
                    vals::Pupdr::PULLUP => Pull::Up,
                    vals::Pupdr::_RESERVED_3 => Pull::None,
                }
            }
        })
    }

I hope that I captured all the cases correctly there. Is it okay for the pull() function to reside inside the SealedPin?

Thje only missing thing (and why this is a draft currently) is that it relies on the MODE bits being set correctly for gpio v1, in order to determine whether the pin is an input or output and this is broken currently because of this bug: #2942

Since it needs changes in the same files, I also tried to mitigate #2942 in a quick way by introducing the Speed::Input so that the set_speed function does not overwrite the input. Speed::None might be a better name though, and I feel like this solution is not ideal, but it is non-invasive. -> Will be moved into separate PR.

[Dirbaio]

Looks good. I'd like to suggest a few additions:

• Add pull fields for mosi, miso, sck, for consistency.
• make the config fields of type Pull. So you can set it to None (default), Up, Down. Reason is who knows, there might be other weird SPI chips which require pull-down.
  • note pullup: bool makes sense for i2c because you never want pulldown for i2c, but this is not the case for spi. This justifies the inconsistency in config between spi and i2c.

[Dirbaio]

Add pull fields for mosi, miso, sck, for consistency.

uhhh sorry for the confusion, I think this doesn't make sense. MOSI and SCK are always driven by the MCU, so the pull would never affect anything, right?

[joelsa]

uhhh sorry for the confusion, I think this doesn't make sense. MOSI and SCK are always driven by the MCU, so the pull would never affect anything, right?

If we think of the MCU as the Master and don't consider the other way, then yes. For the slave-case, one can imagine a Master that want's a pull-up on MISO in order to mimic some sort of chip and I can imagine adding an internal pull to the pins to avoid floating states, but I think we should think about this after #2750 is merged.

So far I've only come accross chips that need a MISO pull-up. The aforementioned MCP33131-10 needs constant VDIO on MOSI as well (it's read-only), but I always solved this in hardware by hardwiring it until now. I guess it might be good to not think too much about these obscure cases, as they are irrelevant for 99.9% of users.

But changing the bool to Pull even makes the code easier (eliminates two match statements), so this seems senible.

[Dirbaio]

but I think we should think about this after #2750 is merged.

SGTM. The SPI slave will probably have a different Config struct anyway.

[Dirbaio]

this shold be miso_pull now.

[Dirbaio]

lgtm, is there any reason this is still draft?

[joelsa]

Yes, I wanted to wait on #2996 being merged :)

[Dirbaio]

ah cool cool

[joelsa]

Wait, can you still abort the merge queue? I might have missed something.

[joelsa]

This modulo is wrong there I think:

embassy/embassy-stm32/src/gpio.rs

Line 702 in 5cba978

match r.pupdr().read().pupdr(n % 8) {