EFM32 NC Pins not supported

Description

EFM32 targets do not allow gpio pins to be specified as NC. In the EFM implementation of gpio_api.c, gpio_init contains MBED_ASSERT(pin != NC).

Other targets such as STM32 do the following

if (pin == (PinName)NC) {
        return;
    }

This behavior can cause problems when interfacing with drivers which have optional pins that may not be used. Is there a reason for difference in behavior?

[X] Question
[ ] Enhancement
[ ] Bug

[0xc0170]

cc @ARMmbed/team-silabs
[Mirrored to Jira]

[stevew817]

Welp, sorry for being unresponsive on this one.

I guess the difference in behavior is due to a different opinion on design philosophy. From our POV, we believe calling gpio_init with pin = NC doesn't make sense, and it is the layer above that should check what it is passing into a HAL call. After all, what are we supposed to do when pin = NC? Ignore the call? Then why was the call made in the first place? If you init pin NC to be an input, and then try to read from it, what value should be returned?

Asserting on pin != NC might uncover actual runtime errors, and using MBED_ASSERT will uncover these errors in debug builds, but 'leave them be' on release builds, if you're absolutely inclined on initializing an NC pin. Note that trying to configure a pin that has been initialized as NC will lead to undefined behaviour on Silicon Labs devices, since we don't runtime-check (other than through that assert) the pin to be valid before passing it off to emlib, which in turn also only applies an assert to the input, which also gets dropped in a release build. You will then start indexing beyond the size of the GPIO peripheral's register map, ending up writing to or reading from a memory location you wouldn't expect. Depending on where those end up, it might lead to anything from a no-op, touching another peripheral, or hardfaulting.

mbed OS doesn't specify what behaviour targets should adopt for gpio_init with pin = NC. If they delegate the responsibility onto us to accept that type of input, we'll change our HAL, but I hope this answer has given you some insight in why our implementation is written the way it is.
[Mirrored to Jira]

Thanks for the response. For us, the specific issue stems from the fact that certain higher level drivers are designed to allow for NC pins to be specified. For example, the Semtech lora radio driver has a default argument of NC for certain pins which are optional in the driver. (https://github.com/ARMmbed/mbed-semtech-lora-rf-drivers/blob/master/SX1276/SX1276_LoRaRadio.h)

   SX1276_LoRaRadio(PinName mosi,
                     PinName miso,
                     PinName sclk,
                     PinName nss,
                     PinName reset,
                     PinName dio0,
                     PinName dio1,
                     PinName dio2,
                     PinName dio3,
                     PinName dio4,
                     PinName dio5,
                     PinName rf_switch_ctl1 = NC,
                     PinName rf_switch_ctl2 = NC,
                     PinName txctl = NC,
                     PinName rxctl = NC,
                     PinName ant_switch = NC,
                     PinName pwr_amp_ctl = NC,
                     PinName tcxo = NC);

This means that the Semtech driver can't be used in its current form on an EFM target.

For some more context, the current mbed gpio api is as follows from mbed_gpio.c :

static inline void _gpio_init_in(gpio_t *gpio, PinName pin, PinMode mode)
{
    gpio_init(gpio, pin);
    if (pin != NC) {
        gpio_dir(gpio, PIN_INPUT);
        gpio_mode(gpio, mode);
    }
}

So it looks like only the init functions need to handle this case.

Additionally, some of the silicon labs implementations DO make this check presumably because those pins are indeed optional. For example in spi_api.c:

void spi_enable_pins(spi_t *obj, uint8_t enable, PinName mosi, PinName miso, PinName clk, PinName cs)
{
    if (enable) {
        if (obj->spi.master) { /* Master mode */
            /* Either mosi or miso can be NC */
            if (mosi != NC) {
                pin_mode(mosi, PushPull);
            }
            if (miso != NC) {
                pin_mode(miso, Input);
            }

I think changing the gpio init and interrupt in apis to simply ignore NC would be sufficient here but I agree that mbed weighing in here on what the expectations are would be helpful.
[Mirrored to Jira]

[adbridge]

Internal Jira reference: https://jira.arm.com/browse/IOTPART-6225

[stevew817]

@ARMmbed/mbed-os-core Can you weigh in here?

[0xc0170]

I also add @ARMmbed/mbed-os-hal who might help with this one as well

[stevew817]

Fix is in #10482
Sorry for the delay, I wanted the mbed maintainers to weigh in here but breakage of SPI on the latest release forced this through.