[U-Boot] [PATCH 0/3] add inital SF tests

[Liam Beguin]

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.

Thanks,
Liam Beguin

[ 1 ] https://patchwork.ozlabs.org/patch/623061/

Liam Beguin (3):
  test/py: README: fix typo
  test/py: README: add HOSTNAME to PYTHONPATH
  test/py: add spi_flash tests

 test/py/README.md|   6 +-
 test/py/tests/test_sf.py | 233 +++
 2 files changed, 236 insertions(+), 3 deletions(-)
 create mode 100644 test/py/tests/test_sf.py

base-commit: 4bafceff0e9e5a36908031e41c69a6b37e82da58
Published-As: https://github.com/Liambeguin/u-boot/releases/tag/test_sf_v1

-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 3/3] test/py: add spi_flash tests

[Liam Beguin]

Add basic tests for the spi_flash subsystem.

Signed-off-by: Liam Beguin 
---
 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 ..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': 0x,
+# 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': 0x1,
+# 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:
+ 

[U-Boot] [PATCH 2/3] test/py: README: add HOSTNAME to PYTHONPATH

[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 
---
 test/py/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/py/README.md b/test/py/README.md
index 000afce93c4a..aed2fd063a81 100644
--- a/test/py/README.md
+++ b/test/py/README.md
@@ -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
 ```
 
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 1/3] test/py: README: fix typo

[Liam Beguin]

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

Signed-off-by: Liam Beguin 
---
 test/py/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/py/README.md b/test/py/README.md
index eefac377567a..000afce93c4a 100644
--- a/test/py/README.md
+++ b/test/py/README.md
@@ -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.
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 0/3] add inital SF tests

[Liam Beguin]

Hi,

On Tue, 27 Feb 2018 at 11:29 Stephen Warren  wrote:
>
> Liam Beguin wrote at Monday, February 26, 2018 9:18 PM:
> > 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.
>
> These patches didn't make it to the U-Boot mailing list, although a couple of 
> replies did. You probably want to resend them.

The patches were delayed because I was not registered on the mailing
list. They now seem to have reached the list but as you said, replies
are not linked to the thread.

What would be the best way to fix this? Should I resend the series?

>
> --
> nvpublic
>

Thanks,
Liam Beguin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] test/py: add spi_flash tests

[Liam Beguin]

Hi Michal,

On 27 February 2018 at 03:51, Michal Simek  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 
>> ---
>>  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 ..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': 0x,
>> +# 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': 0x1,
>> +# 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

Re: [U-Boot] [PATCH 3/3] test/py: add spi_flash tests

[Liam Beguin]

On 1 March 2018 at 01:59, Michal Simek  wrote:
> On 1.3.2018 05:01, Liam Beguin wrote:
>> Hi Michal,
>>
>> On 27 February 2018 at 03:51, Michal Simek  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 
>>>> ---
>>>>  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 ..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': 0x,
>>>> +# 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': 0x1,
>>>> +# 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
>

Re: [U-Boot] [PATCH 1/3] test/py: README: fix typo

[Liam Beguin]

On 1 March 2018 at 16:23, Stephen Warren  wrote:
> On 02/26/2018 09:17 PM, Liam Beguin wrote:
>>
>> fix a minor typo causing vim (and possibly other) to get confused with
>> coloring.
>
>
> Nit: s/fix/Fix/.

Will fix

>
> Reviewed-by: Stephen Warren 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] test/py: add spi_flash tests

[Liam Beguin]

On 1 March 2018 at 16:56, Stephen Warren  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': 0x1,
>
>
> 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&q

[U-Boot] [PATCH v2 1/7] spi: spi_flash: do not fail silently on bad user input

[Liam Beguin]

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

Signed-off-by: Liam Beguin 
---
 drivers/mtd/spi/spi_flash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 294d9f9d79c6..2e61685d3ea4 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -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;
}
 
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 0/8] add inital SF tests

[Liam Beguin]

Hi all,

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

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

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

Thanks,
Liam Beguin

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

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

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


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

-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 5/7] test/py: do not import pytest multiple times

[Liam Beguin]

Signed-off-by: Liam Beguin 
---
 test/py/u_boot_utils.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/py/u_boot_utils.py b/test/py/u_boot_utils.py
index 9acb92ddc448..64584494e463 100644
--- a/test/py/u_boot_utils.py
+++ b/test/py/u_boot_utils.py
@@ -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.
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 2/7] cmd: sf: fix map_physmem check

[Liam Beguin]

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

Signed-off-by: Liam Beguin 
---
 cmd/sf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/sf.c b/cmd/sf.c
index f971eec781cc..e7ff9a646208 100644
--- a/cmd/sf.c
+++ b/cmd/sf.c
@@ -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;
}
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 3/7] test/py: README: fix typo

[Liam Beguin]

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

Signed-off-by: Liam Beguin 
---
 test/py/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/py/README.md b/test/py/README.md
index eefac377567a..000afce93c4a 100644
--- a/test/py/README.md
+++ b/test/py/README.md
@@ -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.
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 6/7] test/py: add generic CRC32 function

[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 
---
 test/py/u_boot_utils.py | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/test/py/u_boot_utils.py b/test/py/u_boot_utils.py
index 64584494e463..de9ee2643f51 100644
--- a/test/py/u_boot_utils.py
+++ b/test/py/u_boot_utils.py
@@ -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)
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 4/7] test/py: README: add HOSTNAME to PYTHONPATH

[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 
---
 test/py/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/py/README.md b/test/py/README.md
index 000afce93c4a..aed2fd063a81 100644
--- a/test/py/README.md
+++ b/test/py/README.md
@@ -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
 ```
 
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v2 7/7] test/py: add spi_flash tests

[Liam Beguin]

Add basic tests for the spi_flash subsystem.

Signed-off-by: Liam Beguin 
---
 test/py/tests/test_sf.py | 223 +++
 1 file changed, 223 insertions(+)
 create mode 100644 test/py/tests/test_sf.py

diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py
new file mode 100644
index ..0cc2a60e68d4
--- /dev/null
+++ b/test/py/tests/test_sf.py
@@ -0,0 +1,223 @@
+# Copyright (c) 2017, Xiphos Systems Corp. All rights reserved.
+#
+# SPDX-License-Identifier: GPL-2.0
+
+import re
+import pytest
+import random
+import u_boot_utils
+
+
+"""
+Note: This test relies on boardenv_* containing configuration values to define
+which SPI Flash areas are available for testing.  Without this, this test will
+be automatically skipped.
+For example:
+
+# A list of sections of Flash memory to be tested.
+env__sf_configs = (
+{
+# Where in SPI Flash should the test operate.
+'offset': 0x,
+# 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': 100,
+# 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': 0x1,
+# This value is optional.
+#   If present, specifies if the test can write to Flash offset
+#   If missing, defaults to False.
+'writeable': False,
+# This value is optional.
+#   If present, specifies the expected CRC32 value of the flash area.
+#   If missing, extra check is ignored.
+'crc32': 0xCAFECAFE,
+},
+)
+"""
+
+
+def sf_prepare(u_boot_console, env__sf_config):
+"""Check global state of the SPI Flash before running any test.
+
+   Args:
+u_boot_console: A U-Boot console connection.
+env__sf_config: The single SPI Flash device configuration on which to
+run the tests.
+
+Returns:
+sf_params: a dictionnary of SPI Flash parameters.
+"""
+
+sf_params = {}
+sf_params['ram_base'] = u_boot_utils.find_ram_base(u_boot_console)
+
+probe_id = env__sf_config.get('id', 0)
+speed = env__sf_config.get('speed', 0)
+if isinstance(speed, list) and len(speed) == 2:
+sf_params['speed'] = random.randint(speed[0], speed[1])
+else:
+sf_params['speed'] = speed
+
+cmd = 'sf probe %d %d' % (probe_id, sf_params['speed'])
+
+output = u_boot_console.run_command(cmd)
+assert 'SF: Detected' in output, 'No Flash device available'
+
+m = re.search('page size (.+?) Bytes', output)
+assert m, 'SPI Flash page size not recognized'
+sf_params['page_size'] = int(m.group(1))
+
+m = re.search('erase size (.+?) KiB', output)
+assert m, 'SPI Flash erase size not recognized'
+sf_params['erase_size'] = int(m.group(1))
+sf_params['erase_size'] *= 1024
+
+m = re.search('total (.+?) MiB', output)
+assert m, 'SPI Flash total size not recognized'
+sf_params['total_size'] = int(m.group(1))
+sf_params['total_size'] *= 1024 * 1024
+
+assert env__sf_config['offset'] is not None, \
+'\'offset\' is required for this test.'
+sf_params['len'] = env__sf_config.get('len', sf_params['erase_size'])
+
+assert not env__sf_config['offset'] % sf_params['erase_size'], \
+'offset not multiple of erase size.'
+assert not sf_params['len'] % sf_params['erase_size'], \
+'erase length not multiple of erase size.'
+
+assert not (env__sf_config.get('writeable', False) and
+env__sf_config.get('crc32', False)), \
+'Cannot check crc32 on  writeable sections'
+
+return sf_params
+
+
+def sf_read(u_boot_console, env__sf_config, sf_params):
+"""Helper function used to read and compute the CRC32 value of a section of
+SPI Flash memory.
+
+Args:
+u_boot_console: A U-Boot console connection.
+env__sf_config: The single SPI Flash device configuration on which to
+run the tests.
+sf_params: SPI Flash parameters.
+
+Returns:
+CRC32 value of S

Re: [U-Boot] [PATCH v2 0/8] add inital SF tests

[Liam Beguin]

Hi everyone,

Any new comments on this iteration?

Thanks,
Liam

On Sun, 4 Mar 2018 at 23:22 Liam Beguin  wrote:

> Hi all,
>
> This is the inital step to adding tests for the SF subsystem plus very
> minor fixes. It is based on work I found on the mailing list[1].
> For now, it doesn't do much but I plan on adding code to reset the flash
> to its initial state (based on an env flag) and more code to test the
> `sf protect` subcommand (which is the main goal of this series).
> I'm sending it now to make sure it's headed in the right direction.
>
> Base on Stephen's comment[2], I haven't added the radomized features.
> I'll see how this iteration goes and maybe add it later.
>
> Changes since v1:
>  - remove unnecessary skip flag from environment
>  - move crc32() to u_boot_utils.py and add extra checks
>  - rewrite sf_prepare to return a dict of parameter
>  - use assert instead of pytest.fail
>  - remove verbose from sf_prepare()
>  - update documentation
>  - improve readability
>  - use ' consistently instead of "
>  - use sf_read() in test_sf_read()
>  - rename crc variables
>  - add speed parameter with optional random range
>  - allow `sf read` to write at 0x00
>
> Thanks,
> Liam Beguin
>
> [ 1 ] https://patchwork.ozlabs.org/patch/623061/
> [ 2 ] https://lists.denx.de/pipermail/u-boot/2018-March/321688.html
>
> Liam Beguin (8):
>   spi: spi_flash: do not fail silently on bad user input
>   cmd: sf: fix map_physmem check
>   test/py: README: fix typo
>   test/py: README: add HOSTNAME to PYTHONPATH
>   test/py: do not import pytest multiple times
>   test/py: add generic CRC32 function
>   test/py: add spi_flash tests
>
>  cmd/sf.c|   2 +-
>  drivers/mtd/spi/spi_flash.c |   2 +-
>  test/py/README.md   |   6 +-
>  test/py/tests/test_sf.py| 223
> 
>  test/py/u_boot_utils.py |  24 -
>  5 files changed, 251 insertions(+), 6 deletions(-)
>  create mode 100644 test/py/tests/test_sf.py
>
>
> base-commit: 77bba970e2372b01156c66585db3d6fc751c7178
> Published-As: https://github.com/Liambeguin/u-boot/releases/tag/test_sf-v2
>
> --
> 2.16.1.72.g5be1f00a9a70
>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] test/py: add spi_flash tests

[Liam Beguin]

Hi Stephen,

On Tue, 13 Mar 2018 at 17:22 Stephen Warren  wrote:

> On 03/03/2018 09:32 AM, Liam Beguin wrote:
> > On 1 March 2018 at 16:56, Stephen Warren  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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 7/7] test/py: add spi_flash tests

[Liam Beguin]

Hi Stephen,

On 13 March 2018 at 17:41, Stephen Warren  wrote:
> On 03/04/2018 09:22 PM, Liam Beguin wrote:
>>
>> Add basic tests for the spi_flash subsystem.
>
>
> Looks good. A few small issues:
>
>> +def sf_prepare(u_boot_console, env__sf_config):
>
> ...
>>
>> +speed = env__sf_config.get('speed', 0)
>> +if isinstance(speed, list) and len(speed) == 2:
>> +sf_params['speed'] = random.randint(speed[0], speed[1])
>> +else:
>> +sf_params['speed'] = speed
>
>
> What if speed is a tuple or other indexable type not a list? Perhaps invert
> the test. Also, perhaps assume any list has two entries, or assert that.
>
> if isintance(speed, int):
>   int case
> else:
>   assert len(speed) == 2, "Speed must have two entries"
>   list/tuple case
>

Right, that's better!

>> +assert env__sf_config['offset'] is not None, \
>> +'\'offset\' is required for this test.'
>
>
> Is this meant to test for:
> a) A key that's present, with value set to None.
> b) A missing key.
>
> It currently tests (a), but testing for (b) seems more likely to catch
> issues. Perhaps:
>
> assert env__sf_config.get('offset', None) is not None
>
> or:
>
> assert 'offset' in env__sf_config.get
>

Thanks for seeing this, will fix.

>> +assert not (env__sf_config.get('writeable', False) and
>> +env__sf_config.get('crc32', False)), \
>> +'Cannot check crc32 on  writeable sections'
>
>
> What if the crc32 value is 0, which IIRC is False? Unlikely admittedly, but
> you never know. Perhaps:
>
> assert not (env__sf_config.get('writeable', False) and
> 'crc32' in env__sf_config.get), \
> 'Cannot check crc32 on  writeable sections'
>

Ok

>> +def sf_read(u_boot_console, env__sf_config, sf_params):
>> +addr = sf_params['ram_base']
>> +offset = env__sf_config['offset']
>> +count = sf_params['len']
>> +pattern = random.randint(0, 0x)
>
>
> 0xFF not 0x since it's bytes.
>

Will fix across file...

>> +crc_expected = env__sf_config.get('crc32', None)
>> +
>> +cmd = 'mw.b %08x %08x %x' % (addr, pattern, count)
>
>
> %02x for pattern.
>
>> +u_boot_console.run_command(cmd)
>> +crc_read = u_boot_utils.crc32(u_boot_console, addr, count)
>
>
> crc_read sounds like a CRC for data that's been read. Perhaps crc_pattern?

crc_pattern sounds good.

>
>> +def sf_update(u_boot_console, env__sf_config, sf_params):
>> +addr = sf_params['ram_base']
>> +offset = env__sf_config['offset']
>> +count = sf_params['len']
>> +pattern = int(random.random() * 0x)
>
>
> 0xFF.
>
>> +cmd = 'mw.b %08x %08x %x' % (addr, pattern, count)
>
>
> %02x for pattern.
>
>> +u_boot_console.run_command(cmd)
>> +crc_read = u_boot_utils.crc32(u_boot_console, addr, count)
>
>
> crc_pattern?
>
>> +def test_sf_erase(u_boot_console, env__sf_config):
>
> ...
>>
>> +cmd = 'mw.b %08x  %x' % (addr, count)
>
>
> Just ff not ?
>
>> +@pytest.mark.buildconfigspec('cmd_sf')
>> +@pytest.mark.buildconfigspec('cmd_memory')
>> +def test_sf_update(u_boot_console, env__sf_config):
>
>
> sf_update() unconditionally calls u_boot_utils.crc32 which asserts if the
> crc32 command isn't available, so this function needs to be
> @pytest.mark.buildconfigspec('cmd_crc32').

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

Thanks for taking the time to review this series,
Liam Beguin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 4/7] test/py: README: add HOSTNAME to PYTHONPATH

[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 
---
 test/py/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/py/README.md b/test/py/README.md
index 000afce93c4a..aed2fd063a81 100644
--- a/test/py/README.md
+++ b/test/py/README.md
@@ -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
 ```
 
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 6/7] test/py: add generic CRC32 function

[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 
---
 test/py/u_boot_utils.py | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/test/py/u_boot_utils.py b/test/py/u_boot_utils.py
index 64584494e463..de9ee2643f51 100644
--- a/test/py/u_boot_utils.py
+++ b/test/py/u_boot_utils.py
@@ -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)
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 3/7] test/py: README: fix typo

[Liam Beguin]

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

Signed-off-by: Liam Beguin 
---
 test/py/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/py/README.md b/test/py/README.md
index eefac377567a..000afce93c4a 100644
--- a/test/py/README.md
+++ b/test/py/README.md
@@ -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.
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 5/7] test/py: do not import pytest multiple times

[Liam Beguin]

Signed-off-by: Liam Beguin 
---
 test/py/u_boot_utils.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/py/u_boot_utils.py b/test/py/u_boot_utils.py
index 9acb92ddc448..64584494e463 100644
--- a/test/py/u_boot_utils.py
+++ b/test/py/u_boot_utils.py
@@ -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.
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 2/7] cmd: sf: fix map_physmem check

[Liam Beguin]

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

Signed-off-by: Liam Beguin 
---
 cmd/sf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/sf.c b/cmd/sf.c
index f971eec781cc..e7ff9a646208 100644
--- a/cmd/sf.c
+++ b/cmd/sf.c
@@ -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;
}
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 1/7] spi: spi_flash: do not fail silently on bad user input

[Liam Beguin]

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

Signed-off-by: Liam Beguin 
---
 drivers/mtd/spi/spi_flash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 294d9f9d79c6..2e61685d3ea4 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -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;
}
 
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 7/7] test/py: add spi_flash tests

[Liam Beguin]

Add basic tests for the spi_flash subsystem.

Signed-off-by: Liam Beguin 
---
 test/py/tests/test_sf.py | 217 +++
 1 file changed, 217 insertions(+)
 create mode 100644 test/py/tests/test_sf.py

diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py
new file mode 100644
index ..8bd1623ff303
--- /dev/null
+++ b/test/py/tests/test_sf.py
@@ -0,0 +1,217 @@
+# 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': 0x,
+# 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': 100,
+# 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': 0x1,
+# 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

[U-Boot] [PATCH v3 0/7] add inital SF tests

[Liam Beguin]

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 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| 217 
 test/py/u_boot_utils.py |  24 -
 5 files changed, 245 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-v3

interdiff vs v2:
diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py
index 0cc2a60e68d4..8bd1623ff303 100644
--- a/test/py/tests/test_sf.py
+++ b/test/py/tests/test_sf.py
@@ -7,7 +7,6 @@ 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
@@ -45,7 +44,6 @@ env__sf_configs = (
 )
 """
 
-
 def sf_prepare(u_boot_console, env__sf_config):
 """Check global state of the SPI Flash before running any test.
 
@@ -63,10 +61,11 @@ def sf_prepare(u_boot_console, env__sf_config):
 
 probe_id = env__sf_config.get('id', 0)
 speed = env__sf_config.get('speed', 0)
-if isinstance(speed, list) and len(speed) == 2:
-sf_params['speed'] = random.randint(speed[0], speed[1])
-else:
+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'])
 
@@ -87,7 +86,7 @@ def sf_prepare(u_boot_console, env__sf_config):
 sf_params['total_size'] = int(m.group(1))
 sf_params['total_size'] *= 1024 * 1024
 
-assert env__sf_config['offset'] is not None, \
+assert 'offset' in env__sf_config, \
 '\'offset\' is required for this test.'
 sf_params['len'] = env__sf_config.get('len', sf_params['erase_size'])
 
@@ -97,12 +96,11 @@ def sf_prepare(u_boot_console, env__sf_config):
 'erase length not multiple of erase size.'
 
 assert not (env__sf_config.get('writeable', False) and
-env__sf_config.get('crc32', False)), \
-'Cannot check crc32 on  writeable sections'
+'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.
@@ -120,26 +118,25 @@ def sf_read(u_boot_console, env__sf_config, sf_params):
 addr = sf_params['ram_base']
 offset = env__sf_config[&#x

Re: [U-Boot] [PATCH v3 7/7] test/py: add spi_flash tests

[Liam Beguin]

Hi Michal,

On Wed, 14 Mar 2018 at 10:35 Michal Simek  wrote:

> On 14.3.2018 03:03, Liam Beguin wrote:
> > Add basic tests for the spi_flash subsystem.
> >
> > Signed-off-by: Liam Beguin 
> > ---
> >  test/py/tests/test_sf.py | 217
> +++
> >  1 file changed, 217 insertions(+)
> >  create mode 100644 test/py/tests/test_sf.py
> >
> > diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py
> > new file mode 100644
> > index ..8bd1623ff303
> > --- /dev/null
> > +++ b/test/py/tests/test_sf.py
> > @@ -0,0 +1,217 @@
> > +# Copyright (c) 2017, Xiphos Systems Corp. All rights reserved.
>
> here should be probably also xilinx copyright because some things were
> taken from that.
>
>
Right, is this ok with you? (copied from your initial patch)

Copyright (c) 2016, Xilinx Inc. Michal Simek



> Michal
>

Liam Beguin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v4 1/7] spi: spi_flash: do not fail silently on bad user input

[Liam Beguin]

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

Signed-off-by: Liam Beguin 
Reviewed-by: Stephen Warren 
---
 drivers/mtd/spi/spi_flash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 294d9f9d79c6..2e61685d3ea4 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -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;
}
 
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v4 0/7] add inital SF tests

[Liam Beguin]

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
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v4 2/7] cmd: sf: fix map_physmem check

[Liam Beguin]

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

Signed-off-by: Liam Beguin 
Reviewed-by: Stephen Warren 
---
 cmd/sf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/sf.c b/cmd/sf.c
index f971eec781cc..e7ff9a646208 100644
--- a/cmd/sf.c
+++ b/cmd/sf.c
@@ -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;
}
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v4 6/7] test/py: add generic CRC32 function

[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 
Reviewed-by: Stephen Warren 
---
 test/py/u_boot_utils.py | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/test/py/u_boot_utils.py b/test/py/u_boot_utils.py
index 64584494e463..de9ee2643f51 100644
--- a/test/py/u_boot_utils.py
+++ b/test/py/u_boot_utils.py
@@ -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)
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v4 7/7] test/py: add spi_flash tests

[Liam Beguin]

Add basic tests for the spi_flash subsystem.

Signed-off-by: Liam Beguin 
Reviewed-by: Stephen Warren 
---
 test/py/tests/test_sf.py | 218 +++
 1 file changed, 218 insertions(+)
 create mode 100644 test/py/tests/test_sf.py

diff --git a/test/py/tests/test_sf.py b/test/py/tests/test_sf.py
new file mode 100644
index ..efcd753c1abe
--- /dev/null
+++ b/test/py/tests/test_sf.py
@@ -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': 0x,
+# 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': 100,
+# 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': 0x1,
+# 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

[U-Boot] [PATCH v4 5/7] test/py: do not import pytest multiple times

[Liam Beguin]

Signed-off-by: Liam Beguin 
Reviewed-by: Stephen Warren 
---
 test/py/u_boot_utils.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/py/u_boot_utils.py b/test/py/u_boot_utils.py
index 9acb92ddc448..64584494e463 100644
--- a/test/py/u_boot_utils.py
+++ b/test/py/u_boot_utils.py
@@ -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.
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v4 3/7] test/py: README: fix typo

[Liam Beguin]

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

Signed-off-by: Liam Beguin 
Reviewed-by: Stephen Warren 
---
 test/py/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/py/README.md b/test/py/README.md
index eefac377567a..000afce93c4a 100644
--- a/test/py/README.md
+++ b/test/py/README.md
@@ -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.
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v4 4/7] test/py: README: add HOSTNAME to PYTHONPATH

[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 
Reviewed-by: Stephen Warren 
---
 test/py/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/py/README.md b/test/py/README.md
index 000afce93c4a..aed2fd063a81 100644
--- a/test/py/README.md
+++ b/test/py/README.md
@@ -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
 ```
 
-- 
2.16.1.72.g5be1f00a9a70

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v4 7/7] test/py: add spi_flash tests

[Liam Beguin]

Hi,

On Wed, 14 Mar 2018 at 19:27 Stephen Warren  wrote:

> On 03/14/2018 05:15 PM, Liam Beguin wrote:
> > Add basic tests for the spi_flash subsystem.
> >
> > Signed-off-by: Liam Beguin 
> > Reviewed-by: Stephen Warren 
>
> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] UBI/UBIFS complete integrity check

[Liam Beguin]

Hi everyone,

I'm currently using a UBIFS root file system (stored on SPI-NOR flash)
and would like to perform a full integrity check before booting it.
The rootfs is read-only and until now, I've been computing an md5sum on
the whole mtd device from an initramfs and comparing it to a stored
md5sum. If both md5sums don't match, I need to stop the boot process
completely.

If possible, I was hoping to drop initramfs and do the integrity check
from U-Boot. I know UBI/UBIFS does a CRC-32 of the data it writes to
flash but the intent here is to prevent booting an image where
even a _single bit_ of flash may have been corrupted.

My question is, does UBI/UBIFS have this kind of complete integrity
check built-in? If not, can I take advantage of these CRC-32, to do
something equivalent to my md5sum check from U-Boot.
Thanks,

Liam Beguin
Xiphos Systems Corp.
http://xiphos.com
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] UBI/UBIFS complete integrity check

[Liam Beguin]

Hi Lukasz,

Thanks for taking the time to answer.

On 11/04/2017 05:17 PM, Lukasz Majewski wrote:

Hi Liam,


Hi everyone,

I'm currently using a UBIFS root file system (stored on SPI-NOR flash)
and would like to perform a full integrity check before booting it.
The rootfs is read-only and until now, I've been computing an md5sum
on the whole mtd device from an initramfs and comparing it to a stored
md5sum. If both md5sums don't match, I need to stop the boot process
completely.

If possible, I was hoping to drop initramfs and do the integrity check
from U-Boot.


U-boot has support for crc32 and sha1 (256). It should be possible to
do the integrity checking in it.

If you have more SDRAM than SPI-NOR, then you can calculate sha1/crc32
of the whole memory.


I know UBI/UBIFS does a CRC-32 of the data it writes to
flash but the intent here is to prevent booting an image where
even a _single bit_ of flash may have been corrupted.


Ok. I see.



My question is, does UBI/UBIFS have this kind of complete integrity
check built-in?


As fair as I'm aware - not. The only recent improvement was the
"encryption/decryption" support


I don't think I have enough time right now but would this integrity check
be an interesting feature to add?




If not, can I take advantage of these CRC-32,


It may be hard to access UBI metadata (from PEB/LEB).


to do
something equivalent to my md5sum check from U-Boot.


It may be possible to read the whole SPI-NOR Memory content to RAM,
calculate crc32/sha1 and compare with some stored value (e.g. in u-boot
envs). This all should be done with u-boot prompt.


This was my backup plan. I should have enough RAM to do it.




Thanks,

Liam Beguin
Xiphos Systems Corp.
http://xiphos.com
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de



Thanks,

Liam Beguin
Xiphos Systems Corp.
http://xiphos.com

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] UBI/UBIFS complete integrity check

[Liam Beguin]

Hi,

Thanks for taking the time to answer.

On 11/05/2017 03:37 AM, Ladislav Michl wrote:

On Tue, Oct 31, 2017 at 11:01:21AM -0400, Liam Beguin wrote:

Hi everyone,

I'm currently using a UBIFS root file system (stored on SPI-NOR flash)
and would like to perform a full integrity check before booting it.
The rootfs is read-only and until now, I've been computing an md5sum on
the whole mtd device from an initramfs and comparing it to a stored
md5sum. If both md5sums don't match, I need to stop the boot process
completely.


Above doesn't sound right even in theory as UBI layer is free to correct
bit-flips (unlikely on SPI-NOR) and shuffle eraseblocks around even
if read only filesystem is sitting on top of it. See this faq:
http://www.linux-mtd.infradead.org/doc/ubi.html#L_ubiblock> 
So, if you are computing md5sum of underlaying mtd device you might get

different checksum even for the same UBI content.



I forgot to mention that the flash I use has a hardware lock which is enabled
after the filesystem is first written (the flash is locked during boot).
I'm quite confident this works as I've been using the md5sum mechanism for
some time now.

As the UBI layer is able to detect/fix bit-flips, what happens if a bit-flip is
detected and UBI cannot write to flash? does it fail to attach?
 

If possible, I was hoping to drop initramfs and do the integrity check
from U-Boot. I know UBI/UBIFS does a CRC-32 of the data it writes to
flash but the intent here is to prevent booting an image where
even a _single bit_ of flash may have been corrupted.

My question is, does UBI/UBIFS have this kind of complete integrity
check built-in? If not, can I take advantage of these CRC-32, to do
something equivalent to my md5sum check from U-Boot.
Thanks,


There is md5sum command, question is whenever you UBI volume fits
into RAM to do calculation at once.


Liam Beguin
Xiphos Systems Corp.
http://xiphos.com
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Thanks,

Liam Beguin
Xiphos Systems Corp.
http://xiphos.com
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] UBI/UBIFS complete integrity check

[Liam Beguin]

On 11/06/2017 12:57 PM, Ladislav Michl wrote:

On Mon, Nov 06, 2017 at 12:31:32PM -0500, Liam Beguin wrote:

Hi,

Thanks for taking the time to answer.

On 11/05/2017 03:37 AM, Ladislav Michl wrote:

On Tue, Oct 31, 2017 at 11:01:21AM -0400, Liam Beguin wrote:

Hi everyone,

I'm currently using a UBIFS root file system (stored on SPI-NOR flash)
and would like to perform a full integrity check before booting it.
The rootfs is read-only and until now, I've been computing an md5sum on
the whole mtd device from an initramfs and comparing it to a stored
md5sum. If both md5sums don't match, I need to stop the boot process
completely.


Above doesn't sound right even in theory as UBI layer is free to correct
bit-flips (unlikely on SPI-NOR) and shuffle eraseblocks around even
if read only filesystem is sitting on top of it. See this faq:
http://www.linux-mtd.infradead.org/doc/ubi.html#L_ubiblock> So, if you
are computing md5sum of underlaying mtd device you might get
different checksum even for the same UBI content.



I forgot to mention that the flash I use has a hardware lock which is enabled
after the filesystem is first written (the flash is locked during boot).
I'm quite confident this works as I've been using the md5sum mechanism for
some time now.


I see. Btw, have you considered using for example squashfs on top of static UBI
volume? Given the fact your fs is read only you get:
- CRC32 checksum of you volume for free
- filesystem compression
- you can easily compute mtd5sum of volume content
- check filesystem consistency using already provided tools (probably overkill).



I have considered squashfs but on rare occasions, I need to be able to write to 
the
rootfs. This involves disabling the hardware lock, remounting RW and updating 
the
reference md5sum after writing.


As the UBI layer is able to detect/fix bit-flips, what happens if a bit-flip is
detected and UBI cannot write to flash? does it fail to attach?


No, it just logs it. It fails once there is enough bitflips that ECC is unable
to correct them. But again, it is not case of NOR, I mentioned it just for
completeness.


I understand that bit-flips are uncommon on NOR flash but in our case, they may
be caused by the environment not the flash technology.




If possible, I was hoping to drop initramfs and do the integrity check
from U-Boot. I know UBI/UBIFS does a CRC-32 of the data it writes to
flash but the intent here is to prevent booting an image where
even a _single bit_ of flash may have been corrupted.


Above mentioned solution should fail instantly in case of CRC error. Not sure if
that's what you want as kernel already took over, so either watchdog will kick 
in
or machine will reboot, if configured to do so after panics.



Ideally, I'd like the attach to fail in U-Boot, before loading the kernel.

Something remains obscure to me... When UBI scans the device and potentially
warns of a bit-flip, has the _whole_ data been checked or was it only the
information contained in the UBI headers?


My question is, does UBI/UBIFS have this kind of complete integrity
check built-in? If not, can I take advantage of these CRC-32, to do
something equivalent to my md5sum check from U-Boot.
Thanks,


There is md5sum command, question is whenever you UBI volume fits
into RAM to do calculation at once.


Liam Beguin
Xiphos Systems Corp.
http://xiphos.com
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Thanks,

Liam Beguin
Xiphos Systems Corp.
http://xiphos.com

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 1/2] spi: zynqmp_qspi: Add support for ZynqMP qspi driver

[Liam Beguin]

Hi Siva,

I've been working on a patch to add support for io-mode [1].
It's based on xilinx/master which is closer to v1 (no include file).
I haven't had much time to test it further but I can load a bitstream
from a ubifs partition with it.
Let me know if you have comments.

On Tue, 10 Apr 2018 at 02:27 Siva Durga Prasad Paladugu 
wrote:

> H Jagan,
>
> Any further comments on this. I am waiting for your reply on some of my
> replies to your comment.
>
> Thanks,
> Siva
>
>
Thanks,
Liam

[1] https://github.com/Liambeguin/u-boot/tree/io-mode
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 1/2] spi: zynqmp_qspi: Add support for ZynqMP qspi driver

[Liam Beguin]

Hi Siva,

I wasn't able to find it on the Xilinx github. Is it in that tree already?

Thanks,
Liam

On Thu, 12 Apr 2018 at 07:18 Siva Durga Prasad Paladugu 
wrote:

> Hi Liam,
>
>
>
> We already have a patch for IO mode that we just verified internally. We
> got it for our tree from NGC guy John Moon. I will be sending that patch
> soon may be with v3 which I am planning to send once I got response from
> Jagan on my v2 queries.
>
>
>
> Regards,
>
> Siva
>
>
>
> *From:* Liam Beguin [mailto:liambeg...@gmail.com]
> *Sent:* Tuesday, April 10, 2018 8:56 PM
> *To:* Siva Durga Prasad Paladugu 
> *Cc:* Jagan Teki ; U-Boot-Denx <
> u-boot@lists.denx.de>; Michal Simek 
>
>
> *Subject:* Re: [U-Boot] [PATCH v2 1/2] spi: zynqmp_qspi: Add support for
> ZynqMP qspi driver
>
>
>
> Hi Siva,
>
>
>
> I've been working on a patch to add support for io-mode [1].
>
> It's based on xilinx/master which is closer to v1 (no include file).
>
> I haven't had much time to test it further but I can load a bitstream
>
> from a ubifs partition with it.
>
> Let me know if you have comments.
>
>
>
> On Tue, 10 Apr 2018 at 02:27 Siva Durga Prasad Paladugu <
> siva...@xilinx.com> wrote:
>
> H Jagan,
>
> Any further comments on this. I am waiting for your reply on some of my
> replies to your comment.
>
> Thanks,
> Siva
>
>
>
> Thanks,
>
> Liam
>
>
>
> [1] https://github.com/Liambeguin/u-boot/tree/io-mode
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] ZynqMP qspi

[Liam Beguin]

Hi,

I'm testing a new Xilinx zynqmp dev board and was not able to probe the
qspi with the latest mainline U-Boot. I see that there is a 'zynqmp_qspi'
driver in the Xilinx tree [1] but nothing in mainline. After a little
digging, I found a thread on the list [2] (and [3]) and was wondering in
what state this was now.

[1] https://github.com/Xilinx/u-boot-xlnx/blob/master/drivers/spi/zynqmp_qspi.c
[2] https://lists.denx.de/pipermail/u-boot/2016-July/261003.html
[3] https://lists.denx.de/pipermail/u-boot/2016-November/273650.html

Thanks,
Liam
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] ZynqMP qspi

[Liam Beguin]

Resending with proper CC since the email came back.

On 24/09/17 09:36 PM, Liam Beguin wrote:
> Hi,
> 
> I'm testing a new Xilinx zynqmp dev board and was not able to probe the
> qspi with the latest mainline U-Boot. I see that there is a 'zynqmp_qspi'
> driver in the Xilinx tree [1] but nothing in mainline. After a little
> digging, I found a thread on the list [2] (and [3]) and was wondering in
> what state this was now.
> 
> [1] 
> https://github.com/Xilinx/u-boot-xlnx/blob/master/drivers/spi/zynqmp_qspi.c
> [2] https://lists.denx.de/pipermail/u-boot/2016-July/261003.html
> [3] https://lists.denx.de/pipermail/u-boot/2016-November/273650.html
> 
> Thanks,
> Liam
> 

Thanks,
Liam
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] ZynqMP qspi

[Liam Beguin]

Hi,

On 26/09/17 01:28 AM, Siva Durga Prasad Paladugu wrote:
> Hi,
> 
>> -Original Message-
>> From: Jagan Teki [mailto:jagannadh.t...@gmail.com]
>> Sent: Tuesday, September 26, 2017 10:17 AM
>> To: Liam Beguin 
>> Cc: u-boot@lists.denx.de; Michal Simek ; Siva Durga
>> Prasad Paladugu 
>> Subject: Re: [U-Boot] ZynqMP qspi
>>
>> On Tue, Sep 26, 2017 at 9:07 AM, Liam Beguin 
>> wrote:
>>>
>>> Resending with proper CC since the email came back.
>>>
>>> On 24/09/17 09:36 PM, Liam Beguin wrote:
>>>> Hi,
>>>>
>>>> I'm testing a new Xilinx zynqmp dev board and was not able to probe
>>>> the qspi with the latest mainline U-Boot. I see that there is a
>> 'zynqmp_qspi'
>>>> driver in the Xilinx tree [1] but nothing in mainline. After a little
>>>> digging, I found a thread on the list [2] (and [3]) and was wondering
>>>> in what state this was now.
> 
> I am planning to send the new series in 1-2 weeks, probably you can wait till 
> then or
> you can try with the patch[2] and see if you are able to get it working with 
> those( those
> are known to be working when I sent ). Also, please note that as of now iam  
> targeting
> only for qspi single mode not the dual modes.
> 

Great, I'll wait for your series and test it as soon as it comes out.

Thanks,
Liam

> Thanks,
> Siva
> 
>>>>
>>>> [1]
>>>> https://github.com/Xilinx/u-boot-
>> xlnx/blob/master/drivers/spi/zynqmp_
>>>> qspi.c [2]
>>>> https://lists.denx.de/pipermail/u-boot/2016-July/261003.html
>>>> [3] https://lists.denx.de/pipermail/u-boot/2016-
>> November/273650.html
>>
>> I think, Siva is working on it. and he would be best to comment this.
>>
>> thanks!
>> --
>> Jagan Teki
>> Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream
>> Maintainer Hyderabad, India.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] ZynqMP qspi

[Liam Beguin]

Hi Siva,

On 26/09/17 01:28 AM, Siva Durga Prasad Paladugu wrote:
> Hi,
> 
...
> 
> I am planning to send the new series in 1-2 weeks, probably you can wait till 
> then or
> you can try with the patch[2] and see if you are able to get it working with 
> those( those
> are known to be working when I sent ). Also, please note that as of now iam  
> targeting
> only for qspi single mode not the dual modes.

I was wondering if you had a chance to work on this in the last few weeks. I 
initially
planned on re-sending your series but I don't currently have the hardware to 
test on.

> 
> Thanks,
> Siva
> 
Thanks,
Liam
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] ZynqMP qspi

[Liam Beguin]

Hi Siva,

On 23/10/17 01:59 AM, Siva Durga Prasad Paladugu wrote:
> Hi Liam,
> 
>> -Original Message-----
>> From: Liam Beguin [mailto:liambeg...@gmail.com]
>> Sent: Saturday, October 21, 2017 11:33 PM
>> To: Siva Durga Prasad Paladugu ; Jagan Teki
>> 
>> Cc: u-boot@lists.denx.de; Michal Simek 
>> Subject: Re: [U-Boot] ZynqMP qspi
>>
>> Hi Siva,
>>
>> On 26/09/17 01:28 AM, Siva Durga Prasad Paladugu wrote:
>>> Hi,
>>>
>> ...
>>>
>>> I am planning to send the new series in 1-2 weeks, probably you can
>>> wait till then or you can try with the patch[2] and see if you are
>>> able to get it working with those( those are known to be working when
>>> I sent ). Also, please note that as of now iam  targeting only for qspi 
>>> single
>> mode not the dual modes.
>>
>> I was wondering if you had a chance to work on this in the last few weeks. I
>> initially planned on re-sending your series but I don't currently have the
>> hardware to test on.
> Sorry for the delay, I am already working on it, need to cleanup and test it. 
> I will send the 
> series in a day.
> 

Great, I'll try to test it if I get the hardware in time.
Thanks,

Liam

> Regards,
> Siva
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [U-Boot, v2, 1/2] spi: zynqmp_qspi: Add support for ZynqMP qspi drivers

[Liam Beguin]

0130:        
0140:        
0150:        
0160:        
0170:        
ZynqMP> sf read 0x100 0x00 0x100
device 0 offset 0x0, size 0x100
SF: 256 bytes @ 0x0 Read: OK
ZynqMP> md.w 0x100
0100: beee beee beee beee beee beee beee beee
0110: beee beee beee beee beee beee beee beee
0120:        
0130:        
0140:        
0150:        
0160:        
0170:        

Here is a case where it failed:

ZynqMP> mw.w 100 abcd 10 ; md.w 100 10
0100: abcd abcd abcd abcd abcd abcd abcd abcd
0110: abcd abcd abcd abcd abcd abcd abcd abcd
ZynqMP> sf update 100 100 10
device 0 offset 0x100, size 0x10
SPI flash failed in erase step
ZynqMP> mw.w 100 00 100 ; sf read 100 100 10 ; md.w 100 10
device 0 offset 0x100, size 0x10
SF: 16 bytes @ 0x100 Read: OK
0100:     ffff ffff ffff 
0110:        

I hope this helps!
Thanks,

Liam Beguin
Xiphos Systems Corp.
http://xiphos.ca

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[PATCH 1/1] arm64: zynqmp: allow overriding board name

[Liam Beguin]

There is no need to use zynqmp name as SYS_BOARD for all boards.
The patch is adding an option to change it.

Signed-off-by: Liam Beguin 
---
 arch/arm/mach-zynqmp/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
index f7b08db35501..716c4d354f34 100644
--- a/arch/arm/mach-zynqmp/Kconfig
+++ b/arch/arm/mach-zynqmp/Kconfig
@@ -25,6 +25,7 @@ config SPL_SPI
default y if ZYNQ_QSPI
 
 config SYS_BOARD
+   string "Board name"
default "zynqmp"
 
 config SYS_VENDOR
-- 
2.32.0.452.g940fe202adcb