[flasher PATCH 1/4] Add crc32 verification of the flash image

[flasher PATCH 1/4] Add crc32 verification of the flash image

[Stephen Warren]

From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Verify the CRC32 of the flash image at two points in time:

1) Before starting the flashing process, to validate the download of the
   image into Tegra's RAM.

2) After writing the image to flash, read it back into RAM, in order to
   validate that it was correctly written to flash.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 tegra-uboot-flasher | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/tegra-uboot-flasher b/tegra-uboot-flasher
index 41879c396b2f..d97b9e43aa45 100755
--- a/tegra-uboot-flasher
+++ b/tegra-uboot-flasher
@@ -25,6 +25,7 @@ import os
 import os.path
 import shutil
 import stat
+import subprocess
 import sys
 import tempfile
 from tegraboardconfigs import *
@@ -51,23 +52,26 @@ def run(dir, cmd):
         raise Exception('Command failed: %d' % ret)
     os.chdir(oldcwd)
 
-def gen_flashcmd_mmc(flash_image_addr, flash_img_size):
+def gen_flashcmd_mmc(flash_image_addr, readback_addr, flash_img_size):
     flash_id = config['flash-id-uboot']
     flash_img_size_sectors = flash_img_size / 512
     flashcmd = 'mmc dev %d 1 ; ' % flash_id
     flashcmd += 'mmc write 0x%08x 0 0x%x ; ' % (flash_image_addr, flash_img_size_sectors)
+    flashcmd += 'mmc read 0x%08x 0 0x%x ; ' % (readback_addr, flash_img_size_sectors)
     return flashcmd
 
-def gen_flashcmd_nand(flash_image_addr, flash_img_size):
+def gen_flashcmd_nand(flash_image_addr, readback_addr, flash_img_size):
     flashcmd = 'nand erase.chip ; '
     flashcmd += 'nand write 0x%08x 0 0x%08x ; ' % (flash_image_addr, flash_img_size)
+    flashcmd += 'nand read 0x%08x 0 0x%08x ; ' % (readback_addr, flash_img_size)
     return flashcmd
 
-def gen_flashcmd_spi(flash_image_addr, flash_img_size):
+def gen_flashcmd_spi(flash_image_addr, readback_addr, flash_img_size):
     flash_id = config.get('flash-id-uboot', '0')
     flashcmd = 'sf probe %s ; ' % flash_id
     flashcmd += 'sf erase 0 0x%08x ; ' % config['flash-erase-size']
     flashcmd += 'sf write 0x%08x 0 0x%08x ; ' % (flash_image_addr, flash_img_size)
+    flashcmd += 'sf read 0x%08x 0 0x%08x ; ' % (readback_addr, flash_img_size)
     return flashcmd
 
 gen_flashcmds = {
@@ -124,6 +128,16 @@ def func_flash():
     flash_img_size = os.path.getsize(flash_img)
     if args.debug:
         print 'flash_img_size %d 0x%x' % (flash_img_size, flash_img_size)
+    flash_img_crc32 = subprocess.check_output(['crc32', flash_img]).strip()
+    if args.debug:
+        print 'flash_img_crc32 %s' % (flash_img_crc32)
+    crc32_i = int(flash_img_crc32, 16)
+    flash_img_crc32_bs = (
+        ((crc32_i & 0xff) << 24) |
+        ((crc32_i & 0xff00) << 8) |
+        ((crc32_i & 0xff0000) >> 8) |
+        ((crc32_i & 0xff000000) >> 24)
+    )
 
     u_boot_plus_dtb_size = u_boot_no_dtb_size + u_boot_dtb_size
     if args.debug:
@@ -144,6 +158,9 @@ def func_flash():
     flash_image_addr = loadaddr + padded_size
     if args.debug:
         print 'flash_image_addr %d 0x%x' % (flash_image_addr, flash_image_addr)
+    readback_addr = flash_image_addr + flash_img_size
+    if args.debug:
+        print 'readback_addr %d 0x%x' % (readback_addr, readback_addr)
 
     flash_type = config['flash-type']
     if not gen_flashcmds.has_key(flash_type):
@@ -165,9 +182,11 @@ def func_flash():
         run(workdir, cmd)
 
         bootcmd = ''
-        if args.debug:
-            bootcmd = 'crc32 0x%08x 0x%08x ; ' % (flash_image_addr, flash_img_size)
-        bootcmd += gen_flashcmd(flash_image_addr, flash_img_size)
+        bootcmd += 'crc32 0x%08x 0x%08x 0x%08x ; ' % (flash_image_addr, flash_img_size, soc['ram-base'])
+        bootcmd += 'if itest.l *0x%08x != 0x%x; then echo CRC MISMATCH of initial image; exit; fi ; ' % (soc['ram-base'], flash_img_crc32_bs)
+        bootcmd += gen_flashcmd(flash_image_addr, readback_addr, flash_img_size)
+        bootcmd += 'crc32 0x%08x 0x%08x 0x%08x ; ' % (readback_addr, flash_img_size, soc['ram-base'])
+        bootcmd += 'if itest.l *0x%08x != 0x%x; then echo CRC MISMATCH of readback image; exit; fi ; ' % (soc['ram-base'], flash_img_crc32_bs)
         bootcmd += 'env default -f -a ; '
         # Perhaps U-Boot should set $boardname based on the ID EEPROM; then we wouldn't need this
         if config['dtbfn-extra'] != '':
-- 
1.8.1.5

[flasher PATCH 2/4] Increase padding between U-Boot binary and flash image

[Stephen Warren]

From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The flasher currently places the flash image in RAM almost directly above
the initial U-Boot binary location, with just a little padding/alignment
in between. This is also where U-Boot's BSS is located, prior to U-Boot
relocating itself to the top of RAM. Hence, if U-Boot touches its BSS, it
will corrupt the flash image. Increase the padding between U-Boot/DT and
the flash image to avoid this. U-Boot's BSS is roughly 300K at present.
Make the pad size 1M to allow plenty of room for growth.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 tegra-uboot-flasher | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tegra-uboot-flasher b/tegra-uboot-flasher
index d97b9e43aa45..06e297730112 100755
--- a/tegra-uboot-flasher
+++ b/tegra-uboot-flasher
@@ -143,9 +143,9 @@ def func_flash():
     if args.debug:
         print 'u_boot_plus_dtb_size %d 0x%x' % (u_boot_plus_dtb_size, u_boot_plus_dtb_size)
 
-    # Add 32k in case size changes due to fdtput
+    # Add 1024k to avoid U-Boot's BSS, and in case the DT size changes due to fdtput
     # Align to 4k, so flash writes don't need a bounce buffer for DMA
-    padded_size = (u_boot_plus_dtb_size + (32 * 1024) + (4 * 1024) - 1) & ~((4 * 1024) - 1)
+    padded_size = (u_boot_plus_dtb_size + (1024 * 1024) + (4 * 1024) - 1) & ~((4 * 1024) - 1)
     if args.debug:
         print 'padded_size %d 0x%x' % (padded_size, padded_size)
 
-- 
1.8.1.5

Re: [flasher PATCH 2/4] Increase padding between U-Boot binary and flash image

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

On Thu, Dec 05, 2013 at 03:13:00PM -0700, Stephen Warren wrote:
> From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> The flasher currently places the flash image in RAM almost directly above
> the initial U-Boot binary location, with just a little padding/alignment
> in between. This is also where U-Boot's BSS is located, prior to U-Boot
> relocating itself to the top of RAM. Hence, if U-Boot touches its BSS, it
> will corrupt the flash image. Increase the padding between U-Boot/DT and
> the flash image to avoid this. U-Boot's BSS is roughly 300K at present.
> Make the pad size 1M to allow plenty of room for growth.
> 
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  tegra-uboot-flasher | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tegra-uboot-flasher b/tegra-uboot-flasher
> index d97b9e43aa45..06e297730112 100755
> --- a/tegra-uboot-flasher
> +++ b/tegra-uboot-flasher
> @@ -143,9 +143,9 @@ def func_flash():
>      if args.debug:
>          print 'u_boot_plus_dtb_size %d 0x%x' % (u_boot_plus_dtb_size, u_boot_plus_dtb_size)
>  
> -    # Add 32k in case size changes due to fdtput
> +    # Add 1024k to avoid U-Boot's BSS, and in case the DT size changes due to fdtput
>      # Align to 4k, so flash writes don't need a bounce buffer for DMA
> -    padded_size = (u_boot_plus_dtb_size + (32 * 1024) + (4 * 1024) - 1) & ~((4 * 1024) - 1)
> +    padded_size = (u_boot_plus_dtb_size + (1024 * 1024) + (4 * 1024) - 1) & ~((4 * 1024) - 1)

You could save a multiplication here by extracting the common factor
1024... =)

Reviewed-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[flasher PATCH 3/4] Allow overriding environment variables during flashing

[Stephen Warren]

From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The flashing process resets the U-Boot environment to the built-in
default. Allow the user to specify some non-default values to be set
before saving the environment. This can be useful e.g. to set up the
default boot target:

./tegra-uboot-flasher flash --env boot_targets dhcp beaver

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 tegra-uboot-flasher | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tegra-uboot-flasher b/tegra-uboot-flasher
index 06e297730112..bd8af7caa888 100755
--- a/tegra-uboot-flasher
+++ b/tegra-uboot-flasher
@@ -191,6 +191,8 @@ def func_flash():
         # Perhaps U-Boot should set $boardname based on the ID EEPROM; then we wouldn't need this
         if config['dtbfn-extra'] != '':
             bootcmd += 'setenv board ' + boardname + config['dtbfn-extra'] + ' ; '
+        for (var, value) in args.env:
+            bootcmd += 'setenv %s %s ; ' % (var, value)
         bootcmd += 'saveenv ; '
         # To update the bootloader, reset.
         # If wanting to run installer, set installer_args.configname in environment, 'run bootcmd'
@@ -279,6 +281,8 @@ parser_flash.add_argument('--flash-image', type=str,
     help='The flash image to write, instead of U-Boot itself')
 parser_flash.add_argument('--gen-only', action='store_true',
     help='Just create the work-dir; don\'t actually flash the image')
+parser_flash.add_argument('--env', type=str, nargs=2, action='append',
+    help='Set a U-Boot environment variable after flashing')
 
 parser_exec = subparsers.add_parser('exec',
     help='Download and execute an unmodified bootloader binary, named ' +
-- 
1.8.1.5

Re: [flasher PATCH 3/4] Allow overriding environment variables during flashing

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 751 bytes --]

On Thu, Dec 05, 2013 at 03:13:01PM -0700, Stephen Warren wrote:
[...]
> diff --git a/tegra-uboot-flasher b/tegra-uboot-flasher
[...]
> @@ -279,6 +281,8 @@ parser_flash.add_argument('--flash-image', type=str,
>      help='The flash image to write, instead of U-Boot itself')
>  parser_flash.add_argument('--gen-only', action='store_true',
>      help='Just create the work-dir; don\'t actually flash the image')
> +parser_flash.add_argument('--env', type=str, nargs=2, action='append',
> +    help='Set a U-Boot environment variable after flashing')

Perhaps this could mention that a key/value pair is expected and that
the option can be repeated? Or perhaps Python is clever enough to say so
given the nargs=2 and action='append' arguments?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[flasher PATCH 4/4] Provide progress messages

[Stephen Warren]

From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Print messages to the U-Boot console while executing the flashing
process. For NAND and SPI at least, IO operations can be quite slow,
and it's useful to know the U-Boot is executing what may be a long-
running operation.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 tegra-uboot-flasher | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/tegra-uboot-flasher b/tegra-uboot-flasher
index bd8af7caa888..87b87697c027 100755
--- a/tegra-uboot-flasher
+++ b/tegra-uboot-flasher
@@ -55,22 +55,32 @@ def run(dir, cmd):
 def gen_flashcmd_mmc(flash_image_addr, readback_addr, flash_img_size):
     flash_id = config['flash-id-uboot']
     flash_img_size_sectors = flash_img_size / 512
-    flashcmd = 'mmc dev %d 1 ; ' % flash_id
+    flashcmd = 'echo >>> Selecting MMC device... ; '
+    flashcmd += 'mmc dev %d 1 ; ' % flash_id
+    flashcmd += 'echo >>> Writing image to MMC... ; '
     flashcmd += 'mmc write 0x%08x 0 0x%x ; ' % (flash_image_addr, flash_img_size_sectors)
+    flashcmd += 'echo >>> Reading image back from MMC... ; '
     flashcmd += 'mmc read 0x%08x 0 0x%x ; ' % (readback_addr, flash_img_size_sectors)
     return flashcmd
 
 def gen_flashcmd_nand(flash_image_addr, readback_addr, flash_img_size):
-    flashcmd = 'nand erase.chip ; '
+    flashcmd = 'echo >>> Erasing NAND... ; '
+    flashcmd += 'nand erase.chip ; '
+    flashcmd += 'echo >>> Writing image to NAND... ; '
     flashcmd += 'nand write 0x%08x 0 0x%08x ; ' % (flash_image_addr, flash_img_size)
+    flashcmd += 'echo >>> Reading image back from NAND... ; '
     flashcmd += 'nand read 0x%08x 0 0x%08x ; ' % (readback_addr, flash_img_size)
     return flashcmd
 
 def gen_flashcmd_spi(flash_image_addr, readback_addr, flash_img_size):
     flash_id = config.get('flash-id-uboot', '0')
-    flashcmd = 'sf probe %s ; ' % flash_id
+    flashcmd = 'echo >>> Selecting SPI device... ; '
+    flashcmd += 'sf probe %s ; ' % flash_id
+    flashcmd += 'echo >>> Erasing SPI... ; '
     flashcmd += 'sf erase 0 0x%08x ; ' % config['flash-erase-size']
+    flashcmd += 'echo >>> Writing image back to SPI... ; '
     flashcmd += 'sf write 0x%08x 0 0x%08x ; ' % (flash_image_addr, flash_img_size)
+    flashcmd += 'echo >>> Reading image back from SPI... ; '
     flashcmd += 'sf read 0x%08x 0 0x%08x ; ' % (readback_addr, flash_img_size)
     return flashcmd
 
@@ -182,11 +192,14 @@ def func_flash():
         run(workdir, cmd)
 
         bootcmd = ''
+        bootcmd += 'echo >>> Verifying image in RAM... ; '
         bootcmd += 'crc32 0x%08x 0x%08x 0x%08x ; ' % (flash_image_addr, flash_img_size, soc['ram-base'])
         bootcmd += 'if itest.l *0x%08x != 0x%x; then echo CRC MISMATCH of initial image; exit; fi ; ' % (soc['ram-base'], flash_img_crc32_bs)
         bootcmd += gen_flashcmd(flash_image_addr, readback_addr, flash_img_size)
+        bootcmd += 'echo >>> Verifying image from flash... ; '
         bootcmd += 'crc32 0x%08x 0x%08x 0x%08x ; ' % (readback_addr, flash_img_size, soc['ram-base'])
         bootcmd += 'if itest.l *0x%08x != 0x%x; then echo CRC MISMATCH of readback image; exit; fi ; ' % (soc['ram-base'], flash_img_crc32_bs)
+        bootcmd += 'echo >>> Setting up environment... ; '
         bootcmd += 'env default -f -a ; '
         # Perhaps U-Boot should set $boardname based on the ID EEPROM; then we wouldn't need this
         if config['dtbfn-extra'] != '':
@@ -194,6 +207,7 @@ def func_flash():
         for (var, value) in args.env:
             bootcmd += 'setenv %s %s ; ' % (var, value)
         bootcmd += 'saveenv ; '
+        bootcmd += 'echo >>> Flashing OK, rebooting... ; '
         # To update the bootloader, reset.
         # If wanting to run installer, set installer_args.configname in environment, 'run bootcmd'
         bootcmd += 'reset'
-- 
1.8.1.5

Re: [flasher PATCH 4/4] Provide progress messages

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

On Thu, Dec 05, 2013 at 03:13:02PM -0700, Stephen Warren wrote:
> From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Print messages to the U-Boot console while executing the flashing
> process. For NAND and SPI at least, IO operations can be quite slow,
> and it's useful to know the U-Boot is executing what may be a long-
> running operation.
> 
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  tegra-uboot-flasher | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)

Reviewed-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [flasher PATCH 1/4] Add crc32 verification of the flash image

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

On Thu, Dec 05, 2013 at 03:12:59PM -0700, Stephen Warren wrote:
[...]
> diff --git a/tegra-uboot-flasher b/tegra-uboot-flasher
[...]
> @@ -124,6 +128,16 @@ def func_flash():
>      flash_img_size = os.path.getsize(flash_img)
>      if args.debug:
>          print 'flash_img_size %d 0x%x' % (flash_img_size, flash_img_size)
> +    flash_img_crc32 = subprocess.check_output(['crc32', flash_img]).strip()

Perhaps do this with binascii.crc32(), which is part of Python's
standard library. That way we don't pull in crc32 as an additional
dependency.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]