Hi Mattias, I think it's not ideal to add two snowball-specific arguments to populate_raw_partition(), but I think it should be easy to get rid of them... see my suggestion below. > === modified file 'linaro-android-media-create' > --- linaro-android-media-create 2012-01-19 06:31:50 +0000 > +++ linaro-android-media-create 2012-02-16 15:27:17 +0000 > @@ -139,7 +139,8 @@ > board_config, media, args.image_size, args.boot_label, > args.should_create_partitions, args.should_align_boot_part) > > - board_config.populate_raw_partition(args.device, BOOT_DIR) > + board_config.populate_raw_partition(args.device, BOOT_DIR, > + os.path.join(BOOT_DIR, 'boot')) > populate_partition(BOOT_DIR + "/boot", BOOT_DISK, boot_partition) > board_config.populate_boot_script(boot_partition, BOOT_DISK, args.consoles) > with partition_mounted(boot_partition, BOOT_DISK): > > === modified file 'linaro_image_tools/media_create/android_boards.py' > --- linaro_image_tools/media_create/android_boards.py 2012-02-16 15:27:17 +0000 > +++ linaro_image_tools/media_create/android_boards.py 2012-02-16 15:27:17 +0000 > @@ -161,8 +161,10 @@ > _userdata_len, sdcard_start) > > @classmethod > - def populate_raw_partition(cls, media, boot_dir): > - super(AndroidBoardConfig, cls).populate_raw_partition(media, boot_dir) > + def populate_raw_partition(cls, media, boot_dir, config_files_dir, > + delete_startupfiles=False): > + super(AndroidBoardConfig, cls).populate_raw_partition( > + media, boot_dir, config_files_dir, delete_startupfiles) > > @classmethod > def install_boot_loader(cls, boot_partition, boot_device_or_file): > @@ -231,6 +233,24 @@ > return '%s,%s,0xDA\n%s' % ( > loader_start, loader_len, command) > > + @classmethod > + def populate_raw_partition(cls, media, boot_dir, config_files_dir, > + delete_startupfiles=False): > + # To avoid adding a Snowball specific command line option, we assume > + # that the user already has unpacked the startfiles to ./startupfiles > + local_config_dir = os.path.join('.', 'startupfiles') > + assert os.path.exists(local_config_dir), "You need to unpack the " > + "Snowball startupfiles to the directory 'startupfiles' in your current " > + "working directory. See igloocommunity.org for more information." This doesn't work as a multi-line string, you need to wrap it with parentheses. And for bonus points you can indent them one extra level. :) > + # We copy the u-boot files from the unpacked boot.tar.bz2 > + # and put it with the startfiles. > + boot_files = ['u-boot.bin'] > + for boot_file in boot_files: > + cmd_runner.run(['cp', os.path.join(boot_dir, 'boot', boot_file), Ain't os.path.join(boot_dir, 'boot') the same as the new argument (config_files_dir)? Maybe not for all boards? > + local_config_dir], as_root=True).wait() > + super(AndroidSnowballEmmcConfig, cls).populate_raw_partition( > + media, boot_dir, local_config_dir, False) > + > > class AndroidMx53LoCoConfig(AndroidBoardConfig, Mx53LoCoConfig): > extra_boot_args_options = ( > > === modified file 'linaro_image_tools/media_create/boards.py' > --- linaro_image_tools/media_create/boards.py 2012-02-16 15:27:17 +0000 > +++ linaro_image_tools/media_create/boards.py 2012-02-16 15:27:17 +0000 > @@ -711,7 +711,8 @@ > > if (cls.snowball_startup_files_config is not None and > cls.board != 'snowball_sd'): > - cls.populate_raw_partition(boot_device_or_file, chroot_dir) > + cls.populate_raw_partition(boot_device_or_file, chroot_dir, > + os.path.join(chroot_dir, 'boot'), True) > > if cls.env_dd: > # Do we need to zero out the env before flashing it? > @@ -816,7 +817,8 @@ > "No kernel found matching %s." % (cls.vmlinuz)) > > @classmethod > - def populate_raw_partition(cls, media, boot_dir): > + def populate_raw_partition(cls, media, boot_dir, chroot_dir, Hmm, in android-boards.py the new argument is called config_files_dir but here it's called chroot_dir. That's confusing. Also, I wonder if it really does make sense to have that as a required argument even though it's only needed for one specific board config? > + delete_startupfiles=False): Same here; delete_startupfiles doesn't make sense for anything other than snowball. I think we could easily get rid of it by having a, say, should_delete_startupfiles @classproperty which returns True if cls.snowball_startup_files_config is not None and cls.board != 'snowball_sd', just like in the conditional above. I wonder if it'd be possible to do the same for the other new argument? > # Override in subclass if needed > pass > > @@ -1070,26 +1072,30 @@ > make_uImage(cls.load_addr, k_img_data, boot_dir) > boot_script_path = os.path.join(boot_dir, cls.boot_script) > make_boot_script(boot_env, boot_script_path) > - cls.populate_raw_partition(boot_device_or_file, chroot_dir) > + cls.populate_raw_partition(boot_device_or_file, chroot_dir, > + os.path.join(chroot_dir, 'boot'), True) > > @classmethod > - def populate_raw_partition(cls, boot_device_or_file, chroot_dir): > + def populate_raw_partition(cls, boot_device_or_file, chroot_dir, > + config_files_dir, delete_startupfiles=False): > # Populate created raw partition with TOC and startup files. > - config_files_path = os.path.join(chroot_dir, 'boot') > _, toc_filename = tempfile.mkstemp() > - new_files = cls.get_file_info(chroot_dir) > + new_files = cls.get_file_info(chroot_dir, config_files_dir) > with open(toc_filename, 'wb') as toc: > cls.create_toc(toc, new_files) > cls.install_snowball_boot_loader(toc_filename, new_files, > - boot_device_or_file, > - cls.SNOWBALL_LOADER_START_S) > + boot_device_or_file, > + cls.SNOWBALL_LOADER_START_S, > + delete_startupfiles) > cls.delete_file(toc_filename) > - cls.delete_file(os.path.join(config_files_path, > - cls.snowball_startup_files_config)) > + if delete_startupfiles: > + cls.delete_file(os.path.join(config_files_dir, > + cls.snowball_startup_files_config)) > > @classmethod > def install_snowball_boot_loader(cls, toc_file_name, files, > - boot_device_or_file, start_sector): > + boot_device_or_file, start_sector, > + delete_startupfiles=False): > ''' Copies TOC and boot files into the boot partition. > A sector size of 1 is used for some files, as they do not > necessarily start on an even address. ''' > @@ -1107,7 +1113,8 @@ > else: > seek_sectors = start_sector + file['offset'] / SECTOR_SIZE > _dd(filename, boot_device_or_file, seek=seek_sectors) > - cls.delete_file(filename) > + if delete_startupfiles: > + cls.delete_file(filename) > > @classmethod > def delete_file(cls, file_path): > @@ -1138,14 +1145,14 @@ > f.write(data) > > @classmethod > - def get_file_info(cls, chroot_dir): > + def get_file_info(cls, chroot_dir, config_files_dir): > ''' Fills in the offsets of files that are located in > non-absolute memory locations depending on their sizes.' > Also fills in file sizes''' > ofs = cls.TOC_SIZE > files = [] > - bin_dir = os.path.join(chroot_dir, 'boot') > - with open(os.path.join(bin_dir, cls.snowball_startup_files_config), > + with open(os.path.join(config_files_dir, > + cls.snowball_startup_files_config), > 'r') as info_file: > for line in info_file: > file_data = line.split() > @@ -1158,7 +1165,7 @@ > filename = os.path.join(chroot_dir, > file_data[1].lstrip('/')) > else: > - filename = os.path.join(bin_dir, file_data[1]) > + filename = os.path.join(config_files_dir, file_data[1]) > assert os.path.exists(filename), "File %s does not exist, " \ > "please check the startfiles config file." % file_data[1] > address = long(file_data[3], 16) > @@ -1398,7 +1405,8 @@ > return uboot_file > > @classmethod > - def populate_raw_partition(cls, boot_device_or_file, chroot_dir): > + def populate_raw_partition(cls, boot_device_or_file, chroot_dir, > + config_files_dir, delete_startupfiles=False): > # Zero the env so that the boot_script will get loaded > _dd("/dev/zero", boot_device_or_file, count=cls.SAMSUNG_V310_ENV_LEN, > seek=cls.SAMSUNG_V310_ENV_START) >