Skip to contentSkip to conversation

add inital SF tests

PATCH New Liam Beguin 3 participants 17 messages Mar 14, 2018 – Mar 22, 2018 Memory technology devices (MTD)

lore
Share & tools
lore
#0 lore raw

v4 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 v3:
- add Xilinx copyright notice since it's loosely based on their patch
- add Reviewed-by tags

Changes since v2:
- remove double blank lines
- in sf_prepare, fix the way speed is read from env__sf_config
- in sf_prepare, fix assert of env__sf_config['offset']
- in sf_prepare, do not fail if env__sf_config['crc32'] == 0
- in sf_{read,update}, pattern is in bytes. Make sure md/mw use the right sizes.
- in sf_{read,update}, rename crc_read to crc_pattern when appropriate
- add missing pytest.mark on test_sf_update

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 (7):
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    | 218 ++++++++++++++++++++++++++++++++++++++++++++
 test/py/u_boot_utils.py     |  24 ++++-
 5 files changed, 246 insertions(+), 6 deletions(-)
 create mode 100644 test/py/tests/test_sf.py

base-commit: f95ab1fb6e37f0601f397091bb011edf7a98b890
Published-As: https://github.com/Liambeguin/u-boot/releases/tag/test_sf-v4

Signature · 1 line
2.16.1.72.g5be1f00a9a70
#1 lore raw

PATCH v4 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 v4 1/7 spi: spi_flash: do not fail silently on bad user input

Quoted 4 lines · Liam Beguin

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

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180322/ca20eeec/attachment.sig>
#3 lore raw

PATCH v4 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
#4 lore raw

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

Quoted 5 lines · 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.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180322/feec1926/attachment.sig>
#5 lore raw

PATCH v4 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
#6 lore raw

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

Quoted 5 lines · Liam Beguin

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

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180322/cfffd4bb/attachment.sig>
#7 lore raw

PATCH v4 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
#8 lore raw

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

Quoted 6 lines · 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.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180322/3af2c6ce/attachment.sig>
#9 lore raw

PATCH v4 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
#10 lore raw

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

Quoted 2 lines · Liam Beguin

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180322/9337f0c9/attachment.sig>
#11 lore raw

PATCH v4 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
#12 lore raw

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

Quoted 5 lines · Liam Beguin

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

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180322/df628363/attachment.sig>
#13 lore raw

PATCH v4 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 | 218 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 218 insertions(+)
 create mode 100644 test/py/tests/test_sf.py
test/py/tests/test_sf.py new file +218 -0
new file mode 100644
index 000000000000..efcd753c1abe
@@ -0,0 +1,218 @@
+# Copyright (c) 2016, Xilinx Inc. Michal Simek
+# 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, int):
+        sf_params['speed'] = speed
+    else:
+        assert len(speed) == 2, "If speed is a list, it must have 2 entries"
+        sf_params['speed'] = random.randint(speed[0], speed[1])
+
+    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 'offset' in env__sf_config, \
+        '\'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
+                'crc32' in env__sf_config), \
+        '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, 0xFF)
+    crc_expected = env__sf_config.get('crc32', None)
+
+    cmd = 'mw.b %08x %02x %x' % (addr, pattern, count)
+    u_boot_console.run_command(cmd)
+    crc_pattern = u_boot_utils.crc32(u_boot_console, addr, count)
+    if crc_expected:
+        assert crc_pattern != 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_pattern != 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() * 0xFF)
+
+    cmd = 'mw.b %08x %02x %x' % (addr, pattern, count)
+    u_boot_console.run_command(cmd)
+    crc_pattern = 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_pattern
+
+ 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 ff %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_crc32')
+ 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
#14 lore raw

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

Quoted 4 lines · Liam Beguin

Add basic tests for the spi_flash subsystem.

It's useful if you put a brief description of what changed between patch versions below the --- line so people know whether they care about re-reviewing the patch. It had to diff the v3/v4 patch content to remember/realize that v4 doesn't change anything except the (c) statement.

#15 lore raw

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

Hi,

Quoted 11 lines · On Wed, 14 Mar 2018 at 19:27 Stephen Warren <[email protected]> wrote:
Quoted 4 lines · Liam Beguin

Add basic tests for the spi_flash subsystem.

It's useful if you put a brief description of what changed between patch versions below the --- line so people know whether they care about re-reviewing the patch. It had to diff the v3/v4 patch content to remember/realize that v4 doesn't change anything except the (c) statement.

I added it to the cover-letter but I'll also include it here next time. Thanks again,

Liam

#16 lore raw

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

In reply to Liam Beguin
Quoted 4 lines · Liam Beguin

Add basic tests for the spi_flash subsystem.

With a minor spelling fix, applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180322/51ffc4cc/attachment.sig>