From 5ed0cecf0ddb57aab381f6a96542c30400cf0fbf Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Wed, 15 Mar 2023 15:43:50 +0100 Subject: [PATCH] nrf: fix memory issue in ADC read There was a very subtle bug in the ADC read code: it stores a pointer to a variable in a register, waits for the hardware to complete the read, and then reads the value again from the local variable. Unfortunately, the compiler doesn't know there is some form of synchronization happening in between. This can be fixed in roughly two ways: * Introduce some sort of synchronization. * Do a volatile read from the variable. I chose the second one as it is probably the least intrusive. We certainly don't need atomic instructions (the chip is single threaded), we just need to tell the compiler the value could have changed by making the read volatile. --- src/machine/machine_nrf528xx.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/machine/machine_nrf528xx.go b/src/machine/machine_nrf528xx.go index ac43c8f3..259f0503 100644 --- a/src/machine/machine_nrf528xx.go +++ b/src/machine/machine_nrf528xx.go @@ -25,7 +25,7 @@ func (a ADC) Configure(ADCConfig) { // Get returns the current value of a ADC pin in the range 0..0xffff. func (a ADC) Get() uint16 { var pwmPin uint32 - var value int16 + var rawValue volatile.Register16 switch a.Pin { case 2: @@ -78,7 +78,7 @@ func (a ADC) Get() uint16 { nrf.SAADC.CH[0].PSELP.Set(pwmPin) // Destination for sample result. - nrf.SAADC.RESULT.PTR.Set(uint32(uintptr(unsafe.Pointer(&value)))) + nrf.SAADC.RESULT.PTR.Set(uint32(uintptr(unsafe.Pointer(&rawValue)))) nrf.SAADC.RESULT.MAXCNT.Set(1) // One sample // Start tasks. @@ -104,6 +104,7 @@ func (a ADC) Get() uint16 { // Disable the ADC. nrf.SAADC.ENABLE.Set(nrf.SAADC_ENABLE_ENABLE_Disabled << nrf.SAADC_ENABLE_ENABLE_Pos) + value := int16(rawValue.Get()) if value < 0 { value = 0 }