Pinmap Pull-Up/Pull-Down Config is Ignored

[mattbrown015]

Description

AFAICT, the pull-up/pull-down config in the STM32 pinmaps has no effect.

For example, from mbed-os\targets\TARGET_STM\TARGET_STM32L4\TARGET_STM32L432xC\TARGET_NUCLEO_L432KC\PeripheralPins.c:

MBED_WEAK const PinMap PinMap_UART_TX[] = {
{PA_2, UART_2, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF7_USART2)}, // Connected to STDIO_UART_TX
// {PA_2, LPUART_1,STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF8_LPUART1)}, // Connected to STDIO_UART_TX
{PA_9, UART_1, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF7_USART1)},
{PB_6, UART_1, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF7_USART1)},
{NC, NC, 0}
};

Specifying GPIO_PULLUP is pointless and achieves nothing.

This is because the generic pinmap_pinout from mbed-os\hal\mbed_pinmap_common.c forces the pull-up/pull-down configuration to PullNone after the STM32 specific pin_function has set it according to the pinmap:

void pinmap_pinout(PinName pin, const PinMap *map)
{
if (pin == NC) {
return;
}

while (map->pin != NC) {
if (map->pin == pin) {
// *** Set pin config according to pinmap including pull-up/pull-down config ***
pin_function(pin, map->function);

// *** Override the mode just set in pin_function! ***
pin_mode(pin, PullNone);
return;
}
map++;
}
MBED_ERROR1(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_PINMAP_INVALID), "could not pinout", pin);
}

It appears, at least some of, the drivers set the pull-up/pull-down config explicitly and appropriately. For example, from mbed-os\targets\TARGET_STM\serial_api.c:

void serial_init(serial_t *obj, PinName tx, PinName rx)
{
...

// Configure UART pins
pinmap_pinout(tx, PinMap_UART_TX);
pinmap_pinout(rx, PinMap_UART_RX);

if (tx != NC) {
pin_mode(tx, PullUp);
}
if (rx != NC) {
pin_mode(rx, PullUp);
}

...
}

I've come to this because it appears the SPI driver, mbed-os\targets\TARGET_STM\serial_api.c, doesn't set the pull-up/pull-down configuration and, at least on our custom board, PullNone is not appropriate.

void spi_init(spi_t *obj, PinName mosi, PinName miso, PinName sclk, PinName ssel)
{
...

// Configure the SPI pins
pinmap_pinout(mosi, PinMap_SPI_MOSI);
pinmap_pinout(miso, PinMap_SPI_MISO);
pinmap_pinout(sclk, PinMap_SPI_SCLK);
spiobj->pin_miso = miso;
spiobj->pin_mosi = mosi;
spiobj->pin_sclk = sclk;
spiobj->pin_ssel = ssel;
if (ssel != NC) {
pinmap_pinout(ssel, PinMap_SPI_SSEL);
handle->Init.NSS = SPI_NSS_HARD_OUTPUT;
} else {
handle->Init.NSS = SPI_NSS_SOFT;
}

...
}

Hacking the code to ensure MISO is configured with a pull-up reduces the STM32 current consumption considerably because, on our board, there is no external pull-up.

I think we've got a number of problems tangled together here!

It seems somewhat belligerent that the generic pinmap_pinout sets PullNone on all pins on all targets. A compromise might be to set PullNone first and allow specific implementations of pin_function to then override it if they want. Given that this is a generic, heavily used function that hasn't changed for years, I can imagine that changing it will be difficult!

The STM32 specific pinmap configuration is misleading and pointless. The pull-up/pull-down config should be either:

• Removed
• Made to work as implied
• Heavily annotated as documentation only

The SPI driver needs to either set the pull-up/pull-down configuration appropriately or provide some configuration to do it.

Of course any app using the SPI driver can set the pull-up/pull-down configuration directly after initialising the driver but this would not be obvious and isn't necessary for other drivers.

In our particular situation we not using the SPI driver directly. We're actually using mbed-semtech-lora-rf-drivers and therefore the SPI is a bit remote from our app code.

Issue request type

[ ] Question
[ ] Enhancement
[x] Bug

[LMESTM]

@mattbrown015 - clear thanks fr the report.
do you confirm that commenting out the below line solves the issue ?

            // *** Override the mode just set in pin_function! ***
            pin_mode(pin, PullNone);

I'd like to get arm views on

It seems somewhat belligerent that the generic pinmap_pinout sets PullNone on all pins on all targets. A compromise might be to set PullNone first and allow specific implementations of pin_function to then override it if they want. Given that this is a generic, heavily used function that hasn't changed for years, I can imagine that changing it will be difficult!

@bulislaw @0xc0170

[mattbrown015]

do you confirm that commenting out the below line solves the issue ?

Commenting out this line (and changing the pin map) solves my issue and, I believe, is the correct thing to do for STM32.

What should happen on other targets with different styles of pin map is less clear to me.

Perhaps STM32 is the only target that includes the pull-up/pull-down config in the pin map which would mean this is probably fine for all other targets. Let's see what @bulislaw and @0xc0170 have to say! :-)

I've solved my specific and immediate problem by explicitly setting the SPI MISO pull-up in the application code after the SPI object initialisation has completed.

[DataDrake]

I agree that this is a problem as I just finished debugging an issue with an SPI CS pin that needed the internal Pull Up for CS to register as 1 and set our SPI Flash to its low-power idle mode.

[kjbracey]

Pull ups and pull downs are board-specific, so I don't feel like they should be something for the application or C++ driver classes to deal with.

BUT - we already expect applications to do board-specific things by specifying the pin number to use for a function in the first place.

HOWEVER - the need for a pull or not on a given pin is independent of what software is actually doing, Even when a piece of application software is not using the function on a pin, the default state of the pin after power-up must have the pull correct (either by being in GPIO mode with appropriate pull configured, or autoconfigured to a function with appropriate inherent pull).

So I submit that board initialisation code should be getting the pulls right for the board, and then any function selection should not interfere with that, unless the application code makes an explicit pull specifying request.

So I would concur with removing that automatic "pull:=none" in principle.

You could move it to above the pin_function call - that would seem like a plausible backwards-compatibility to minimise risk of change for existing targets, but that forces pull selection into the HAL target code, and would preclude a board-aware application making a GPIO pull request before activating a function on the same pin.

[DataDrake]

Setting it before pin_function seems to make the most sense. Targets that don't change the pull selection will be unaffected, but others will at least have an opportunity to override the default setting.

[LMESTM]

@ARMmbed/mbed-os-core
could you take care of reviewing and possibly pushing the change proposed by @kjbracey-arm above ?

[bcostm]

Why adding the "devices: st" label ? The proposed change has to be made in the core files and not in ST ones...

It would be nice also to remove "STM32" in the title...

[cmonr]

@bcostm Updated

[ciarmcom]

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/IOTOSM-2226

[ciarmcom]

We closed this issue because it has been inactive for quite some time and we believe it to be low priority. If you think that the priority should be higher, then please reopen with your justification for increasing the priority.

[stefan-npsys]

Still a problem it seems. In: ../mbed-os/targets/TARGET_STM/stm_spi_api.c, the line(232):

pin_mode(pinmap->miso_pin, PullNone);

Will override any PullUp/PullDown specified in a board-specific PeripheralPins.c file, leaving the MISO floating and possibly causing a much higher power usage than needed. [Just seen this on a SEED Lora E5 Development board, mbed-os 6.14 with a Winbond NOR SPI Flash - Workaround is using external pulldown resistor or re-defining the port-pull state in the application after using the SPI/blockdevice].

I'm not sure why it is forced to 'PullNone', when a pretty neat system with board-specific PeripheralPin definitions is in place ?

[jeromecoutant]

Workaround is using external pulldown resistor or re-defining the port-pull state in the application after using the SPI/blockdevice

Agree

Just seen this on a SEED Lora E5 Development board

@hallard FYI

[hallard]

My 2 cents,

CS pin external pull-up is a must have, remember some Arduino with RFM95 module with no pull-up was a pain to flash with ISCP because RFM95 module answered to SPI command because of no pull up and avoided correct flashing (gave me some headaches that I still remember after years).
By default SPI devices should not answer and drive SPI bus until explicitly pulling down their CS pin (that is why this pull up is a must have).

About power consumption when sleeping, sometimes I saw that with some custom driving, but never had this issue with mbed. If you got current leak and too much power consumption drawn by SPI pins, a quick workaround is to deallocate SPI before sleep or set all SPI pins to input, but once again never had this issue on mbed, even with Murata module and semtech transceiver. Just had one issue due to misconfiguration of the SX module, now fixed with this following PR #15181

[stefan-npsys]

Thx for the comments. It's very easy to workaround the issue (once you find it) in the application (as illustrated below for reference). I did expect that removing the BlockDevice (and underlying SPI) would have returned the system to LowPower. It didn't... not without forcing a PullDown.

// Global
  BlockDevice *g_bd = new SPIFBlockDevice(
    MBED_CONF_SPIF_DRIVER_SPI_MOSI,
    MBED_CONF_SPIF_DRIVER_SPI_MISO, 
    MBED_CONF_SPIF_DRIVER_SPI_CLK,
    MBED_CONF_SPIF_DRIVER_SPI_CS);

  // Main-stuff
  int bd_cc = g_bd->init();
 
  // Do stuff with the BlockDevice
  
  // Remove blockDevice
  g_bd->deinit();   // Ohh.. SPI is till using too much power (~40uA). Try to disintegrate it. (I like old SiFi movies)
  delete g_bd;      // Ouch.. SPI still using too much (~40uA)
  
  // Workaround - do this after at least the de-init
  pin_mode(MBED_CONF_SPIF_DRIVER_SPI_MISO, PullDown); // this kills it! Now we're at the expected ~3uA when sleeping

... but perhaps it should not be fixed at the application level? I see in the 'stm_spi_api.c' that all SPI pins MISO/MOSI/SCLK/SSEL are forced to PullNone ? They are not controlled by the (pretty clever) board specific PeripheralPins.c setup? I understand that MOST SPI setups dont need anything special, but apparently some cases exist, where a Pullup/PullDown is needed. So why not allow it (internally)?
I think we will extend the 'SPIFBlockDevice' and add the PullDown-fix to the deinit() method for now.

@hallard: Had not really thought about the CS-pullup, so thx :-). Im no hardware guy. Thx for the PR-reference. I did try the Lora on the SEED E5 module, seems to work OK (only tried class A, basic example, though).

[hallard]

I got an SPI Flash nor on the board with the LoRa-E5, as far as I remember I did not had this issue but I need to check it back but il all case it should not happens (except a default on the device)
To be sure, did you force your SPI device to go to sleep mode before? not doing that may cause device not configuring it own pins to go to sleep, writing here brings that point back to my memory, remember I had this kind of issue with a SPI flash some years ago.
LoRa E5 in EU868 has too much consumption during transmit (80mA) the new RAK3172SIP has this fixed :-)
https://twitter.com/ch_2i/status/1557762476678291456?s=20&t=52w84_JjvBw9o6TNniEfPw

[stefan-npsys]

Thanks for comments. Good to know!

I got an SPI Flash nor on the board with the LoRa-E5, as far as I remember I did not had this issue but I need to check it back but il all case it should not happens (except a default on the device)

I too, thought of the SPI-bus as "pretty digital", but aparrently this can happen.. when I Google it, many replies starts with something like "no restistors needed on SPI for proper function, however, <something, something> MISO can float.. <something, something>, causing leakage" (ahh well, live and learn)

To be sure, did you force your SPI device to go to sleep mode before? not doing that may cause device not configuring it own pins to go to sleep, writing here brings that point back to my memory, remember I had this kind of issue with a SPI flash some years ago.

Im not doing anything special to the SPI (or the FLASH). I assumed that MBED would take care of the nitty-gritty. I remove the BlockDevice (and hence the associated SPI device), as shown in my example above. That should really do it I suppose (but it dosent)... but workaround works. Yey! (Though I still believe it should be fixed in MBED)

LoRa E5 in EU868 has too much consumption during transmit (80mA) the new RAK3172SIP has this fixed :-) https://twitter.com/ch_2i/status/1557762476678291456?s=20&t=52w84_JjvBw9o6TNniEfPw

Great!! More module options available! The SEED Lora E5 module power consumption was around 9-10 mA when transmitting the 24 bytes test-data from the default MBED example (Class A, DR2/SF10/BW125), running the MCU @ 4MHz, without trace and printf's).