You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2021/02/18 13:23:39 UTC
[GitHub] [mynewt-mcumgr] de-nordic opened a new pull request #115: Support for software update to Direct-XIP built applications
de-nordic opened a new pull request #115:
URL: https://github.com/apache/mynewt-mcumgr/pull/115
Depends on https://github.com/apache/mynewt-mcumgr/pull/114
The PR contains two PRs that
1) Add automatic selection of running application slot at compile time
2) Remove restrictions on uploading applications to "pending" slots for Direct-XIP applications
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] de-nordic commented on a change in pull request #115: Support for software update to Direct-XIP built applications
Posted by GitBox <gi...@apache.org>.
de-nordic commented on a change in pull request #115:
URL: https://github.com/apache/mynewt-mcumgr/pull/115#discussion_r580317263
##########
File path: cmd/img_mgmt/port/zephyr/src/zephyr_img_mgmt.c
##########
@@ -400,6 +400,7 @@ int img_mgmt_impl_erase_if_needed(uint32_t off, uint32_t len)
int
img_mgmt_impl_swap_type(void)
{
+#if !defined(CONFIG_IMG_MGMT_DIRECT_XIP)
Review comment:
Does the direct-xip support test runs?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] de-nordic commented on a change in pull request #115: Support for software update to Direct-XIP built applications
Posted by GitBox <gi...@apache.org>.
de-nordic commented on a change in pull request #115:
URL: https://github.com/apache/mynewt-mcumgr/pull/115#discussion_r594168908
##########
File path: cmd/img_mgmt/port/zephyr/src/zephyr_img_mgmt.c
##########
@@ -400,6 +400,7 @@ int img_mgmt_impl_erase_if_needed(uint32_t off, uint32_t len)
int
img_mgmt_impl_swap_type(void)
{
+#if !defined(CONFIG_IMG_MGMT_DIRECT_XIP)
Review comment:
@sjanc In this mode application will run from the slot it has been uploaded to. The problem is that you could still set the "opposite" slot in pending mode which means it would not be possible to overwrite and the flag would never be lifted as in XIP mode it will not switch slots, example:
0) let have app_v0.0.0 and app_v0.0.1
1) the app_v0.0.0 is running
2) set the "opposite" slot to pending
3) try to upload app_v0.0.1, will fail
4) reboot
5) the app_v0.0.0 is running, the "opposite" slot is in pending state, it is not possible to overwrite it at this point anymore.
6) reboot and you are at 5 again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] nvlsianpu commented on a change in pull request #115: Support for software update to Direct-XIP built applications
Posted by GitBox <gi...@apache.org>.
nvlsianpu commented on a change in pull request #115:
URL: https://github.com/apache/mynewt-mcumgr/pull/115#discussion_r580289546
##########
File path: cmd/img_mgmt/include/img_mgmt/img_mgmt_config.h
##########
@@ -32,13 +32,41 @@
#elif defined __ZEPHYR__
+#include <devicetree.h>
+
#define IMG_MGMT_UL_CHUNK_SIZE CONFIG_IMG_MGMT_UL_CHUNK_SIZE
#define IMG_MGMT_VERBOSE_ERR CONFIG_IMG_MGMT_VERBOSE_ERR
#define IMG_MGMT_LAZY_ERASE CONFIG_IMG_ERASE_PROGRESSIVELY
#define IMG_MGMT_DUMMY_HDR CONFIG_IMG_MGMT_DUMMY_HDR
-#define IMG_MGMT_BOOT_CURR_SLOT 0
+#define DT_CODE_PARTITION_NODE DT_CHOSEN(zephyr_code_partition)
Review comment:
I wonder whether this can be included as a zephyr-rtos header?
##########
File path: cmd/img_mgmt/port/zephyr/src/zephyr_img_mgmt.c
##########
@@ -400,6 +400,7 @@ int img_mgmt_impl_erase_if_needed(uint32_t off, uint32_t len)
int
img_mgmt_impl_swap_type(void)
{
+#if !defined(CONFIG_IMG_MGMT_DIRECT_XIP)
Review comment:
It is possible to distinguish test-run while app is polling the swap type as well
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] de-nordic commented on pull request #115: Support for software update to Direct-XIP built applications
Posted by GitBox <gi...@apache.org>.
de-nordic commented on pull request #115:
URL: https://github.com/apache/mynewt-mcumgr/pull/115#issuecomment-781358369
@mlaz , @nvlsianpu , @utzig Can you take a look?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] sjanc commented on a change in pull request #115: Support for software update to Direct-XIP built applications
Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #115:
URL: https://github.com/apache/mynewt-mcumgr/pull/115#discussion_r594180276
##########
File path: cmd/img_mgmt/port/zephyr/src/zephyr_img_mgmt.c
##########
@@ -400,6 +400,7 @@ int img_mgmt_impl_erase_if_needed(uint32_t off, uint32_t len)
int
img_mgmt_impl_swap_type(void)
{
+#if !defined(CONFIG_IMG_MGMT_DIRECT_XIP)
Review comment:
yes, this is what I mean. Isn't the whole point of pending flag to prevent it?
it == uploading new image before current proces of testing 'previous new image' is done?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] de-nordic commented on a change in pull request #115: Support for software update to Direct-XIP built applications
Posted by GitBox <gi...@apache.org>.
de-nordic commented on a change in pull request #115:
URL: https://github.com/apache/mynewt-mcumgr/pull/115#discussion_r594168908
##########
File path: cmd/img_mgmt/port/zephyr/src/zephyr_img_mgmt.c
##########
@@ -400,6 +400,7 @@ int img_mgmt_impl_erase_if_needed(uint32_t off, uint32_t len)
int
img_mgmt_impl_swap_type(void)
{
+#if !defined(CONFIG_IMG_MGMT_DIRECT_XIP)
Review comment:
@sjanc In this mode application will run from the slot it has been uploaded to. The problem is that you could still set the "opposite" slot in pending mode which means it would not be possible to overwrite and the flag would never be lifted as in XIP mode it will not switch slots, example:
0) let have app_v0.0.0 and app_v0.0.1
1) the app_v0.0.0 is running
2) set the "opposite" slot to pending
3) try to upload app_v0.0.1, will fail
4) reboot
5) the app_v0.0.0 is running, the "opposite" slot is in pending state, it is not possible to overwrite it at this point anymore.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] sjanc commented on a change in pull request #115: Support for software update to Direct-XIP built applications
Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #115:
URL: https://github.com/apache/mynewt-mcumgr/pull/115#discussion_r594180276
##########
File path: cmd/img_mgmt/port/zephyr/src/zephyr_img_mgmt.c
##########
@@ -400,6 +400,7 @@ int img_mgmt_impl_erase_if_needed(uint32_t off, uint32_t len)
int
img_mgmt_impl_swap_type(void)
{
+#if !defined(CONFIG_IMG_MGMT_DIRECT_XIP)
Review comment:
yes, this is what I mean. Isn't the whole point of pending flag is to prevent it?
it == uploading new image before current proces of testing 'previous new image' is done?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] sjanc commented on a change in pull request #115: Support for software update to Direct-XIP built applications
Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #115:
URL: https://github.com/apache/mynewt-mcumgr/pull/115#discussion_r594162611
##########
File path: cmd/img_mgmt/port/zephyr/src/zephyr_img_mgmt.c
##########
@@ -400,6 +400,7 @@ int img_mgmt_impl_erase_if_needed(uint32_t off, uint32_t len)
int
img_mgmt_impl_swap_type(void)
{
+#if !defined(CONFIG_IMG_MGMT_DIRECT_XIP)
Review comment:
Not sure if I understand reasoning behind this patch.
If in direct-xip mode one can upload image to pending slot than why bother with pending flag at all in this mode?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] de-nordic commented on a change in pull request #115: Support for software update to Direct-XIP built applications
Posted by GitBox <gi...@apache.org>.
de-nordic commented on a change in pull request #115:
URL: https://github.com/apache/mynewt-mcumgr/pull/115#discussion_r580316387
##########
File path: cmd/img_mgmt/include/img_mgmt/img_mgmt_config.h
##########
@@ -32,13 +32,41 @@
#elif defined __ZEPHYR__
+#include <devicetree.h>
+
#define IMG_MGMT_UL_CHUNK_SIZE CONFIG_IMG_MGMT_UL_CHUNK_SIZE
#define IMG_MGMT_VERBOSE_ERR CONFIG_IMG_MGMT_VERBOSE_ERR
#define IMG_MGMT_LAZY_ERASE CONFIG_IMG_ERASE_PROGRESSIVELY
#define IMG_MGMT_DUMMY_HDR CONFIG_IMG_MGMT_DUMMY_HDR
-#define IMG_MGMT_BOOT_CURR_SLOT 0
+#define DT_CODE_PARTITION_NODE DT_CHOSEN(zephyr_code_partition)
Review comment:
You mean that it should be in separate header taken directly from Zrephyr code base?
I was also thinking about that and that is one of reasons behind https://github.com/zephyrproject-rtos/zephyr/issues/32257
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org