chip/stm32: Sample time off-by-one

The STM32L4/5 ADCs use three bits to configure the time to sample the
input voltage, that is the time during which an analog MUX conducts,
allowing the internal holding capacitor to settle.  The enums declared
in adc_chip.h has eight usable values, plus an additional value called
DEFAULT.

It makes sense to have the DEFAULT be represented by a zero value, such
that if a board.c file does not mention the sample_time field in its
adc_t, then due to the C convention of data being zero-initialized, the
code can detect it and apply a sensible default.

However, the body of adc_configure() has no such logic, but takes the
raw enum value and attempts to stuff it into a 3-bit field, which will
be off by one, and overrun for the largest enum value of 8.

Also, I find the term "sample rate" misleading, as it implies a
continuous process.  This sample time setting applies equally to one-off
injected conversions, as it refers to the duration of the sampling of
the input signal, before conversion begins, not a rate of conversions
happening.

BUG=b:269621551
TEST=Made measurements with the longest same time setting

Change-Id: Id8d297fcec883565dea1e09d6bbbfa1ab564778d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4265222
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
Tested-by: Jes Klinke <jbk@chromium.org>
Commit-Queue: Jes Klinke <jbk@chromium.org>

3 files changed, 15 insertions, 10 deletions

diff --git a/chip/stm32/adc-stm32f0.c b/chip/stm32/adc-stm32f0.c
index d1b1cc0e02..597861cb33 100644
--- a/chip/stm32/adc-stm32f0.c
+++ b/chip/stm32/adc-stm32f0.c
@@ -108,14 +108,14 @@ static void adc_init(const struct adc_t *adc)
 		STM32_ADC_CR = STM32_ADC_CR_ADEN;
 }
 
-static void adc_configure(int ain_id, enum stm32_adc_smpr sample_rate)
+static void adc_configure(int ain_id, enum stm32_adc_smpr sample_time)
 {
 	/* Sampling time */
-	if (sample_rate == STM32_ADC_SMPR_DEFAULT ||
-	    sample_rate >= STM32_ADC_SMPR_COUNT)
+	if (sample_time == STM32_ADC_SMPR_DEFAULT ||
+	    sample_time >= STM32_ADC_SMPR_COUNT)
 		STM32_ADC_SMPR = profile.smpr_reg;
 	else
-		STM32_ADC_SMPR = STM32_ADC_SMPR_SMP(sample_rate);
+		STM32_ADC_SMPR = STM32_ADC_SMPR_SMP(sample_time);
 
 	/* Select channel to convert */
 	STM32_ADC_CHSELR = BIT(ain_id);
@@ -321,7 +321,7 @@ int adc_read_channel(enum adc_channel ch)
 		adc_disable_watchdog_no_lock();
 	}
 
-	adc_configure(adc->channel, adc->sample_rate);
+	adc_configure(adc->channel, adc->sample_time);
 
 	/* Clear flags */
 	STM32_ADC_ISR = 0xe;
diff --git a/chip/stm32/adc-stm32l4.c b/chip/stm32/adc-stm32l4.c
index 23a49e2908..c8eb989f3e 100644
--- a/chip/stm32/adc-stm32l4.c
+++ b/chip/stm32/adc-stm32l4.c
@@ -100,16 +100,21 @@ static void adc_init(const struct adc_t *adc)
 	STM32_ADC1_CFGR &= ~STM32_ADC1_CFGR_AUTDLY;
 }
 
+BUILD_ASSERT(CONFIG_ADC_SAMPLE_TIME > 0 && CONFIG_ADC_SAMPLE_TIME <= 8);
+
 static void adc_configure(int ain_id, int ain_rank,
-			  enum stm32_adc_smpr sample_rate)
+			  enum stm32_adc_smpr sample_time)
 {
 	/* Select Sampling time and channel to convert */
+	if (sample_time == STM32_ADC_SMPR_DEFAULT)
+		sample_time = CONFIG_ADC_SAMPLE_TIME;
+
 	if (ain_id <= 10) {
 		STM32_ADC1_SMPR1 &= ~(7 << ((ain_id - 1) * 3));
-		STM32_ADC1_SMPR1 |= (sample_rate << ((ain_id - 1) * 3));
+		STM32_ADC1_SMPR1 |= ((sample_time - 1) << ((ain_id - 1) * 3));
 	} else {
 		STM32_ADC1_SMPR2 &= ~(7 << ((ain_id - 11) * 3));
-		STM32_ADC1_SMPR2 |= (sample_rate << ((ain_id - 11) * 3));
+		STM32_ADC1_SMPR2 |= ((sample_time - 1) << ((ain_id - 11) * 3));
 	}
 
 	/* Setup Rank */
@@ -146,7 +151,7 @@ int adc_read_channel(enum adc_channel ch)
 			const struct adc_t *adc = adc_channels + i;
 
 			adc_configure(adc->channel, adc->rank,
-				      adc->sample_rate);
+				      adc->sample_time);
 		}
 
 		if ((STM32_ADC1_CR & STM32_ADC1_CR_ADEN) !=
diff --git a/chip/stm32/adc_chip.h b/chip/stm32/adc_chip.h
index 18a280a9d8..8cd53a748a 100644
--- a/chip/stm32/adc_chip.h
+++ b/chip/stm32/adc_chip.h
@@ -49,7 +49,7 @@ struct adc_t {
 
 #if defined(CHIP_FAMILY_STM32F0) || defined(CHIP_FAMILY_STM32L4) || \
 	defined(CHIP_FAMILY_STM32L5)
-	enum stm32_adc_smpr sample_rate; /* Sampling Rate of the channel */
+	enum stm32_adc_smpr sample_time; /* Sampling Time of the channel */
 #endif
 };