Agree- but there should be some way of easily knowing whether called by boot loader or actual system.
Sterling > On Apr 10, 2017, at 6:53 PM, will sanfilippo <wi...@runtime.io> wrote: > > Thinking about this more… (which is almost never a good thing with me): > > Cortex-M MCU manufacturers produce a file called system_xxxx.c (where xxx is > the name of the mcu). There is a function in that file called SystemInit. > Generally, this is something that should be called as early as possible and > is called in the startup code. What I am now considering doing is adding our > own version of “SystemInit” as I do not think it proper to modify the > system_xxx.c module unless absolutely necessary. > > To be more concise: > > * SystemInit() will still be called and will remain exactly the way it is now. > * Add a function in hal_system.c (which is in hw/mcu) called > hal_system_init(). > * This function will be called by the startup code after SystemInit is called. > * The configuration variable for DCDCEN will live in the MCU (default to 0) > and will be overridden by any BSP that can support it. > > NOTE: I will comment on this in the hal_system_init() function, but this > function will be called by both the bootloader and the app. If there is a > case where doing something twice is undesirable that will have to be dealt > with by anyone adding code to hal_system_init(). > > Comments? > >> On Apr 7, 2017, at 4:50 PM, Sterling Hughes >> <sterling.hughes.pub...@gmail.com> wrote: >> >> Hi, >> >> Couple of thoughts: >> >> - I think this function/syscfg belongs in the MCU definition, as a >> configuration item that can be controlled by the BSP. >> >> - I think it should be called as early as possible, so probably >> hal_bsp_init(). >> >> - It’s a bid odd that hal_bsp_init() is the same for bootloader and running >> image, we should probably have an option to make this different. I’d lean >> to having more functionality in the image, because it’s upgradable. >> >> Sterling >> >>> On 7 Apr 2017, at 16:32, will sanfilippo wrote: >>> >>> Hello: >>> >>> I want to add some code that enables the DC/DC regulator for the nordic >>> chips. Enabling this regulator reduces power consumption (considerably). >>> For example, using the LDO when running from flash (cache enabled) is >>> typically 7.4mA; using the DC/DC regulator it goes to 3.7 mA. >>> >>> It would be best to turn this on as soon as possible but it should only be >>> enabled if there is some external circuitry attached to some of the pins >>> (see the product specifications for more details). For all the BSP’s >>> currently in the repo, the DC/DC regulator can (and should) be enabled. >>> GIven that there is external circuitry involved I was going to create a >>> syscfg variable that would either exist in the BSP or be overridden by the >>> BSP. What I am having a bit of trouble figuring out is where should the >>> code to enable the DC/DC regulator go? >>> >>> We have a choice of putting it in an existing place or doing something new. >>> It seems to me that if we choose an exisiting place it would go in either >>> hal_bsp_init() or hal_system_start(). >>> >>> Some comments about the existing functions: >>> >>> hal_system_start(): >>> * Code would only need to be modified in one place (in hw/mcu). >>> * This function is called after the bootloader does some work, so more >>> power savings could be realized earlier on. >>> * If you build an image with no bootloader I do not think this is called. >>> * It might be a bit of an odd place to put this code (enabled the DC/DC >>> regulator). >>> >>> hal_bsp_init(): >>> * This is called early on by the bootloader. It is also called by the >>> application which is a bit confusing to me. I am not super familiar with >>> the bootloader but unless this function exists in some place I do not see >>> or we override bsp syscfg variables in the bootloader app, hal_bsp_init() >>> is going to do things twice. Is this true or am I missing something? >>> * We would have to modify hal_bsp_init() in all the bsps. >>> >>> Honestly, I am not too concerned about having to modify all the bsps that >>> use the nordic chip if the community thinks this code belongs in >>> hal_bsp_init(). >>> >>> Any comments/suggestions? >>> >>> Thanks! >>> >>> PS If we decide that this code should exist in hw/mcu what I would do is to >>> create a syscfg variable in hw/mcu/nordic (or hw/mcu/nordic/nrf52xxx and >>> hw/mcu/nordic/nrf51xxx) called MCU_DCDC_ENABLE (or some such). By default, >>> this will be 0. All the bsp syscfg.yml files will override this setting. >
