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 2020/11/30 01:18:55 UTC

[GitHub] [mynewt-mcumgr] utzig opened a new pull request #102: Add image upload support for flash erased at 0

utzig opened a new pull request #102:
URL: https://github.com/apache/mynewt-mcumgr/pull/102


   Add a new abstraction to the image port implementation to get the value a flash device is erased to, and update the image manager to expect the correct values when reading erased data at the image header, etc.


----------------------------------------------------------------
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 #102: Add image upload support for flash erased at 0

Posted by GitBox <gi...@apache.org>.
de-nordic commented on a change in pull request #102:
URL: https://github.com/apache/mynewt-mcumgr/pull/102#discussion_r532480115



##########
File path: cmd/img_mgmt/src/img_mgmt.c
##########
@@ -137,21 +137,29 @@ img_mgmt_read_info(int image_slot, struct image_version *ver, uint8_t *hash,
     size_t data_off;
     size_t data_end;
     bool hash_found;
+    uint8_t erased_val;
+    uint32_t erased_val_32;
     int rc;
 
+    rc = img_mgmt_impl_erased_val(image_slot, &erased_val);
+    if (rc != 0) {
+        return MGMT_ERR_EUNKNOWN;
+    }
+
     rc = img_mgmt_impl_read(image_slot, 0, &hdr, sizeof hdr);
     if (rc != 0) {
         return MGMT_ERR_EUNKNOWN;
     }
 
     if (ver != NULL) {
-        memset(ver, 0xff, sizeof(*ver));
+        memset(ver, erased_val, sizeof(*ver));
     }
+    erased_val_32 = erased_val << 24 | erased_val << 16 | erased_val << 8 | erased_val;

Review comment:
       Could be done via some macro as it repeats also at zephyr_img_mgmt.c:61




----------------------------------------------------------------
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] utzig commented on a change in pull request #102: Add image upload support for flash erased at 0

Posted by GitBox <gi...@apache.org>.
utzig commented on a change in pull request #102:
URL: https://github.com/apache/mynewt-mcumgr/pull/102#discussion_r532985151



##########
File path: cmd/img_mgmt/src/img_mgmt.c
##########
@@ -137,21 +137,29 @@ img_mgmt_read_info(int image_slot, struct image_version *ver, uint8_t *hash,
     size_t data_off;
     size_t data_end;
     bool hash_found;
+    uint8_t erased_val;
+    uint32_t erased_val_32;
     int rc;
 
+    rc = img_mgmt_impl_erased_val(image_slot, &erased_val);
+    if (rc != 0) {
+        return MGMT_ERR_EUNKNOWN;
+    }

Review comment:
       What old functionality cannot be used with this feature?




----------------------------------------------------------------
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] utzig commented on pull request #102: Add image upload support for flash erased at 0

Posted by GitBox <gi...@apache.org>.
utzig commented on pull request #102:
URL: https://github.com/apache/mynewt-mcumgr/pull/102#issuecomment-735494318


   /cc @de-nordic @nvlsianpu 


----------------------------------------------------------------
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] vrahane commented on a change in pull request #102: Add image upload support for flash erased at 0

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #102:
URL: https://github.com/apache/mynewt-mcumgr/pull/102#discussion_r533070011



##########
File path: cmd/img_mgmt/src/img_mgmt.c
##########
@@ -137,21 +137,29 @@ img_mgmt_read_info(int image_slot, struct image_version *ver, uint8_t *hash,
     size_t data_off;
     size_t data_end;
     bool hash_found;
+    uint8_t erased_val;
+    uint32_t erased_val_32;
     int rc;
 
+    rc = img_mgmt_impl_erased_val(image_slot, &erased_val);
+    if (rc != 0) {
+        return MGMT_ERR_EUNKNOWN;
+    }

Review comment:
       Well, the hardcoded value of 0xff, but you are right all hal flash set it to 0xff anyway. So, it is fine.




----------------------------------------------------------------
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] utzig commented on a change in pull request #102: Add image upload support for flash erased at 0

Posted by GitBox <gi...@apache.org>.
utzig commented on a change in pull request #102:
URL: https://github.com/apache/mynewt-mcumgr/pull/102#discussion_r532985020



##########
File path: cmd/img_mgmt/src/img_mgmt.c
##########
@@ -137,21 +137,29 @@ img_mgmt_read_info(int image_slot, struct image_version *ver, uint8_t *hash,
     size_t data_off;
     size_t data_end;
     bool hash_found;
+    uint8_t erased_val;
+    uint32_t erased_val_32;
     int rc;
 
+    rc = img_mgmt_impl_erased_val(image_slot, &erased_val);
+    if (rc != 0) {
+        return MGMT_ERR_EUNKNOWN;
+    }
+
     rc = img_mgmt_impl_read(image_slot, 0, &hdr, sizeof hdr);
     if (rc != 0) {
         return MGMT_ERR_EUNKNOWN;
     }
 
     if (ver != NULL) {
-        memset(ver, 0xff, sizeof(*ver));
+        memset(ver, erased_val, sizeof(*ver));
     }
+    erased_val_32 = erased_val << 24 | erased_val << 16 | erased_val << 8 | erased_val;

Review comment:
       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] utzig merged pull request #102: Add image upload support for flash erased at 0

Posted by GitBox <gi...@apache.org>.
utzig merged pull request #102:
URL: https://github.com/apache/mynewt-mcumgr/pull/102


   


----------------------------------------------------------------
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] utzig commented on pull request #102: Add image upload support for flash erased at 0

Posted by GitBox <gi...@apache.org>.
utzig commented on pull request #102:
URL: https://github.com/apache/mynewt-mcumgr/pull/102#issuecomment-736130779


   Tested on Mynewt 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] vrahane commented on a change in pull request #102: Add image upload support for flash erased at 0

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #102:
URL: https://github.com/apache/mynewt-mcumgr/pull/102#discussion_r532660985



##########
File path: cmd/img_mgmt/src/img_mgmt.c
##########
@@ -137,21 +137,29 @@ img_mgmt_read_info(int image_slot, struct image_version *ver, uint8_t *hash,
     size_t data_off;
     size_t data_end;
     bool hash_found;
+    uint8_t erased_val;
+    uint32_t erased_val_32;
     int rc;
 
+    rc = img_mgmt_impl_erased_val(image_slot, &erased_val);
+    if (rc != 0) {
+        return MGMT_ERR_EUNKNOWN;
+    }

Review comment:
       Maybe this feature could be made conditional based on syscfg so that old functionality can still be used. 




----------------------------------------------------------------
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