chip/stm32: ADC fixes

The ADC drivers for STM32L4 (and L5) seems to have significant flaws.

The chip has the ability to continuously perform ADC conversions
according to a set sequence of channels and bit depths.  In addition to
that, the chip can perform a one-off "injected" sequence of up to four
channels, in response to a hardware or software trigger.

The driver uses only a software triggered "injected" sequence to convert
two pre-set channels.

The code of `adc_read_channel()` has some odd properties, though.  On
first invocation, it sets up the injected sequence to consist of the two
configured ADC channels.  Each invocation takes one particular ADC
channel number as input, but the code simply kicks of the sequence of
two channels to read, and then returns the relevant of the two,
discarding the other reading.

The has the needless limitation that it cannot be used with more than
two ADC channels.

Since it is permitted to modify the list of channels in the injected
sequence as long as no conversion is in progress, it would be more
straightforward, if each invocation of `adc_read_channel()` would
reconfigure the injected sequence to consist of a single channel, the
one requested.  This CL makes that change.

BUG=b:269621551
TEST=Read and of a dozen ADC channels on HyperDebug

Change-Id: I62387979faf494cfefc3b6e7dd1d9a1954017ae6
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4265223
Tested-by: Jes Klinke <jbk@chromium.org>
Commit-Queue: Eric Yilun Lin <yllin@google.com>
Auto-Submit: Jes Klinke <jbk@chromium.org>
Reviewed-by: Eric Yilun Lin <yllin@google.com>

4 files changed, 24 insertions, 37 deletions

diff --git a/board/bellis/board.c b/board/bellis/board.c
index a776f119fe..2fe9bd5099 100644
--- a/board/bellis/board.c
+++ b/board/bellis/board.c
@@ -59,10 +59,8 @@ static void tcpc_alert_event(enum gpio_signal signal)
 /******************************************************************************/
 /* ADC channels. Must be in the exactly same order as in enum adc_channel. */
 const struct adc_t adc_channels[] = {
-	[ADC_BOARD_ID] = { "BOARD_ID", 3300, 4096, 0, STM32_AIN(5),
-			   STM32_RANK(1) },
-	[ADC_EC_SKU_ID] = { "EC_SKU_ID", 3300, 4096, 0, STM32_AIN(15),
-			    STM32_RANK(2) },
+	[ADC_BOARD_ID] = { "BOARD_ID", 3300, 4096, 0, STM32_AIN(5) },
+	[ADC_EC_SKU_ID] = { "EC_SKU_ID", 3300, 4096, 0, STM32_AIN(15) },
 };
 BUILD_ASSERT(ARRAY_SIZE(adc_channels) == ADC_CH_COUNT);
 
diff --git a/board/munna/board.c b/board/munna/board.c
index b09605cae4..d7c497cadd 100644
--- a/board/munna/board.c
+++ b/board/munna/board.c
@@ -59,10 +59,8 @@ static void tcpc_alert_event(enum gpio_signal signal)
 /******************************************************************************/
 /* ADC channels. Must be in the exactly same order as in enum adc_channel. */
 const struct adc_t adc_channels[] = {
-	[ADC_BOARD_ID] = { "BOARD_ID", 3300, 4096, 0, STM32_AIN(5),
-			   STM32_RANK(1) },
-	[ADC_EC_SKU_ID] = { "EC_SKU_ID", 3300, 4096, 0, STM32_AIN(15),
-			    STM32_RANK(2) },
+	[ADC_BOARD_ID] = { "BOARD_ID", 3300, 4096, 0, STM32_AIN(5) },
+	[ADC_EC_SKU_ID] = { "EC_SKU_ID", 3300, 4096, 0, STM32_AIN(15) },
 };
 BUILD_ASSERT(ARRAY_SIZE(adc_channels) == ADC_CH_COUNT);
 
diff --git a/chip/stm32/adc-stm32l4.c b/chip/stm32/adc-stm32l4.c
index c8eb989f3e..c81978d6ce 100644
--- a/chip/stm32/adc-stm32l4.c
+++ b/chip/stm32/adc-stm32l4.c
@@ -41,10 +41,10 @@ struct adc_profile_t {
 #define ADC_ENABLE_TIMEOUT_US 200000U
 #define ADC_CONVERSION_TIMEOUT_US 200000U
 
-#define NUMBER_OF_ADC_CHANNEL 2
 static uint8_t adc1_initialized;
 
 #ifdef CONFIG_ADC_PROFILE_FAST_CONTINUOUS
+#error "Continuous ADC sampling not implemented for STM32L4/5"
 
 #ifndef CONFIG_ADC_SAMPLE_TIME
 #define CONFIG_ADC_SAMPLE_TIME STM32_ADC_SMPR_1_5_CY
@@ -71,7 +71,7 @@ static const struct adc_profile_t profile = {
 };
 #endif
 
-static void adc_init(const struct adc_t *adc)
+static void adc_init(void)
 {
 	/*
 	 * If clock is already enabled, and ADC module is enabled
@@ -102,10 +102,9 @@ static void adc_init(const struct adc_t *adc)
 
 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_time)
+static void adc_configure_channel(int ain_id, enum stm32_adc_smpr sample_time)
 {
-	/* Select Sampling time and channel to convert */
+	/* Select Sampling time for channel to convert */
 	if (sample_time == STM32_ADC_SMPR_DEFAULT)
 		sample_time = CONFIG_ADC_SAMPLE_TIME;
 
@@ -116,16 +115,12 @@ static void adc_configure(int ain_id, int ain_rank,
 		STM32_ADC1_SMPR2 &= ~(7 << ((ain_id - 11) * 3));
 		STM32_ADC1_SMPR2 |= ((sample_time - 1) << ((ain_id - 11) * 3));
 	}
+}
 
-	/* Setup Rank */
-	STM32_ADC1_JSQR &= ~(0x03);
-	STM32_ADC1_JSQR |= NUMBER_OF_ADC_CHANNEL - 1;
-
-	STM32_ADC1_JSQR &= ~(0x1F << (((ain_rank - 1) * 6) + 8));
-	STM32_ADC1_JSQR |= (ain_id << (((ain_rank - 1) * 6) + 8));
-
-	/* Disable DMA */
-	STM32_ADC1_CFGR &= ~STM32_ADC1_CFGR_DMAEN;
+static void adc_select_channel(int ain_id)
+{
+	/* Setup an "injected sequence" consisting of only this one channel. */
+	STM32_ADC1_JSQR = ain_id << 8;
 }
 
 static void stm32_adc1_isr_clear(uint32_t bitmask)
@@ -144,16 +139,18 @@ int adc_read_channel(enum adc_channel ch)
 	mutex_lock(&adc_lock);
 
 	if (adc1_initialized == 0) {
-		adc_init(adc);
+		adc_init();
 
-		/* Configure Injected Channel N */
-		for (uint8_t i = 0; i < NUMBER_OF_ADC_CHANNEL; i++) {
+		/* Configure Channel N */
+		for (uint8_t i = 0; i < ADC_CH_COUNT; i++) {
 			const struct adc_t *adc = adc_channels + i;
 
-			adc_configure(adc->channel, adc->rank,
-				      adc->sample_time);
+			adc_configure_channel(adc->channel, adc->sample_time);
 		}
 
+		/* Disable DMA */
+		STM32_ADC1_CFGR &= ~STM32_ADC1_CFGR_DMAEN;
+
 		if ((STM32_ADC1_CR & STM32_ADC1_CR_ADEN) !=
 		    STM32_ADC1_CR_ADEN) {
 			/* Disable ADC deep power down (enabled by default after
@@ -206,6 +203,9 @@ int adc_read_channel(enum adc_channel ch)
 		adc1_initialized = 1;
 	}
 
+	/* Configure Injected Channel N */
+	adc_select_channel(adc->channel);
+
 	/* Start injected conversion */
 	STM32_ADC1_CR |= BIT(3); /* JADSTART */
 
@@ -221,10 +221,7 @@ int adc_read_channel(enum adc_channel ch)
 	stm32_adc1_isr_clear(BIT(6));
 
 	/* read converted value */
-	if (adc->rank == 1)
-		value = STM32_ADC1_JDR1;
-	if (adc->rank == 2)
-		value = STM32_ADC1_JDR2;
+	value = STM32_ADC1_JDR1;
 
 	mutex_unlock(&adc_lock);
 
diff --git a/chip/stm32/adc_chip.h b/chip/stm32/adc_chip.h
index 8cd53a748a..2677f1d9cc 100644
--- a/chip/stm32/adc_chip.h
+++ b/chip/stm32/adc_chip.h
@@ -43,9 +43,6 @@ struct adc_t {
 	int factor_div;
 	int shift;
 	int channel;
-#if defined(CHIP_FAMILY_STM32L4) || defined(CHIP_FAMILY_STM32L5)
-	int rank;
-#endif
 
 #if defined(CHIP_FAMILY_STM32F0) || defined(CHIP_FAMILY_STM32L4) || \
 	defined(CHIP_FAMILY_STM32L5)
@@ -63,7 +60,4 @@ void adc_disable(void);
 /* Just plain id mapping for code readability */
 #define STM32_AIN(x) (x)
 
-/* Add for ADCs with RANK */
-#define STM32_RANK(x) (x)
-
 #endif /* __CROS_EC_ADC_CHIP_H */