Merge lp://qastaging/~fboudra/linaro-image-tools/initial-arndale-bl1-support into lp://qastaging/linaro-image-tools/11.11

Proposed by Fathi Boudra
Status: Merged
Approved by: Fathi Boudra
Approved revision: 605
Merged at revision: 600
Proposed branch: lp://qastaging/~fboudra/linaro-image-tools/initial-arndale-bl1-support
Merge into: lp://qastaging/linaro-image-tools/11.11
Diff against target: 263 lines (+171/-31)
2 files modified
linaro_image_tools/media_create/boards.py (+103/-31)
linaro_image_tools/media_create/tests/test_media_create.py (+68/-0)
To merge this branch: bzr merge lp://qastaging/~fboudra/linaro-image-tools/initial-arndale-bl1-support
Reviewer Review Type Date Requested Status
Milo Casagrande (community) Approve
Fathi Boudra Needs Resubmitting
Georgy Redkozubov Pending
Review via email: mp+143023@code.qastaging.launchpad.net

Description of the change

* Initial support for Arndale BL1 binary (samsung_bl0_*)

It assumes a pre-defined binary file. Long term, it should be a set of new keys to add in Arndale hardware pack configuration.

* Add ethact/ethaddr to Arndale boot_env (LP: #1097265)

Arndale board doesn't have a MAC address. To get network working out of the box, we define one in U-Boot environment.

To post a comment you must log in.
Revision history for this message
Milo Casagrande (milo) wrote :

The first part of the merge is similar if not identical to the previous one.

A couple of remarks:
- We need some tests for the new board, and the overridden methods (_make_boot_files_v2 and get_boot_env)

One thing I'm not so sure about:

81 class ArndaleConfig(SamsungConfig):
82 def __init__(self):
83 super(ArndaleConfig, self).__init__()
84 + self.bl0_file = 'lib/firmware/arndale/arndale-bl1.bin'
85 self.boot_script = 'boot.scr'
86 self.bootloader_flavor = 'arndale'
87 self.dtb_addr = '0x41f00000'
88 @@ -1572,12 +1584,84 @@
89 self.load_addr = '0x40008000'
90 self.mmc_option = '0:2'
91 self.mmc_part_offset = 1
92 + self.samsung_bl0_start = 1
93 + self.samsung_bl0_len = 16
94 self.samsung_bl1_start = 17
95 + self.samsung_bl1_len = 32
96 self.samsung_bl2_start = 49
97 + self.samsung_bl2_len = 1024
98 self.samsung_env_start = 1073
99 + self.samsung_env_len = 32
100 self.serial_tty = 'ttySAC2'
101 self._extra_serial_options = 'console=%s,115200n8'

I guess there will be a hwpack config file, used to generate the hwpack and the corresponding metadata.
All the serial_tty and _extra_serial_options where done for hwpack format 1.0 compatibility and backward-compatibility. I think we should avoid them with newly defined config and with hwpack 3.0 format. What I would do, is define only the necessary fields that are not present in the hwpack v3.0 definition (even if it wouldn't be that hard to introduce them). So, all the samsung_* defined fields, load_addr, dtb_addr, ..., move them in the hwpack config file, keep only the bare minimum in the class and define only the overridden methods.

review: Needs Fixing
603. By Fathi Boudra

Move *_addr and samsung_* fields in Arndale hwpack config file

604. By Fathi Boudra

Add test_arndale to cover _get_boot_env overridden method

605. By Fathi Boudra

Add test_arndale_make_boot_files_v2 test

Revision history for this message
Fathi Boudra (fboudra) wrote :

Concerns have been fixed (despite I'm clueless with mock).
Moving the fields in the hwpack config had some side effects like None values (tests doesn't mock a hwpack config).

review: Needs Resubmitting
Revision history for this message
Milo Casagrande (milo) wrote :

Hi Fathi,

thanks for the effort on addressing all the issues!
To me it looks OK to go, there is only one small glitch:

./linaro_image_tools/media_create/tests/test_media_create.py:823:48: E231 missing whitespace after ','

PEP8 complaining. You can fix that while merging.
Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches