[riot-notifications] [RIOT-OS/RIOT] stm32-common/spi: allow custom pin modes on spi to minimize power consumption (#11542)

Francisco notifications at github.com
Fri May 17 19:21:53 CEST 2019


fjmolinas commented on this pull request.

Although PUSH-PULL shouldn't need PU/PD to achieve GND/VDD I've seen that in stm32 boards when GPIO's are defined as AF an that AF is deactivated (e.g. releasing SPI) the pins do no go completely to GND or VDD. This means that although keeping their inactive state should be enough to minimize consumption some current leak is seen because of the GPIOS having a voltage of ~1mV instead of 0. So I agree with the idea behind this PR although it might only be useful for stm32 (don't know if this problem is present in other families).

I'm not sure on where pin modes are handled though, see comments below.


> @@ -226,6 +226,11 @@ static const spi_conf_t spi_config[] = {
         .miso_pin = GPIO_PIN(PORT_A, 6),
         .sclk_pin = GPIO_PIN(PORT_A, 5),
         .cs_pin   = GPIO_UNDEF,
+#ifdef MODULE_PERIPH_SPI_GPIO_MODE

I think by default this should be set as it is now, so:

```
        .mosi_pin_mode = (GPIO_OUT),
        .miso_pin_mode = (GPIO_IN),
        .sclk_pin_mode = (GPIO_OUT),
```

But in concerned driver add `FEATURES_OPTIONAL = periph_spi_gpio_mode`. And change the gpio mode accordingly.

IMO having it as a default configuration for a board isn't the best. If an spi device uses CPOL=1 you would want to have a PU and not PD. A driver could have pull ups or pull down on the board witch could affect everything, for example some devices can operate ass SPI/I2C and would then have pull ups, if you pull it down by default the current leak will be higher.

I think you could implement this in a way where a device driver can modify the default mosi/miso/sclk pin mode. I'm not sure how this would be implemented... I can think of using CFLAGS (not ideal) or adding an spi function that changes the pins modes having a separate structure for this?

```
void spi_init_pins_mode(spi_t bus, spi_pin_mode_t mode)
{
    spi_config[bus].mosi_pin_mode = mode.mosi;
    spi_config[bus].miso_pin_mode = mode.miso;
    spi_config[bus].sclk_pin_mode = mode.sclk;
}

// or just

void spi_init_pins_mode(spi_t bus, spi_pin_mode_t mode)
{
    gpio_init(spi_config[bus].mosi_pin, mode.mosi);
    gpio_init(spi_config[bus].miso_pin, mode.miso);
    gpio_init(spi_config[bus].sclk_pin, mode.sclk);
}
```

There could still be conflicts between spi devices but that is up to the user to realize that using spi_devices with different inactive state is not a good idea.

What do you think?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11542#pullrequestreview-239040828
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190517/b50910d4/attachment.html>


More information about the notifications mailing list