From 23ff7f6517ede8faa6fb257cb2740b7d22d153de Mon Sep 17 00:00:00 2001 From: Alec Berg Date: Fri, 1 Nov 2013 10:11:09 -0700 Subject: 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 Reviewed-on: https://chromium-review.googlesource.com/175404 Reviewed-by: Randall Spangler --- chip/lm4/adc.c | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) (limited to 'chip') 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; -- cgit v1.2.1