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/06/10 09:28:28 UTC

[GitHub] [mynewt-mcumgr] de-nordic opened a new pull request #124: Zephyr fixes

de-nordic opened a new pull request #124:
URL: https://github.com/apache/mynewt-mcumgr/pull/124


   The PR is series of fixes that to the Zephyr fork that:
    - change slot selection logic in `img_mgmt_find_best_area_id` to just check if slot1 is available, as basically that was what the function was doing, but with unneeded checks to slot 0, etc; change the name of the function to reflect what it does from `img_mgmt_find_best_area_id` to `img_mgmt_get_second_slot_area_id`
    - add missing check, of `img_mgmt_get_second_slot_area_id` return, in `img_mgmt_impl_erase_slot`;
    - changes the `img_mgmt_impl_write_image_data` to use the `area_id` from the state of mcumgr, so the currently selected for operation, instead of calling `img_mgmt_get_second_slot_area_id` (formerly `img_mgmt_find_best_area_id`) to figure it out 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] utzig commented on pull request #124: Zephyr fixes

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


   > @utzig Thanks for quick review!
   
   I will wait for @nvlsianpu approval!


-- 
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 #124: Zephyr fixes

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



##########
File path: cmd/img_mgmt/port/zephyr/src/zephyr_img_mgmt.c
##########
@@ -108,48 +108,23 @@ zephyr_img_mgmt_flash_area_id(int slot)
         break;
 
     default:
-        assert(0);
-        fa_id = FLASH_AREA_ID(image_1);
+        fa_id = -1;
         break;
     }
 
     return fa_id;
 }
 
+/**
+ * The function will check if second(ary) slot is available, and allowed,
+ * for DFU; if checks area positive, then area ID is returned, -1 is returned
+ * otherwise.
+ */
 static int
-img_mgmt_find_best_area_id(void)
+img_mgmt_get_second_slot_area_id(void)
 {
-    struct image_version ver;
-    int best = -1;
-    int i;
-    int rc;
-
-    for (i = 0; i < 2; i++) {
-        rc = img_mgmt_read_info(i, &ver, NULL, NULL);
-        if (rc < 0) {
-            continue;
-        }
-        if (rc == 0) {
-            /* Image in slot is ok. */
-            if (img_mgmt_slot_in_use(i)) {
-                /* Slot is in use; can't use this. */
-                continue;
-            } else {
-                /*
-                 * Not active slot, but image is ok. Use it if there are
-                 * no better candidates.
-                 */
-                best = i;
-            }
-            continue;
-        }
-        best = i;
-        break;
-    }
-    if (best >= 0) {
-        best = zephyr_img_mgmt_flash_area_id(best);
-    }
-    return best;
+    /* In case of Zephyr slot 0 is always unavailable */
+    return img_mgmt_slot_in_use(1) == 0 ? zephyr_img_mgmt_flash_area_id(1) : -1;

Review comment:
       You are right. I have completely forgotten about the Direct-XIP. Thanks!




-- 
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 #124: Zephyr fixes

Posted by GitBox <gi...@apache.org>.
de-nordic commented on pull request #124:
URL: https://github.com/apache/mynewt-mcumgr/pull/124#issuecomment-860810894


   @utzig Thanks for quick review!


-- 
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 #124: Zephyr fixes

Posted by GitBox <gi...@apache.org>.
de-nordic commented on pull request #124:
URL: https://github.com/apache/mynewt-mcumgr/pull/124#issuecomment-858466003


   Please take a look @mlaz, @nvlsianpu , @utzig, Thanks!


-- 
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 #124: Zephyr fixes

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



##########
File path: cmd/img_mgmt/port/zephyr/src/zephyr_img_mgmt.c
##########
@@ -108,48 +108,23 @@ zephyr_img_mgmt_flash_area_id(int slot)
         break;
 
     default:
-        assert(0);
-        fa_id = FLASH_AREA_ID(image_1);
+        fa_id = -1;
         break;
     }
 
     return fa_id;
 }
 
+/**
+ * The function will check if second(ary) slot is available, and allowed,
+ * for DFU; if checks area positive, then area ID is returned, -1 is returned
+ * otherwise.
+ */
 static int
-img_mgmt_find_best_area_id(void)
+img_mgmt_get_second_slot_area_id(void)
 {
-    struct image_version ver;
-    int best = -1;
-    int i;
-    int rc;
-
-    for (i = 0; i < 2; i++) {
-        rc = img_mgmt_read_info(i, &ver, NULL, NULL);
-        if (rc < 0) {
-            continue;
-        }
-        if (rc == 0) {
-            /* Image in slot is ok. */
-            if (img_mgmt_slot_in_use(i)) {
-                /* Slot is in use; can't use this. */
-                continue;
-            } else {
-                /*
-                 * Not active slot, but image is ok. Use it if there are
-                 * no better candidates.
-                 */
-                best = i;
-            }
-            continue;
-        }
-        best = i;
-        break;
-    }
-    if (best >= 0) {
-        best = zephyr_img_mgmt_flash_area_id(best);
-    }
-    return best;
+    /* In case of Zephyr slot 0 is always unavailable */
+    return img_mgmt_slot_in_use(1) == 0 ? zephyr_img_mgmt_flash_area_id(1) : -1;

Review comment:
       I'm afraid that this will made update impossible for DIRECT-XIP MCUboot compatible mode, when either primary or secondary slot might be the update target.




-- 
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 #124: Zephyr fixes

Posted by GitBox <gi...@apache.org>.
de-nordic commented on pull request #124:
URL: https://github.com/apache/mynewt-mcumgr/pull/124#issuecomment-861584931


   @utzig @sjanc Sorry for inconvenience, can you look 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