Skip to content

Config: fix bootloader config errors - propagate errors #12059

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ ITCM-RAM: 16K (0x3FFF) @0x00000000
if (!isdefinedsymbol(MBED_ROM_START)) { define symbol MBED_ROM_START = 0x8000000; }
if (!isdefinedsymbol(MBED_ROM_SIZE)) { define symbol MBED_ROM_SIZE = 0x200000; }
if (!isdefinedsymbol(MBED_RAM_START)) { define symbol MBED_RAM_START = 0x20010000; }
if (!isdefinedsymbol(MBED_RAM_START)) { define symbol MBED_RAM_SIZE = 0x40000; }
if (!isdefinedsymbol(MBED_RAM_SIZE)) { define symbol MBED_RAM_SIZE = 0x40000; }
if (!isdefinedsymbol(MBED_RAM1_START)) { define symbol MBED_RAM1_START = 0x20000000; }
if (!isdefinedsymbol(MBED_RAM1_SIZE)) { define symbol MBED_RAM1_SIZE = 0x10000; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ ITCM-RAM: 16K (0x3FFF) @0x00000000
if (!isdefinedsymbol(MBED_ROM_START)) { define symbol MBED_ROM_START = 0x8000000; }
if (!isdefinedsymbol(MBED_ROM_SIZE)) { define symbol MBED_ROM_SIZE = 0x200000; }
if (!isdefinedsymbol(MBED_RAM_START)) { define symbol MBED_RAM_START = 0x20010000; }
if (!isdefinedsymbol(MBED_RAM_START)) { define symbol MBED_RAM_SIZE = 0x40000; }
if (!isdefinedsymbol(MBED_RAM_SIZE)) { define symbol MBED_RAM_SIZE = 0x40000; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

But as I am a copy paste expert... you need to update also stm32f746xg.icf, stm32f767xi.icf, and stm32f769xi.icf...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how come they don't fail the build? will check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a copy paste expert!

if (!isdefinedsymbol(MBED_RAM1_START)) { define symbol MBED_RAM1_START = 0x20000000; }
if (!isdefinedsymbol(MBED_RAM1_SIZE)) { define symbol MBED_RAM1_SIZE = 0x10000; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ ITCM-RAM: 16K (0x3FFF) @0x00000000
if (!isdefinedsymbol(MBED_ROM_START)) { define symbol MBED_ROM_START = 0x8000000; }
if (!isdefinedsymbol(MBED_ROM_SIZE)) { define symbol MBED_ROM_SIZE = 0x200000; }
if (!isdefinedsymbol(MBED_RAM_START)) { define symbol MBED_RAM_START = 0x20020000; }
if (!isdefinedsymbol(MBED_RAM_START)) { define symbol MBED_RAM_SIZE = 0x60000; }
if (!isdefinedsymbol(MBED_RAM_SIZE)) { define symbol MBED_RAM_SIZE = 0x60000; }
if (!isdefinedsymbol(MBED_RAM1_START)) { define symbol MBED_RAM1_START = 0x20000000; }
if (!isdefinedsymbol(MBED_RAM1_SIZE)) { define symbol MBED_RAM1_SIZE = 0x20000; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ ITCM-RAM: 16K (0x3FFF) @0x00000000
if (!isdefinedsymbol(MBED_ROM_START)) { define symbol MBED_ROM_START = 0x8000000; }
if (!isdefinedsymbol(MBED_ROM_SIZE)) { define symbol MBED_ROM_SIZE = 0x200000; }
if (!isdefinedsymbol(MBED_RAM_START)) { define symbol MBED_RAM_START = 0x20020000; }
if (!isdefinedsymbol(MBED_RAM_START)) { define symbol MBED_RAM_SIZE = 0x60000; }
if (!isdefinedsymbol(MBED_RAM_SIZE)) { define symbol MBED_RAM_SIZE = 0x60000; }
if (!isdefinedsymbol(MBED_RAM1_START)) { define symbol MBED_RAM1_START = 0x20000000; }
if (!isdefinedsymbol(MBED_RAM1_SIZE)) { define symbol MBED_RAM1_SIZE = 0x20000; }

Expand Down
30 changes: 13 additions & 17 deletions targets/targets.json
Original file line number Diff line number Diff line change
Expand Up @@ -5878,7 +5878,7 @@
"device_has_add": ["USBDEVICE", "EMAC", "FLASH", "LPTICKER"],
"release_versions": ["2", "5"],
"device_name": "R7S72100",
"bootloader_supported": true
"bootloader_supported": false
},
"VK_RZ_A1H": {
"inherits": ["RZ_A1XX"],
Expand All @@ -5895,7 +5895,7 @@
"device_has_remove": ["ETHERNET"],
"release_versions": ["2", "5"],
"device_name": "R7S72103",
"bootloader_supported": true,
"bootloader_supported": false,
"overrides": {
"network-default-interface-type": null
}
Expand Down Expand Up @@ -6753,7 +6753,8 @@
},
"overrides": {
"network-default-interface-type": "MESH"
}
},
"bootloader_supported": false
},
"EFM32PG12B500F1024GL125": {
"inherits": ["EFM32"],
Expand Down Expand Up @@ -8475,7 +8476,7 @@
"MPU"
],
"release_versions": ["2", "5"],
"bootloader_supported": true
"bootloader_supported": false
},
"NUCLEO_WB55RG": {
"inherits": ["FAMILY_STM32"],
Expand Down Expand Up @@ -8509,7 +8510,7 @@
],
"features": ["BLE"],
"release_versions": ["2", "5"],
"bootloader_supported": true
"bootloader_supported": false
},
"VBLUNO52": {
"supported_form_factors": ["ARDUINO"],
Expand Down Expand Up @@ -9184,7 +9185,7 @@
"post_binary_hook": {
"function": "PSOC6Code.complete"
},
"bootloader_supported": true,
"bootloader_supported": false,
"sectors": [[268435456, 512]],
"overrides": {
"network-default-interface-type": "WIFI"
Expand All @@ -9197,7 +9198,7 @@
"post_binary_hook": {
"function": "PSOC6Code.complete"
},
"bootloader_supported": true,
"bootloader_supported": false,
"sectors": [[268435456, 512]]
},
"CY8CPROTO_062S3_4343W": {
Expand Down Expand Up @@ -9229,7 +9230,7 @@
"function": "PSOC6Code.complete"
},
"sectors": [[268443648, 512]],
"bootloader_supported": true
"bootloader_supported": false
},
"CY8CPROTO_063_BLE": {
"inherits": ["MCU_PSOC6_M4"],
Expand All @@ -9245,7 +9246,7 @@
"function": "PSOC6Code.complete"
},
"sectors": [[268443648, 512]],
"bootloader_supported": true
"bootloader_supported": false
},
"CY8CPROTO_064_SB": {
"inherits": ["MCU_PSOC6_M4"],
Expand Down Expand Up @@ -9279,7 +9280,7 @@
"post_binary_hook": {
"function": "PSOC6Code.complete"
},
"bootloader_supported": true,
"bootloader_supported": false,
"sectors": [[268435456, 512]],
"overrides": {
"network-default-interface-type": "WIFI"
Expand Down Expand Up @@ -9461,15 +9462,10 @@
"inherits": ["RDA5981X"],
"detect_code": ["8001"],
"components_add": ["FLASHIAP"],
"bootloader_supported": true,
"mbed_ram_start": "0x00100080",
"mbed_ram_size": "0x1ff80",
"mbed_rom_start": "0x18001000",
"mbed_rom_size": "0x1F4000",
"sectors": [[0,4096]],
"overrides": {
"network-default-interface-type" : "WIFI"
}
},
"bootloader_supported": false
},
"GD32_Target": {
"inherits": ["Target"],
Expand Down
91 changes: 42 additions & 49 deletions tools/toolchains/mbed_toolchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -894,63 +894,56 @@ def _add_all_regions(self, region_list, active_region_name):
def add_regions(self):
"""Add regions to the build profile, if there are any.
"""

if not getattr(self.target, "bootloader_supported", False):
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather encode the default behaviour in self.target rather than define the default here, other places where this is used may assume a different default. There is also a danger of typos using this technique that would not cause exceptions as if there is a typos the result is always False. Maybe these are relatively minor points but they do somewhat hinder maintainability so should be avoided in code we wish to maintain in the long term.

Copy link
Contributor Author

@0xc0170 0xc0170 Dec 9, 2019

Choose a reason for hiding this comment

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

This is how it is done in regions().
It should be done properly but I don't have entire picture how config works yet . I found this the easiest way to fix now (same check is used inside config but in the later phase when its too late to go back) and later to even rewrite this add_regions to catch errors better.

Do you suggest to have target method to return if bootloader is supported and add_regions() would not even be called if not?

return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the addition for #12007 . The rest was approved already.


if self.config.has_regions:
try:
regions = list(self.config.regions)
regions.sort(key=lambda x: x.start)
self.notify.info("Using ROM region%s %s in this build." % (
"s" if len(regions) > 1 else "",
", ".join(r.name for r in regions)
))
self._add_all_regions(regions, "MBED_APP")
except ConfigException:
pass
regions = list(self.config.regions)
regions.sort(key=lambda x: x.start)
self.notify.info("Using ROM region%s %s in this build." % (
"s" if len(regions) > 1 else "",
", ".join(r.name for r in regions)
))
self._add_all_regions(regions, "MBED_APP")

if self.config.has_ram_regions:
try:
regions = list(self.config.ram_regions)
self.notify.info("Using RAM region%s %s in this build." % (
"s" if len(regions) > 1 else "",
", ".join(r.name for r in regions)
))
self._add_all_regions(regions, None)
except ConfigException:
pass
regions = list(self.config.ram_regions)
self.notify.info("Using RAM region%s %s in this build." % (
"s" if len(regions) > 1 else "",
", ".join(r.name for r in regions)
))
self._add_all_regions(regions, None)

Region = namedtuple("Region", "name start size")

try:
# Add all available ROM regions to build profile
if not getattr(self.target, "static_memory_defines", False):
raise ConfigException()
rom_available_regions = self.config.get_all_active_memories(
ROM_ALL_MEMORIES

# Add all available ROM regions to build profile
if not getattr(self.target, "static_memory_defines", False):
raise ConfigException()
rom_available_regions = self.config.get_all_active_memories(
ROM_ALL_MEMORIES
)
for key, value in rom_available_regions.items():
rom_start, rom_size = value
self._add_defines_from_region(
Region("MBED_" + key, rom_start, rom_size),
True,
suffixes=["_START", "_SIZE"]
)
for key, value in rom_available_regions.items():
rom_start, rom_size = value
self._add_defines_from_region(
Region("MBED_" + key, rom_start, rom_size),
True,
suffixes=["_START", "_SIZE"]
)
except ConfigException:
pass
try:
# Add all available RAM regions to build profile
if not getattr(self.target, "static_memory_defines", False):
raise ConfigException()
ram_available_regions = self.config.get_all_active_memories(
RAM_ALL_MEMORIES
# Add all available RAM regions to build profile
if not getattr(self.target, "static_memory_defines", False):
raise ConfigException()
ram_available_regions = self.config.get_all_active_memories(
RAM_ALL_MEMORIES
)
for key, value in ram_available_regions.items():
ram_start, ram_size = value
self._add_defines_from_region(
Region("MBED_" + key, ram_start, ram_size),
True,
suffixes=["_START", "_SIZE"]
)
for key, value in ram_available_regions.items():
ram_start, ram_size = value
self._add_defines_from_region(
Region("MBED_" + key, ram_start, ram_size),
True,
suffixes=["_START", "_SIZE"]
)
except ConfigException:
pass

STACK_PARAM = "target.boot-stack-size"
TFM_LVL_PARAM = "tfm.level"
Expand Down