I have found an interesting behavior from this code:
NRF_LOG_INFO("ID1(P0.%d) is LOW.",7);
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
**26488: bf00 nop
**2648a: f8d3 2510 ldr.w r2, [r3, #1296] ; 0x510
2648e: f012 0f80 tst.w r2, #128 ; 0x80
NRF_LOG_INFO("ID1(P0.%d) is LOW.",ID_PIN1);
26492: 6862 ldr r2, [r4, #4]
26494: d10f bne.n 264b6 <board_init+0x42>
And the working one is:
26484: f8c3 271c str.w r2, [r3, #1820] ; 0x71c
26488: bf00 nop
2648a: bf00 nop
2648c: f8d3 2510 ldr.w r2, [r3, #1296] ; 0x510
26490: f012 0f80 tst.w r2, #128 ; 0x80
NRF_LOG_INFO("ID1(P0.%d) is LOW.",ID_PIN1);
26494: 6862 ldr r2, [r4, #4]
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?
I suppose your 10k resistor and the nRF52's pad capacitance can slow down the voltage rise time on the input just enough so that it doesn't reach a logic '1' fast enough for nrf_gpio_pin_read().
But before I report this to the SDK team, is your nRF52's supply voltage also 1.8V? To register as a logic 1 the input voltage must be higher than 0.7*Vdd (electrical GPIO specs).
Yes, it is a 1.8V system. I mentioned above that we have suspected the capacitance so we have tried to tie the gpio directly to 1.8V as well. If you want we could try to replicate this on one of the nrf52 dks as well, but that might take some time.
Can you try to add a "dummy instruction" after nrf_gpio_cfg_input()? Something like this:
You can also use __ISB() eller __DMB(). The theory being that it might be because the nRF52's peripherals run at 16 MHz while the CPU runs at 64 MHz. It might be worth a shot to use one of these instructions to ensure that the CPU buffers are empty. Here is a somewhat sparse documentation of the issue.
Thanks for the possible solutions, but would it be better to incorporate these into the SDK where it can cause undeterministic output? I mean everywhere else this just manifests as a slight latency (or jitter in it), but in the scenario where IO changes direction it leads to a wrong value read under certain conditions (like optimized enough code generated by the compiler that the two instructions follow closely). The idea about NRF_GPIO->OUT is the same as nops, one is not enough :). ISB and DMB could be a solution, I will test it later, but still I think it should be in the SDK, so from the user's point of view it is consistent.
I tend to agree regarding incorporation in the SDK. However, the argument against it is that fixing this corner case will affect other implementations and that we should rather improve our documentation to help people decide on an implementation themselves. I have notified the SDK team so that they can look into it.
Please keep me posted.