Skip to content
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

Changes to support building combined Photon and P1 images for the recent releases #1887

Merged
merged 6 commits into from Aug 26, 2019

Conversation

@avtolstoy
Copy link
Member

commented Aug 16, 2019

Problem

We haven't done a manufacturing release for Photons and P1s since 0.5.5 and DeviceOS has changed quite significantly since then:

  1. DCT functions do not fit into the bootloader and are dynamically imported from system parts
  2. System flags are no longer stored in DCT
  3. Tinker is no longer being built by default if no app is specified

Solution

This PR along with the changes in https://github.com/particle-iot/photon-wiced/tree/feature/combined_fw introduces the following changes.

Changes the layout of the binaries in the combined image

Current layout:                            Previous layout:

+-----------------------------+ 0x08100000 +----------------------------------+
| mfg_test monolithic binary  |            |                                  |
|-----------------------------+ 0x080E5000 |                                  |
| Tinker (factory image)      |            | System part 2                    |
+-----------------------------+ 0x080E0000 |                                  |
|                             |            |                                  |
| mfg_test monolithic binary  | 0x080C0000 |----------------------------------|
|                             |            |
|                             |            |                                  |
+-----------------------------+ 0x080A0000 | System part 1                    |
|                             |            |                                  |
|                             |            |
| System part 2               | 0x08080000 +----------------------------------+
|                             |            |                                  |
|                             |            |                                  |
+-----------------------------+ 0x08060000 |                                  |
|                             |            |                                  |
|                             |            | mfg_test monolithic binary       |
| System part 1               |            |                                  |
|                             |            |                                  |
|                             |            |                                  |
+-----------------------------+ 0x08020000 +----------------------------------+
| EEPROM 2                    |            | Tinker (factory image)           |
+-----------------------------+ 0x08010000 +----------------------------------+
| EEPROM 1                    |            | EEPROM 1                         |
+-----------------------------+ 0x0800C000 +----------------------------------+
| DCT 2                       |            | DCT 2                            |
+-----------------------------+ 0x08008000 +----------------------------------+
| DCT 1                       |            | DCT 1                            |
+-----------------------------+ 0x08004000 +----------------------------------+
| Bootloader                  |            | Bootloader                       |
+-----------------------------+ 0x08000000 +----------------------------------+

Previously after the manufacturing app via the wl tool was done, the system parts were copied into their appropriate location by the bootloader and tinker was copied into the user part by the system parts.

Now, most of the things are in place and only the tinker is copied (from factory location) into its rightful place after the manufacturing app is done.

NOTE: tinker is placed into a gap in mfg_test binary, but is located at the correct factory image location (0x080E0000).

Introduces MONO_MFG_FIRMWARE_AT_USER_PART build flag

This flag causes the bootloader to be built with MODULAR_FIRMWARE=y and enables two things:

  1. Jump into the user part location directly if there is a valid monolithic binary present @ 0x080a0000 (and is built to be run from 0x080a0000)
  2. Certain system flags are read out from the DCT if condition 1 is satisfied

Build script/makefile

This PR removes obsolete targets from the makefile and introduces changes to layout the binaries in the order mentioned above.

Steps to Test

  1. set WICED_SDK env variable to point to the directory containing the photon-wiced repo, with the feature/combined-fw branch checked out
  2. (optional) set DEVICE_OS_DIR env variable to point to the directory containing the device-os repo with the necessary release tag checked out
  3. Run hal/src/photon/make_combined.sh, specifying the target PLATFORM_ID, e.g.: ./make_combined.sh PLATFORM_ID=6
  4. This will build the artefacts to $(DEVICE_OS_DIR)/build/releases/release-$(DEVICE_OS_VERSION)-p$(PLATFORM_ID) e.g. $(DEVICE_OS_DIR)/build/releases/release-1.2.1-p6 for Photon
  5. Run openocd-flash or openocd-flash-stlink target depending on whether particle-ftdi or stlinkv2 debugger is used: ./make_combined.sh PLATFORM_ID=6 openocd-flash-stlink
  6. Connect a USB-to-UART adapter to TX/RX pins of your Photon and P1
  7. The device should be booted into the manufacturing mfg_test application
  8. Run the wl.exe tool from $(DEVICE_OS_DIR)/build/releases/release-1.2.1-p6 (wine can be used): wine wl.exe --serial 33 --interactive (33 = COM33)
  9. Execute stm32id, setup_code, factory_reset. All commands should execute successfully.
  10. Reset the device, put into dfu. Run particle keys doctor <deviceId>. The device should reset and normally boot into tinker.
  11. (This is an extra step added by @m-mcgowan that isn't part of the manufacturing process. It's to validate that when building the bootloader with MODULAR_FIRMWARE=y now the monolithic firmware still works) Flash monolithic firmware to the 0x8020000 and verify that it works.

Example App

N/A

References


Changelog

  • [internal] Changes to support building combined Photon and P1 images for the recent releases

@avtolstoy avtolstoy added this to the 1.4.0 milestone Aug 16, 2019

@avtolstoy avtolstoy requested review from technobly and m-mcgowan Aug 16, 2019

@m-mcgowan
Copy link
Contributor

left a comment

Fantastic way to solve this problem. Things were complicated by the fact that this originally had to work with monolithic images. So good to remove obsolete cruft!

Since we are no longer using the COMBINED_FIRMWARE_IMAGE symbol we should probably remove all the conditional logic using that.

@avtolstoy avtolstoy changed the title Changes in order to support building combined Photon and P1 images for the recent release Changes to support building combined Photon and P1 images for the recent releases Aug 16, 2019

@avtolstoy

This comment has been minimized.

Copy link
Member Author

commented Aug 16, 2019

Since we are no longer using the COMBINED_FIRMWARE_IMAGE symbol we should probably remove all the conditional logic using that.

👍 I would also propose that we build the bootloaders with MODULAR_FIRMWARE=y by default.

@avtolstoy avtolstoy force-pushed the feature/photon-p1-combined-image-update branch from 747a346 to 69df339 Aug 23, 2019

@avtolstoy avtolstoy requested review from m-mcgowan and technobly and removed request for technobly Aug 23, 2019

@avtolstoy avtolstoy modified the milestones: 1.4.0, 1.4.0-rc.1 Aug 23, 2019

@avtolstoy avtolstoy force-pushed the feature/photon-p1-combined-image-update branch from 60c10d2 to 41a307e Aug 23, 2019

@avtolstoy

This comment has been minimized.

Copy link
Member Author

commented Aug 23, 2019

COMBINED_FIRMWARE_IMAGE removed.

👍 I would also propose that we build the bootloaders with MODULAR_FIRMWARE=y by default.

IMPORTANT: I've implemented this in the latest commit. This will probably require us to modify the concourse release pipeline as the target name changed (e.g. from platform-6-lto to platform-6-m-lto).

We can also simply opt not to do this and build with MODULAR_FIRMWARE=y for combined images.

@m-mcgowan
Copy link
Contributor

left a comment

I'm not sure about building modular firmware by default - seems fine in principle but might lead to some downstream regressions. No strong opinion either way!

hal/src/nRF52840/image_bootloader.sh Show resolved Hide resolved
@technobly
Copy link
Member

left a comment

After completing steps 1-5 to test, I end up with a Photon that is SOS 1 hard-faulting. If I flash tinker particle flash --usb tinker to the Photon it boots ok afterwards. Something it's not likely about the user image. Please see my previous comments on how I got through steps 1-5

exit 1
fi

make -f wiced_test_mfg_combined.mk FIRMWARE="${DEVICE_OS_DIR}" WICED_SDK="${WICED_SDK}" $*

This comment has been minimized.

Copy link
@technobly

technobly Aug 23, 2019

Member

There are several places where OUT=$(FIRMWARE_BUILD)/releases/ is used but not checked if it exists first, so it will fail if DEVICE_OS_DIR points to a brand new clone of device-os repo for released firmware version. Suggest adding mkdir -p $(FIRMWARE_BUILD)/releases/ in several places, or possibly one place in make_combined.sh.

This comment has been minimized.

Copy link
@technobly

technobly Aug 23, 2019

Member

To get this to work for 1.2.1, I had to cherry pick these 5 commits on top of the checked out release/v1.2.1 branch. I could not run this checked out branch and target a fresh clone of device-os pointed to by DEVICE_OS_DIR of v1.2.1.

I found that before building the platform with build $ time ./make_release.sh --platform=photon --publish=1.2.1 I was getting this error:

make: [system-full] Error 1 (ignored)
/Applications/Xcode.app/Contents/Developer/usr/bin/make -s -C /Users/brett/code/firmware/modules COMPILE_LTO=n PLATFORM_ID=6 PRODUCT_FIRMWARE_VERSION=1213 PRODUCT_ID=6 all
  /Users/brett/code/firmware/modules/photon/system-part1/makefile /Users/brett/code/firmware/modules/photon/system-part2/makefile /Users/brett/code/firmware/modules/photon/user-part/makefile
   text	   data	    bss	    dec	    hex	filename
 237348	    208	    336	 237892	  3a144	../../../build/target/system-part1/platform-6-m/system-part1.elf
make[3]: *** No rule to make target `src/malloc.cpp', needed by `../build/target/newlib_nano/platform-6-m/./src/malloc.o'.  Stop.
make[2]: *** [newlib_nano] Error 2
make[1]: *** [/Users/brett/code/firmware/modules/photon/system-part2/makefile] Error 2
make: *** [system-full] Error 2

This comment has been minimized.

Copy link
@technobly

technobly Aug 23, 2019

Member

With all of the errors I encountered, I wished that when this finally completed it printed something in terminal to indicate it completed successfully since it does a lot of output prior to that. Maybe something like "\r\n\r\nComplete!"

This comment has been minimized.

Copy link
@technobly

technobly Aug 23, 2019

Member

It would also be good to add the full steps to test to the header of make_combined.sh

This comment has been minimized.

Copy link
@avtolstoy

avtolstoy Aug 24, 2019

Author Member

I found that before building the platform with build $ time ./make_release.sh --platform=photon --publish=1.2.1 I was getting this error:

That's probably a side-effect of not running a clean target when switching branches.

@avtolstoy

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2019

After completing steps 1-5 to test, I end up with a Photon that is SOS 1 hard-faulting.

@technobly Could you clarify the steps you took? The device shouldn't even boot into DeviceOS. It's supposed to go straight into the manufacturing firmware, which doesn't do fancy stuff like SOS signaling.

@technobly

This comment has been minimized.

Copy link
Member

commented Aug 24, 2019

@avtolstoy To get this to work for 1.2.1, I had to cherry pick these 5 commits on top of the checked out release/v1.2.1 branch. After that I just ran your steps as is. Is there a different way I should have tried to make this work for 1.2.1? When I just check out this branch and run steps 1-5 it builds a 1.3.1-rc.1 image and doesn't SOS. Did you have to do something slightly different to make it work for 1.2.1?

I'll continue testing with the 1.3.1-rc.1 combined image and see if that completes properly.

@technobly

This comment has been minimized.

Copy link
Member

commented Aug 24, 2019

Hmm, so apparently the 1.3.1-rc.1 combined image doesn't actually boot. It's just a dim D7 LED and no USB.

@avtolstoy

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2019

Hmm, so apparently the 1.3.1-rc.1 combined image doesn't actually boot. It's just a dim D7 LED and no USB.

That's intended. The device booted into manufacturing test firmware. See the following steps about wl.exe tool and the commands to erase the manufacturing test firmware and flash tinker from the factory location (factory_reset).

@technobly

This comment has been minimized.

Copy link
Member

commented Aug 24, 2019

I figured the wl.exe app would communicate to the mfg_test firmware via the USB serial connection, but I guess it should be with a FTDI tool on the TX/RX pins?

@avtolstoy

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2019

  1. Connect a USB-to-UART adapter to TX/RX pins of your Photon and P1

It does use hardware serial, hence this step.

@technobly

This comment has been minimized.

Copy link
Member

commented Aug 24, 2019

LOL, totally invisible somehow 😄

@technobly

This comment has been minimized.

Copy link
Member

commented Aug 24, 2019

All steps tested and working with 1.3.1-rc.1. Still not sure how to get it working for 1.2.1, but hopefully that doesn't matter for the sake of merging this. I'm not sure how to test step 11, do I need to rebuild the bootloader and a monolithic tinker app? Please specify make commands if so.

@avtolstoy

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2019

I'm not sure how to test step 11, do I need to rebuild the bootloader and a monolithic tinker app? Please specify make commands if so.

Simply flashing the bootloader built out of this branch and a monolithic firmware e.g. tinker (MODULAR=n) should be enough to verify this.

@avtolstoy

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2019

Still not sure how to get it working for 1.2.1, but hopefully that doesn't matter for the sake of merging this.

I'm also not sure what went wrong, but I did produce 1.2.1 binaries with the changes from this branch and they are going to be used for manufacturing.

@technobly

This comment has been minimized.

Copy link
Member

commented Aug 24, 2019

Simply flashing the bootloader built out of this branch and a monolithic firmware e.g. tinker (MODULAR=n) should be enough to verify this.

particle flash --usb photon-tinker-serial1-debugging\@1.3.1-rc.1+debug.jtag.bin

Works fine!

@avtolstoy

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2019

@technobly Thanks for verifying everything. I'm going to fix the issue with the releases folder and perhaps improve output before merging this.

avtolstoy added 6 commits Aug 16, 2019
[bootloader] adds MONO_MFG_FIRMWARE_AT_USER_PART compile flag that al…
…lows the bootloader to conditionally jump to user-part address if there is a valid mono binary there with the correct start address
[photon/p1] script: improves error/success output and ensures 'releas…
…es' folder is created in the makefile for building combined binaries

@avtolstoy avtolstoy requested a review from technobly Aug 26, 2019

@avtolstoy avtolstoy force-pushed the feature/photon-p1-combined-image-update branch from 41a307e to 619a77d Aug 26, 2019

@avtolstoy avtolstoy merged commit 93b2734 into develop Aug 26, 2019

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@avtolstoy avtolstoy deleted the feature/photon-p1-combined-image-update branch Aug 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.