=== modified file 'linaro-media-create'
--- linaro-media-create	2011-02-02 17:32:10 +0000
+++ linaro-media-create	2011-02-11 15:49:23 +0000
@@ -130,7 +130,7 @@
     boot_partition, root_partition = setup_partitions(
         board_config, media, args.image_size, args.boot_label, args.rfs_label,
         args.rootfs, args.should_create_partitions, args.should_format_bootfs,
-        args.should_format_rootfs)
+        args.should_format_rootfs, args.should_align_boot_part)
 
     rootfs_uuid = get_uuid(root_partition)
 

=== modified file 'linaro_media_create/__init__.py'
--- linaro_media_create/__init__.py	2011-02-10 16:58:11 +0000
+++ linaro_media_create/__init__.py	2011-02-11 15:49:23 +0000
@@ -105,4 +105,8 @@
     parser.add_argument(
         '--no-part', dest='should_create_partitions', action='store_false',
         help='Reuse existing partitions on the given media.')
+    parser.add_argument(
+        '--align-boot-part', dest='should_align_boot_part',
+        action='store_true',
+        help='Align boot partition too (might break older x-loaders).')
     return parser

=== modified file 'linaro_media_create/boards.py'
--- linaro_media_create/boards.py	2011-02-10 16:58:11 +0000
+++ linaro_media_create/boards.py	2011-02-14 16:26:08 +0000
@@ -31,6 +31,54 @@
 import tempfile
 
 from linaro_media_create import cmd_runner
+from linaro_media_create.partitions import SECTOR_SIZE
+
+# Notes:
+# * geometry is currently always 255 heads and 63 sectors due to limitations of
+#   older OMAP3 boot ROMs
+# * apparently some OMAP3 ROMs don't tolerate vfat length of an odd number of
+#   sectors (only sizes rounded to 1 KiB seem to boot)
+# * we want partitions aligned on 4 MiB as to get the best performance and
+#   limit wear-leveling
+# * image_size is passed on the command-line and should preferably be a power
+#   of 2; it should be used as a "don't go over this size" information for a
+#   real device, and a "give me a file exactly this big" requirement for an
+#   image file.  Having exactly a power of 2 helps with QEMU; there seem to be
+#   some truncating issues otherwise. XXX to be researched
+
+# align on 4 MiB
+PART_ALIGN_S = 4 * 1024 * 1024 / SECTOR_SIZE
+
+def align_up(value, align):
+    """Round value to the next multiple of align."""
+    return (value + align - 1) / align * align
+
+# optional bootloader partition; at least 1 MiB; in theory, an i.MX5 bootloader
+# partition could hold RedBoot, FIS table, RedBoot config, kernel, and initrd,
+# but we typically use U-Boot which is about 167 KiB as of 2011/02/11 and
+# currently doesn't even store its environment there, so this should be enough
+LOADER_MIN_SIZE_S = align_up(1 * 1024 * 1024, SECTOR_SIZE) / SECTOR_SIZE
+# boot partition; at least 50 MiB; XXX this shouldn't be hardcoded
+BOOT_MIN_SIZE_S = align_up(50 * 1024 * 1024, SECTOR_SIZE) / SECTOR_SIZE
+# root partition; at least 50 MiB; XXX this shouldn't be hardcoded
+ROOT_MIN_SIZE_S = align_up(50 * 1024 * 1024, SECTOR_SIZE) / SECTOR_SIZE
+
+
+def align_partition(min_start, min_length, start_alignment, end_alignment):
+    """Compute partition start and end offsets based on specified constraints.
+
+    :param min_start: Minimal start offset of partition
+    :param min_lengh: Minimal length of partition
+    :param start_alignment: Alignment of this partition
+    :param end_alignment: Alignment of the data following this partition
+    :return: start offset, end offset (inclusive), length
+    """
+    start = align_up(min_start, start_alignment)
+    # end offset is inclusive, so substact one
+    end = align_up(start + min_length, end_alignment) - 1
+    # and add one to length
+    length = end - start + 1
+    return start, end, length
 
 
 class BoardConfig(object):
@@ -53,17 +101,41 @@
     serial_tty = None
 
     @classmethod
-    def get_sfdisk_cmd(cls):
-        """Return the sfdisk command to partition the media."""
+    def get_sfdisk_cmd(cls, should_align_boot_part=False):
+        """Return the sfdisk command to partition the media.
+
+        :param should_align_boot_part: Whether to align the boot partition too.
+
+        This default implementation returns a boot vfat partition of type FAT16
+        or FAT32, followed by a root partition.
+        """
         if cls.fat_size == 32:
             partition_type = '0x0C'
         else:
             partition_type = '0x0E'
 
-        # This will create a partition of the given type, containing 9
-        # cylinders (74027520 bytes, ~70 MiB), followed by a Linux-type
-        # partition containing the rest of the free space.
-        return ',9,%s,*\n,,,-' % partition_type
+        # align on sector 63 for compatibility with broken versions of x-loader
+        # unless align_boot_part is set
+        boot_align = 63
+        if should_align_boot_part:
+            boot_align = PART_ALIGN_S
+
+        # can only start on sector 1 (sector 0 is MBR / partition table)
+        boot_start, boot_end, boot_len = align_partition(
+            1, BOOT_MIN_SIZE_S, boot_align, PART_ALIGN_S)
+        # apparently OMAP3 ROMs require the vfat length to be an even number
+        # of sectors (multiple of 1 KiB); decrease the length if it's odd,
+        # there should still be enough room
+        boot_len = boot_len - boot_len % 2
+        boot_end = boot_start + boot_len - 1
+        # we ignore _root_end / _root_len and return a sfdisk command to
+        # instruct the use of all remaining space; XXX if we had some root size
+        # config, we could do something more sensible
+        root_start, _root_end, _root_len = align_partition(
+            boot_end + 1, ROOT_MIN_SIZE_S, PART_ALIGN_S, PART_ALIGN_S)
+
+        return '%s,%s,%s,*\n%s,,,-' % (
+            boot_start, boot_len, partition_type, root_start)
 
     @classmethod
     def _get_boot_cmd(cls, is_live, is_lowmem, consoles, rootfs_uuid):
@@ -287,12 +359,32 @@
     mmc_option = '0:2'
 
     @classmethod
-    def get_sfdisk_cmd(cls):
-        # Create a one cylinder partition for fixed-offset bootloader data at
-        # the beginning of the image (size is one cylinder, so 8224768 bytes
-        # with the first sector for MBR).
-        sfdisk_cmd = super(Mx51evkConfig, cls).get_sfdisk_cmd()
-        return ',1,0xDA\n%s' % sfdisk_cmd
+    def get_sfdisk_cmd(cls, should_align_boot_part=None):
+        """Return the sfdisk command to partition the media.
+
+        :param should_align_boot_part: Ignored.
+
+        This i.MX5 implementation returns a non-FS data bootloader partition,
+        followed by a FAT32 boot partition, followed by a root partition.
+        """
+        # boot ROM expects bootloader at 0x400 which is sector 2 with the usual
+        # SECTOR_SIZE of 512; we could theoretically leave sector 1 unused, but
+        # older bootloaders like RedBoot might store the environment from 0x0
+        # onwards, so it's safer to just start at the first sector, sector 1
+        # (sector 0 is MBR / partition table)
+        loader_start, loader_end, loader_len = align_partition(
+            1, LOADER_MIN_SIZE_S, 1, PART_ALIGN_S)
+
+        boot_start, boot_end, boot_len = align_partition(
+            loader_end + 1, BOOT_MIN_SIZE_S, PART_ALIGN_S, PART_ALIGN_S)
+        # we ignore _root_end / _root_len and return a sfdisk command to
+        # instruct the use of all remaining space; XXX if we had some root size
+        # config, we could do something more sensible
+        root_start, _root_end, _root_len = align_partition(
+            boot_end + 1, ROOT_MIN_SIZE_S, PART_ALIGN_S, PART_ALIGN_S)
+
+        return '%s,%s,0xDA\n%s,%s,0x0C,*\n%s,,,-' % (
+            loader_start, loader_len, boot_start, boot_len, root_start)
 
     @classmethod
     def _make_boot_files(cls, uboot_parts_dir, boot_cmd, chroot_dir,

=== modified file 'linaro_media_create/partitions.py'
--- linaro_media_create/partitions.py	2011-02-10 17:16:48 +0000
+++ linaro_media_create/partitions.py	2011-02-11 15:49:23 +0000
@@ -47,15 +47,24 @@
 # might want to do it.
 def setup_partitions(board_config, media, image_size, bootfs_label,
                      rootfs_label, rootfs_type, should_create_partitions,
-                     should_format_bootfs, should_format_rootfs):
+                     should_format_bootfs, should_format_rootfs,
+                     should_align_boot_part=False):
     """Make sure the given device is partitioned to boot the given board.
 
     :param board_config: A BoardConfig class.
     :param media: The Media we should partition.
     :param image_size: The size of the image file, in case we're setting up a
         QEMU image.
+    :param bootfs_label: Label for the boot partition.
+    :param rootfs_label: Label for the root partition.
+    :param rootfs_type: Filesystem for the root partition.
     :param should_create_partitions: Whether or not we should erase existing
         partitions and create new ones.
+    :param should_format_bootfs: Whether to reuse the filesystem on the boot
+        partition.
+    :param should_format_rootfs: Whether to reuse the filesystem on the root
+        partition.
+    :param should_align_boot_part: Whether to align the boot partition too.
     """
     cylinders = None
     if not media.is_block_device:
@@ -69,7 +78,8 @@
 
     if should_create_partitions:
         create_partitions(
-            board_config, media, HEADS, SECTORS, cylinders)
+            board_config, media, HEADS, SECTORS, cylinders,
+            should_align_boot_part=should_align_boot_part)
 
     if media.is_block_device:
         bootfs, rootfs = get_boot_and_root_partitions_for_media(
@@ -264,17 +274,6 @@
         raise ValueError("Unknown size format: %s.  Use K[bytes], M[bytes] "
                          "or G[bytes]" % size)
 
-    # Round the size of the raw disk image up to a multiple of 256K so it is
-    # an exact number of SD card erase blocks in length.  Otherwise Linux
-    # under qemu cannot access the last part of the card and is likely to
-    # complain that the last partition on the disk has been truncated.  This
-    # doesn't appear to work in all cases, though, as can be seen on
-    # https://bugs.launchpad.net/linux-linaro/+bug/673335.
-    if real_size % (1024 * 256):
-        cylinders = real_size / CYLINDER_SIZE
-        real_size = cylinders * CYLINDER_SIZE
-        real_size = ((((real_size - 1) / (1024 * 256)) + 1) * (1024 * 256))
-
     return real_size
 
 
@@ -289,8 +288,13 @@
     :param commands: A string of sfdisk commands; each on a separate line.
     :return: A 2-tuple containing the subprocess' stdout and stderr.
     """
+    # --force is unfortunate, but a consequence of having partitions not
+    # starting on cylinder boundaries: sfdisk will abort with "Warning:
+    # partition 2 does not start at a cylinder boundary"
     args = ['sfdisk',
+            '--force',
             '-D',
+            '-uS',
             '-H', str(heads),
             '-S', str(sectors)]
     if cylinders is not None:
@@ -302,7 +306,8 @@
     return proc.communicate("%s\n" % commands)
 
 
-def create_partitions(board_config, media, heads, sectors, cylinders=None):
+def create_partitions(board_config, media, heads, sectors, cylinders=None,
+                      should_align_boot_part=False):
     """Partition the given media according to the board requirements.
 
     :param board_config: A BoardConfig class.
@@ -313,6 +318,7 @@
         partitions.
     :param cylinders: The number of cylinders to pass to sfdisk's -C argument.
         If None the -C argument is not passed.
+    :param should_align_boot_part: Whether to align the boot partition too.
     """
     if media.is_block_device:
         # Overwrite any existing partition tables with a fresh one.
@@ -320,7 +326,8 @@
             ['parted', '-s', media.path, 'mklabel', 'msdos'], as_root=True)
         proc.wait()
 
-    sfdisk_cmd = board_config.get_sfdisk_cmd()
+    sfdisk_cmd = board_config.get_sfdisk_cmd(
+        should_align_boot_part=should_align_boot_part)
     run_sfdisk_commands(sfdisk_cmd, heads, sectors, cylinders, media.path)
 
     # Sync and sleep to wait for the partition to settle.

=== modified file 'linaro_media_create/tests/test_media_create.py'
--- linaro_media_create/tests/test_media_create.py	2011-02-10 19:08:06 +0000
+++ linaro_media_create/tests/test_media_create.py	2011-02-11 19:36:32 +0000
@@ -43,6 +43,8 @@
     )
 import linaro_media_create
 from linaro_media_create.boards import (
+    align_up,
+    align_partition,
     BoardConfig,
     board_configs,
     make_boot_script,
@@ -283,6 +285,27 @@
         self.assertEqual(expected, self.funcs_calls)
 
 
+class TestAlignPartition(TestCase):
+    def test_align_up_none(self):
+        self.assertEqual(1024, align_up(1024, 1))
+
+    def test_align_up_no_rounding(self):
+        self.assertEqual(512, align_up(512, 512))
+
+    def test_align_up_rounding(self):
+        self.assertEqual(512, align_up(1, 512))
+
+    def test_align_partition_4_mib_4_mib(self):
+        expected = (4 * 1024 * 1024, 8 * 1024 * 1024 - 1, 4 * 1024 * 1024)
+        self.assertEqual(expected,
+            align_partition(1, 1, 4 * 1024 * 1024, 4 * 1024 * 1024))
+
+    def test_align_partition_none_4_mib(self):
+        expected = (1, 4 * 1024 * 1024 - 1, 4 * 1024 * 1024 - 1)
+        self.assertEqual(expected,
+            align_partition(1, 1, 1, 4 * 1024 * 1024))
+
+
 class TestFixForBug697824(TestCaseWithFixtures):
 
     def mock_set_appropriate_serial_tty(self, config):
@@ -331,12 +354,18 @@
 class TestGetSfdiskCmd(TestCase):
 
     def test_default(self):
-        self.assertEquals(
-            ',9,0x0C,*\n,,,-', boards.BoardConfig.get_sfdisk_cmd())
+        self.assertEqual(
+            '63,106432,0x0C,*\n106496,,,-',
+            boards.BoardConfig.get_sfdisk_cmd())
+
+    def test_default_aligned(self):
+        self.assertEqual(
+            '8192,106496,0x0C,*\n114688,,,-',
+            boards.BoardConfig.get_sfdisk_cmd(should_align_boot_part=True))
 
     def test_mx51evk(self):
-        self.assertEquals(
-            ',1,0xDA\n,9,0x0C,*\n,,,-',
+        self.assertEqual(
+            '1,8191,0xDA\n8192,106496,0x0C,*\n114688,,,-',
             board_configs['mx51evk'].get_sfdisk_cmd())
 
 
@@ -633,7 +662,7 @@
         # every time we run sfdisk it actually repartitions the device,
         # erasing any partitions created previously.
         self.assertEqual(
-            [(',1,0xDA\n,9,0x0C,*\n,,,-', 255, 63, '', self.media.path)],
+            [('1,8191,0xDA\n8192,106496,0x0C,*\n114688,,,-', 255, 63, '', self.media.path)],
             sfdisk_fixture.mock.calls)
 
     def test_create_partitions_for_beagle(self):
@@ -648,7 +677,7 @@
              ['sync']],
             popen_fixture.mock.calls)
         self.assertEqual(
-            [(',9,0x0C,*\n,,,-', 255, 63, '', self.media.path)],
+            [('63,106432,0x0C,*\n106496,,,-', 255, 63, '', self.media.path)],
             sfdisk_fixture.mock.calls)
 
     def test_create_partitions_with_img_file(self):
@@ -665,7 +694,7 @@
         self.assertEqual([['sync']], popen_fixture.mock.calls)
 
         self.assertEqual(
-            [(',9,0x0C,*\n,,,-', 255, 63, '', tempfile)],
+            [('63,106432,0x0C,*\n106496,,,-', 255, 63, '', tempfile)],
             sfdisk_fixture.mock.calls)
 
     def test_run_sfdisk_commands(self):
@@ -675,7 +704,7 @@
             stdout=subprocess.PIPE)
         proc.communicate()
         stdout, stderr = run_sfdisk_commands(
-            ',1,0xDA', 5, 63, '', tempfile, as_root=False,
+            '2,16063,0xDA', 255, 63, '', tempfile, as_root=False,
             stderr=subprocess.PIPE)
         self.assertIn('Successfully wrote the new partition table', stdout)
 
@@ -684,7 +713,7 @@
         self.assertRaises(
             cmd_runner.SubcommandNonZeroReturnValue,
             run_sfdisk_commands,
-            ',1,0xDA', 5, 63, '', tempfile, as_root=False,
+            ',1,0xDA', 255, 63, '', tempfile, as_root=False,
             stderr=subprocess.PIPE)
 
 
@@ -700,6 +729,11 @@
         super(TestPartitionSetup, self).tearDown()
         time.sleep = self.orig_sleep
 
+    def _create_tempfile(self):
+        # boot part at +8 MiB, root part at +16 MiB
+        return self._create_qemu_img_with_partitions(
+            '16384,15746,0x0C,*\n32768,,,-')
+
     def test_convert_size_in_kbytes_to_bytes(self):
         self.assertEqual(512 * 1024, convert_size_to_bytes('512K'))
 
@@ -709,28 +743,23 @@
     def test_convert_size_in_gbytes_to_bytes(self):
         self.assertEqual(12 * 1024**3, convert_size_to_bytes('12G'))
 
-    def test_convert_size_in_kbytes_to_bytes_rounds_to_256k_multiple(self):
-        # See comment in convert_size_to_bytes as to why we need to do this.
-        self.assertEqual(
-            3891 * (1024 * 256), convert_size_to_bytes('1000537K'))
-
     def test_calculate_partition_size_and_offset(self):
-        tempfile = self._create_qemu_img_with_partitions(',1,0x0C,*\n,,,-')
+        tempfile = self._create_tempfile()
         vfat_size, vfat_offset, linux_size, linux_offset = (
             calculate_partition_size_and_offset(tempfile))
         self.assertEqual(
-            [129024L, 32256L, 10321920L, 161280L],
+            [8061952L, 8388608L, 14680064L, 16777216L],
             [vfat_size, vfat_offset, linux_size, linux_offset])
 
     def test_partition_numbering(self):
-        # another Linux partition after the boot/root parts
+        # another Linux partition at +24 MiB after the boot/root parts
         tempfile = self._create_qemu_img_with_partitions(
-            ',1,0x0C,*\n,1,,-\n,,,-')
+            '16384,15746,0x0C,*\n32768,15427,,-\n49152,,,-')
         vfat_size, vfat_offset, linux_size, linux_offset = (
             calculate_partition_size_and_offset(tempfile))
-        # check that the linux partition offset starts on second cylinder so
-        # that it's the partition immediately following the vfat one
-        self.assertEqual(linux_offset, 5 * 63 * 512)
+        # check that the linux partition offset starts at +16 MiB so that it's
+        # the partition immediately following the vfat one
+        self.assertEqual(linux_offset, 32768 * 512)
 
     def test_get_boot_and_root_partitions_for_media_beagle(self):
         self.useFixture(MockSomethingFixture(
@@ -759,11 +788,11 @@
     def _create_qemu_img_with_partitions(self, sfdisk_commands):
         tempfile = self.createTempFileAsFixture()
         proc = cmd_runner.run(
-            ['qemu-img', 'create', '-f', 'raw', tempfile, '10M'],
+            ['qemu-img', 'create', '-f', 'raw', tempfile, '30M'],
             stdout=subprocess.PIPE)
         proc.communicate()
         stdout, stderr = run_sfdisk_commands(
-            sfdisk_commands, 5, 63, '', tempfile, as_root=False,
+            sfdisk_commands, 255, 63, '', tempfile, as_root=False,
             # Throw away stderr as sfdisk complains a lot when operating on a
             # qemu image.
             stderr=subprocess.PIPE)
@@ -786,7 +815,7 @@
         self.assertEqual(None, popen_fixture.mock.calls)
 
     def test_get_boot_and_root_loopback_devices(self):
-        tempfile = self._create_qemu_img_with_partitions(',1,0x0C,*\n,,,-')
+        tempfile = self._create_tempfile()
         atexit_fixture = self.useFixture(MockSomethingFixture(
             atexit, 'register', AtExitRegister()))
         popen_fixture = self.useFixture(MockCmdRunnerPopenFixture())
@@ -796,9 +825,9 @@
         get_boot_and_root_loopback_devices(tempfile)
         self.assertEqual(
             [['sudo', 'losetup', '-f', '--show', tempfile, '--offset',
-              '32256', '--sizelimit', '129024'],
+              '8388608', '--sizelimit', '8061952'],
              ['sudo', 'losetup', '-f', '--show', tempfile, '--offset',
-              '161280', '--sizelimit', '10321920']],
+              '16777216', '--sizelimit', '14680064']],
             popen_fixture.mock.calls)
 
         # get_boot_and_root_loopback_devices will also setup two exit handlers
@@ -818,7 +847,7 @@
         # but here we mock Popen() and thanks to that the image is not setup
         # (via qemu-img) inside setup_partitions.  That's why we pass an
         # already setup image file.
-        tempfile = self._create_qemu_img_with_partitions(',1,0x0C,*\n,,,-')
+        tempfile = self._create_tempfile()
         popen_fixture = self.useFixture(MockCmdRunnerPopenFixture())
         self.useFixture(MockSomethingFixture(
             sys, 'stdout', open('/dev/null', 'w')))
@@ -838,11 +867,11 @@
             board_configs['beagle'], Media(tempfile), '2G', 'boot',
             'root', 'ext3', True, True, True)
         self.assertEqual(
-             # This is the call that would create the image file.
+             # This is the call that would create a 2 GiB image file.
             [['qemu-img', 'create', '-f', 'raw', tempfile, '2147483648'],
              # This call would partition the image file.
-             ['sudo', 'sfdisk', '-D', '-H', '255', '-S', '63', '-C', '261',
-              tempfile],
+             ['sudo', 'sfdisk', '--force', '-D', '-uS', '-H', '255', '-S',
+              '63', '-C', '261', tempfile],
              # Make sure changes are written to disk.
              ['sync'],
              ['sudo', 'mkfs.vfat', '-F', '32', bootfs_dev, '-n', 'boot'],
@@ -855,7 +884,7 @@
         # Pretend the partitions are mounted.
         self.useFixture(MockSomethingFixture(
             partitions, 'is_partition_mounted', lambda part: True))
-        tempfile = self._create_qemu_img_with_partitions(',1,0x0C,*\n,,,-')
+        tempfile = self._create_tempfile()
         self.useFixture(MockSomethingFixture(
             partitions, '_get_device_file_for_partition_number',
             lambda dev, partition: '%s%d' % (tempfile, partition)))
@@ -868,7 +897,8 @@
             True, True, True)
         self.assertEqual(
             [['sudo', 'parted', '-s', tempfile, 'mklabel', 'msdos'],
-             ['sudo', 'sfdisk', '-D', '-H', '255', '-S', '63', tempfile],
+             ['sudo', 'sfdisk', '--force', '-D', '-uS', '-H', '255', '-S',
+              '63', tempfile],
              ['sync'],
              # Since the partitions are mounted, setup_partitions will umount
              # them before running mkfs.

