Bug in low_power_pwm.c causing incorrect pin configurations

[gnaux]

Hi, I'm running nRF5 SDK v15.2, and tried to run the led softblink example on port 1 of a nRF52840, which did not seem to work. I found a problem in low_power_pwm.c, which causes incorrect pin configurations. The code is located in the function low_power_pwm_init, and looks like this:

    while (bit_mask)
    {
        if (bit_mask & 0x1UL)
        {
            nrf_gpio_cfg_output(pin_number);
        }

        pin_number++;
        bit_mask >>= 1UL;
    }

As you can see above, the call to nrf_gpio_cfg_output does not take the port into consideration, it was supplied through the p_pwm_config struct. The code below fixes the problem for me, but will of course not work if you have more than 2 ports.

    uint32_t portNr = (p_pwm_config->p_port == NRF_P0) ? 0 : 1;

    while (bit_mask)
    {
        if (bit_mask & 0x1UL)
        {
            nrf_gpio_cfg_output(NRF_GPIO_PIN_MAP(portNr, pin_number));
        }

        pin_number++;
        bit_mask >>= 1UL;
    }

I would appreciate if this could be fixed in the next revision of the SDK.

[Edvin]

Hello,

Thank you for reporting.

I have not tested this right now, but I didn't understand why the bit mask will not work with the pins on P1. Have you tried setting the bit mask in the p_pwm_config, i.e. in main.c:

    APP_TIMER_DEF(lpp_timer_0);
    low_power_pwm_config.active_high    = false;
    low_power_pwm_config.period         = 220;
    low_power_pwm_config.bit_mask       = PIN_MASK(_pin);
    low_power_pwm_config.p_timer_id     = &lpp_timer_0;
    low_power_pwm_config.p_port         = PIN_PORT(_pin);

    err_code = low_power_pwm_init((&low_power_pwm_0), &low_power_pwm_config, pwm_handler);

where _pin is the pin that you want to use? Alternatively a custom pin mask consisting of more pins?

Best regards,

Edvin

[gnaux]

Hi Edvin,

Thanks for getting back to me. Yes, I have tried setting the mask with the appropriate pins, instead of setting up the pins on port 1, it sets up the pins on port 0, and proceeds to try and toggle unconfigured pins on port 1.

The way this particular code is constructed, it is meant to allow the user to toggle multiple LEDs simultaneously. You can see the while(bit_mask) loop extracting the pin numbers to configure as outputs. The pin number itself is not very useful if you don't know which port it operates on. The calls to nrf_gpio_cfg_output assumes that the pin number is held in the lower 5 bits, and that the port is held in the sixth, as indicated by the macro:

#define NRF_GPIO_PIN_MAP(port, pin) (((port) << 5) | ((pin) & 0x1F))

Thus, calls to nrf_gpio_cfg_output() need to be made with the port put in the sixth bit (idx 5). It proceeds to call nrf_gpio_pin_port_decode(), which assumes that pins that are equal or above to 32 belong to Port 1, thus confirming the above statement.

[Edvin]

Hello,

Yes. You are sorry. I mixed up the numbers in my head (thinking 32 bits would fit numbers well over 32, but it will of course not work with a bit mask. 

I agree. The port should also be used in the low_power_pwm_init(), as you describe. Especially since it is already a parameter of the p_pwm_config.

I will report this internally.

Best regards,

Edvin

[gnaux]

No worries! Thanks for the quick feedback!