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.

Reply via email to