Skip to contentSkip to conversation

add inital SF tests

PATCH Superseded Liam Beguin 2 participants 12 messages Mar 5, 2018 – Mar 14, 2018 Memory technology devices (MTD)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover add inital SF tests

Hi all,

This is the inital step to adding tests for the SF subsystem plus very minor fixes. It is based on work I found on the mailing list[1]. For now, it doesn't do much but I plan on adding code to reset the flash to its initial state (based on an env flag) and more code to test the sf protect subcommand (which is the main goal of this series). I'm sending it now to make sure it's headed in the right direction.

Base on Stephen's comment[2], I haven't added the radomized features. I'll see how this iteration goes and maybe add it later.

Changes since v1:
- remove unnecessary skip flag from environment
- move crc32() to u_boot_utils.py and add extra checks
- rewrite sf_prepare to return a dict of parameter
- use assert instead of pytest.fail
- remove verbose from sf_prepare()
- update documentation
- improve readability
- use ' consistently instead of "
- use sf_read() in test_sf_read()
- rename crc variables
- add speed parameter with optional random range
- allow sf read to write at 0x00

Thanks,
Liam Beguin

[ 1 ] https://patchwork.ozlabs.org/patch/623061/
[ 2 ] https://lists.denx.de/pipermail/u-boot/2018-March/321688.html

Liam Beguin (8):
spi: spi_flash: do not fail silently on bad user input
cmd: sf: fix map_physmem check
test/py: README: fix typo
test/py: README: add HOSTNAME to PYTHONPATH test/py: do not import pytest multiple times test/py: add generic CRC32 function
test/py: add spi_flash tests

 cmd/sf.c                    |   2 +-
 drivers/mtd/spi/spi_flash.c |   2 +-
 test/py/README.md           |   6 +-
 test/py/tests/test_sf.py    | 223 ++++++++++++++++++++++++++++++++++++++++++++
 test/py/u_boot_utils.py     |  24 ++++-
 5 files changed, 251 insertions(+), 6 deletions(-)
 create mode 100644 test/py/tests/test_sf.py

base-commit: 77bba970e2372b01156c66585db3d6fc751c7178
Published-As: https://github.com/Liambeguin/u-boot/releases/tag/test_sf-v2

Signature · 1 line
2.16.1.72.g5be1f00a9a70
#1 lore raw

PATCH v2 1/7 spi: spi_flash: do not fail silently on bad user input

Make sure the user is notified instead of silently returning an error.

---
 drivers/mtd/spi/spi_flash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/spi/spi_flash.c +1 -1
index 294d9f9d79c6..2e61685d3ea4 100644
@@ -320,7 +320,7 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
 
 	erase_size = flash->erase_size;
 	if (offset % erase_size || len % erase_size) {
-		debug("SF: Erase offset/length not multiple of erase size\n");
+		printf("SF: Erase offset/length not multiple of erase size\n");
 		return -1;
 	}
 
Signature · 1 line
2.16.1.72.g5be1f00a9a70
#2 lore raw

PATCH v2 2/7 cmd: sf: fix map_physmem check

In reply to Liam Beguin

Make sure 0x00 is a valid address to read to. If addr is 0x00 then map_physmem() will return 0 which should be a valid address.

---
 cmd/sf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
cmd/sf.c +1 -1
index f971eec781cc..e7ff9a646208 100644
@@ -287,7 +287,7 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 	}
 
 	buf = map_physmem(addr, len, MAP_WRBACK);
-	if (!buf) {
+	if (!buf && addr) {
 		puts("Failed to map physical memory\n");
 		return 1;
 	}
Signature · 1 line
2.16.1.72.g5be1f00a9a70
#3 lore raw

PATCH v2 3/7 test/py: README: fix typo

In reply to Liam Beguin

Fix a minor typo causing vim (and possibly other) to get confused with coloring.

---
 test/py/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
test/py/README.md +1 -1
index eefac377567a..000afce93c4a 100644
@@ -150,7 +150,7 @@ processing.
   option takes a single argument which is used to filter test names. Simple
   logical operators are supported. For example:
   - `'ums'` runs only tests with "ums" in their name.
-  - ``ut_dm'` runs only tests with "ut_dm" in their name. Note that in this
+  - `'ut_dm'` runs only tests with "ut_dm" in their name. Note that in this
     case, "ut_dm" is a parameter to a test rather than the test name. The full
     test name is e.g. "test_ut[ut_dm_leak]".
   - `'not reset'` runs everything except tests with "reset" in their name.
Signature · 1 line
2.16.1.72.g5be1f00a9a70
#4 lore raw

PATCH v2 4/7 test/py: README: add HOSTNAME to PYTHONPATH

In reply to Liam Beguin

As opposed to PATH, HOSTNAME is not appended to PYTHONPATH automatically. Lets add it to the examples to make it more obvious to new users.

---
 test/py/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
test/py/README.md +2 -2
index 000afce93c4a..aed2fd063a81 100644
@@ -320,7 +320,7 @@ If U-Boot has already been built:
 
 ```bash
 PATH=$HOME/ubtest/bin:$PATH \
-    PYTHONPATH=${HOME}/ubtest/py:${PYTHONPATH} \
+    PYTHONPATH=${HOME}/ubtest/py/${HOSTNAME}:${PYTHONPATH} \
     ./test/py/test.py --bd seaboard
 ```
 
@@ -331,7 +331,7 @@ follow:
 ```bash
 CROSS_COMPILE=arm-none-eabi- \
     PATH=$HOME/ubtest/bin:$PATH \
-    PYTHONPATH=${HOME}/ubtest/py:${PYTHONPATH} \
+    PYTHONPATH=${HOME}/ubtest/py/${HOSTNAME}:${PYTHONPATH} \
     ./test/py/test.py --bd seaboard --build
 ```
 
Signature · 1 line
2.16.1.72.g5be1f00a9a70
#5 lore raw

PATCH v2 5/7 test/py: do not import pytest multiple times

In reply to Liam Beguin
---
 test/py/u_boot_utils.py | 1 -
 1 file changed, 1 deletion(-)
test/py/u_boot_utils.py +0 -1
index 9acb92ddc448..64584494e463 100644
@@ -11,7 +11,6 @@ import os.path
 import pytest
 import sys
 import time
-import pytest
 
 def md5sum_data(data):
     """Calculate the MD5 hash of some data.
Signature · 1 line
2.16.1.72.g5be1f00a9a70
#6 lore raw

PATCH v2 6/7 test/py: add generic CRC32 function

In reply to Liam Beguin

Add a generic function which can be used to compute the CRC32 value of a region of RAM.

---
 test/py/u_boot_utils.py | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
test/py/u_boot_utils.py +23 -0
index 64584494e463..de9ee2643f51 100644
@@ -11,6 +11,7 @@ import os.path
 import pytest
 import sys
 import time
+import re
 
 def md5sum_data(data):
     """Calculate the MD5 hash of some data.
@@ -310,3 +311,25 @@ def persistent_file_helper(u_boot_log, filename):
     """
 
     return PersistentFileHelperCtxMgr(u_boot_log, filename)
+
+def crc32(u_boot_console, address, count):
+    """Helper function used to compute the CRC32 value of a section of RAM.
+
+    Args:
+        u_boot_console: A U-Boot console connection.
+        address: Address where data starts.
+        count: Amount of data to use for calculation.
+
+    Returns:
+        CRC32 value
+    """
+
+    bcfg = u_boot_console.config.buildconfig
+    has_cmd_crc32 = bcfg.get('config_cmd_crc32', 'n') == 'y'
+    assert has_cmd_crc32, 'Cannot compute crc32 without CONFIG_CMD_CRC32.'
+    output = u_boot_console.run_command('crc32 %08x %x' % (address, count))
+
+    m = re.search('==> ([0-9a-fA-F]{8})$', output)
+    assert m, 'CRC32 operation failed.'
+
+    return m.group(1)
Signature · 1 line
2.16.1.72.g5be1f00a9a70
#8 lore raw

PATCH v2 7/7 test/py: add spi_flash tests

In reply to Liam Beguin

Add basic tests for the spi_flash subsystem.

---
 test/py/tests/test_sf.py | 223 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 223 insertions(+)
 create mode 100644 test/py/tests/test_sf.py
test/py/tests/test_sf.py new file +223 -0
new file mode 100644
index 000000000000..0cc2a60e68d4
@@ -0,0 +1,223 @@
+# Copyright (c) 2017, Xiphos Systems Corp. All rights reserved.
+#
+# SPDX-License-Identifier: GPL-2.0
+
+import re
+import pytest
+import random
+import u_boot_utils
+
+
+"""
+Note: This test relies on boardenv_* containing configuration values to define
+which SPI Flash areas are available for testing.  Without this, this test will
+be automatically skipped.
+For example:
+
+# A list of sections of Flash memory to be tested.
+env__sf_configs = (
+    {
+        # Where in SPI Flash should the test operate.
+        'offset': 0x00000000,
+        # This value is optional.
+        #   If present, specifies the [[bus:]cs] argument used in `sf probe`
+        #   If missing, defaults to 0.
+        'id': '0:1',
+        # This value is optional.
+        #   If set as a number, specifies the speed of the SPI Flash.
+        #   If set as an array of 2, specifies a range for a random speed.
+        #   If missing, defaults to 0.
+        'speed': 1000000,
+        # This value is optional.
+        #   If present, specifies the size to use for read/write operations.
+        #   If missing, the SPI Flash page size is used as a default (based on
+        #   the `sf probe` output).
+        'len': 0x10000,
+        # This value is optional.
+        #   If present, specifies if the test can write to Flash offset
+        #   If missing, defaults to False.
+        'writeable': False,
+        # This value is optional.
+        #   If present, specifies the expected CRC32 value of the flash area.
+        #   If missing, extra check is ignored.
+        'crc32': 0xCAFECAFE,
+    },
+)
+"""
+
+
+def sf_prepare(u_boot_console, env__sf_config):
+    """Check global state of the SPI Flash before running any test.
+
+   Args:
+        u_boot_console: A U-Boot console connection.
+        env__sf_config: The single SPI Flash device configuration on which to
+            run the tests.
+
+    Returns:
+        sf_params: a dictionnary of SPI Flash parameters.
+    """
+
+    sf_params = {}
+    sf_params['ram_base'] = u_boot_utils.find_ram_base(u_boot_console)
+
+    probe_id = env__sf_config.get('id', 0)
+    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
+
+    cmd = 'sf probe %d %d' % (probe_id, sf_params['speed'])
+
+    output = u_boot_console.run_command(cmd)
+    assert 'SF: Detected' in output, 'No Flash device available'
+
+    m = re.search('page size (.+?) Bytes', output)
+    assert m, 'SPI Flash page size not recognized'
+    sf_params['page_size'] = int(m.group(1))
+
+    m = re.search('erase size (.+?) KiB', output)
+    assert m, 'SPI Flash erase size not recognized'
+    sf_params['erase_size'] = int(m.group(1))
+    sf_params['erase_size'] *= 1024
+
+    m = re.search('total (.+?) MiB', output)
+    assert m, 'SPI Flash total size not recognized'
+    sf_params['total_size'] = int(m.group(1))
+    sf_params['total_size'] *= 1024 * 1024
+
+    assert env__sf_config['offset'] is not None, \
+        '\'offset\' is required for this test.'
+    sf_params['len'] = env__sf_config.get('len', sf_params['erase_size'])
+
+    assert not env__sf_config['offset'] % sf_params['erase_size'], \
+        'offset not multiple of erase size.'
+    assert not sf_params['len'] % sf_params['erase_size'], \
+        'erase length not multiple of erase size.'
+
+    assert not (env__sf_config.get('writeable', False) and
+                env__sf_config.get('crc32', False)), \
+        'Cannot check crc32 on  writeable sections'
+
+    return sf_params
+
+
+def sf_read(u_boot_console, env__sf_config, sf_params):
+    """Helper function used to read and compute the CRC32 value of a section of
+    SPI Flash memory.
+
+    Args:
+        u_boot_console: A U-Boot console connection.
+        env__sf_config: The single SPI Flash device configuration on which to
+            run the tests.
+        sf_params: SPI Flash parameters.
+
+    Returns:
+        CRC32 value of SPI Flash section
+    """
+
+    addr = sf_params['ram_base']
+    offset = env__sf_config['offset']
+    count = sf_params['len']
+    pattern = random.randint(0, 0xFFFFFFFF)
+    crc_expected = env__sf_config.get('crc32', None)
+
+    cmd = 'mw.b %08x %08x %x' % (addr, pattern, count)
+    u_boot_console.run_command(cmd)
+    crc_read = u_boot_utils.crc32(u_boot_console, addr, count)
+    if crc_expected:
+        assert crc_read != crc_expected
+
+    cmd = 'sf read %08x %08x %x' % (addr, offset, count)
+    response = u_boot_console.run_command(cmd)
+    assert 'Read: OK' in response, 'Read operation failed'
+    crc_readback = u_boot_utils.crc32(u_boot_console, addr, count)
+    assert crc_read != crc_readback, 'sf read did not update RAM content.'
+    if crc_expected:
+        assert crc_readback == crc_expected
+
+    return crc_readback
+
+
+def sf_update(u_boot_console, env__sf_config, sf_params):
+    """Helper function used to update a section of SPI Flash memory.
+
+   Args:
+        u_boot_console: A U-Boot console connection.
+        env__sf_config: The single SPI Flash device configuration on which to
+           run the tests.
+
+    Returns:
+        CRC32 value of SPI Flash section
+    """
+
+    addr = sf_params['ram_base']
+    offset = env__sf_config['offset']
+    count = sf_params['len']
+    pattern = int(random.random() * 0xFFFFFFFF)
+
+    cmd = 'mw.b %08x %08x %x' % (addr, pattern, count)
+    u_boot_console.run_command(cmd)
+    crc_read = u_boot_utils.crc32(u_boot_console, addr, count)
+
+    cmd = 'sf update %08x %08x %x' % (addr, offset, count)
+    u_boot_console.run_command(cmd)
+    crc_readback = sf_read(u_boot_console, env__sf_config, sf_params)
+
+    assert crc_readback == crc_read
+
+
+ at pytest.mark.buildconfigspec('cmd_sf')
+ at pytest.mark.buildconfigspec('cmd_crc32')
+ at pytest.mark.buildconfigspec('cmd_memory')
+def test_sf_read(u_boot_console, env__sf_config):
+    sf_params = sf_prepare(u_boot_console, env__sf_config)
+    sf_read(u_boot_console, env__sf_config, sf_params)
+
+
+ at pytest.mark.buildconfigspec('cmd_sf')
+ at pytest.mark.buildconfigspec('cmd_crc32')
+ at pytest.mark.buildconfigspec('cmd_memory')
+def test_sf_read_twice(u_boot_console, env__sf_config):
+    sf_params = sf_prepare(u_boot_console, env__sf_config)
+
+    crc1 = sf_read(u_boot_console, env__sf_config, sf_params)
+    sf_params['ram_base'] += 0x100
+    crc2 = sf_read(u_boot_console, env__sf_config, sf_params)
+
+    assert crc1 == crc2, 'CRC32 of two successive read operation do not match'
+
+
+ at pytest.mark.buildconfigspec('cmd_sf')
+ at pytest.mark.buildconfigspec('cmd_crc32')
+ at pytest.mark.buildconfigspec('cmd_memory')
+def test_sf_erase(u_boot_console, env__sf_config):
+    if not env__sf_config.get('writeable', False):
+        pytest.skip('Flash config is tagged as not writeable')
+
+    sf_params = sf_prepare(u_boot_console, env__sf_config)
+    addr = sf_params['ram_base']
+    offset = env__sf_config['offset']
+    count = sf_params['len']
+
+    cmd = 'sf erase %08x %x' % (offset, count)
+    output = u_boot_console.run_command(cmd)
+    assert 'Erased: OK' in output, 'Erase operation failed'
+
+    cmd = 'mw.b %08x ffffffff %x' % (addr, count)
+    u_boot_console.run_command(cmd)
+    crc_ffs = u_boot_utils.crc32(u_boot_console, addr, count)
+
+    crc_read = sf_read(u_boot_console, env__sf_config, sf_params)
+    assert crc_ffs == crc_read, 'Unexpected CRC32 after erase operation.'
+
+
+ at pytest.mark.buildconfigspec('cmd_sf')
+ at pytest.mark.buildconfigspec('cmd_memory')
+def test_sf_update(u_boot_console, env__sf_config):
+    if not env__sf_config.get('writeable', False):
+        pytest.skip('Flash config is tagged as not writeable')
+
+    sf_params = sf_prepare(u_boot_console, env__sf_config)
+    sf_update(u_boot_console, env__sf_config, sf_params)
Signature · 1 line
2.16.1.72.g5be1f00a9a70
#9 lore raw

PATCH v2 7/7 test/py: add spi_flash tests

Quoted 1 line · Liam Beguin

Add basic tests for the spi_flash subsystem.

Looks good. A few small issues:

Quoted 1 line

+def sf_prepare(u_boot_console, env__sf_config):

...

test/py/tests/test_sf.py line 69

Quoted 5 lines
test/py/tests/test_sf.py new file +5 -0
+    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

test/py/tests/test_sf.py line 91

Quoted 2 lines
test/py/tests/test_sf.py new file +2 -0
+    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

test/py/tests/test_sf.py line 101

Quoted 3 lines
test/py/tests/test_sf.py new file +3 -0
+    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'

test/py/tests/test_sf.py line 123

Quoted 5 lines
test/py/tests/test_sf.py new file +5 -0
+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.

test/py/tests/test_sf.py line 126

Quoted 3 lines
test/py/tests/test_sf.py new file +3 -0
+    crc_expected = env__sf_config.get('crc32', None)
+
+    cmd = 'mw.b %08x %08x %x' % (addr, pattern, count)

%02x for pattern.

Review

Quoted 2 lines
(quoted diff) +2 -0
+    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?

test/py/tests/test_sf.py line 158

Quoted 5 lines
test/py/tests/test_sf.py new file +5 -0
+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.

Quoted 1 line

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

%02x for pattern.

Review

Quoted 2 lines
(quoted diff) +2 -0
+    u_boot_console.run_command(cmd)
+    crc_read = u_boot_utils.crc32(u_boot_console, addr, count)

crc_pattern?

Quoted 1 line

+def test_sf_erase(u_boot_console, env__sf_config):

...

Quoted 1 line

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

Just ff not ffffffff?

Quoted 3 lines

+ at pytest.mark.buildconfigspec('cmd_sf')
+ at 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').

#10 lore raw

PATCH v2 7/7 test/py: add spi_flash tests

Hi Stephen,

Quoted 18 lines · Stephen Warren
Quoted 2 lines · Liam Beguin

Add basic tests for the spi_flash subsystem.

Looks good. A few small issues:

Quoted 1 line

+def sf_prepare(u_boot_console, env__sf_config):

...

test/py/tests/test_sf.py line 69

Quoted 6 lines
test/py/tests/test_sf.py new file +5 -0
+    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
Quoted 9 lines

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

Right, that's better!

Quoted 8 lines

test/py/tests/test_sf.py line 91

Quoted 2 lines
test/py/tests/test_sf.py new file +2 -0
+    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.

Quoted 9 lines

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

Thanks for seeing this, will fix.

Quoted 5 lines

test/py/tests/test_sf.py line 101

Quoted 3 lines
test/py/tests/test_sf.py new file +3 -0
+    assert not (env__sf_config.get('writeable', False) and
+                env__sf_config.get('crc32', False)), \
+        'Cannot check crc32 on  writeable sections'
Quoted 7 lines

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'

Ok

Quoted 9 lines

test/py/tests/test_sf.py line 123

Quoted 5 lines
test/py/tests/test_sf.py new file +5 -0
+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.

Will fix across file...

Quoted 5 lines

test/py/tests/test_sf.py line 126

Quoted 3 lines
test/py/tests/test_sf.py new file +3 -0
+    crc_expected = env__sf_config.get('crc32', None)
+
+    cmd = 'mw.b %08x %08x %x' % (addr, pattern, count)
Quoted 7 lines

%02x for pattern.

Review

Quoted 2 lines
(quoted diff) +2 -0
+    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?

crc_pattern sounds good.

Quoted 30 lines

test/py/tests/test_sf.py line 158

Quoted 6 lines
test/py/tests/test_sf.py new file +5 -0
+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.

Quoted 1 line

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

%02x for pattern.

Review

Quoted 2 lines
(quoted diff) +2 -0
+    u_boot_console.run_command(cmd)
+    crc_read = u_boot_utils.crc32(u_boot_console, addr, count)

crc_pattern?

Quoted 1 line

+def test_sf_erase(u_boot_console, env__sf_config):

...

Quoted 2 lines

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

Just ff not ffffffff?

Quoted 8 lines
Quoted 3 lines

+ at pytest.mark.buildconfigspec('cmd_sf')
+ at 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').

You're right, I missed this one...

Thanks for taking the time to review this series,
Liam Beguin

#11 lore raw

v2 cover add inital SF tests

In reply to Liam Beguin

Hi everyone,

Any new comments on this iteration?

Thanks,
Liam

Quoted 58 lines · On Sun, 4 Mar 2018 at 23:22 Liam Beguin <[email protected]> wrote:

Hi all,

This is the inital step to adding tests for the SF subsystem plus very minor fixes. It is based on work I found on the mailing list[1]. For now, it doesn't do much but I plan on adding code to reset the flash to its initial state (based on an env flag) and more code to test the sf protect subcommand (which is the main goal of this series). I'm sending it now to make sure it's headed in the right direction.

Base on Stephen's comment[2], I haven't added the radomized features. I'll see how this iteration goes and maybe add it later.

Changes since v1:
- remove unnecessary skip flag from environment
- move crc32() to u_boot_utils.py and add extra checks
- rewrite sf_prepare to return a dict of parameter
- use assert instead of pytest.fail
- remove verbose from sf_prepare()
- update documentation
- improve readability
- use ' consistently instead of "
- use sf_read() in test_sf_read()
- rename crc variables
- add speed parameter with optional random range
- allow sf read to write at 0x00

Thanks,
Liam Beguin

[ 1 ] https://patchwork.ozlabs.org/patch/623061/
[ 2 ] https://lists.denx.de/pipermail/u-boot/2018-March/321688.html

Liam Beguin (8):
spi: spi_flash: do not fail silently on bad user input
cmd: sf: fix map_physmem check
test/py: README: fix typo
test/py: README: add HOSTNAME to PYTHONPATH test/py: do not import pytest multiple times test/py: add generic CRC32 function
test/py: add spi_flash tests

 cmd/sf.c                    |   2 +-
 drivers/mtd/spi/spi_flash.c |   2 +-
 test/py/README.md           |   6 +-
 test/py/tests/test_sf.py    | 223
++++++++++++++++++++++++++++++++++++++++++++
 test/py/u_boot_utils.py     |  24 ++++-
 5 files changed, 251 insertions(+), 6 deletions(-)
 create mode 100644 test/py/tests/test_sf.py

base-commit: 77bba970e2372b01156c66585db3d6fc751c7178
Published-As: https://github.com/Liambeguin/u-boot/releases/tag/test_sf-v2

--
2.16.1.72.g5be1f00a9a70