cpu/sam0_common: derive ROM_LEN & RAM_LEN from part number#13607
cpu/sam0_common: derive ROM_LEN & RAM_LEN from part number#13607dylad merged 1 commit intoRIOT-OS:masterfrom
Conversation
dba83fa to
bc84822
Compare
|
@benpicco: |
|
I'm afraid those are not available to |
Yeah but I was thinking of using sed to parse the right file from the Makefile. CPU model should be known so It might be doable ? This way, we just have to add vendor files for new MCUs and Makefile will adapt automatically. |
Good idea! |
Nice, this looks cleaner this way and even more generic ! Let's wait for Murdock. |
This could be very bad performance wise, it could lead to very big build times. In general anything that uses non make builtins is slow, so any shell call is slow. Parsing files can be very slow and IMO should be avoided, see #13174 for an example. It is in general better to have a big file declaring everything (in make) than using magic parsing to retrieve that. |
|
I reverted the last change, it was broken anyway according to Murdock. |
I understand it may be faster for make but this is hell to maintain. |
|
Not sure why Parsing the part number with # get vendor file to extract RAM size
VENDOR_FILE := $(shell find $(RIOTCPU)/sam0_common/include/vendor/sam$(FAMILY) -name $(CPU_MODEL).h | grep include*/sam)
RAM_LEN := $(shell sed -E -n 's/\#define (HMCRAMC0_SIZE|HSRAM_SIZE).*\((.*)\).*/\2/p' $(VENDOR_FILE))But then again the compiler will do just the same thing and the file will be in the cache. |
ec9fe67 to
51af582
Compare
|
Why speculate when we can measure it? mastersemi-generic version with per-family formulageneric version parsing vendor files(This was on a Crucial CT250MX2 SDD with a i5-2500k and 16 GiB RAM) |
fa65ece to
ce4cae2
Compare
|
So I'd say the impact of the fully generic version is negligible. |
|
I am happy with this. then run make BOARD=samr34-xpro -C tests/leds print-info I tested a bunch of SAM0-based board, I also modified CPU_MODEL in some cases to check if output was changed accordingly. @fjmolinas Are you happy with this too ? |
|
@dylad you can also test with
Performance was evaluated by @benpicco so I'm fine with this. |
That's the command I wanted to use but I was unable to remember it so it was faster to rewrite something ! |
The ROM size is encoded in the part number of the Atmel SAM chips. RAM size is not encoded directly, so get it by parsing the chip's vendor file. The file remains in the page cache for the compiler to use, so the overhead should be minimal: on master: Benchmark #1: make BOARD=samr21-xpro -j Time (mean ± σ): 527.9 ms ± 4.9 ms [User: 503.1 ms, System: 69.6 ms] Range (min … max): 519.7 ms … 537.2 ms 10 runs with this patch: Benchmark #1: make BOARD=samr21-xpro -j Time (mean ± σ): 535.6 ms ± 4.0 ms [User: 507.6 ms, System: 75.1 ms] Range (min … max): 530.6 ms … 542.0 ms 10 runs
|
Should I squash? |
Please, go ahead. |
ce4cae2 to
7796110
Compare
dylad
left a comment
There was a problem hiding this comment.
I checked manually Makefile output, this looks good and buildsystem isn't impact that much so let's go.
Contribution description
The ROM size is encoded in the part number of the Atmel SAM chips.
RAM size is not encoded directly, but it's always a fixed fraction of the ROM size.
Testing procedure
I manually confirmed that the resulting
ROM_LENandRAM_LENs are still the same for the chips that were listed here before.Issues/PRs references
This is the last missing piece missing to do away with having to do changes in RIOT for using a different part number of a supported chip.