Mbed-os: UBLOX C030 hal setting min battery voltage brining 4k unncessary code to binary

[LiyouZhou]

Description

• Type: Enhancement

Enhancement

https://github.com/ARMmbed/mbed-os/blob/806b762473f3f8807d77bba62acb28439cef5487/targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F437xG/TARGET_UBLOX_C030/min_battery_voltage.cpp#L35

The C030 Hal makes a call to battery management unit to set minimum battery level. On the code comment in the file above, it states that this operation is not absolutely necessary.

Reason to enhance or problem with existing solution

The downside of doing this is that it brings in the I2C driving which add 4k of binary size to every single build. We are building bootloaders and it is essential to get the size down.

Suggested enhancement

Is it possible to make this an option or can be turned off using some macro please?

[0xc0170]

@ARMmbed/team-ublox Please review

[RobMeades]

We're going to have to make a change here to fix #5995 so I guess we could add a "disable" switch around this but do bear in mind that in this case there is a risk that the C030 board will NOT boot or, more likely, will function unreliably, when connected to some PC USB interfaces or with longer USB cables. We would add a note explaining this risk and that the user invokes the switch at their own peril.

[RobMeades]

Thinking about it overnight, I don't think we should do this. It wouldn't make sense to create a boot loader that has less chance of booting than a normal build. The comment you refer to about this I2C operation not being essential only applies in the case where we have tried and failed to do it (when it is better to continue and complete the boot process, as a "best effort" service, than to just stop).

Does the I2C driver really take 4 kbytes of code space on a release build? It doesn't do a great deal, so that seems like a lot.

[LiyouZhou]

Compiling mbed blinky with the battery manager code turned off result in a binary almost 5k smaller. See the step by step below. You can use this tool to see what is taking up the binary size.

> git clone [email protected]:ARMmbed/mbed-os-example-blinky.git
Cloning into 'mbed-os-example-blinky'...
remote: Counting objects: 366, done.
remote: Compressing objects: 100% (4/4), done.
remote: Total 366 (delta 0), reused 2 (delta 0), pack-reused 362
Receiving objects: 100% (366/366), 212.03 KiB | 0 bytes/s, done.
Resolving deltas: 100% (117/117), done.
> cd mbed-os-example-blinky/
> mbed deploy
[mbed] Adding library "mbed-os" from "ssh://[email protected]/ARMmbed/mbed-os.git" at rev #caeaa49d68c6
 /tmp/mbed-os-example-blinky  ⑂ master    mbed compile -m UBLOX_C030_U201 -t GCC_ARM
[mbed] WARNING: Could not find mbed program in current path "/private/tmp/mbed-os-example-blinky".
[mbed] WARNING: You can fix this by calling "mbed new ." in the root of your program.
---
Building project mbed-os-example-blinky (UBLOX_C030_U201, GCC_ARM)
...
Compile [100.0%]: test_env.cpp
Link: mbed-os-example-blinky
Elf2Bin: mbed-os-example-blinky
+------------------+-------+-------+-------+
| Module           | .text | .data |  .bss |
+------------------+-------+-------+-------+
| [fill]           |    98 |     4 |    18 |
| [lib]/c.a        | 24793 |  2472 |    89 |
| [lib]/gcc.a      |  3112 |     0 |     0 |
| [lib]/misc       |   208 |    12 |    28 |
| main.o           |    64 |     4 |    28 |
| mbed-os/drivers  |   136 |     4 |   100 |
| mbed-os/features |    44 |     0 | 12556 |
| mbed-os/hal      |  1501 |     4 |    66 |
| mbed-os/platform |  1396 |     4 |   270 |
| mbed-os/rtos     |  8783 |   168 |  5989 |
| mbed-os/targets  | 11883 |     4 |   672 |
| Subtotals        | 52018 |  2676 | 19816 |
+------------------+-------+-------+-------+
Total Static RAM memory (data + bss): 22492 bytes
Total Flash memory (text + data): 54694 bytes

Image: ./BUILD/UBLOX_C030_U201/GCC_ARM/mbed-os-example-blinky.bin
> cd mbed-os && git diff
diff --git a/targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F437xG/TARGET_UBLOX_C030/min_battery_voltage.cpp b/targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F437xG/TARGET_UBLOX_C030/min_battery_voltage.cpp
index fbcd25047..106ef889c 100644
--- a/targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F437xG/TARGET_UBLOX_C030/min_battery_voltage.cpp
+++ b/targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F437xG/TARGET_UBLOX_C030/min_battery_voltage.cpp
@@ -23,7 +23,7 @@

 void HAL_MspInit(void)
 {
-       set_minimum_battery_voltage();
+       // set_minimum_battery_voltage();
 }

 void set_minimum_battery_voltage()
> cd .. && mbed compile -m UBLOX_C030_U201 -t GCC_ARM
Building project mbed-os-example-blinky (UBLOX_C030_U201, GCC_ARM)
Scan: .
Scan: mbed
Scan: env
Scan: FEATURE_LWIP
Compile [100.0%]: min_battery_voltage.cpp
Link: mbed-os-example-blinky
Elf2Bin: mbed-os-example-blinky
+------------------+-------+-------+-------+
| Module           | .text | .data |  .bss |
+------------------+-------+-------+-------+
| [fill]           |    89 |     4 |    22 |
| [lib]/c.a        | 24777 |  2472 |    89 |
| [lib]/gcc.a      |  3112 |     0 |     0 |
| [lib]/misc       |   208 |    12 |    28 |
| main.o           |    64 |     4 |    28 |
| mbed-os/drivers  |   136 |     4 |   100 |
| mbed-os/features |    44 |     0 | 12556 |
| mbed-os/hal      |  1501 |     4 |    66 |
| mbed-os/platform |  1392 |     4 |   270 |
| mbed-os/rtos     |  8783 |   168 |  5989 |
| mbed-os/targets  |  6824 |     4 |   652 |
| Subtotals        | 46930 |  2676 | 19800 |
+------------------+-------+-------+-------+
Total Static RAM memory (data + bss): 22476 bytes
Total Flash memory (text + data): 49606 bytes

Image: ./BUILD/UBLOX_C030_U201/GCC_ARM/mbed-os-example-blinky.bin

[LiyouZhou]

I tried release build these are the numbers: 28616 vs 23624, still 5k difference.

[RobMeades]

Understood, seems to be all in the target code. In our case this is all ST standard driver code for the STM32F4 MCU. @ARMmbed/team-st-mcd: is 4k bytes what you would expect for the I2C driver code size in a release build? LiyouZhou is very tight on space for a boot loading image. If there's anything he could do to reduce it that would be interesting.

[LMESTM]

@RobMeades Have you disabled I2C_SLAVE and I2C_ASYNCH in a first step ?

[LiyouZhou]

mbed compile -m UBLOX_C030_U201 -t GCC_ARM --profile release -DDEVICE_I2C_SLAVE=0 -DDEVICE_I2C_ASYNCH=0 Had no effect on the bin size. The STM HAL does not seem to contain these defines.

[LMESTM]

@LiyouZhou @RobMeades several points to share after quick testing

• it's DEVICE_I2CSLAVE not DEVICE_I2C_SLAVE
• Using command mbed compile -m UBLOX_C030_U201 -t GCC_ARM --profile release -DDEVICE_I2C_SLAVE=0 -DDEVICE_I2C_ASYNCH=0 does not seem to work. Testing with a basi I2C test, I've been able to save about 230 bytes by disabling those device feature directly in targets.json.
• From a quick test, Replacing calls to
  int write(int address, const char *data, int length, bool repeated = false);
  by calls to
  int write(int data);
  (and same for read) provides much more savings (> 3KB in my test). At HAL level, this is
  int i2c_byte_write(i2c_t *obj, int data) and int i2c_byte_read(i2c_t *obj, int last)
  This is maybe worth a try to move the battery driver to this simple API if this is simple enough ...

[RobMeades]

Thanks @LMESTM, that's very useful data.

@bqam-ublox: could we try re-coding set_minimum_battery_voltage() to use i2c_object.write() and i2c_object.read()?

[bqam-ublox]

@RobMeades do you mean using i2c_byte_read and i2c_byte_write functions from i2c_api.h? I will definitely look into it.

[LMESTM]

@RobMeades @bqam-ublox one question out of curiosity: why do you use the HAL layer API and not an mbed I2C object ? and as a consequence how do you manage the I2C lock against multiple accesses of several drivers using the same I2C bus (that is done in mbed I2C driver)

[RobMeades]

I don't think you can use mbed I2C objects from HAL_MspInit() as it's before LIBC has been initialised, C++ is not up yet (and I have a feeling that the HAL include paths don't get brought into this portion of the build process).

[bqam-ublox]

We cannot use the I2C driver provided in mbed because it needs locks and acquires the mutex which is not allowed in ISR context. I tried once, CMSIS-RTOS gives us osErrorISR when using the mbed driver. Since, this is at boot time, no one else can access the same I2C.

[LMESTM]

@RobMeades @bqam-ublox ok clear - thanks. for the info

[0xc0170]

6117 was integrated, @LiyouZhou Please test and reopen if this issue persist

[LiyouZhou]

Thank you for the swift fix. I tested it and the binary size reduction is significant. @0xc0170 when will this be released?

[0xc0170]

Thank you for the swift fix. I tested it and the binary size reduction is significant. @0xc0170 when will this be released?

Based on the version label - the next week