STM32L1xxx ADC HAL driver bug Topic is solved

[radar_macgyver]

I think there's a bug in the STM32L1xxx device-specific ADC HAL driver.

When doing a multi-channel conversion in a sequence, only the first channel was getting converted. For example, I had a conversion group with num_channels = 4, SQ1 = CH4, SQ2 = CH8, SQ3 = VREF, SQ4 = SENSOR. When I run adcConvert(&ADCD1, &adcgrp, samples, 2), my buffer (of 8 adcsample_t's) contains almost identical values all corresponding to CH4.

Changing the first sequence to a different channel (for example, if I set SQ_1 to CH8), then I get almost identical samples of a different value corresponding to the analog voltage on CH8.

Inspecting the code,

os/hal/ports/STM32/STM32L1xx/hal_adc_lld.c

line 227 sets SQR1 to the specified value, but ignores grpp->num_channels.

By contrast,

os/hal/ports/STM32/LLD/ADCv2/hal_adc_lld.c

line 375 sets SQR1 to the specified value, OR'd with ADC_SQR1_NUM_CH(grpp->num_channels).

If I manually set sqr1 = ADC_SQR1_NUM_CH(4) in my ADC Conversion Group structure, all works as expected. The code comments in STM32L1xx/hal_adc_lld.h do state that sqr1 contains the sequence length, but the behavior is not the same as the other ADC drivers and the documentation.

I'm using the current master from github. My apologies in advance if I'm not submitting this the right way - I'm new to ChibiOS and the community. Also, thank you so much to all the ChibiOS developers!

[Giovanni]

Hi,

Fixed as bug #1247.

Giovanni

[radar_macgyver]

Hello Giovanni,
Thank you for the quick turnaround. I did a git pull today and noticed that there was a patch to os/hal/ports/STM32/STM32L1xx/hal_adc_lld.c. Looks like there was a change, but the write is to the wrong register: ADC_SQR1_NUM_CH(grpp->num_channels) is being written to SMPR1 instead of SQR1. I applied the following:

Code: Select all

diff --git a/os/hal/ports/STM32/STM32L1xx/hal_adc_lld.c b/os/hal/ports/STM32/STM32L1xx/hal_adc_lld.c
index 78c000186..926301f86 100644
--- a/os/hal/ports/STM32/STM32L1xx/hal_adc_lld.c
+++ b/os/hal/ports/STM32/STM32L1xx/hal_adc_lld.c
@@ -224,10 +224,10 @@ void adc_lld_start_conversion(ADCDriver *adcp) {

   /* ADC setup.*/
   adcp->adc->SR    = 0;
-  adcp->adc->SMPR1 = grpp->smpr1 | ADC_SQR1_NUM_CH(grpp->num_channels);
+  adcp->adc->SMPR1 = grpp->smpr1;
   adcp->adc->SMPR2 = grpp->smpr2;
   adcp->adc->SMPR3 = grpp->smpr3;
-  adcp->adc->SQR1  = grpp->sqr1;
+  adcp->adc->SQR1  = grpp->sqr1 | ADC_SQR1_NUM_CH(grpp->num_channels);
   adcp->adc->SQR2  = grpp->sqr2;
   adcp->adc->SQR3  = grpp->sqr3;
   adcp->adc->SQR4  = grpp->sqr4;

Now things work, but they didn't previously due to writing to the incorrect register.

[Giovanni]

Hi,

Thanks for finding, fixed in repository.

Giovanni