add inital SF tests
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
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
@@ -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
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.
Signed-off-by Liam Beguin <[email protected]>Reviewed-by Stephen Warren <[email protected]>
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>
PATCH v4 2/7 cmd: sf: fix map_physmem check
In reply to Liam BeguinMake 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
@@ -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
PATCH v4 2/7 cmd: sf: fix map_physmem check
Quoted 5 lines · Liam Beguin
Make sure
0x00is a valid address to read to. Ifaddris0x00thenmap_physmem()will return 0 which should be a valid address.Signed-off-by Liam Beguin <[email protected]>Reviewed-by Stephen Warren <[email protected]>
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>
PATCH v4 3/7 test/py: README: fix typo
In reply to Liam BeguinFix 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
@@ -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
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.
Signed-off-by Liam Beguin <[email protected]>Reviewed-by Stephen Warren <[email protected]>
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>
PATCH v4 4/7 test/py: README: add HOSTNAME to PYTHONPATH
In reply to Liam BeguinAs 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
@@ -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
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.
Signed-off-by Liam Beguin <[email protected]>Reviewed-by Stephen Warren <[email protected]>
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>
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
@@ -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
PATCH v4 5/7 test/py: do not import pytest multiple times
Quoted 2 lines · Liam Beguin
Signed-off-by Liam Beguin <[email protected]>Reviewed-by Stephen Warren <[email protected]>
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>
PATCH v4 6/7 test/py: add generic CRC32 function
In reply to Liam BeguinAdd 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
@@ -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
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.
Signed-off-by Liam Beguin <[email protected]>Reviewed-by Stephen Warren <[email protected]>
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>
PATCH v4 7/7 test/py: add spi_flash tests
In reply to Liam BeguinAdd 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
@@ -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
PATCH v4 7/7 test/py: add spi_flash tests
Quoted 4 lines · Liam Beguin
Add basic tests for the
spi_flashsubsystem.Signed-off-by Liam Beguin <[email protected]>Reviewed-by Stephen Warren <[email protected]>
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.
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_flashsubsystem.Signed-off-by Liam Beguin <[email protected]>Reviewed-by Stephen Warren <[email protected]>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
PATCH v4 7/7 test/py: add spi_flash tests
In reply to Liam BeguinQuoted 4 lines · Liam Beguin
Add basic tests for the
spi_flashsubsystem.Signed-off-by Liam Beguin <[email protected]>Reviewed-by Stephen Warren <[email protected]>
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>