Skip to content

Fix AnalogIn implementation on STM32G4 series #13601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 30, 2020
Merged

Fix AnalogIn implementation on STM32G4 series #13601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e9d6c9c
Fix AnalogIn implementation on STM32G4 series
AGlass0fMilk Sep 11, 2020
bbc15f6
Implemented adc deinitialization functionality
AGlass0fMilk Sep 21, 2020
bbc7355
Merge internal ADC channel changes from ST
jeromecoutant Sep 21, 2020
76d488d
Revert sampling time decrease and remove todos.
AGlass0fMilk Sep 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions targets/TARGET_STM/TARGET_STM32G4/TARGET_STM32G474xx/TARGET_NUCLEO_G474RE/PeripheralPins.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,10 @@ MBED_WEAK const PinMap PinMap_ADC[] = {
{NC, NC, 0}
};

// !!! SECTION TO BE CHECKED WITH DEVICE REFERENCE MANUAL
MBED_WEAK const PinMap PinMap_ADC_Internal[] = {
{ADC_TEMP, ADC_1, STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 16, 0)},
{ADC_VREF, ADC_1, STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 17, 0)},
{ADC_VBAT, ADC_1, STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 18, 0)},
{ADC_TEMP, ADC_1, STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 16, 0)}, // ADC1_IN16
{ADC_VREF, ADC_1, STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 18, 0)}, // ADC1_IN18
{ADC_VBAT, ADC_1, STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 17, 0)}, // ADC1_IN17
{NC, NC, 0}
};

Expand Down
189 changes: 149 additions & 40 deletions targets/TARGET_STM/TARGET_STM32G4/analogin_device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,43 @@
#include "mbed_error.h"
#include "mbed_debug.h"
#include "PeripheralPins.h"
#include "stm32g4xx_ll_bus.h"
#include "string.h"

#if defined(ADC1)
static uint8_t adc1_en_counter = 0;
#define ADC1_EN_CTR adc1_en_counter
#else
#define ADC1_EN_CTR 0
#endif

#if defined(ADC2)
static uint8_t adc2_en_counter = 0;
#define ADC2_EN_CTR adc2_en_counter
#else
#define ADC2_EN_CTR 0
#endif

#if defined(ADC3)
static uint8_t adc3_en_counter = 0;
#define ADC3_EN_CTR adc3_en_counter
#else
#define ADC3_EN_CTR 0
#endif

#if defined(ADC4)
static uint8_t adc4_en_counter = 0;
#define ADC4_EN_CTR adc4_en_counter
#else
#define ADC4_EN_CTR 0
#endif

#if defined(ADC5)
static uint8_t adc5_en_counter = 0;
#define ADC5_EN_CTR adc5_en_counter
#else
#define ADC5_EN_CTR 0
#endif

#if STATIC_PINMAP_READY
#define ANALOGIN_INIT_DIRECT analogin_init_direct
Expand Down Expand Up @@ -72,6 +109,7 @@ static void _analogin_init_direct(analogin_t *obj, const PinMap *pinmap)

// Configure ADC object structures
obj->handle.State = HAL_ADC_STATE_RESET;
memset(&obj->handle.Init, 0, sizeof(obj->handle.Init));
obj->handle.Init.ClockPrescaler = ADC_CLOCK_SYNC_PCLK_DIV2;
obj->handle.Init.Resolution = ADC_RESOLUTION_12B;
obj->handle.Init.DataAlign = ADC_DATAALIGN_RIGHT;
Expand All @@ -82,40 +120,48 @@ static void _analogin_init_direct(analogin_t *obj, const PinMap *pinmap)
obj->handle.Init.NbrOfConversion = 1;
obj->handle.Init.DiscontinuousConvMode = DISABLE;
obj->handle.Init.NbrOfDiscConversion = 0;
obj->handle.Init.ExternalTrigConv = ADC_EXTERNALTRIG_T1_CC1;
obj->handle.Init.ExternalTrigConv = ADC_SOFTWARE_START;
obj->handle.Init.ExternalTrigConvEdge = ADC_EXTERNALTRIGCONVEDGE_NONE;
obj->handle.Init.DMAContinuousRequests = DISABLE;
obj->handle.Init.Overrun = ADC_OVR_DATA_OVERWRITTEN;
obj->handle.Init.GainCompensation = 0;
obj->handle.Init.OversamplingMode = DISABLE;
obj->handle.Init.SamplingMode = ADC_SAMPLING_MODE_NORMAL;

#if defined(ADC1)
if ((ADCName)obj->handle.Instance == ADC_1) {
__HAL_RCC_ADC12_CONFIG(RCC_ADC12CLKSOURCE_SYSCLK); // TODO - which clock?
// SYSCLK or PLL?
LL_AHB2_GRP1_EnableClock(LL_AHB2_GRP1_PERIPH_ADC12);
__HAL_RCC_ADC12_CONFIG(RCC_ADC12CLKSOURCE_SYSCLK);
adc1_en_counter++;
}
#endif
#if defined(ADC2)
if ((ADCName)obj->handle.Instance == ADC_2) {
__HAL_RCC_ADC12_CONFIG(RCC_ADC12CLKSOURCE_SYSCLK); // TODO - which clock?
// SYSCLK or PLL?
LL_AHB2_GRP1_EnableClock(LL_AHB2_GRP1_PERIPH_ADC12);
__HAL_RCC_ADC12_CONFIG(RCC_ADC12CLKSOURCE_SYSCLK);
adc2_en_counter++;
}
#endif
#if defined(ADC3)
if ((ADCName)obj->handle.Instance == ADC_3) {
__HAL_RCC_ADC345_CONFIG(RCC_ADC345CLKSOURCE_SYSCLK); // TODO - which clock?
// SYSCLK or PLL?
LL_AHB2_GRP1_EnableClock(LL_AHB2_GRP1_PERIPH_ADC345);
__HAL_RCC_ADC345_CONFIG(RCC_ADC345CLKSOURCE_SYSCLK);
adc3_en_counter++;
}
#endif
#if defined(ADC4)
if ((ADCName)obj->handle.Instance == ADC_4) {
__HAL_RCC_ADC345_CONFIG(RCC_ADC345CLKSOURCE_SYSCLK); // TODO - which clock?
// SYSCLK or PLL?
LL_AHB2_GRP1_EnableClock(LL_AHB2_GRP1_PERIPH_ADC345);
__HAL_RCC_ADC345_CONFIG(RCC_ADC345CLKSOURCE_SYSCLK);
adc4_en_counter++;
}
#endif

#if defined(ADC5)
if ((ADCName)obj->handle.Instance == ADC_4) {
__HAL_RCC_ADC345_CONFIG(RCC_ADC345CLKSOURCE_SYSCLK); // TODO - which clock?
// SYSCLK or PLL?
if ((ADCName)obj->handle.Instance == ADC_5) {
LL_AHB2_GRP1_EnableClock(LL_AHB2_GRP1_PERIPH_ADC345);
__HAL_RCC_ADC345_CONFIG(RCC_ADC345CLKSOURCE_SYSCLK);
adc5_en_counter++;
}

#endif
Expand Down Expand Up @@ -153,7 +199,7 @@ uint16_t adc_read(analogin_t *obj)

// Configure ADC channel
sConfig.Rank = ADC_REGULAR_RANK_1;
sConfig.SamplingTime = ADC_SAMPLETIME_24CYCLES_5;
sConfig.SamplingTime = ADC_SAMPLETIME_247CYCLES_5;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really need so much time ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If my math is correct, with the PCLK @ 180MHz and the ADC prescaler set to DIV2, a sample time of 24.5 clock cycles is only ~272 nanoseconds... I think this is quite short for most of the slow signals Mbed users will be trying to read with the simple AnalogIn API.

My comment from before:

Also, since the clock speed on this target is so high by default I am seeing small offsets in my ADC readings due to the short accumulation time.
Setting the ADC prescaler to divide by 4 and setting the sampling time to 640.5 cycles greatly improves the offset but maybe there's a way to get make it so I'm not maxing out the settings?

Perhaps we could increase the ADC prescaler?

sConfig.SingleDiff = ADC_SINGLE_ENDED;
sConfig.OffsetNumber = ADC_OFFSET_NONE;
sConfig.Offset = 0;
Expand Down Expand Up @@ -202,45 +248,31 @@ uint16_t adc_read(analogin_t *obj)
sConfig.Channel = ADC_CHANNEL_14;
break;
case 15:
if ((ADCName)obj->handle.Instance == ADC_1) {
sConfig.Channel = ADC_CHANNEL_VOPAMP1;
sConfig.SamplingTime = ADC_SAMPLETIME_247CYCLES_5;
} else {
sConfig.Channel = ADC_CHANNEL_15;
}
sConfig.Channel = ADC_CHANNEL_15;
break;
case 16:
sConfig.Channel = ADC_CHANNEL_16;

if ((ADCName)obj->handle.Instance == ADC_1) {
sConfig.Channel = ADC_CHANNEL_TEMPSENSOR_ADC1;
sConfig.SamplingTime = ADC_SAMPLETIME_247CYCLES_5;
} else {
sConfig.Channel = ADC_CHANNEL_16;
sConfig.SamplingTime = ADC_SAMPLETIME_640CYCLES_5;
}
break;
case 17:
sConfig.SamplingTime = ADC_SAMPLETIME_247CYCLES_5;
sConfig.Channel = ADC_CHANNEL_17;

if ((ADCName)obj->handle.Instance == ADC_1) {
sConfig.Channel = ADC_CHANNEL_VBAT;
sConfig.SamplingTime = ADC_SAMPLETIME_640CYCLES_5;
}
#if defined(ADC2)
if ((ADCName)obj->handle.Instance == ADC_2) {
sConfig.Channel = ADC_CHANNEL_VOPAMP2;
}
#endif
#if defined(ADC3)
if ((ADCName)obj->handle.Instance == ADC_3) {
sConfig.Channel = ADC_CHANNEL_VOPAMP3_ADC3;
}
#endif
#if defined(ADC4)
if ((ADCName)obj->handle.Instance == ADC_4) {
sConfig.Channel = ADC_CHANNEL_VOPAMP4;
}
#endif
break;
case 18:
sConfig.SamplingTime = ADC_SAMPLETIME_247CYCLES_5;
sConfig.Channel = ADC_CHANNEL_VREFINT;
sConfig.Channel = ADC_CHANNEL_18;

if ((ADCName)obj->handle.Instance == ADC_1) {
sConfig.Channel = ADC_CHANNEL_VREFINT;
sConfig.SamplingTime = ADC_SAMPLETIME_640CYCLES_5;
}
break;
default:
return 0;
Expand All @@ -261,7 +293,9 @@ uint16_t adc_read(analogin_t *obj)
} else {
debug("HAL_ADC_PollForConversion issue\n");
}

LL_ADC_SetCommonPathInternalCh(__LL_ADC_COMMON_INSTANCE((&obj->handle)->Instance), LL_ADC_PATH_INTERNAL_NONE);

if (HAL_ADC_Stop(&obj->handle) != HAL_OK) {
debug("HAL_ADC_Stop issue\n");;
}
Expand All @@ -274,4 +308,79 @@ const PinMap *analogin_pinmap()
return PinMap_ADC;
}

void analogin_free(analogin_t *obj)
{
#if defined(ADC1)
if ((ADCName)obj->handle.Instance == ADC_1) {
adc1_en_counter--;
if(ADC1_EN_CTR == 0)
{
HAL_ADC_DeInit(&obj->handle);

// Disable clock if ADC2 is also unused
if(ADC2_EN_CTR == 0) {
LL_AHB2_GRP1_DisableClock(LL_AHB2_GRP1_PERIPH_ADC12);
}
}
}
#endif
#if defined(ADC2)
if ((ADCName)obj->handle.Instance == ADC_2) {
adc2_en_counter--;
if(ADC2_EN_CTR == 0)
{
HAL_ADC_DeInit(&obj->handle);

// Disable clock if ADC1 is also unused
if(ADC1_EN_CTR == 0) {
LL_AHB2_GRP1_DisableClock(LL_AHB2_GRP1_PERIPH_ADC12);
}
}
}
#endif
#if defined(ADC3)
if ((ADCName)obj->handle.Instance == ADC_3) {
adc3_en_counter--;
if(ADC3_EN_CTR == 0)
{
HAL_ADC_DeInit(&obj->handle);

// Disable clock if ADC4 and ADC5 are also unused
if((ADC4_EN_CTR + ADC5_EN_CTR) == 0) {
LL_AHB2_GRP1_DisableClock(LL_AHB2_GRP1_PERIPH_ADC345);
}
}
}
#endif
#if defined(ADC4)
if ((ADCName)obj->handle.Instance == ADC_4) {
adc4_en_counter--;
if(ADC4_EN_CTR == 0)
{
HAL_ADC_DeInit(&obj->handle);

// Disable clock if ADC3 and ADC5 are also unused
if((ADC3_EN_CTR + ADC5_EN_CTR) == 0) {
LL_AHB2_GRP1_DisableClock(LL_AHB2_GRP1_PERIPH_ADC345);
}
}
}
#endif

#if defined(ADC5)
if ((ADCName)obj->handle.Instance == ADC_5) {
adc5_en_counter--;
if(ADC5_EN_CTR == 0)
{
HAL_ADC_DeInit(&obj->handle);

// Disable clock if ADC3 and ADC4 are also unused
if((ADC3_EN_CTR + ADC4_EN_CTR) == 0) {
LL_AHB2_GRP1_DisableClock(LL_AHB2_GRP1_PERIPH_ADC345);
}
}
}
#endif
}

#endif
3 changes: 0 additions & 3 deletions targets/targets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2469,9 +2469,6 @@
"EXTRA_IDLE_STACK_REQUIRED",
"MBED_TICKLESS"
],
"device_has_remove": [
"ANALOGIN"
],
"overrides": {
"lpticker_delay_ticks": 0
},
Expand Down