Refactoring the STM core code

RogerClark
Fri Sep 16, 2016 11:14 pm
I’ve started to refactor the code uploaded by Frederic, so that each series has its own separate repo e.g. STM32F1xx, STM32L4xx etc

The Tools are now also in their own separate repo.

At the moment each board “variant” is still inside its parent folder e.g. the Nucleo_F103RB source is inside the STM32F1xx folder, however if we expect a lot of different boards to be created by the community and my STM, it may be worthwhile making each board into its own small repo and then referencing them as submodules

What does however seem to be a problem at the moment is that the system/libstm32f1 folder is really the source for a specifc variant not the whole series

I was able to rebuild the Nucleo variant, on WIndows, just by changing the path to gcc (as the dev’s must have used linux and had gcc installed in usr/bin)

So I think the quickest solution is simply to rename the libstm32f1 folder to lib_nucleo_f103rb, and then copy duplicate this folder to make other variants

It may be possible to move the variant source to the variant folder to speed up debugging of a new core, but I’ve not had time to investigate that.


Slammer
Fri Sep 16, 2016 11:39 pm
libstm32f1 must be remain common (in source level) for all F1 members otherwise the development and support will be a nightmare, of course each variant will have its own binary version of library in variants directory. Any source differences must be implemented in variants directory.
It is possible to move some functions from system/lib to variant if it is necessarily but to maintain almost the same copies of sources so many times it is not wise.
Just my opinion….

PS: I did not study the code yet….


RogerClark
Sat Sep 17, 2016 1:17 am
@slammer

I think libstm32f1 will need to be split up, it contains both variant specific code as well as code thats common to the stm32f1 core :-(

e.g. the variant initiation, in variant.cpp
calls hw_config_init() which is in libstm32f1 (in hw_config.c)

But in hw_config.c, hw_config_init() calls SystemClock_Config() which is variant specific e.g. in the case of the Nucleo it sets up the HSI clock etc

So I think inside libstm32f1 there would need to be a common and a variants folder, then inside the variants folder would be source, include and build_gcc folders.

Or perhaps have a variants folder under the system folder and that has subfolders for each board

I’m not sure how the development company planned on releasing more than one variant per MCU series, perhaps this wasn’t part of their remit :-(


Rick Kimball
Sat Sep 17, 2016 1:21 am
Maybe just make the hw_config_init() function a weak reference .. put a default one in the lib and let boards override in variant

RogerClark
Sat Sep 17, 2016 1:24 am
Good idea

Where will hw_config.c for each board go ?


Rick Kimball
Sat Sep 17, 2016 1:26 am
in the variant folder

RogerClark
Sat Sep 17, 2016 1:29 am
OK. So its compiled by the IDE..

Of course the USB CDCACM stuff is current not in the source code at all because that Nucleo board doesnt use it ;-(


RogerClark
Sat Sep 17, 2016 3:07 am
Rick

I’m not sure about the weak reference idea

Isn’t there an issue that libstm32f1 could be compiled without some HAL functions which the individual variant needs, or could possibly burden some variants functions that are not used

e.g. the Nucleo doesn’t use USB CDCACM but the blue pill needs it

Well, perhaps USB CDCACM would end up in all variants’s lib file but would not then get linked into the binary if it was not called in the real (strong) hw_config_init function

It still seems more flexible to me, to have variants within system folder.


Slammer
Sat Sep 17, 2016 9:26 am
The initialization of the clock, normally happens in HALMX in function SetSysClock in file system_stm32f1xx.c. This file must be specific for each board and thus must be in variant directory.
It is not possible to have a generic function in system library as weak, because of arduino build system, the system library is in archive form during linking, the same thing happens for all files of core and variant (arduino builds everything in a temporary library). It is not possible for the linker to select which function must be resolved because both versions are on different libraries. I cant see a reason to define anywhere the generic version (actually empty, that means that MCU will run at default internal clock), this function must be defined in variant directory of any variant.

RogerClark
Sat Sep 17, 2016 10:28 am
Slammer wrote:The initialization of the clock, normally happens in HALMX in function SetSysClock in file system_stm32f1xx.c. This file must be specific for each board and thus must be in variant directory.
It is not possible to have a generic function in system library as weak, because of arduino build system, the system library is in archive form during linking, the same thing happens for all files of core and variant (arduino builds everything in a temporary library). It is not possible for the linker to select which function must be resolved because both versions are on different libraries. I cant see a reason to define anywhere the generic version (actually empty, that means that MCU will run at default internal clock), this function must be defined in variant directory of any variant.

Slammer
Sat Sep 17, 2016 10:41 am
Maybe it is time to make some changes in the repository, I will have some time during the weekend. The first target is to support 2 variants (eg. Nucleo and BluePill) by separating the needed functions. As Rick is working in his own repository we need some way to work better, what is your suggestion?

RogerClark
Sat Sep 17, 2016 11:02 am
Probably the best approach is to clone the repo, like Rick did.

I have the master version cloned to my local machine, but I may also fork the repo to my own personal github account (not the stm32duino account), and try some approaches

Ricks original approach was just to use #ifdef’s to select different code in hw_config, but I think that will get very messy

We then briefly discussed using a weak definition of hw_config_init() but I have concerns about this.

So I was going to probably just add a variants folder somewhere in the system folder, e.g. inside libstm32f1, and separate the common code from the variant specific code

I think that solution would work OK, and I may have time tomorrow (sunday) to try it (I am UTC +10 so its already 21:00 local and Saturday is almost over)


Slammer
Sat Sep 17, 2016 2:00 pm
OK Roger I will fork the F1 repo stm32duino for the beginning…. and we see, it is a bit tricky now…. but as I understand this repo must be connecting link of all of us.

Rick Kimball
Sat Sep 17, 2016 6:21 pm
I was kind of surprised at the amount of ram used so I did a little digging.

$ arm-none-eabi-readelf -a BlinkNoDelay.elf >/tmp/a
$ sed -e ‘s/ \+/ /g’ /tmp/a | sort -t’ ‘ -n -k 3,3 | grep -v ‘ 08’ | grep -v ‘ 0 ‘
Num: Value Size Type Bind Vis Ndx Name
455: 20000d0c 1 OBJECT GLOBAL DEFAULT 7 g_rx_data
438: 20000004 4 OBJECT GLOBAL DEFAULT 6 SystemCoreClock
468: 20000b58 4 OBJECT GLOBAL DEFAULT 7 ledState
504: 20000000 4 OBJECT GLOBAL DEFAULT 6 interval
537: 20000b54 4 OBJECT GLOBAL DEFAULT 7 previousMillis
86: 20000b98 4 OBJECT LOCAL DEFAULT 7 uwTick
423: 20000b70 20 OBJECT GLOBAL DEFAULT 7 Serial1
477: 20000b5c 20 OBJECT GLOBAL DEFAULT 7 Serial
566: 20000b84 20 OBJECT GLOBAL DEFAULT 7 Serial2
180: 20000b9c 128 OBJECT LOCAL DEFAULT 7 g_UartHandle
548: 20001080 140 OBJECT GLOBAL DEFAULT 7 g_anInputPinConfigured
226: 200001a8 208 OBJECT LOCAL DEFAULT 6 g_timer_config
228: 20000c1c 240 OBJECT LOCAL DEFAULT 7 g_TimerHandle
287: 20000278 320 OBJECT LOCAL DEFAULT 6 gpio_irq_conf
178: 20000008 416 OBJECT LOCAL DEFAULT 6 g_uart_config
391: 20000ec8 440 OBJECT GLOBAL DEFAULT 7 g_anOutputPinConfigured
409: 20000d10 440 OBJECT GLOBAL DEFAULT 7 g_digPinConfigured
526: 200003b8 1920 OBJECT GLOBAL DEFAULT 6 g_analog_config

1920, 440, 440 416 bytes in ram

seems like g_analog_config, g_digPinConfigured, g_anOutputPinConfigured might be some structures to take a look at to see if they can be optimized

typedef struct {
GPIO_TypeDef *port;
uint32_t pin;
void (*alFunction)(void);
ADC_TypeDef *adcInstance;
ADC_ChannelConfTypeDef adcChannelConf;

#if defined (STM32F100xB) || defined (STM32F100xE) || defined (STM32F101xE) || defined (STM32F101xG) || defined (STM32F103xE) || defined (STM32F103xG) || defined (STM32F105xC) || defined (STM32F107xC)
DAC_TypeDef *dacInstance;
uint32_t dacChannel;
DAC_ChannelConfTypeDef dacChannelConf;
#endif

TIM_TypeDef *timInstance;
uint32_t timChannel;
uint32_t useNchannel;
TIM_OC_InitTypeDef timConfig;
TIM_HandleTypeDef timHandle;
} analog_config_str;

... a sample of an entry ...

analog_config_str g_analog_config[NB_ANALOG_CHANNELS] = {
{
.port = GPIOA,
.pin = GPIO_PIN_0,
.adcInstance = ADC1,
.adcChannelConf = {
.Channel = ADC_CHANNEL_0,
.Rank = ADC_REGULAR_RANK_1,
.SamplingTime = SAMPLINGTIME
},
#if defined (STM32F100xB) || defined (STM32F100xE) || defined (STM32F101xE) || defined (STM32F101xG) || defined (STM32F103xE) || defined (STM32F103xG) || defined (STM32F105xC) || defined (STM32F107xC)
.dacInstance = NULL,
#endif
.timInstance = NULL
},


Slammer
Sat Sep 17, 2016 8:38 pm
HALMX is making extensive use of large structures as arguments for functions. Nobody expects that HALMX core will be the smaller….

RogerClark
Wed Sep 21, 2016 9:29 pm
I think we can probably discuss this for ever and not do anything

The safest option is probably to duplicate the whole of libstm32f1 and call it libBluePill, then we can hack about without doing any damage to the Nucelo version.

We can determine what is common and what changes, after we have produced a few board variants.

Note. I am waiting or STM to get back to me about how we can go forward and modify the code without any risk of breaking their official Nucleo board(s) functionality.

duplicating libstm32f1 seems to do this, unless we also need to change files in the cores folder


Leave a Reply

Your email address will not be published. Required fields are marked *