-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add Riotboot support for CC2538 #11665
Conversation
Note, riotboot/flash-extended-slot0 does not work as yet. And when i flash using riotboot/flash-slotX, the other slot's image address is 0xffffffff. |
I know work still needs to be done before this can be merged in RIOT-master. To get slots 0 and 1 to work together. The flash cca section needs to be removed after flashing the bootloader onto the board. I am still working on this project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please provide the test commands with debug output you have while testing for reference. I would ease reviewing :)
The whole problem you will have is with the cca
as it currently not taken into account at all with riotboot
. If you have more infos on how this work and how it should be handled, it would help.
The whole "memory at the end that should not be touched" is a bit tricky.
The way riotboot/flash-slot0-extended
and combined
works is by creating a binary with only the bootloader, truncated to its size, plus the header and slot firmware. So everything that was at the end of the memory would be removed. So I understand that currently they do not work.
For the cca
, I am not sure how to handle it. Somehow I think we need to keep a full flash
sector for it. Otherwise, when updating over the air, bytes nearby may need to be updated and so require erasing the whole sector when erasing memory. There is a ROM_RESERVED
variable that was set for the arduino bootloader to describe reserved memory at the end of the rom but is currently not yet taken into account.
Could it be possible to assume it is always there on the board and RIOT should never touch it ? Or have a flash-cca-sector
target to setup the board.
However then, the flasher will need to never erase the memory.
boards/cc2538dk/Makefile.include
Outdated
@@ -23,7 +23,7 @@ RESET ?= $(RIOTBOARD)/$(BOARD)/dist/reset.sh | |||
export PROGRAMMER ?= cc2538-bsl | |||
|
|||
ifeq ($(PROGRAMMER),cc2538-bsl) | |||
FLASHER = $(RIOTTOOLS)/cc2538-bsl/cc2538-bsl.py | |||
FLASHER = $(RIOTTOOLS)/cc2538-bsl/cc2538-bsl.py -a $(IMAGE_OFFSET)+$(ROM_START_ADDR) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would need to go in FFLAGS
instead of FLASHER
.
Also, I would prefer with somehow only adding it only if IMAGE_OFFSET
is set. Like in
RIOT/makefiles/tools/edbg.inc.mk
Lines 14 to 15 in fc95770
# Set offset according to IMAGE_OFFSET if it's defined | |
EDBG_ARGS += $(if $(IMAGE_OFFSET),--offset $(IMAGE_OFFSET)) |
Even if
cc2538-bsl.py
handles if the -a
is just +rom_start as it does eval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does it behave if you remove the -e
in FFLAGS
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be possible to assume the CCA is always there, and just work around that memory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disabled the -e in FFLAGS so that i could flash the images to slots 0 and 1 individually. However it doesn't select the latest firmware. I posted my test results below. I'm not sure whether the issue is the linking of the bootloader, or the checksum
cpu/cc2538/ldscripts/cc2538sf53.ld
Outdated
_rom_offset = DEFINED( _rom_offset ) ? _rom_offset : 0x0; | ||
_fw_rom_length = DEFINED( _fw_rom_length ) ? _fw_rom_length : _rom_length - _rom_offset; | ||
|
||
ASSERT((_fw_rom_length <= _rom_length - _rom_offset), "Specified firmware size does not fit in ROM"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has now been put in common with cortexm_rom_offset.ld
so you can reuse it
RIOT/cpu/kinetis/ldscripts/kinetis.ld
Line 28 in fc95770
INCLUDE cortexm_rom_offset.ld |
Hmm, the shell command does not allow printing the header of the other slot. I think it would be good to know how the flasher behaves when you do not have Maybe even try to set the whole memory to 0 (except the cca maybe) and flash only the header the
And check the written memory through I checked the flash sector erase size in case a slot may not be aligned. It may be that it is not possible to do it through flashing with the bsl. |
|
||
/* Memory Space Definitions: */ | ||
MEMORY | ||
{ | ||
rom (rx) : ORIGIN = 0x00200000, LENGTH = 512K - 44 | ||
rom (rx) : ORIGIN = _rom_start_addr + _rom_offset, LENGTH = _fw_rom_length | ||
cca : ORIGIN = 0x0027ffd4, LENGTH = 44 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a problem. when saying that cca will start at 0x0027ffd4
you will have this linked for both files, so what happens is that when flashing slot0
you are completely erasing slot1
, you can see this by looking at the generated binaries (on my local computer.)
-rw-rw-r-- 1 francisco francisco 505008 Jun 18 18:09 tests_riotboot.map
-rwxrwxr-x 1 francisco francisco 519936 Jun 18 18:09 tests_riotboot-slot0.bin
-rwxrwxr-x 1 francisco francisco 599032 Jun 18 18:09 tests_riotboot-slot0.elf
-rw-rw-r-- 1 francisco francisco 256 Jun 18 18:09 tests_riotboot-slot0.hdr
-rw-rw-r-- 1 francisco francisco 520192 Jun 18 18:09 tests_riotboot-slot0.riot.bin
-rwxrwxr-x 1 francisco francisco 259840 Jun 18 18:09 tests_riotboot-slot1.bin
-rwxrwxr-x 1 francisco francisco 599032 Jun 18 18:09 tests_riotboot-slot1.elf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I hadn't read correctly in the description that you had disabled it manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small question, what do you mean exactly by disabling the cca region? Do you comment out the section and memory definition? Could you paste the modified ld script? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, i just commented out "SECTIONS"
`
_rom_offset = DEFINED( _rom_offset ) ? _rom_offset : 0x0;
_fw_rom_length = DEFINED( _fw_rom_length ) ? _fw_rom_length : _rom_length - _rom_offset;
ASSERT((_fw_rom_length <= _rom_length - _rom_offset), "Specified firmware size does not fit in ROM");
MEMORY
{
rom (rx) : ORIGIN = _rom_start_addr + _rom_offset, LENGTH = _fw_rom_length
cca : ORIGIN = 0x0027ffd4, LENGTH = 44
ram (w!rx) : ORIGIN = _ram_start_addr, LENGTH = _ram_length
}
/* MCU Specific Section Definitions /
*/
SECTIONS
{
.flashcca :
{
KEEP((.flashcca))
} > cca
}
/*
INCLUDE cortexm_base.ld`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I have similar HW as you do, I have a cc2538em + smartrf06eb board. I'll try to replicate your setup (never used any of these board or TI software so it might take me some time to get back to you). What exact hardware are you using? Just to have an Idea of the differences we might have (if any)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I am using a SmartRF06 Evaluation Board, with a CC2538EM 1.2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some problems with how the CCFA field is handled. Not sure how this should be handled.. I'm reading about it..
Re-wrote my comment upon reading the test description better: an approach for the cca field is having the bootloader be flashed first covering the whole firmware image and have the cca field be located based on the available rom. It would be wrongly located for each slot but correctly for the bootloader (although wasting 88 bytes) |
Hey guys, I used the exact binary files using RIOTs cc2538-bsl and the Uniflash tool and did a comparison.
Test procedure:
Results
|
@brent7984 I got it too work in a hacky way. I followed the same procedure as you and got the same results. I had two problems which I think explain why it was working with "cc2538-bsl.py", but once fixed it managed to make it work. 1.- Since you removed In the
So this solved the problem of writing to flash. But weirdly I was still booting always from the same slot. 2.- The second problem comes from the cca array.
You can see the entry point to the application is also defined here, so depending when you deleted the Ok so the Can you try these changes? Do you think these problems can apply to what you are doing? I will try to pr the |
Trying to PR the changes in |
I think it is because of point 1 in my comments, the version is never being re-written (the rest of the firmware is always the same anyway. Can you check if that works, you could also use JelmerT/cc2538-bsl#87. |
Thank you for the help. It works. I'm moving on to OTA update now :) |
What else needs to be done, for riotboot support for the CC2538 to be merged with RIOT's master base? |
boards/cc2538dk/Makefile.include
Outdated
@@ -7,7 +7,7 @@ PROGRAMMER_SERIAL ?= 06EB | |||
|
|||
# setup serial terminal | |||
# the debug UART is always the second tty with the matching serial number: | |||
PORT_LINUX ?= $(word 2,$(shell $(RIOTTOOLS)/usb-serial/find-tty.sh '^$(PROGRAMMER_SERIAL)')) | |||
PORT_LINUX ?= $(word 1,$(shell $(RIOTTOOLS)/usb-serial/find-tty.sh '^$(PROGRAMMER_SERIAL)')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this changed? Shouldn't it always be the second USB?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, when I changed it to 1, it always picked up the second USB. When it was 2, it didn't pick up any USB
cpu/cc2538/Makefile.include
Outdated
@@ -1,3 +1,7 @@ | |||
export CPU_ARCH := cortex-m3 | |||
|
|||
ROM_LEN ?= 0x80000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the ROM_LEN allways the same? can it change depending on the cpu version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure whether if I need to decrease the ROM_LEN to account for the CCA. However when I tried 0x7FFD4 (which is 512K - 44), the flashing failed
Writing 260116 bytes starting at address 0x002407EA
Target returned: 0x43, Invalid address
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok so since it has different length we need to set ROM_LEN and RAM_LEN according to CPU_MODEL so its taking into account for all options since this is in a common cc2538 file.
I'm not sure whether if I need to decrease the ROM_LEN to account for the CCA.
I don't think so. The problem right now is that the entry point is in the CCA field so we NEED to write it every time for normal applications, but make it always be the bootloader for riotboot.
However when I tried 0x7FFD4 (which is 512K - 44), the flashing failed
How did it fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it didn't flash:
Writing 260116 bytes starting at address 0x002407EA
Target returned: 0x43, Invalid address
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when i tried 0x7FF00, it flashed correctly
I don't think you should close the pull-request. The work you did is really nice. But for it to get merged in RIOT there are still some work to be done. The changes I made target an external tool. The proper approach is to get this merged upstream and then update the reference in RIOT's tools. Which is what I'm trying to do with JelmerT/cc2538-bsl#87. Hopefully this PR or a variance of this will get merged. For now you can change the py script and commit something like: The main problem I still see is the You should also address the changes @cladmi pointed out. Usually what we do here is add a new commit addressing the change or a "fixup" so the changes you make and why they are made are correctly documented. A guide on this Here. I also added some comments now. Then there is formatting stuff and coding convention styles, as well as commit style. You can find a guide here: https://github.com/RIOT-OS/RIOT/wiki/Coding-conventions Anyway it would be awesome if you keep working on this, its already progressing fast, with @cladmi we can keep giving you pointers and figure out the pest solution for the |
@brent7984 For
|
boards/cc2538dk/Makefile.include
Outdated
@@ -24,7 +24,7 @@ export PROGRAMMER ?= cc2538-bsl | |||
|
|||
ifeq ($(PROGRAMMER),cc2538-bsl) | |||
FLASHER = $(RIOTTOOLS)/cc2538-bsl/cc2538-bsl.py | |||
FFLAGS = -p "$(PORT)" -e -w -v $(FLASHFILE) | |||
FFLAGS = -p "$(PORT)" -a $(IMAGE_OFFSET)+$(ROM_START_ADDR) -w -v $(FLASHFILE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should still be edited according to @cladmi suggestion, I would maybe split it in something like:
FFLAGS = -p "$(PORT)" -w -v $(FLASHFILE)
FFLAGS += $(if $(IMAGE_OFFSET), -a $(shell printf "0x%08x" $$(($(IMAGE_OFFSET) + $(ROM_START_ADDR)))))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[@fjmolinas] This solution is looking good. I tested riotboot/flash-extended-slot0, and riotboot/flash-combined-slot0. Both works :)
But we currently have to flash the bootloader first (riotboot/flash-bootloader), and then the firmware. How do you suggest we integrate the 2, so we only need to use 1 command (i.e. riotboot/flash-slot0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we currently have to flash the bootloader first (riotboot/flash-bootloader), and then the firmware. How do you suggest we integrate the 2, so we only need to use 1 command (i.e. riotboot/flash-slot0)
why? Because of the cca field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested, by flashing the hello-world example, then riotboot/flashXXXX.
riotboot/flash-extended-slot0 and riotboot/flash-combined-slot0 works fine,
However, riotboot/flash-slotX fails unless if I use riotboot/flash-bootloader first.
I believe it is because of the CCA entry point as you mentioned previously.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To program the CC2538 on the smartrf06eb, I currently have to hold the select button whilst pressing the reset button, before I flash it. Is there any way for me to bypass this procedure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, riotboot/flash-slotX fails unless if I use riotboot/flash-bootloader first
Hmm. Maybe the filter I defined isn't correct. What I wanted to achieve was that cca
never be updated for any slot and when compiling riotboot/flash-extended-slot0 and riotboot/flash-combined-slot0
it shouldn't either. Maybe look at the bin files and see it isn't defined? If not, changing it to:
# The entry point `cortex_vector_base` is defined in the cca field.
# If the cca field is updated when flashing a slot then the entry
# point will never be the bootloader but the respective slot. This
# ensures it never happens .
ifneq (,$(filter riotboot,$(FEATURES_REQUIRED)))
CFLAGS+=-DUPDATE_CCA=0
endif
would work as long as you flash some RIOT application before, like hello-world. But I'm not sure of this approach. The ideal would be that when doing riotboot/flash-extended-slot0
, riotboot/flash-combined-slot0
or riotboot/flash-bootloader
the cca is updated and points to the bootloader, but in no other case. I'll try to figure something out. but I don't have that much time right now, soy I might take a while to respond :) (I'll be AFK most of next week too just so you now).
I would suggest you look into ways of using UPDATE_CCA
flags to achieve what I'm suggesting :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the help. I will continue to work on it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer a second variable over adding to FFLAGS
as it prevents from overwriting it from outside. I am slowly trying to remove the FFLAGS +=
.
For the cca
, can we not include it in the linker if there is a rom_offset
instead of a compile time hack ?
The hack does not work, as the bootloader uses riotboot
.
General note, checking FEATURES_REQUIRED
is forbidden (there is a sanity check for it) and should be done with FEATURES_USED
.
Somehow I would still like that the combined-extended
firmwares work because it is what should be distributed and is required for the tests to pass. But I do not know how to handle it properly yet.
boards/cc2538dk/Makefile.include
Outdated
@@ -7,7 +7,7 @@ PROGRAMMER_SERIAL ?= 06EB | |||
|
|||
# setup serial terminal | |||
# the debug UART is always the second tty with the matching serial number: | |||
PORT_LINUX ?= $(word 2,$(shell $(RIOTTOOLS)/usb-serial/find-tty.sh '^$(PROGRAMMER_SERIAL)')) | |||
PORT_LINUX ?= $(word 1,$(shell $(RIOTTOOLS)/usb-serial/find-tty.sh '^$(PROGRAMMER_SERIAL)')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this on my setup and it works fine. There might be a problem with how this is defined but lets not mix things. If there is a problem we should open another PR about it, and figure out why it works for me (and presumably others) but not you.
Hey guys, I managed to get the CC2538 to update OTA via riotboot_flashwrite |
cpu/cc2538/Makefile.include
Outdated
ROM_START_ADDR ?= 0x00200000 | ||
RAM_START_ADDR ?= 0x20000000 | ||
LINKER_SCRIPT ?= cc2538sf53.ld | ||
USEMODULE += periph_common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this USEMODULE
needed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's needed. It was in the original RIOT base which I was working with. I've noticed it's no longer in the master base, so I'll remove it now
@brent7984 what is the status here from your point of view? |
Hi @emmanuelsearch , I tested these changes with PR #11818. Note, on this PR, everything works fine, however there is one issue. In order to use the riotboot/flash-slotX command, you have to flash the bootloader first. i.e. after using riotboot/flash-bootloader or riotboot/flash-combined-slot0, then you can use riotboot/flash-slotX correctly. |
@brent7984 ok thanks for the info! Actually I meant: using your added riotboot support, can you successfully update the firmware of your cc2538 via the workflow described in the README from #11818 for simplicity using the wired version (over ethos)? |
Your board would currently be broken if rebased on the current master due to #12003. Flashing |
I'm still trying to get the SUIT update working on the SAMR21 first. Once I get that working I'll test on the CC2538 board. For some reason, I'm getting an error |
The SUIT PR is merged now so it should just work™ on SAMR21. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions. |
Sorry that it took the stale bot to remind us of this - I have an |
Thanks a lot for putting the initial work in for this @brent7984 I'm sorry at the time I was not more active to push this one through :/ |
Hi @fjmolinas <https://github.com/fjmolinas>
Thank you for all of your assistance, you've helped me a lot. I managed to
accomplish all of my required tasks on time :)
Hopefully next year, I can make a bigger contribution towards RIOT
With much thanks and appreciation
Brenton
…On Fri, Nov 13, 2020 at 5:28 PM Francisco ***@***.***> wrote:
Thanks a lot for putting the initial work in for this @brent7984
<https://github.com/brent7984> I'm sorry at the time I was not more
active to push this one through :/
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#11665 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGFSPKBMN2TKDQQOSGHHRWDSPVGCRANCNFSM4HWQ5WXQ>
.
|
Contribution description
Testing procedure
Issues/PRs references