Skip to content

Conversation

@rgundi
Copy link
Contributor

@rgundi rgundi commented Jan 7, 2019

The documentation for intel_s1000 is updated with the steps
to use it with MCUbootloader.

Signed-off-by: Rajavardhan Gundi [email protected]

@zephyrbot
Copy link

zephyrbot commented Jan 7, 2019

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace MCUbootloader with MCUBoot Bootloader

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it's Zephyr on Intel S1000, not Intel S1000 on Zephyr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it's Zephyr on Intel S1000, not Intel S1000 on Zephyr.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

USB DFU is not exactly over the "air", please remove "over the air". Also change "upgradation" to "upgrades".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this procedure (and upgrading the S1000 firmware) work on the board referenced at the top of this doc (https://software.intel.com/en-us/iot/speech-enabling-dev-kit)? I suspect not, and we need to be more explicit at the top of this doc about the board and procedures being documented here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbkinder This procedure will work on the board referenced at the top of this doc.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block is similar to the app build and should be able to use the zephyr-app-commands directive. See around line 136

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zephyr-app-commands are applicable only to those apps which are within the zephyr tree. mcubootloader is outside of it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modified as suggested.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_spi_flash utility is not available publicly. need to make it or a variant of it as part of the build system before updating the document.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. noted. I will keep this for the time being while i work on the utility

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this now. #12662 addresses this.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, when the active image is switched to image1, will the DFU capability be lost? If the DFU capability is part of the image, then both image0 and image1 should have the capability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. updated the doc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add one more space on the indent...

Suggested change
`MCUboot with Zephyr`_ documentation for details.)
`MCUboot with Zephyr`_ documentation for details.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the comment below, this title should likely be MCUBoot bootloader support (and adjust the title underline)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and change all references to MCUBoot bootloader

Suggested change
MCUbootloader in conjuntion with USB DFU (Device Firmware Upgrade)
MCUBoot bootloader, in conjunction with USB DFU (Device Firmware Upgrade),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
over the air can be used for Field upgradation of Intel S1000 Firmware.
can be used to upgrade Intel S1000 firmware.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The MCUbootloader source code itself is hosted in the
The MCUBoot bootloader source code is hosted in the

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
looking at boards/xtensa/intel_s1000_crb/intel_s1000_crb.dts. Refer to
looking at ``boards/xtensa/intel_s1000_crb/intel_s1000_crb.dts``. Refer to

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In order to build an MCUboot-compatible Intel S1000 Firmware image
Build an MCUboot-compatible Intel S1000 Firmware image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(image0/image1), you need to do the following:
(image0/image1), by following these steps:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to indent the code-block directives to be within the bullet list items.
How about more simply:

1. Edit your application's :file:`.conf` file to enable the
   :option:`CONFIG_BOOTLOADER_MCUBOOT` option.
2. Use the ``scripts/imagetool.py`` script from the MCUboot GitHub repo`_
   to sign the ``image0`` image (see `MCUboot with Zephyr`_ documentation for details):

   .. code-block:: console

      scripts/imgtool.py sign \
           --key root-rsa-2048.pem \
           --header-size 0x100 \
           --align 8 \
           --version 1.2 \
           --slot-size 0x1D0000 \
           ./zephyr/zephyr.bin \
           signed-zephyr0.bin

3. Sign the ``image1`` upgrade image with the ``--pad`` option:

   .. code-block:: console

      scripts/imgtool.py sign \
           --key root-rsa-2048.pem \
           --header-size 0x100 \
           --align 8 \
           --version 1.2 \
           --slot-size 0x1D0000 \
           --pad
           ./zephyr/zephyr.bin \
           signed-zephyr1.bin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this procedure (and upgrading the S1000 firmware) work on the board referenced at the top of this doc (https://software.intel.com/en-us/iot/speech-enabling-dev-kit)? I suspect not, and we need to be more explicit at the top of this doc about the board and procedures being documented here.

@rgundi
Copy link
Contributor Author

rgundi commented Jan 9, 2019

@dbkinder @sathishkuttan addressed your comments. Plz have a look.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs look better, but still need to resolve use of ./create_spi_flash

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but it's still here in this version of the documentation being reviewed.

@rgundi
Copy link
Contributor Author

rgundi commented Jan 25, 2019

Docs look better, but still need to resolve use of ./create_spi_flash

The reference to create_spi_flash is now removed. Please review again.

@rgundi rgundi force-pushed the mcuboot_doc branch 2 times, most recently from 6da0dc5 to 92eeccc Compare January 29, 2019 04:55
Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for docs, thanks!

The documentation for intel_s1000 is updated with the steps
to use it with MCUbootloader.

Signed-off-by: Rajavardhan Gundi <[email protected]>
@rgundi
Copy link
Contributor Author

rgundi commented Mar 20, 2019

@nashif .. please check if this can be merged. Shippable failures seem to be unrelated.

@galak galak added this to the v1.14.0 milestone Mar 28, 2019
@galak galak modified the milestones: v1.14.0, future Apr 16, 2019
@nashif nashif removed the request for review from SavinayDharmappa November 29, 2019 18:02
@zephyrproject-rtos zephyrproject-rtos deleted a comment from codecov-io Dec 21, 2019
@nashif
Copy link
Member

nashif commented Dec 21, 2019

stale

@nashif nashif closed this Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants