On 03/04/2018 09:22 PM, Liam Beguin wrote:
Add basic tests for the spi_flash subsystem.


Looks good. A few small issues:

+def sf_prepare(u_boot_console, env__sf_config):
...
+    speed = env__sf_config.get('speed', 0)
+    if isinstance(speed, list) and len(speed) == 2:
+        sf_params['speed'] = random.randint(speed[0], speed[1])
+    else:
+        sf_params['speed'] = speed

What if speed is a tuple or other indexable type not a list? Perhaps invert the test. Also, perhaps assume any list has two entries, or assert that.

if isintance(speed, int):
  int case
else:
  assert len(speed) == 2, "Speed must have two entries"
  list/tuple case

+    assert env__sf_config['offset'] is not None, \
+        '\'offset\' is required for this test.'

Is this meant to test for:
a) A key that's present, with value set to None.
b) A missing key.

It currently tests (a), but testing for (b) seems more likely to catch issues. Perhaps:

assert env__sf_config.get('offset', None) is not None

or:

assert 'offset' in env__sf_config.get

+    assert not (env__sf_config.get('writeable', False) and
+                env__sf_config.get('crc32', False)), \
+        'Cannot check crc32 on  writeable sections'

What if the crc32 value is 0, which IIRC is False? Unlikely admittedly, but you never know. Perhaps:

assert not (env__sf_config.get('writeable', False) and
            'crc32' in env__sf_config.get), \
        'Cannot check crc32 on  writeable sections'

+def sf_read(u_boot_console, env__sf_config, sf_params):
+    addr = sf_params['ram_base']
+    offset = env__sf_config['offset']
+    count = sf_params['len']
+    pattern = random.randint(0, 0xFFFFFFFF)

0xFF not 0xFFFFFFFF since it's bytes.

+    crc_expected = env__sf_config.get('crc32', None)
+
+    cmd = 'mw.b %08x %08x %x' % (addr, pattern, count)

%02x for pattern.

+    u_boot_console.run_command(cmd)
+    crc_read = u_boot_utils.crc32(u_boot_console, addr, count)

crc_read sounds like a CRC for data that's been read. Perhaps crc_pattern?

+def sf_update(u_boot_console, env__sf_config, sf_params):
+    addr = sf_params['ram_base']
+    offset = env__sf_config['offset']
+    count = sf_params['len']
+    pattern = int(random.random() * 0xFFFFFFFF)

0xFF.

+    cmd = 'mw.b %08x %08x %x' % (addr, pattern, count)

%02x for pattern.

+    u_boot_console.run_command(cmd)
+    crc_read = u_boot_utils.crc32(u_boot_console, addr, count)

crc_pattern?

+def test_sf_erase(u_boot_console, env__sf_config):
...
+    cmd = 'mw.b %08x ffffffff %x' % (addr, count)

Just ff not ffffffff?

+@pytest.mark.buildconfigspec('cmd_sf')
+@pytest.mark.buildconfigspec('cmd_memory')
+def test_sf_update(u_boot_console, env__sf_config):

sf_update() unconditionally calls u_boot_utils.crc32 which asserts if the crc32 command isn't available, so this function needs to be @pytest.mark.buildconfigspec('cmd_crc32').
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to