nrf_gpio_pin_read and optimized code might return wrong value

[Tamisoft]

Hi, I have found an interesting behavior from this code:

nrf_gpio_cfg_input(7,NRF_GPIO_PIN_NOPULL);
if (nrf_gpio_pin_read(7)==0)
{
 NRF_LOG_INFO("ID1(P0.%d) is LOW.",7);
}else{
 NRF_LOG_INFO("ID1(P0.%d) is HIGH.",7);
}

the nrf_gpio_pin_read will always return 0 as if the pin was low, but the pin is in fact connected to 1.8V through a 10k resistor. Further investigating first added a delay_ms(1) before the if, and that fixed the issue, but still didn't make sense. Then rebuilt the code with

#pragma GCC push_options
#pragma GCC optimize ("O0")
...
#pragma GCC pop_options

around the gpio_init function and it was working without the delay. Started suspecting that maybe it is an analog issue, so we tried without the 10k resistor, but the behavior was the same.

After this we started dissecting the generated assembly code and found that with the default optimized code output has the str.w instruction (used to set the pin direction) directly followed by the read of the pin ldr.w. So started adding nops between the two and found that it won't work with one, but will work with 2 or more. The non-working one looks like this (this is with 1 nop, 0 no is the same without that):

   reg->PIN_CNF[pin_number] = ((uint32_t)dir << GPIO_PIN_CNF_DIR_Pos)
   2647e:       f04f 43a0       mov.w   r3, #1342177280 ; 0x50000000
   26482:       2200            movs    r2, #0
   //------------INTERESTING PART FROM HERE---------------
   **26484:       f8c3 271c       str.w   r2, [r3, #1820] ; 0x71c
        __asm__("nop");
   **26488:       bf00            nop
    return p_reg->IN;
   **2648a:       f8d3 2510       ldr.w   r2, [r3, #1296] ; 0x510
        if (nrf_gpio_pin_read(ID_PIN1)==0)
   2648e:       f012 0f80       tst.w   r2, #128        ; 0x80
        {
                NRF_LOG_INFO("ID1(P0.%d) is LOW.",ID_PIN1);
   26492:       6862            ldr     r2, [r4, #4]
        if (nrf_gpio_pin_read(ID_PIN1)==0)
   26494:       d10f            bne.n   264b6 <board_init+0x42>

And the working one is:

 26484:       f8c3 271c       str.w   r2, [r3, #1820] ; 0x71c
        __asm__("nop");
   26488:       bf00            nop
        __asm__("nop");
   2648a:       bf00            nop
    return p_reg->IN;
   2648c:       f8d3 2510       ldr.w   r2, [r3, #1296] ; 0x510
        if (nrf_gpio_pin_read(ID_PIN1)==0)
   26490:       f012 0f80       tst.w   r2, #128        ; 0x80
        {
                NRF_LOG_INFO("ID1(P0.%d) is LOW.",ID_PIN1);
   26494:       6862            ldr     r2, [r4, #4]
        if (nrf_gpio_pin_read(ID_PIN1)==0)
   26496:       d10f            bne.n   264b8 <board_init+0x44>

So it seems that if the direction change and the read gets within 3 clock cycles it will return the wrong value. An obvious fix of course is to make the assembly longer eg. by creating a volatile uint32_t zero=0 and make the if use that instead of "0", but I think this could be fixed in a nicer way (especially since this is not bullet proof). Is this a known behavior of the nRF52832? Should it be fixed in the SDK by simply adding 2 nops to the direction change static inline function?

Thanks Levente

[Tamisoft]

Will do! Do you want me to close this ticket, or the SDK team will write their response here?

[MartinBL]

You can just leave it open and we will see what happens.

[MartinBL]

I accidentally deleted a post here, and I'm unable to undo it or find the name of the poster. I copied the text before I deleted the post though, and here it is:

POST:

We've experienced the same issue. When a read of the IN register is executed right after the instruction
where the PIN_CFG is written, it returns a wrong value (always 0?). When we add 4 consecutive (dummy)
reads of PIN_CFG, as suggested in the workaround for errata 173, it works.

Shouldn't this issue be mentioned in the errata? And also include it in the SDK?

The following code works for me:

__STATIC_INLINE void nrf_gpio_cfg(
uint32_t pin_number,
nrf_gpio_pin_dir_t 
dir,
nrf_gpio_pin_input_t input,
nrf_gpio_pin_pull_t pull,
nrf_gpio_pin_drive_t 
drive,
nrf_gpio_pin_sense_t sense)
{
NRF_GPIO->PIN_CNF[pin_number] = ((uint32_t)dir << 
GPIO_PIN_CNF_DIR_Pos)
| ((uint32_t)input << GPIO_PIN_CNF_INPUT_Pos)

| ((uint32_t)pull << GPIO_PIN_CNF_PULL_Pos)
| 
((uint32_t)drive << GPIO_PIN_CNF_DRIVE_Pos)
| ((uint32_t)sense << 
GPIO_PIN_CNF_SENSE_Pos);

#if defined(NRF52) || defined(NRF52_SERIES)
/*
* A read of the IN register 
immediately after a write to PIN_CFG[pin] might result in a wrong value.
* We suspect a 
synchronization issue with the CPU and GPIO clock, similar to PAN173.
*
* Any read on the system 
bus will require the write buffer to be emptied first.
* Synchronize with the 16MHz GPIO clock. This 
takes at least 3 cycles to take effect.
*/
(void)NRF_GPIO->PIN_CNF[pin_number];
(void)NRF_GPIO- 
>PIN_CNF[pin_number];
(void)NRF_GPIO->PIN_CNF[pin_number];
(void)NRF_GPIO->PIN_CNF[pin_number];
#endif
}

 

ANSWER:
You are right. There is a propagation delay on the GPIO->IN register of a couple of cycles. The number of cycles appears to depend on the IC variant and use case, but should never be more than 3. The system architects are currently discussing how to improve the description of errata 173.

[martin-ve]

It was mine, I was already wondering why I couldn't find my post or your reply xD. 

Anyway, thanks for your answer. 

[MartinBL]

Happy to help. My apologies about the post.