| Message ID | 20180227041746.2509-4-liambeguin@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | add inital SF tests | expand |
Hi Michal, On 27 February 2018 at 03:51, Michal Simek <michal.simek@xilinx.com> wrote: > Hi Liam, > > On 27.2.2018 05:17, Liam Beguin wrote: >> Add basic tests for the spi_flash subsystem. > > Good to see this patch. > FYI: We have created qspi tests too but didn't push it out because of > missing sf_configs options as you have below. We are testing the whole > flash all the time. I tried to write this based on your test plus the feedback from when you sent it on the mailing list. > Test was designed to use more randomization to make sure that every run > is working with different data. We found several issues thanks to this. > (Also keep in your mind that some tests depends on order which is wrong > but we didn't have time to clean them yet) > IIRC I have sent that patch to mainline in past and using sf_configs was > suggested too. > > https://github.com/Xilinx/u-boot-xlnx/blob/master/test/py/tests/test_qspi.py > Looking at the test, I see you randomize the spi frequency and the R/W size. I could probably add these as env configs with default values. >> >> Signed-off-by: Liam Beguin <liambeguin@gmail.com> >> --- >> test/py/tests/test_sf.py | 233 +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 233 insertions(+) >> >> diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py >> new file mode 100644 >> index 000000000000..7017d8072ea9 >> --- /dev/null >> +++ b/test/py/tests/test_sf.py >> @@ -0,0 +1,233 @@ >> +# Copyright (c) 2017, Xiphos Systems Corp. All rights reserved. >> +# >> +# SPDX-License-Identifier: GPL-2.0 >> + >> +import re >> +import pytest >> +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: >> + >> +# Boolean indicating whether the SF tests should be skipped. >> +env__sf_skip = False >> + >> +# A list of sections of flash memory to be tested. >> +env__sf_configs = ( >> + { >> + # Optional, [[bus:]cs] argument used in `sf probe` >> + 'id': "0", >> + # Where in SPI flash should the test operate. >> + 'offset': 0x00000000, >> + # 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, >> + # Specifies if the test can write to offset >> + 'writeable': False, >> + }, >> +) >> +""" >> + >> + >> +def sf_prepare(u_boot_console, env__sf_config, verbose=False): >> + """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: >> + Nothing. >> + """ >> + >> + if u_boot_console.config.env.get('env__sf_skip', True): >> + pytest.skip('sf test disabled in environment') >> + >> + # NOTE: sf read at address 0 fails because map_physmem 'converts' it >> + # address 0 to a pointer. >> + ram_address = u_boot_utils.find_ram_base(u_boot_console) + 0x10 > > Maybe you can add it to sf_configs too. I'm not sure what it would add to the test but I could make the configuration optional and default to find_ram_base(). > > >> + >> + probe_id = env__sf_config.get('id', '') >> + output = u_boot_console.run_command('sf probe ' + probe_id) >> + if 'SF: Detected' not in output: >> + pytest.fail('No flash device available') >> + >> + m = re.search('page size (.+?) Bytes', output) >> + if m: >> + try: >> + page_size = int(m.group(1)) >> + except ValueError: >> + pytest.fail('SPI Flash page size not recognized') >> + >> + m = re.search('erase size (.+?) KiB', output) >> + if m: >> + try: >> + erase_size = int(m.group(1)) >> + except ValueError: >> + pytest.fail('SPI Flash erase size not recognized') >> + >> + erase_size *= 1024 >> + >> + m = re.search('total (.+?) MiB', output) >> + if m: >> + try: >> + total_size = int(m.group(1)) >> + except ValueError: >> + pytest.fail('SPI Flash total size not recognized') >> + >> + total_size *= 1024 * 1024 >> + >> + if verbose: >> + u_boot_console.log.info('Page size is: ' + str(page_size) + ' B') >> + u_boot_console.log.info('Erase size is: ' + str(erase_size) + ' B') >> + u_boot_console.log.info('Total size is: ' + str(total_size) + ' B') >> + >> + env__sf_config['len'] = env__sf_config.get('len', erase_size) >> + if env__sf_config['offset'] % erase_size or \ >> + env__sf_config['len'] % erase_size: >> + u_boot_console.log.warning("erase offset/length not multiple of " >> + "erase size") > > Is this only warning or you can destroy any data by that? It's a warning because `sf erase` will fail if this condition is true. It shouldn't destroy any data. I'm not really sure on how this should be handled because if this warning appears, the erase and update operations will fail. > > > >> + >> + env__sf_config['ram_address'] = ram_address >> + >> + >> +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 >> + """ >> + >> + output = u_boot_console.run_command('crc32 %08x %x' % (address, count)) >> + >> + m = re.search('==> ([0-9a-fA-F]{8})$', output) >> + if not m: >> + pytest.fail('CRC32 failed') >> + >> + return m.group(1) >> + >> + >> +def sf_read(u_boot_console, env__sf_config, size=None): >> + """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. >> + size: Optional, used to override env__sf_config value. >> + >> + Returns: >> + CRC32 value of SPI Flash section >> + """ >> + >> + if size is None: >> + size = env__sf_config['len'] >> + >> + u_boot_console.run_command('mw %08x 0 %x' % (env__sf_config['ram_address'], >> + size)) > > Any reason for this write? It should be rewritten by sf read below anyway. Yes, it is rewritten by `sf read`. I added it as an extra precaution to make sure that the data changes if we do two consecutive reads of the same address. > >> + >> + response = u_boot_console.run_command('sf read %08x %08x %x' % >> + (env__sf_config['ram_address'], >> + env__sf_config['offset'], >> + size)) >> + assert 'Read: OK' in response, "Read operation failed" >> + >> + return crc32(u_boot_console, env__sf_config['ram_address'], >> + env__sf_config['len']) >> + >> + >> +def sf_update(u_boot_console, env__sf_config): >> + """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 >> + """ >> + from time import time >> + >> + u_boot_console.run_command('mw %08x %08x %x' % >> + (env__sf_config['ram_address'], time(), >> + env__sf_config['len'])) > > ditto same here. > >> + crc_ram = crc32(u_boot_console, env__sf_config['ram_address'], >> + env__sf_config['len']) >> + u_boot_console.run_command('sf update %08x %08x %x' % >> + (env__sf_config['ram_address'], >> + env__sf_config['offset'], >> + env__sf_config['len'])) >> + >> + crc2 = sf_read(u_boot_console, env__sf_config) >> + >> + return (crc2 == crc_ram) >> + >> + >> +@pytest.mark.buildconfigspec("cmd_sf") >> +def test_sf_read(u_boot_console, env__sf_config): >> + sf_prepare(u_boot_console, env__sf_config) >> + >> + output = u_boot_console.run_command('sf read %08x %08x %x' % >> + (env__sf_config['ram_address'], >> + env__sf_config['offset'], >> + env__sf_config['len'])) >> + assert 'Read: OK' in output, "Read operation failed" >> + >> + >> +@pytest.mark.buildconfigspec("cmd_sf") >> +@pytest.mark.buildconfigspec("cmd_crc32") >> +@pytest.mark.buildconfigspec("cmd_memory") >> +def test_sf_read_twice(u_boot_console, env__sf_config): >> + sf_prepare(u_boot_console, env__sf_config) >> + >> + crc1 = sf_read(u_boot_console, env__sf_config) >> + crc2 = sf_read(u_boot_console, env__sf_config) > > The test is using the same memory. I tried to avoid that to make sure > that there is no issue with memory itself. Small offset should be enough. This is a reason why I added the extra `mw` above. Do you think it's not enough? > Also these sf read should be able to write bytes to non align addresses > to make sure that driver is handling that properly. s/write/read/ ? I guess I could add an extra test to read unaligned addresses. would that work? should the extra offset also be randomized? > > >> + >> + assert crc1 == crc2, "CRC32 of two successive read operation do not match" >> + >> + >> +@pytest.mark.buildconfigspec("cmd_sf") >> +@pytest.mark.buildconfigspec("cmd_crc32") >> +@pytest.mark.buildconfigspec("cmd_memory") >> +def test_sf_erase(u_boot_console, env__sf_config): >> + if not env__sf_config['writeable']: >> + pytest.skip('flash config is tagged as not writeable') >> + >> + sf_prepare(u_boot_console, env__sf_config) >> + output = u_boot_console.run_command('sf erase %08x %x' % >> + (env__sf_config['offset'], >> + env__sf_config['len'])) >> + assert 'Erased: OK' in output, "Erase operation failed" >> + >> + u_boot_console.run_command('mw %08x ffffffff %x' % >> + (env__sf_config['ram_address'], >> + env__sf_config['len'])) >> + crc1 = crc32(u_boot_console, env__sf_config['ram_address'], >> + env__sf_config['len']) >> + >> + crc2 = sf_read(u_boot_console, env__sf_config) >> + assert crc1 == crc2, "CRC32 of erase section does not match expected value" >> + >> + >> +@pytest.mark.buildconfigspec("cmd_sf") >> +@pytest.mark.buildconfigspec("cmd_memory") >> +def test_sf_update(u_boot_console, env__sf_config): >> + if not env__sf_config['writeable']: >> + pytest.skip('flash config is tagged as not writeable') >> + >> + sf_prepare(u_boot_console, env__sf_config) >> + assert sf_update(u_boot_console, env__sf_config) is True >> > > Also I wanted to design these tests that you will check bytes in front > of and behind region in DDR to make sure that your driver is writing > just amount of data you requested not aligned with words for example. It's a good point! In a follow up series, I was planning on doing this on the flash after write/erase operations to make sure we don't go beyond `offset + size`. > > Anyway good to see this patch. > > Thanks, > Michal > > Thanks for the feedback, Liam
On 1.3.2018 05:01, Liam Beguin wrote: > Hi Michal, > > On 27 February 2018 at 03:51, Michal Simek <michal.simek@xilinx.com> wrote: >> Hi Liam, >> >> On 27.2.2018 05:17, Liam Beguin wrote: >>> Add basic tests for the spi_flash subsystem. >> >> Good to see this patch. >> FYI: We have created qspi tests too but didn't push it out because of >> missing sf_configs options as you have below. We are testing the whole >> flash all the time. > > I tried to write this based on your test plus the feedback from when you > sent it on the mailing list. > >> Test was designed to use more randomization to make sure that every run >> is working with different data. We found several issues thanks to this. >> (Also keep in your mind that some tests depends on order which is wrong >> but we didn't have time to clean them yet) >> IIRC I have sent that patch to mainline in past and using sf_configs was >> suggested too. >> >> https://github.com/Xilinx/u-boot-xlnx/blob/master/test/py/tests/test_qspi.py >> > > Looking at the test, I see you randomize the spi frequency and the R/W size. > I could probably add these as env configs with default values. > >>> >>> Signed-off-by: Liam Beguin <liambeguin@gmail.com> >>> --- >>> test/py/tests/test_sf.py | 233 +++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 233 insertions(+) >>> >>> diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py >>> new file mode 100644 >>> index 000000000000..7017d8072ea9 >>> --- /dev/null >>> +++ b/test/py/tests/test_sf.py >>> @@ -0,0 +1,233 @@ >>> +# Copyright (c) 2017, Xiphos Systems Corp. All rights reserved. >>> +# >>> +# SPDX-License-Identifier: GPL-2.0 >>> + >>> +import re >>> +import pytest >>> +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: >>> + >>> +# Boolean indicating whether the SF tests should be skipped. >>> +env__sf_skip = False >>> + >>> +# A list of sections of flash memory to be tested. >>> +env__sf_configs = ( >>> + { >>> + # Optional, [[bus:]cs] argument used in `sf probe` >>> + 'id': "0", >>> + # Where in SPI flash should the test operate. >>> + 'offset': 0x00000000, >>> + # 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, >>> + # Specifies if the test can write to offset >>> + 'writeable': False, >>> + }, >>> +) >>> +""" >>> + >>> + >>> +def sf_prepare(u_boot_console, env__sf_config, verbose=False): >>> + """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: >>> + Nothing. >>> + """ >>> + >>> + if u_boot_console.config.env.get('env__sf_skip', True): >>> + pytest.skip('sf test disabled in environment') >>> + >>> + # NOTE: sf read at address 0 fails because map_physmem 'converts' it >>> + # address 0 to a pointer. >>> + ram_address = u_boot_utils.find_ram_base(u_boot_console) + 0x10 >> >> Maybe you can add it to sf_configs too. > > I'm not sure what it would add to the test but I could make the > configuration optional and default to find_ram_base(). > >> >> >>> + >>> + probe_id = env__sf_config.get('id', '') >>> + output = u_boot_console.run_command('sf probe ' + probe_id) >>> + if 'SF: Detected' not in output: >>> + pytest.fail('No flash device available') >>> + >>> + m = re.search('page size (.+?) Bytes', output) >>> + if m: >>> + try: >>> + page_size = int(m.group(1)) >>> + except ValueError: >>> + pytest.fail('SPI Flash page size not recognized') >>> + >>> + m = re.search('erase size (.+?) KiB', output) >>> + if m: >>> + try: >>> + erase_size = int(m.group(1)) >>> + except ValueError: >>> + pytest.fail('SPI Flash erase size not recognized') >>> + >>> + erase_size *= 1024 >>> + >>> + m = re.search('total (.+?) MiB', output) >>> + if m: >>> + try: >>> + total_size = int(m.group(1)) >>> + except ValueError: >>> + pytest.fail('SPI Flash total size not recognized') >>> + >>> + total_size *= 1024 * 1024 >>> + >>> + if verbose: >>> + u_boot_console.log.info('Page size is: ' + str(page_size) + ' B') >>> + u_boot_console.log.info('Erase size is: ' + str(erase_size) + ' B') >>> + u_boot_console.log.info('Total size is: ' + str(total_size) + ' B') >>> + >>> + env__sf_config['len'] = env__sf_config.get('len', erase_size) >>> + if env__sf_config['offset'] % erase_size or \ >>> + env__sf_config['len'] % erase_size: >>> + u_boot_console.log.warning("erase offset/length not multiple of " >>> + "erase size") >> >> Is this only warning or you can destroy any data by that? > > It's a warning because `sf erase` will fail if this condition is true. > It shouldn't destroy any data. I'm not really sure on how this should > be handled because if this warning appears, the erase and update > operations will fail. > >> >> >> >>> + >>> + env__sf_config['ram_address'] = ram_address >>> + >>> + >>> +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 >>> + """ >>> + >>> + output = u_boot_console.run_command('crc32 %08x %x' % (address, count)) >>> + >>> + m = re.search('==> ([0-9a-fA-F]{8})$', output) >>> + if not m: >>> + pytest.fail('CRC32 failed') >>> + >>> + return m.group(1) >>> + >>> + >>> +def sf_read(u_boot_console, env__sf_config, size=None): >>> + """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. >>> + size: Optional, used to override env__sf_config value. >>> + >>> + Returns: >>> + CRC32 value of SPI Flash section >>> + """ >>> + >>> + if size is None: >>> + size = env__sf_config['len'] >>> + >>> + u_boot_console.run_command('mw %08x 0 %x' % (env__sf_config['ram_address'], >>> + size)) >> >> Any reason for this write? It should be rewritten by sf read below anyway. > > Yes, it is rewritten by `sf read`. I added it as an extra precaution > to make sure > that the data changes if we do two consecutive reads of the same address. > >> >>> + >>> + response = u_boot_console.run_command('sf read %08x %08x %x' % >>> + (env__sf_config['ram_address'], >>> + env__sf_config['offset'], >>> + size)) >>> + assert 'Read: OK' in response, "Read operation failed" >>> + >>> + return crc32(u_boot_console, env__sf_config['ram_address'], >>> + env__sf_config['len']) >>> + >>> + >>> +def sf_update(u_boot_console, env__sf_config): >>> + """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 >>> + """ >>> + from time import time >>> + >>> + u_boot_console.run_command('mw %08x %08x %x' % >>> + (env__sf_config['ram_address'], time(), >>> + env__sf_config['len'])) >> >> ditto > > same here. > >> >>> + crc_ram = crc32(u_boot_console, env__sf_config['ram_address'], >>> + env__sf_config['len']) >>> + u_boot_console.run_command('sf update %08x %08x %x' % >>> + (env__sf_config['ram_address'], >>> + env__sf_config['offset'], >>> + env__sf_config['len'])) >>> + >>> + crc2 = sf_read(u_boot_console, env__sf_config) >>> + >>> + return (crc2 == crc_ram) >>> + >>> + >>> +@pytest.mark.buildconfigspec("cmd_sf") >>> +def test_sf_read(u_boot_console, env__sf_config): >>> + sf_prepare(u_boot_console, env__sf_config) >>> + >>> + output = u_boot_console.run_command('sf read %08x %08x %x' % >>> + (env__sf_config['ram_address'], >>> + env__sf_config['offset'], >>> + env__sf_config['len'])) >>> + assert 'Read: OK' in output, "Read operation failed" >>> + >>> + >>> +@pytest.mark.buildconfigspec("cmd_sf") >>> +@pytest.mark.buildconfigspec("cmd_crc32") >>> +@pytest.mark.buildconfigspec("cmd_memory") >>> +def test_sf_read_twice(u_boot_console, env__sf_config): >>> + sf_prepare(u_boot_console, env__sf_config) >>> + >>> + crc1 = sf_read(u_boot_console, env__sf_config) >>> + crc2 = sf_read(u_boot_console, env__sf_config) >> >> The test is using the same memory. I tried to avoid that to make sure >> that there is no issue with memory itself. Small offset should be enough. > > This is a reason why I added the extra `mw` above. > Do you think it's not enough? I am not working with broken HW now but on incorrectly setup DDR controller you can get systematic error for certain bits that's why if you simply shift the second read you should be to find these issues. > >> Also these sf read should be able to write bytes to non align addresses >> to make sure that driver is handling that properly. > > s/write/read/ ? right. > > I guess I could add an extra test to read unaligned addresses. would that > work? should the extra offset also be randomized? two tests are fine. We were talking internally about have more randomization in connection to data and test flow that's why we have changed that test to work with random values in that certain range. Also the part of that is randomized test order but I found that pytest-random-order does work properly. Also these randomized data/lengths were able to find out more issues than fixed one. I was playing with that randomized test order and I see that not all current tests are self contained and this will be good to run too. > >> >> >>> + >>> + assert crc1 == crc2, "CRC32 of two successive read operation do not match" >>> + >>> + >>> +@pytest.mark.buildconfigspec("cmd_sf") >>> +@pytest.mark.buildconfigspec("cmd_crc32") >>> +@pytest.mark.buildconfigspec("cmd_memory") >>> +def test_sf_erase(u_boot_console, env__sf_config): >>> + if not env__sf_config['writeable']: >>> + pytest.skip('flash config is tagged as not writeable') >>> + >>> + sf_prepare(u_boot_console, env__sf_config) >>> + output = u_boot_console.run_command('sf erase %08x %x' % >>> + (env__sf_config['offset'], >>> + env__sf_config['len'])) >>> + assert 'Erased: OK' in output, "Erase operation failed" >>> + >>> + u_boot_console.run_command('mw %08x ffffffff %x' % >>> + (env__sf_config['ram_address'], >>> + env__sf_config['len'])) >>> + crc1 = crc32(u_boot_console, env__sf_config['ram_address'], >>> + env__sf_config['len']) >>> + >>> + crc2 = sf_read(u_boot_console, env__sf_config) >>> + assert crc1 == crc2, "CRC32 of erase section does not match expected value" >>> + >>> + >>> +@pytest.mark.buildconfigspec("cmd_sf") >>> +@pytest.mark.buildconfigspec("cmd_memory") >>> +def test_sf_update(u_boot_console, env__sf_config): >>> + if not env__sf_config['writeable']: >>> + pytest.skip('flash config is tagged as not writeable') >>> + >>> + sf_prepare(u_boot_console, env__sf_config) >>> + assert sf_update(u_boot_console, env__sf_config) is True >>> >> >> Also I wanted to design these tests that you will check bytes in front >> of and behind region in DDR to make sure that your driver is writing >> just amount of data you requested not aligned with words for example. > > It's a good point! In a follow up series, I was planning on doing this on > the flash after write/erase operations to make sure we don't go > beyond `offset + size`. Ok good. Thanks, Michal
On 1 March 2018 at 01:59, Michal Simek <michal.simek@xilinx.com> wrote: > On 1.3.2018 05:01, Liam Beguin wrote: >> Hi Michal, >> >> On 27 February 2018 at 03:51, Michal Simek <michal.simek@xilinx.com> wrote: >>> Hi Liam, >>> >>> On 27.2.2018 05:17, Liam Beguin wrote: >>>> Add basic tests for the spi_flash subsystem. >>> >>> Good to see this patch. >>> FYI: We have created qspi tests too but didn't push it out because of >>> missing sf_configs options as you have below. We are testing the whole >>> flash all the time. >> >> I tried to write this based on your test plus the feedback from when you >> sent it on the mailing list. >> >>> Test was designed to use more randomization to make sure that every run >>> is working with different data. We found several issues thanks to this. >>> (Also keep in your mind that some tests depends on order which is wrong >>> but we didn't have time to clean them yet) >>> IIRC I have sent that patch to mainline in past and using sf_configs was >>> suggested too. >>> >>> https://github.com/Xilinx/u-boot-xlnx/blob/master/test/py/tests/test_qspi.py >>> >> >> Looking at the test, I see you randomize the spi frequency and the R/W size. >> I could probably add these as env configs with default values. >> >>>> >>>> Signed-off-by: Liam Beguin <liambeguin@gmail.com> >>>> --- >>>> test/py/tests/test_sf.py | 233 +++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 233 insertions(+) >>>> >>>> diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py >>>> new file mode 100644 >>>> index 000000000000..7017d8072ea9 >>>> --- /dev/null >>>> +++ b/test/py/tests/test_sf.py >>>> @@ -0,0 +1,233 @@ >>>> +# Copyright (c) 2017, Xiphos Systems Corp. All rights reserved. >>>> +# >>>> +# SPDX-License-Identifier: GPL-2.0 >>>> + >>>> +import re >>>> +import pytest >>>> +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: >>>> + >>>> +# Boolean indicating whether the SF tests should be skipped. >>>> +env__sf_skip = False >>>> + >>>> +# A list of sections of flash memory to be tested. >>>> +env__sf_configs = ( >>>> + { >>>> + # Optional, [[bus:]cs] argument used in `sf probe` >>>> + 'id': "0", >>>> + # Where in SPI flash should the test operate. >>>> + 'offset': 0x00000000, >>>> + # 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, >>>> + # Specifies if the test can write to offset >>>> + 'writeable': False, >>>> + }, >>>> +) >>>> +""" >>>> + >>>> + >>>> +def sf_prepare(u_boot_console, env__sf_config, verbose=False): >>>> + """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: >>>> + Nothing. >>>> + """ >>>> + >>>> + if u_boot_console.config.env.get('env__sf_skip', True): >>>> + pytest.skip('sf test disabled in environment') >>>> + >>>> + # NOTE: sf read at address 0 fails because map_physmem 'converts' it >>>> + # address 0 to a pointer. >>>> + ram_address = u_boot_utils.find_ram_base(u_boot_console) + 0x10 >>> >>> Maybe you can add it to sf_configs too. >> >> I'm not sure what it would add to the test but I could make the >> configuration optional and default to find_ram_base(). >> >>> >>> >>>> + >>>> + probe_id = env__sf_config.get('id', '') >>>> + output = u_boot_console.run_command('sf probe ' + probe_id) >>>> + if 'SF: Detected' not in output: >>>> + pytest.fail('No flash device available') >>>> + >>>> + m = re.search('page size (.+?) Bytes', output) >>>> + if m: >>>> + try: >>>> + page_size = int(m.group(1)) >>>> + except ValueError: >>>> + pytest.fail('SPI Flash page size not recognized') >>>> + >>>> + m = re.search('erase size (.+?) KiB', output) >>>> + if m: >>>> + try: >>>> + erase_size = int(m.group(1)) >>>> + except ValueError: >>>> + pytest.fail('SPI Flash erase size not recognized') >>>> + >>>> + erase_size *= 1024 >>>> + >>>> + m = re.search('total (.+?) MiB', output) >>>> + if m: >>>> + try: >>>> + total_size = int(m.group(1)) >>>> + except ValueError: >>>> + pytest.fail('SPI Flash total size not recognized') >>>> + >>>> + total_size *= 1024 * 1024 >>>> + >>>> + if verbose: >>>> + u_boot_console.log.info('Page size is: ' + str(page_size) + ' B') >>>> + u_boot_console.log.info('Erase size is: ' + str(erase_size) + ' B') >>>> + u_boot_console.log.info('Total size is: ' + str(total_size) + ' B') >>>> + >>>> + env__sf_config['len'] = env__sf_config.get('len', erase_size) >>>> + if env__sf_config['offset'] % erase_size or \ >>>> + env__sf_config['len'] % erase_size: >>>> + u_boot_console.log.warning("erase offset/length not multiple of " >>>> + "erase size") >>> >>> Is this only warning or you can destroy any data by that? >> >> It's a warning because `sf erase` will fail if this condition is true. >> It shouldn't destroy any data. I'm not really sure on how this should >> be handled because if this warning appears, the erase and update >> operations will fail. >> >>> >>> >>> >>>> + >>>> + env__sf_config['ram_address'] = ram_address >>>> + >>>> + >>>> +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 >>>> + """ >>>> + >>>> + output = u_boot_console.run_command('crc32 %08x %x' % (address, count)) >>>> + >>>> + m = re.search('==> ([0-9a-fA-F]{8})$', output) >>>> + if not m: >>>> + pytest.fail('CRC32 failed') >>>> + >>>> + return m.group(1) >>>> + >>>> + >>>> +def sf_read(u_boot_console, env__sf_config, size=None): >>>> + """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. >>>> + size: Optional, used to override env__sf_config value. >>>> + >>>> + Returns: >>>> + CRC32 value of SPI Flash section >>>> + """ >>>> + >>>> + if size is None: >>>> + size = env__sf_config['len'] >>>> + >>>> + u_boot_console.run_command('mw %08x 0 %x' % (env__sf_config['ram_address'], >>>> + size)) >>> >>> Any reason for this write? It should be rewritten by sf read below anyway. >> >> Yes, it is rewritten by `sf read`. I added it as an extra precaution >> to make sure >> that the data changes if we do two consecutive reads of the same address. >> >>> >>>> + >>>> + response = u_boot_console.run_command('sf read %08x %08x %x' % >>>> + (env__sf_config['ram_address'], >>>> + env__sf_config['offset'], >>>> + size)) >>>> + assert 'Read: OK' in response, "Read operation failed" >>>> + >>>> + return crc32(u_boot_console, env__sf_config['ram_address'], >>>> + env__sf_config['len']) >>>> + >>>> + >>>> +def sf_update(u_boot_console, env__sf_config): >>>> + """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 >>>> + """ >>>> + from time import time >>>> + >>>> + u_boot_console.run_command('mw %08x %08x %x' % >>>> + (env__sf_config['ram_address'], time(), >>>> + env__sf_config['len'])) >>> >>> ditto >> >> same here. >> >>> >>>> + crc_ram = crc32(u_boot_console, env__sf_config['ram_address'], >>>> + env__sf_config['len']) >>>> + u_boot_console.run_command('sf update %08x %08x %x' % >>>> + (env__sf_config['ram_address'], >>>> + env__sf_config['offset'], >>>> + env__sf_config['len'])) >>>> + >>>> + crc2 = sf_read(u_boot_console, env__sf_config) >>>> + >>>> + return (crc2 == crc_ram) >>>> + >>>> + >>>> +@pytest.mark.buildconfigspec("cmd_sf") >>>> +def test_sf_read(u_boot_console, env__sf_config): >>>> + sf_prepare(u_boot_console, env__sf_config) >>>> + >>>> + output = u_boot_console.run_command('sf read %08x %08x %x' % >>>> + (env__sf_config['ram_address'], >>>> + env__sf_config['offset'], >>>> + env__sf_config['len'])) >>>> + assert 'Read: OK' in output, "Read operation failed" >>>> + >>>> + >>>> +@pytest.mark.buildconfigspec("cmd_sf") >>>> +@pytest.mark.buildconfigspec("cmd_crc32") >>>> +@pytest.mark.buildconfigspec("cmd_memory") >>>> +def test_sf_read_twice(u_boot_console, env__sf_config): >>>> + sf_prepare(u_boot_console, env__sf_config) >>>> + >>>> + crc1 = sf_read(u_boot_console, env__sf_config) >>>> + crc2 = sf_read(u_boot_console, env__sf_config) >>> >>> The test is using the same memory. I tried to avoid that to make sure >>> that there is no issue with memory itself. Small offset should be enough. >> >> This is a reason why I added the extra `mw` above. >> Do you think it's not enough? > > I am not working with broken HW now but on incorrectly setup DDR > controller you can get systematic error for certain bits that's why if > you simply shift the second read you should be to find these issues. > I understand what you mean here but I'm wondering if this shouldn't be added to test_md.py instead since it's more about testing the DDR. >> >>> Also these sf read should be able to write bytes to non align addresses >>> to make sure that driver is handling that properly. >> >> s/write/read/ ? > > right. > >> >> I guess I could add an extra test to read unaligned addresses. would that >> work? should the extra offset also be randomized? > > two tests are fine. We were talking internally about have more > randomization in connection to data and test flow that's why we have > changed that test to work with random values in that certain range. > Also the part of that is randomized test order but I found that > pytest-random-order does work properly. I'll make sure test_sf.py runs with pytest-random-order enabled. > > Also these randomized data/lengths were able to find out more issues > than fixed one. I was playing with that randomized test order and I see > that not all current tests are self contained and this will be good to > run too. > I'll add options to randomize in v2. >> >>> >>> >>>> + >>>> + assert crc1 == crc2, "CRC32 of two successive read operation do not match" >>>> + >>>> + >>>> +@pytest.mark.buildconfigspec("cmd_sf") >>>> +@pytest.mark.buildconfigspec("cmd_crc32") >>>> +@pytest.mark.buildconfigspec("cmd_memory") >>>> +def test_sf_erase(u_boot_console, env__sf_config): >>>> + if not env__sf_config['writeable']: >>>> + pytest.skip('flash config is tagged as not writeable') >>>> + >>>> + sf_prepare(u_boot_console, env__sf_config) >>>> + output = u_boot_console.run_command('sf erase %08x %x' % >>>> + (env__sf_config['offset'], >>>> + env__sf_config['len'])) >>>> + assert 'Erased: OK' in output, "Erase operation failed" >>>> + >>>> + u_boot_console.run_command('mw %08x ffffffff %x' % >>>> + (env__sf_config['ram_address'], >>>> + env__sf_config['len'])) >>>> + crc1 = crc32(u_boot_console, env__sf_config['ram_address'], >>>> + env__sf_config['len']) >>>> + >>>> + crc2 = sf_read(u_boot_console, env__sf_config) >>>> + assert crc1 == crc2, "CRC32 of erase section does not match expected value" >>>> + >>>> + >>>> +@pytest.mark.buildconfigspec("cmd_sf") >>>> +@pytest.mark.buildconfigspec("cmd_memory") >>>> +def test_sf_update(u_boot_console, env__sf_config): >>>> + if not env__sf_config['writeable']: >>>> + pytest.skip('flash config is tagged as not writeable') >>>> + >>>> + sf_prepare(u_boot_console, env__sf_config) >>>> + assert sf_update(u_boot_console, env__sf_config) is True >>>> >>> >>> Also I wanted to design these tests that you will check bytes in front >>> of and behind region in DDR to make sure that your driver is writing >>> just amount of data you requested not aligned with words for example. >> >> It's a good point! In a follow up series, I was planning on doing this on >> the flash after write/erase operations to make sure we don't go >> beyond `offset + size`. > > Ok good. > > Thanks, > Michal Thanks, Liam
On 1.3.2018 12:48, Liam Beguin wrote: > On 1 March 2018 at 01:59, Michal Simek <michal.simek@xilinx.com> wrote: >> On 1.3.2018 05:01, Liam Beguin wrote: >>> Hi Michal, >>> >>> On 27 February 2018 at 03:51, Michal Simek <michal.simek@xilinx.com> wrote: >>>> Hi Liam, >>>> >>>> On 27.2.2018 05:17, Liam Beguin wrote: >>>>> Add basic tests for the spi_flash subsystem. >>>> >>>> Good to see this patch. >>>> FYI: We have created qspi tests too but didn't push it out because of >>>> missing sf_configs options as you have below. We are testing the whole >>>> flash all the time. >>> >>> I tried to write this based on your test plus the feedback from when you >>> sent it on the mailing list. >>> >>>> Test was designed to use more randomization to make sure that every run >>>> is working with different data. We found several issues thanks to this. >>>> (Also keep in your mind that some tests depends on order which is wrong >>>> but we didn't have time to clean them yet) >>>> IIRC I have sent that patch to mainline in past and using sf_configs was >>>> suggested too. >>>> >>>> https://github.com/Xilinx/u-boot-xlnx/blob/master/test/py/tests/test_qspi.py >>>> >>> >>> Looking at the test, I see you randomize the spi frequency and the R/W size. >>> I could probably add these as env configs with default values. >>> >>>>> >>>>> Signed-off-by: Liam Beguin <liambeguin@gmail.com> >>>>> --- >>>>> test/py/tests/test_sf.py | 233 +++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 233 insertions(+) >>>>> >>>>> diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py >>>>> new file mode 100644 >>>>> index 000000000000..7017d8072ea9 >>>>> --- /dev/null >>>>> +++ b/test/py/tests/test_sf.py >>>>> @@ -0,0 +1,233 @@ >>>>> +# Copyright (c) 2017, Xiphos Systems Corp. All rights reserved. >>>>> +# >>>>> +# SPDX-License-Identifier: GPL-2.0 >>>>> + >>>>> +import re >>>>> +import pytest >>>>> +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: >>>>> + >>>>> +# Boolean indicating whether the SF tests should be skipped. >>>>> +env__sf_skip = False >>>>> + >>>>> +# A list of sections of flash memory to be tested. >>>>> +env__sf_configs = ( >>>>> + { >>>>> + # Optional, [[bus:]cs] argument used in `sf probe` >>>>> + 'id': "0", >>>>> + # Where in SPI flash should the test operate. >>>>> + 'offset': 0x00000000, >>>>> + # 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, >>>>> + # Specifies if the test can write to offset >>>>> + 'writeable': False, >>>>> + }, >>>>> +) >>>>> +""" >>>>> + >>>>> + >>>>> +def sf_prepare(u_boot_console, env__sf_config, verbose=False): >>>>> + """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: >>>>> + Nothing. >>>>> + """ >>>>> + >>>>> + if u_boot_console.config.env.get('env__sf_skip', True): >>>>> + pytest.skip('sf test disabled in environment') >>>>> + >>>>> + # NOTE: sf read at address 0 fails because map_physmem 'converts' it >>>>> + # address 0 to a pointer. >>>>> + ram_address = u_boot_utils.find_ram_base(u_boot_console) + 0x10 >>>> >>>> Maybe you can add it to sf_configs too. >>> >>> I'm not sure what it would add to the test but I could make the >>> configuration optional and default to find_ram_base(). >>> >>>> >>>> >>>>> + >>>>> + probe_id = env__sf_config.get('id', '') >>>>> + output = u_boot_console.run_command('sf probe ' + probe_id) >>>>> + if 'SF: Detected' not in output: >>>>> + pytest.fail('No flash device available') >>>>> + >>>>> + m = re.search('page size (.+?) Bytes', output) >>>>> + if m: >>>>> + try: >>>>> + page_size = int(m.group(1)) >>>>> + except ValueError: >>>>> + pytest.fail('SPI Flash page size not recognized') >>>>> + >>>>> + m = re.search('erase size (.+?) KiB', output) >>>>> + if m: >>>>> + try: >>>>> + erase_size = int(m.group(1)) >>>>> + except ValueError: >>>>> + pytest.fail('SPI Flash erase size not recognized') >>>>> + >>>>> + erase_size *= 1024 >>>>> + >>>>> + m = re.search('total (.+?) MiB', output) >>>>> + if m: >>>>> + try: >>>>> + total_size = int(m.group(1)) >>>>> + except ValueError: >>>>> + pytest.fail('SPI Flash total size not recognized') >>>>> + >>>>> + total_size *= 1024 * 1024 >>>>> + >>>>> + if verbose: >>>>> + u_boot_console.log.info('Page size is: ' + str(page_size) + ' B') >>>>> + u_boot_console.log.info('Erase size is: ' + str(erase_size) + ' B') >>>>> + u_boot_console.log.info('Total size is: ' + str(total_size) + ' B') >>>>> + >>>>> + env__sf_config['len'] = env__sf_config.get('len', erase_size) >>>>> + if env__sf_config['offset'] % erase_size or \ >>>>> + env__sf_config['len'] % erase_size: >>>>> + u_boot_console.log.warning("erase offset/length not multiple of " >>>>> + "erase size") >>>> >>>> Is this only warning or you can destroy any data by that? >>> >>> It's a warning because `sf erase` will fail if this condition is true. >>> It shouldn't destroy any data. I'm not really sure on how this should >>> be handled because if this warning appears, the erase and update >>> operations will fail. >>> >>>> >>>> >>>> >>>>> + >>>>> + env__sf_config['ram_address'] = ram_address >>>>> + >>>>> + >>>>> +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 >>>>> + """ >>>>> + >>>>> + output = u_boot_console.run_command('crc32 %08x %x' % (address, count)) >>>>> + >>>>> + m = re.search('==> ([0-9a-fA-F]{8})$', output) >>>>> + if not m: >>>>> + pytest.fail('CRC32 failed') >>>>> + >>>>> + return m.group(1) >>>>> + >>>>> + >>>>> +def sf_read(u_boot_console, env__sf_config, size=None): >>>>> + """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. >>>>> + size: Optional, used to override env__sf_config value. >>>>> + >>>>> + Returns: >>>>> + CRC32 value of SPI Flash section >>>>> + """ >>>>> + >>>>> + if size is None: >>>>> + size = env__sf_config['len'] >>>>> + >>>>> + u_boot_console.run_command('mw %08x 0 %x' % (env__sf_config['ram_address'], >>>>> + size)) >>>> >>>> Any reason for this write? It should be rewritten by sf read below anyway. >>> >>> Yes, it is rewritten by `sf read`. I added it as an extra precaution >>> to make sure >>> that the data changes if we do two consecutive reads of the same address. >>> >>>> >>>>> + >>>>> + response = u_boot_console.run_command('sf read %08x %08x %x' % >>>>> + (env__sf_config['ram_address'], >>>>> + env__sf_config['offset'], >>>>> + size)) >>>>> + assert 'Read: OK' in response, "Read operation failed" >>>>> + >>>>> + return crc32(u_boot_console, env__sf_config['ram_address'], >>>>> + env__sf_config['len']) >>>>> + >>>>> + >>>>> +def sf_update(u_boot_console, env__sf_config): >>>>> + """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 >>>>> + """ >>>>> + from time import time >>>>> + >>>>> + u_boot_console.run_command('mw %08x %08x %x' % >>>>> + (env__sf_config['ram_address'], time(), >>>>> + env__sf_config['len'])) >>>> >>>> ditto >>> >>> same here. >>> >>>> >>>>> + crc_ram = crc32(u_boot_console, env__sf_config['ram_address'], >>>>> + env__sf_config['len']) >>>>> + u_boot_console.run_command('sf update %08x %08x %x' % >>>>> + (env__sf_config['ram_address'], >>>>> + env__sf_config['offset'], >>>>> + env__sf_config['len'])) >>>>> + >>>>> + crc2 = sf_read(u_boot_console, env__sf_config) >>>>> + >>>>> + return (crc2 == crc_ram) >>>>> + >>>>> + >>>>> +@pytest.mark.buildconfigspec("cmd_sf") >>>>> +def test_sf_read(u_boot_console, env__sf_config): >>>>> + sf_prepare(u_boot_console, env__sf_config) >>>>> + >>>>> + output = u_boot_console.run_command('sf read %08x %08x %x' % >>>>> + (env__sf_config['ram_address'], >>>>> + env__sf_config['offset'], >>>>> + env__sf_config['len'])) >>>>> + assert 'Read: OK' in output, "Read operation failed" >>>>> + >>>>> + >>>>> +@pytest.mark.buildconfigspec("cmd_sf") >>>>> +@pytest.mark.buildconfigspec("cmd_crc32") >>>>> +@pytest.mark.buildconfigspec("cmd_memory") >>>>> +def test_sf_read_twice(u_boot_console, env__sf_config): >>>>> + sf_prepare(u_boot_console, env__sf_config) >>>>> + >>>>> + crc1 = sf_read(u_boot_console, env__sf_config) >>>>> + crc2 = sf_read(u_boot_console, env__sf_config) >>>> >>>> The test is using the same memory. I tried to avoid that to make sure >>>> that there is no issue with memory itself. Small offset should be enough. >>> >>> This is a reason why I added the extra `mw` above. >>> Do you think it's not enough? >> >> I am not working with broken HW now but on incorrectly setup DDR >> controller you can get systematic error for certain bits that's why if >> you simply shift the second read you should be to find these issues. >> > > I understand what you mean here but I'm wondering if this shouldn't be > added to test_md.py instead since it's more about testing the DDR. crc test should be there as it is the part of cmd/mem.c. On the other hand none is saying that the same DDR configuration is shared for the whole DDR in the system. On our devices you can have one DDR setup to hard code and the second for soft core that's why I would suggest to never use the same offsets for repeated read. Also in connection to address randomization will be also good to cover cases above 4GB address space. It means for systems with multiple memory banks test "read" to every bank. If this is continues then cross that boundary with data. But this could be done as follow up patch. Thanks, Michal
On 03/01/2018 04:48 AM, Liam Beguin wrote: > On 1 March 2018 at 01:59, Michal Simek <michal.simek@xilinx.com> wrote: ... >> Also these randomized data/lengths were able to find out more issues >> than fixed one. I was playing with that randomized test order and I see >> that not all current tests are self contained and this will be good to >> run too. > > I'll add options to randomize in v2. I'd strongly prefer the tests to be deterministic and not do random different actions each time they're run. That way, any failures are much easier to track down. Writing randomly generated data is fine, but not random access patterns or sets of operations. Although I suppose you could implement two sets of tests; one that does a static set of operations driven by a table in the board config file, and one that does a more randomized set of operations controlled by limits set by a table in the board config file.
On 02/26/2018 09:17 PM, Liam Beguin wrote: > Add basic tests for the spi_flash subsystem. > diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py > +import re > +import pytest > +import u_boot_utils > + > + Nit: Double blank line. The same issue exists in many other places too. > +""" > +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: > + > +# Boolean indicating whether the SF tests should be skipped. > +env__sf_skip = False Why do we need this variable? Let's just have the test rely upon whether env__sf_configs is present or not. If your test function takes env__sf_config as an argument, it'll automatically be skipped by the core test code if env__sf_configs isn't defined, or is defined by empty. > +# A list of sections of flash memory to be tested. > +env__sf_configs = ( > + { > + # Optional, [[bus:]cs] argument used in `sf probe` > + 'id': "0", What is the default value if this isn't specified? I assume "0", but that would be good to document. > + # 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, Is len in u8s or u32s? I would assume bytes. len is used inconsistently below; as a parameter to "sf read" where it's interpreted as bytes, and as a parameter to "mw" where it's interpreted as words IIRC. > +def sf_prepare(u_boot_console, env__sf_config, verbose=False): > + # NOTE: sf read at address 0 fails because map_physmem 'converts' it > + # address 0 to a pointer. > + ram_address = u_boot_utils.find_ram_base(u_boot_console) + 0x10 Can we fix that bug instead? Other tests don't need that workaround, so I'm not sure why the SPI test does. > + probe_id = env__sf_config.get('id', '') > + output = u_boot_console.run_command('sf probe ' + probe_id) > + if 'SF: Detected' not in output: > + pytest.fail('No flash device available') assert 'SF: Detected' in output ? > + m = re.search('page size (.+?) Bytes', output) > + if m: > + try: > + page_size = int(m.group(1)) > + except ValueError: > + pytest.fail('SPI Flash page size not recognized') What if not m? Currently the code leaves page_size uninitialized. Shouldn't the logic be: m = re.search ... assert m try: ... > + if verbose: > + u_boot_console.log.info('Page size is: ' + str(page_size) + ' B') Shorter might be: u_boot_console.log.info('Page size is: %dB' % page_size) > + env__sf_config['len'] = env__sf_config.get('len', erase_size) Over-writing len changes the value in the original structure which could affect tests that want to handle missing/inconsistent data in other ways. I'd rather store the data using a different key that the user will never set, or even better store derived/calculated data in a different dictionary altogether. > + if env__sf_config['offset'] % erase_size or \ > + env__sf_config['len'] % erase_size: > + u_boot_console.log.warning("erase offset/length not multiple of " > + "erase size") I'd assert that here, rather than just warning. > + env__sf_config['ram_address'] = ram_address ram_address isn't user-supplied configuration, and doesn't vary from test-to-test. Let's either return it, or write it to a global, or just call find_ram_base() whenever the value is required. > +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 > + """ > + > + output = u_boot_console.run_command('crc32 %08x %x' % (address, count)) > + > + m = re.search('==> ([0-9a-fA-F]{8})$', output) > + if not m: > + pytest.fail('CRC32 failed') > + > + return m.group(1) Should we put this into u_boot_utils.py? I was going to assert that other tests must already do this, but I guess other tests check for whether a known-ahead-of-time CRC value is on the crc32 command output, rather than parsing out the actual value. So, maybe this doesn't need to be a utility function. > +def sf_read(u_boot_console, env__sf_config, size=None): Is size ever used? If not, let's delete it. > + u_boot_console.run_command('mw %08x 0 %x' % (env__sf_config['ram_address'], > + size)) mw.b instead? Same in sf_update(). This function could benefit from doing: a) Fill target memory with known pattern. b) sf read. c) Check that target memory doesn't contain the known pattern. > +def sf_update(u_boot_console, env__sf_config): > + from time import time That's not needed. It should be at the top of the file if it was. > + u_boot_console.run_command('mw %08x %08x %x' % > + (env__sf_config['ram_address'], time(), > + env__sf_config['len'])) > + crc_ram = crc32(u_boot_console, env__sf_config['ram_address'], > + env__sf_config['len']) > + u_boot_console.run_command('sf update %08x %08x %x' % > + (env__sf_config['ram_address'], > + env__sf_config['offset'], > + env__sf_config['len'])) Here the RAM should be cleared, although I guess that can happen inside sf_read(). > + crc2 = sf_read(u_boot_console, env__sf_config) > + > + return (crc2 == crc_ram) > +@pytest.mark.buildconfigspec("cmd_sf") > +def test_sf_read(u_boot_console, env__sf_config): > + sf_prepare(u_boot_console, env__sf_config) > + > + output = u_boot_console.run_command('sf read %08x %08x %x' % > + (env__sf_config['ram_address'], > + env__sf_config['offset'], > + env__sf_config['len'])) > + assert 'Read: OK' in output, "Read operation failed" This doesn't verify that the command actually read anything, since it doesn't look for changes in the RAM. Let's "mw.b ram_address offset len" first and check that the CRC changes before/after the read to ensure that some bytes were actually transferred. If you take a look at the test_mmc_rd patch I sent recently, you can see how to do that. It'd be nice to structure the two tests as similarly as possible since they're doing roughly the same thing. Also, it'd be good if env__sf_configs specified a known CRC for data that should be in the SPI flash, so we can verify not only that /some/ data was read, but that the /correct/ data was read. Why doesn't this function use the sf_read helper function like test_sf_read_twice() does? > +@pytest.mark.buildconfigspec("cmd_sf") > +@pytest.mark.buildconfigspec("cmd_crc32") > +@pytest.mark.buildconfigspec("cmd_memory") > +def test_sf_read_twice(u_boot_console, env__sf_config): > + sf_prepare(u_boot_console, env__sf_config) > + > + crc1 = sf_read(u_boot_console, env__sf_config) > + crc2 = sf_read(u_boot_console, env__sf_config) > + > + assert crc1 == crc2, "CRC32 of two successive read operation do not match" Similarly here, the test will pass if neither commands reads anything, or if only the first does, or if they both read the wrong thing. > +def test_sf_erase(u_boot_console, env__sf_config): ... > + u_boot_console.run_command('mw %08x ffffffff %x' % > + (env__sf_config['ram_address'], > + env__sf_config['len'])) > + crc1 = crc32(u_boot_console, env__sf_config['ram_address'], > + env__sf_config['len']) It'd be nice to name that crc_ffs or something like that ... > + > + crc2 = sf_read(u_boot_console, env__sf_config) ... and name that crc_read or crc_readback, just to give a quick/easy idea of what each variable holds.
On 1 March 2018 at 16:56, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 02/26/2018 09:17 PM, Liam Beguin wrote: >> >> Add basic tests for the spi_flash subsystem. > > >> diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py > > >> +import re >> +import pytest >> +import u_boot_utils >> + >> + > > > Nit: Double blank line. The same issue exists in many other places too. This is because I use flake8 and it was complaining if I didn't. Apparently PEP8 suggests you surround top level functions with 2 blank lines. I really have no preference. > >> +""" >> +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: >> + >> +# Boolean indicating whether the SF tests should be skipped. >> +env__sf_skip = False > > > Why do we need this variable? Let's just have the test rely upon whether > env__sf_configs is present or not. If your test function takes > env__sf_config as an argument, it'll automatically be skipped by the core > test code if env__sf_configs isn't defined, or is defined by empty. Will do. > >> +# A list of sections of flash memory to be tested. >> +env__sf_configs = ( >> + { >> + # Optional, [[bus:]cs] argument used in `sf probe` >> + 'id': "0", > > > What is the default value if this isn't specified? I assume "0", but that > would be good to document. The default value is an empty string. I'll change it to 0 so I can test different SPI frequencies. > >> + # 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, > > > Is len in u8s or u32s? I would assume bytes. len is used inconsistently > below; as a parameter to "sf read" where it's interpreted as bytes, and as a > parameter to "mw" where it's interpreted as words IIRC. Thanks for pointing this out! I'll update call to `mw` to use bytes instead of words. > >> +def sf_prepare(u_boot_console, env__sf_config, verbose=False): > > >> + # NOTE: sf read at address 0 fails because map_physmem 'converts' it >> + # address 0 to a pointer. >> + ram_address = u_boot_utils.find_ram_base(u_boot_console) + 0x10 > > > Can we fix that bug instead? Other tests don't need that workaround, so I'm > not sure why the SPI test does. Good point. Let me see what I can do. > >> + probe_id = env__sf_config.get('id', '') >> + output = u_boot_console.run_command('sf probe ' + probe_id) >> + if 'SF: Detected' not in output: >> + pytest.fail('No flash device available') > > > assert 'SF: Detected' in output ? Yes, I'll also replace all other `pytest.fail` with asserts. > >> + m = re.search('page size (.+?) Bytes', output) >> + if m: >> + try: >> + page_size = int(m.group(1)) >> + except ValueError: >> + pytest.fail('SPI Flash page size not recognized') > > > What if not m? Currently the code leaves page_size uninitialized. Shouldn't > the logic be: > > m = re.search ... > assert m > try: > ... > Will fix. >> + if verbose: >> + u_boot_console.log.info('Page size is: ' + str(page_size) + ' B') > > > Shorter might be: > > u_boot_console.log.info('Page size is: %dB' % page_size) > >> + env__sf_config['len'] = env__sf_config.get('len', erase_size) > > > Over-writing len changes the value in the original structure which could > affect tests that want to handle missing/inconsistent data in other ways. > I'd rather store the data using a different key that the user will never > set, or even better store derived/calculated data in a different dictionary > altogether. Sound like a better solution. I'll update `sf_prepare` to return a dictionary of calculated data. > >> + if env__sf_config['offset'] % erase_size or \ >> + env__sf_config['len'] % erase_size: >> + u_boot_console.log.warning("erase offset/length not multiple of " >> + "erase size") > > > I'd assert that here, rather than just warning. Ok. > >> + env__sf_config['ram_address'] = ram_address > > > ram_address isn't user-supplied configuration, and doesn't vary from > test-to-test. Let's either return it, or write it to a global, or just call > find_ram_base() whenever the value is required. I'll add it to the returned dictionary. > >> +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 >> + """ >> + >> + output = u_boot_console.run_command('crc32 %08x %x' % (address, >> count)) >> + >> + m = re.search('==> ([0-9a-fA-F]{8})$', output) >> + if not m: >> + pytest.fail('CRC32 failed') >> + >> + return m.group(1) > > > Should we put this into u_boot_utils.py? I was going to assert that other > tests must already do this, but I guess other tests check for whether a > known-ahead-of-time CRC value is on the crc32 command output, rather than > parsing out the actual value. So, maybe this doesn't need to be a utility > function. As it is quite generic, I think it's a good idea to move it to u_boot_utils.py. > >> +def sf_read(u_boot_console, env__sf_config, size=None): > > > Is size ever used? If not, let's delete it. I forgot to remove it from a previous iteration. Will remove. > >> + u_boot_console.run_command('mw %08x 0 %x' % >> (env__sf_config['ram_address'], >> + size)) > > > mw.b instead? Same in sf_update(). > > This function could benefit from doing: > > a) Fill target memory with known pattern. > b) sf read. > c) Check that target memory doesn't contain the known pattern. > Will do. >> +def sf_update(u_boot_console, env__sf_config): > > >> + from time import time > > > That's not needed. It should be at the top of the file if it was. > >> + u_boot_console.run_command('mw %08x %08x %x' % >> + (env__sf_config['ram_address'], time(), >> + env__sf_config['len'])) >> + crc_ram = crc32(u_boot_console, env__sf_config['ram_address'], >> + env__sf_config['len']) >> + u_boot_console.run_command('sf update %08x %08x %x' % >> + (env__sf_config['ram_address'], >> + env__sf_config['offset'], >> + env__sf_config['len'])) > > > Here the RAM should be cleared, although I guess that can happen inside > sf_read(). > >> + crc2 = sf_read(u_boot_console, env__sf_config) >> + >> + return (crc2 == crc_ram) > > >> +@pytest.mark.buildconfigspec("cmd_sf") >> +def test_sf_read(u_boot_console, env__sf_config): >> + sf_prepare(u_boot_console, env__sf_config) >> + >> + output = u_boot_console.run_command('sf read %08x %08x %x' % >> + (env__sf_config['ram_address'], >> + env__sf_config['offset'], >> + env__sf_config['len'])) >> + assert 'Read: OK' in output, "Read operation failed" > > > This doesn't verify that the command actually read anything, since it > doesn't look for changes in the RAM. Let's "mw.b ram_address offset len" > first and check that the CRC changes before/after the read to ensure that > some bytes were actually transferred. If you take a look at the test_mmc_rd > patch I sent recently, you can see how to do that. It'd be nice to structure > the two tests as similarly as possible since they're doing roughly the same > thing. Ok, I'll have a look at that. If it's similar, I can use the updated `sf_read()` you propose above. > > Also, it'd be good if env__sf_configs specified a known CRC for data that > should be in the SPI flash, so we can verify not only that /some/ data was > read, but that the /correct/ data was read. Ok, I'll add something like `expected_crc32` to env__sf_configs. > > Why doesn't this function use the sf_read helper function like > test_sf_read_twice() does? > >> +@pytest.mark.buildconfigspec("cmd_sf") >> +@pytest.mark.buildconfigspec("cmd_crc32") >> +@pytest.mark.buildconfigspec("cmd_memory") >> +def test_sf_read_twice(u_boot_console, env__sf_config): >> + sf_prepare(u_boot_console, env__sf_config) >> + >> + crc1 = sf_read(u_boot_console, env__sf_config) >> + crc2 = sf_read(u_boot_console, env__sf_config) >> + >> + assert crc1 == crc2, "CRC32 of two successive read operation do not >> match" > > > Similarly here, the test will pass if neither commands reads anything, or if > only the first does, or if they both read the wrong thing. It's true that this will pass if both commands don't read anything. I think what you suggested for `sf_read` will fix this. > >> +def test_sf_erase(u_boot_console, env__sf_config): > > ... >> >> + u_boot_console.run_command('mw %08x ffffffff %x' % >> + (env__sf_config['ram_address'], >> + env__sf_config['len'])) >> + crc1 = crc32(u_boot_console, env__sf_config['ram_address'], >> + env__sf_config['len']) > > > It'd be nice to name that crc_ffs or something like that ... > >> + >> + crc2 = sf_read(u_boot_console, env__sf_config) > > > ... and name that crc_read or crc_readback, just to give a quick/easy idea > of what each variable holds. Will do. Thanks for your time, Liam
On 1.3.2018 22:18, Stephen Warren wrote: > On 03/01/2018 04:48 AM, Liam Beguin wrote: >> On 1 March 2018 at 01:59, Michal Simek <michal.simek@xilinx.com> wrote: > ... >>> Also these randomized data/lengths were able to find out more issues >>> than fixed one. I was playing with that randomized test order and I see >>> that not all current tests are self contained and this will be good to >>> run too. >> >> I'll add options to randomize in v2. > > I'd strongly prefer the tests to be deterministic and not do random > different actions each time they're run. That way, any failures are much > easier to track down. Writing randomly generated data is fine, but not > random access patterns or sets of operations. > > Although I suppose you could implement two sets of tests; one that does > a static set of operations driven by a table in the board config file, > and one that does a more randomized set of operations controlled by > limits set by a table in the board config file. The thing is that I have real experience with random tests which were able to find out issues in the driver compare to fixed tests. But I have not a problem with two set of tests. Thanks, Michal
On 03/03/2018 09:32 AM, Liam Beguin wrote: > On 1 March 2018 at 16:56, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 02/26/2018 09:17 PM, Liam Beguin wrote: >>> >>> Add basic tests for the spi_flash subsystem. >> >> >>> diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py >> >> >>> +import re >>> +import pytest >>> +import u_boot_utils >>> + >>> + >> >> >> Nit: Double blank line. The same issue exists in many other places too. > > This is because I use flake8 and it was complaining if I didn't. Apparently > PEP8 suggests you surround top level functions with 2 blank lines. > I really have no preference. Hmm. There are some things about PEP8 I'm not so found of. Either way though, I don't believe any current code in test/py uses double blank lines, and I /think/ the same is true for other Python code in U-Boot. Looks like I don't have any other comments on your replies.
Hi Stephen, On Tue, 13 Mar 2018 at 17:22 Stephen Warren <swarren@wwwdotorg.org> wrote: > On 03/03/2018 09:32 AM, Liam Beguin wrote: > > On 1 March 2018 at 16:56, Stephen Warren <swarren@wwwdotorg.org> wrote: > >> On 02/26/2018 09:17 PM, Liam Beguin wrote: > >>> > >>> Add basic tests for the spi_flash subsystem. > >> > >> > >>> diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py > >> > >> > >>> +import re > >>> +import pytest > >>> +import u_boot_utils > >>> + > >>> + > >> > >> > >> Nit: Double blank line. The same issue exists in many other places too. > > > > This is because I use flake8 and it was complaining if I didn't. > Apparently > > PEP8 suggests you surround top level functions with 2 blank lines. > > I really have no preference. > > Hmm. There are some things about PEP8 I'm not so found of. Either way > though, I don't believe any current code in test/py uses double blank > lines, and I /think/ the same is true for other Python code in U-Boot. > As I said, I don't have any preference but you're right, consistency is best. I'll remove them in v3. > > Looks like I don't have any other comments on your replies. > Thanks, Liam Beguin
diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py new file mode 100644 index 000000000000..7017d8072ea9 --- /dev/null +++ b/test/py/tests/test_sf.py @@ -0,0 +1,233 @@ +# Copyright (c) 2017, Xiphos Systems Corp. All rights reserved. +# +# SPDX-License-Identifier: GPL-2.0 + +import re +import pytest +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: + +# Boolean indicating whether the SF tests should be skipped. +env__sf_skip = False + +# A list of sections of flash memory to be tested. +env__sf_configs = ( + { + # Optional, [[bus:]cs] argument used in `sf probe` + 'id': "0", + # Where in SPI flash should the test operate. + 'offset': 0x00000000, + # 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, + # Specifies if the test can write to offset + 'writeable': False, + }, +) +""" + + +def sf_prepare(u_boot_console, env__sf_config, verbose=False): + """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: + Nothing. + """ + + if u_boot_console.config.env.get('env__sf_skip', True): + pytest.skip('sf test disabled in environment') + + # NOTE: sf read at address 0 fails because map_physmem 'converts' it + # address 0 to a pointer. + ram_address = u_boot_utils.find_ram_base(u_boot_console) + 0x10 + + probe_id = env__sf_config.get('id', '') + output = u_boot_console.run_command('sf probe ' + probe_id) + if 'SF: Detected' not in output: + pytest.fail('No flash device available') + + m = re.search('page size (.+?) Bytes', output) + if m: + try: + page_size = int(m.group(1)) + except ValueError: + pytest.fail('SPI Flash page size not recognized') + + m = re.search('erase size (.+?) KiB', output) + if m: + try: + erase_size = int(m.group(1)) + except ValueError: + pytest.fail('SPI Flash erase size not recognized') + + erase_size *= 1024 + + m = re.search('total (.+?) MiB', output) + if m: + try: + total_size = int(m.group(1)) + except ValueError: + pytest.fail('SPI Flash total size not recognized') + + total_size *= 1024 * 1024 + + if verbose: + u_boot_console.log.info('Page size is: ' + str(page_size) + ' B') + u_boot_console.log.info('Erase size is: ' + str(erase_size) + ' B') + u_boot_console.log.info('Total size is: ' + str(total_size) + ' B') + + env__sf_config['len'] = env__sf_config.get('len', erase_size) + if env__sf_config['offset'] % erase_size or \ + env__sf_config['len'] % erase_size: + u_boot_console.log.warning("erase offset/length not multiple of " + "erase size") + + env__sf_config['ram_address'] = ram_address + + +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 + """ + + output = u_boot_console.run_command('crc32 %08x %x' % (address, count)) + + m = re.search('==> ([0-9a-fA-F]{8})$', output) + if not m: + pytest.fail('CRC32 failed') + + return m.group(1) + + +def sf_read(u_boot_console, env__sf_config, size=None): + """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. + size: Optional, used to override env__sf_config value. + + Returns: + CRC32 value of SPI Flash section + """ + + if size is None: + size = env__sf_config['len'] + + u_boot_console.run_command('mw %08x 0 %x' % (env__sf_config['ram_address'], + size)) + + response = u_boot_console.run_command('sf read %08x %08x %x' % + (env__sf_config['ram_address'], + env__sf_config['offset'], + size)) + assert 'Read: OK' in response, "Read operation failed" + + return crc32(u_boot_console, env__sf_config['ram_address'], + env__sf_config['len']) + + +def sf_update(u_boot_console, env__sf_config): + """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 + """ + from time import time + + u_boot_console.run_command('mw %08x %08x %x' % + (env__sf_config['ram_address'], time(), + env__sf_config['len'])) + crc_ram = crc32(u_boot_console, env__sf_config['ram_address'], + env__sf_config['len']) + u_boot_console.run_command('sf update %08x %08x %x' % + (env__sf_config['ram_address'], + env__sf_config['offset'], + env__sf_config['len'])) + + crc2 = sf_read(u_boot_console, env__sf_config) + + return (crc2 == crc_ram) + + +@pytest.mark.buildconfigspec("cmd_sf") +def test_sf_read(u_boot_console, env__sf_config): + sf_prepare(u_boot_console, env__sf_config) + + output = u_boot_console.run_command('sf read %08x %08x %x' % + (env__sf_config['ram_address'], + env__sf_config['offset'], + env__sf_config['len'])) + assert 'Read: OK' in output, "Read operation failed" + + +@pytest.mark.buildconfigspec("cmd_sf") +@pytest.mark.buildconfigspec("cmd_crc32") +@pytest.mark.buildconfigspec("cmd_memory") +def test_sf_read_twice(u_boot_console, env__sf_config): + sf_prepare(u_boot_console, env__sf_config) + + crc1 = sf_read(u_boot_console, env__sf_config) + crc2 = sf_read(u_boot_console, env__sf_config) + + assert crc1 == crc2, "CRC32 of two successive read operation do not match" + + +@pytest.mark.buildconfigspec("cmd_sf") +@pytest.mark.buildconfigspec("cmd_crc32") +@pytest.mark.buildconfigspec("cmd_memory") +def test_sf_erase(u_boot_console, env__sf_config): + if not env__sf_config['writeable']: + pytest.skip('flash config is tagged as not writeable') + + sf_prepare(u_boot_console, env__sf_config) + output = u_boot_console.run_command('sf erase %08x %x' % + (env__sf_config['offset'], + env__sf_config['len'])) + assert 'Erased: OK' in output, "Erase operation failed" + + u_boot_console.run_command('mw %08x ffffffff %x' % + (env__sf_config['ram_address'], + env__sf_config['len'])) + crc1 = crc32(u_boot_console, env__sf_config['ram_address'], + env__sf_config['len']) + + crc2 = sf_read(u_boot_console, env__sf_config) + assert crc1 == crc2, "CRC32 of erase section does not match expected value" + + +@pytest.mark.buildconfigspec("cmd_sf") +@pytest.mark.buildconfigspec("cmd_memory") +def test_sf_update(u_boot_console, env__sf_config): + if not env__sf_config['writeable']: + pytest.skip('flash config is tagged as not writeable') + + sf_prepare(u_boot_console, env__sf_config) + assert sf_update(u_boot_console, env__sf_config) is True
Add basic tests for the spi_flash subsystem. Signed-off-by: Liam Beguin <liambeguin@gmail.com> --- test/py/tests/test_sf.py | 233 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 233 insertions(+)