This issue was referenced in #10857

For SLOT0_LEN, the ROM_LEN is expected to be in KB by default.

It is not expected to be in `KB, it is just that we handle it when it is defined as 512K or something.

However I still do not get why we have an error when it is defined as 512 * 1024 and I would like to understand it before changing it.

I somehow remember issues while defining values only as $$() without giving it to the shell but do not remember what. Maybe @kYc0o remembers where we had the issue ?

Change this PR with another fix as discussed IRL with @cladmi

I really do not like having to add a subshell here for an unknown reason. Does it work if you just remove the spaces inside the parentheses and keep the whole thing as it is in master? The spaves might have messed up the processing in the other place.

having to add a subshell here for an unknown reason

The problem is because we need to perform the variable expansion before the replacement that is done on it for computing the SLOT0_LEN here. This SO answer mention that the $$ requires this expansion to be done by the shell.

Does it work if you just remove the spaces inside the parentheses and keep the whole thing as it is in master?

I tried and it doesn't make any difference. Another potential fix is to use the shell command when evaluating the value of ROM_LEN here.

The reason why it would be needed to change to $(shell) or change to an explicit value is that the variable value must not only be used from recipes where $(()) will correctly be evaluated but also but also directly from within the makefiles.

It was not the case before but now ROM_LEN is used and evaluated from within the makefiles so is supposed to be a variable with a valid value. It is currently not needed for kinetis, but if we want to try using the same riotboot/ and cortexm_common mechanism, it would need to be harmonized.

The simplest solution would be to do ROM_LEN = $(KINETIS_ROMSIZE)K
This requires no shell evaluation and the linker and cortexm_common/Makefile.include handles the 'K'.
RAM_LEN cannot as it is can be 256/8 and adding a K after breaks it but it is not needed for riotboot.

Another solution, would be to change KINETIS_ROMSIZE to be in bytes and not in kibytes and then also change KINETIS_RAMSIZE and KINETIS_SRAM_L_SIZE to be in bytes.
But it would be a lot more line changes than the other one for maybe no real benefit.

I could run make -C examples/hello-world buildtest with this diff:

diff --git a/cpu/kinetis/Makefile.include b/cpu/kinetis/Makefile.include
index a5f36b4ed..7d60f9c22 100644
--- a/cpu/kinetis/Makefile.include
+++ b/cpu/kinetis/Makefile.include
@@ -13,7 +13,10 @@ LINKER_SCRIPT = kinetis.ld
 ROM_START_ADDR = 0x00000000
 RAM_BASE_ADDR  = 0x20000000
 RAM_START_ADDR = $$(($(RAM_BASE_ADDR)-($(KINETIS_SRAM_L_SIZE) * 1024)))
-ROM_LEN = $$(($(KINETIS_ROMSIZE) * 1024))
+# Define ROM_LEN with a non arithmetic value as it must be
+# evaluated in `cortexm_common` without a shell context
+# The `K` is correctly handled by both the linker and `cortexm_common`.
+ROM_LEN = $(KINETIS_ROMSIZE)K
 RAM_LEN = $$(($(KINETIS_RAMSIZE) * 1024))
 
 CFLAGS += \

It is also not showing errors anymore:

for board in $(git grep 'CPU = kinetis' | sed 's|boards/||;s|/.*||'); do echo ${board}; BOARD=${board} make --no-print-directory -C examples/hello-world/ clean;  done
frdm-k22f
frdm-k64f
frdm-kw41z
mulle
pba-d-01-kw2x
phynode-kw41z
teensy31
usb-kw41z

In master they all have the Syntax error:

for board in $(git grep 'CPU = kinetis' | sed 's|boards/||;s|/.*||'); do echo ${board}; BOARD=${board} make --no-print-directory -C examples/hello-world/ clean;  done
frdm-k22f
/bin/sh: 1: Syntax error: Missing '))'
frdm-k64f
/bin/sh: 1: Syntax error: Missing '))'
frdm-kw41z
/bin/sh: 1: Syntax error: Missing '))'
mulle
/bin/sh: 1: Syntax error: Missing '))'
pba-d-01-kw2x
/bin/sh: 1: Syntax error: Missing '))'
phynode-kw41z
/bin/sh: 1: Syntax error: Missing '))'
teensy31
/bin/sh: 1: Syntax error: Missing '))'
usb-kw41z
/bin/sh: 1: Syntax error: Missing '))'

And even more, now that I had test to report. The _rom_length linker symbol is the same with my proposed solution:

Setup:

git clean -xdf examples/hello-world/; for board in $(git grep 'CPU = kinetis' | sed 's|boards/||;s|/.*||'); do echo ${board}; BOARD=${board} make --no-print-directory -C examples/hello-world/; done

The output is the same with the $(KINETIS_ROMSIZE)K

for board in $(git grep 'CPU = kinetis' | sed 's|boards/||;s|/.*||'); do echo ${board}; arm-none-eabi-readelf --symbols examples/hello-world/bin/${board}/hello-world.elf | grep _rom_length | awk '{print $2}'; done | tee rom_length_output
frdm-k22f
00080000
frdm-k64f
00100000
frdm-kw41z
00080000
mulle
00080000
pba-d-01-kw2x
00040000
phynode-kw41z
00080000
teensy31
00040000
usb-kw41z
00080000

@cladmi, thanks and sorry for the long delay. I confirm your solution works. If it's fine for you, I'll drop the initial commit and the revert one from this PR.

@cladmi: I integrated your comment into this PR (and you are now co-author of the commit). Let me know what you think.

Thank you I will retest

All green, let's go!

Thanks for handling this one.

Thanks for proposing a better fix than the one initially proposed ;)