lm4: ADC clock management bug fix

Fixed bug in which two different tasks reading two different ADC
channels could interfere and cause the ADC clock to get disabled
when a read is still in progress, thus causing a reboot.

Added a runtime assert to verify that developers don't sample the
same ADC channel from multiple different tasks, which could cause
hard to trace reboots.

BRANCH=none
BUG=chromium:313872
TEST=1) Added console command to continuously read an ADC channel not
read anywhere else. Verified that when running this console command
I could reproduce the bug every few minutes.
2) Wrote code in adc.c to protect the ADC clock resource.
3) Ran console command from step 1 for ~2 hours with no reboots.

Change-Id: Ic1905dde12871a4b93957702f7f31a25a2762bb4
Signed-off-by: Alec Berg <alecaberg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/175404
Reviewed-by: Randall Spangler <rspangler@chromium.org>

1 files changed, 27 insertions, 3 deletions

diff --git a/chip/lm4/adc.c b/chip/lm4/adc.c
index e41386c25f..1e4fdf16c5 100644
--- a/chip/lm4/adc.c
+++ b/chip/lm4/adc.c
@@ -7,6 +7,7 @@
 
 #include "adc.h"
 #include "adc_chip.h"
+#include "atomic.h"
 #include "clock.h"
 #include "console.h"
 #include "common.h"
@@ -121,17 +122,40 @@ static void adc_configure(const struct adc_t *adc)
 int adc_read_channel(enum adc_channel ch)
 {
 	const struct adc_t *adc = adc_channels + ch;
+	static uint32_t ch_busy_mask;
+	static struct mutex adc_clock;
 	int rv;
 
-	/* Enable ADC0 module in run and sleep modes. */
+	/*
+	 * TODO(crbug.com/314121): Generalize ADC reads such that any task can
+	 * trigger a read of any channel.
+	 */
+
+	/*
+	 * Enable ADC clock and set a bit in ch_busy_mask to signify that this
+	 * channel is busy. Note, this function may be called from multiple
+	 * tasks, but each channel may be read by only one task. If assert
+	 * fails, then it means multiple tasks are trying to read same channel.
+	 */
+	mutex_lock(&adc_clock);
+	ASSERT(!(ch_busy_mask & (1UL << ch)));
 	clock_enable_peripheral(CGC_OFFSET_ADC, 0x1,
 			CGC_MODE_RUN | CGC_MODE_SLEEP);
+	ch_busy_mask |= (1UL << ch);
+	mutex_unlock(&adc_clock);
 
 	rv = flush_and_read(adc->sequencer);
 
-	/* Disable ADC0 module to conserve power. */
-	clock_disable_peripheral(CGC_OFFSET_ADC, 0x1,
+	/*
+	 * If no ADC channels are busy, then disable ADC clock to conserve
+	 * power.
+	 */
+	mutex_lock(&adc_clock);
+	ch_busy_mask &= ~(1UL << ch);
+	if (!ch_busy_mask)
+		clock_disable_peripheral(CGC_OFFSET_ADC, 0x1,
 			CGC_MODE_RUN | CGC_MODE_SLEEP);
+	mutex_unlock(&adc_clock);
 
 	if (rv == ADC_READ_ERROR)
 		return ADC_READ_ERROR;