Skip to content
This repository was archived by the owner on Jun 5, 2019. It is now read-only.

Fix to build Discovery 4 using GCC 5.4.1 #475

Merged
merged 2 commits into from
Aug 10, 2016

Conversation

josesimoes
Copy link
Contributor

  • add new symbol to missing symbols for gcc scatter file
  • add new-lib nano setting to tinybooter proj
  • add platform emulated setting to solution settings
  • add compiler flags to enable C11

@@ -23,6 +23,8 @@
<ExtraTargets>$(ExtraTargets);CompressBin</ExtraTargets>
<ScatterFileDefinition>scatterfile_bootloader_$(COMPILER_TOOL).$(SCATTER_EXT)</ScatterFileDefinition>
<EXEScatterFileDefinition>scatterfile_bootloader_$(COMPILER_TOOL).$(SCATTER_EXT)</EXEScatterFileDefinition>
<!-- newlib-nano does not support long-long and long-double formatting, use standard newlib instead -->
Copy link
Member

@smaillet-ms smaillet-ms Jul 9, 2016

Choose a reason for hiding this comment

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

What is it in Tinybooter that is requiring long-long and long-double formatting? Perhaps we should fix/remove that so we don't need to do this and can use newlib-nano?

Well actually scratch that, it seems like the comment is redundant as it is setting NewLibNano == true for GCC...

Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC, the Tinybooter code does not require long-long and long-double formatting support, but the TinyCLR does (uses "%I64" and "%lld" printf formats). The newlib-nano included in GCC toolchain does not have those options enabled (i.e. long.ToString() and double.ToString() does not work correctly), so there are basically two options:

  1. Use default newlib-nano in TinyBooter and standard newlib in TinyCLR,
  2. Build a custom newlib-nano with long-long and long-double formatting support and use it in both TinyBooter and TinyCLR.

The ideal solution would be to use default newlib-nano in Tinybooter and a custom newlib-nano in TinyCLR - I guess that could be done with some additional tweaking of linker settings, but it would probably be too confusing...

Unfortunately, I have not yet had a chance to review the recent GCC versions, sometimes they introduce changes in newlib-nano, so if they for example added long-long and long-double formatting by default, it could made things a little bit easier.

Copy link
Member

Choose a reason for hiding this comment

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

Another option would be to change the CLR code so it doesn't use printf style formatting from the CRT for long and double and instead does it's own thing. Thus, allowing for newlib-nano in all cases. It would be ideal if we could figure out how to eliminate the duplication of the CRT completely (e.g. Tinybooter has a copy of the CRT and so does TinyCLR ) do we really NEED two copies of the CRT in the device?)

Copy link
Contributor

Choose a reason for hiding this comment

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

The obvious/naïve solution to eliminate Tinybooter's CRT is to eliminate the Tinybooter itself :)

Copy link
Member

Choose a reason for hiding this comment

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

A fair point, though it presumes that all devices have some sort of firmware update support built into the silicon to allow updates even if the main application image is completely hosed.

Doing that for NETMF requires some major re-work of the wireProtocol and debugger communication layers (and MFDeploy). Effectively abstracting out the entire wire protocol so that a plug-in of some sort could manage all the actions needed by MFDeploy to update the NETMF image. Unfortunately most such facilities built into silicon and devices requires some sort of external hardware/jumper to engage the update. (Although for development purposes the abstraction could also allow for standard JTAG/SWD/CMSIS-DAP access too which would provide full access and control [Yeah, I've been thinking about this one a while!])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I vote for changing the ToString implementation instead of sacrificing the newlib nano use.

@@ -16,6 +16,7 @@
<IsSolutionWizardVisible>True</IsSolutionWizardVisible>
<ENDIANNESS>le</ENDIANNESS>
<NO_BOOTLOADER_COMPRESSION>true</NO_BOOTLOADER_COMPRESSION>
<PLATFORM_EMULATED_FLOATINGPOINT Condition="'$(COMPILER_TOOL)'=='GCC'">true</PLATFORM_EMULATED_FLOATINGPOINT>
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand this is required to enable use of newlib-nano, but is it really worth it to use emulation on a CPU with a hardware FPU just to save a few percents of the firmware size?

ER_FLASH size built with GCC 5.4.1 and standard newlib is 327084, which is about 6% larger than 308380 reported in issue #474; the practical difference is even smaller due to the block size in the flash memory code area.

How about moving this setting to a separate 'size-optimized' solution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this because of the crypto libs. GCC link complains about a mismatch in the VFP flags between the crypto libs and the netmf. This has been discussed a number of times on other threads.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I have found the issues, thanks.

So, if I understand it correctly, this happens only when crypto.lib is used (resp. installed)? Because IMHO it would be much better to fix the cause in crypto.lib (as you suggest in the other comment) than modifying floating point setting (which would be needed for all solutions configured to use newlib-nano).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants