test/py: move find_ram_base() into u_boot_utils

PATCH New Stephen Warren 3 participants 13 messages Jan 21, 2016 – Jan 26, 2016

lore

Share & tools

lore

Stephen Warren

author

2016-01-21 23:05 UTC #0 lore raw

PATCH 1/2 test/py: move find_ram_base() into u_boot_utils

From: Stephen Warren <[email protected]>

find_ram_base() is a shared utility function, not a core part of the U-Boot console interaction.

Signed-off-by Stephen Warren <[email protected]>

---
These two patches depend on my previous series starting with:
test/py: fix timeout to be absolute
and ending with:
test/py: add DFU test

---
 test/py/tests/test_md.py       |  5 +++--
 test/py/u_boot_console_base.py | 37 -------------------------------------
 test/py/u_boot_utils.py        | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 39 deletions(-)

test/py/tests/test_md.py +3 -2

index 94603c7df609..32cce4f15c53 100644
@@ -4,13 +4,14 @@
 # SPDX-License-Identifier: GPL-2.0
 
 import pytest
+import u_boot_utils
 
 @pytest.mark.buildconfigspec('cmd_memory')
 def test_md(u_boot_console):
     '''Test that md reads memory as expected, and that memory can be modified
     using the mw command.'''
 
-    ram_base = u_boot_console.find_ram_base()
+    ram_base = u_boot_utils.find_ram_base(u_boot_console)
     addr = '%08x' % ram_base
     val = 'a5f09876'
     expected_response = addr + ': ' + val

@@ -26,7 +27,7 @@ def test_md_repeat(u_boot_console):
     '''Test command repeat (via executing an empty command) operates correctly
     for "md"; the command must repeat and dump an incrementing address.'''
 
-    ram_base = u_boot_console.find_ram_base()
+    ram_base = u_boot_utils.find_ram_base(u_boot_console)
     addr_base = '%08x' % ram_base
     words = 0x10
     addr_repeat = '%08x' % (ram_base + (words * 4))

test/py/u_boot_console_base.py +0 ~~-37~~

index 06f61f987180..51163bc0db68 100644
@@ -86,7 +86,6 @@ class ConsoleBase(object):
 
         self.at_prompt = False
         self.at_prompt_logevt = None
-        self.ram_base = None
 
     def close(self):
         '''Terminate the connection to the U-Boot console.

@@ -378,39 +377,3 @@ class ConsoleBase(object):
         '''
 
         return ConsoleDisableCheck(self, check_type)
-
-    def find_ram_base(self):
-        '''Find the running U-Boot's RAM location.
-
-        Probe the running U-Boot to determine the address of the first bank
-        of RAM. This is useful for tests that test reading/writing RAM, or
-        load/save files that aren't associated with some standard address
-        typically represented in an environment variable such as
-        ${kernel_addr_r}. The value is cached so that it only needs to be
-        actively read once.
-
-        Args:
-            None.
-
-        Returns:
-            The address of U-Boot's first RAM bank, as an integer.
-        '''
-
-        if self.config.buildconfig.get('config_cmd_bdi', 'n') != 'y':
-            pytest.skip('bdinfo command not supported')
-        if self.ram_base == -1:
-            pytest.skip('Previously failed to find RAM bank start')
-        if self.ram_base is not None:
-            return self.ram_base
-
-        with self.log.section('find_ram_base'):
-            response = self.run_command('bdinfo')
-            for l in response.split('\n'):
-                if '-> start' in l:
-                    self.ram_base = int(l.split('=')[1].strip(), 16)
-                    break
-            if self.ram_base is None:
-                self.ram_base = -1
-                raise Exception('Failed to find RAM bank start in `bdinfo`')
-
-        return self.ram_base

test/py/u_boot_utils.py +38 -0

index 539af618dbf2..522390a207ef 100644
@@ -169,3 +169,41 @@ def run_and_log(u_boot_console, cmd, ignore_errors=False):
     runner = u_boot_console.log.get_runner(cmd[0], sys.stdout)
     runner.run(cmd, ignore_errors=ignore_errors)
     runner.close()
+
+ram_base = None
+def find_ram_base(u_boot_console):
+    '''Find the running U-Boot's RAM location.
+
+    Probe the running U-Boot to determine the address of the first bank
+    of RAM. This is useful for tests that test reading/writing RAM, or
+    load/save files that aren't associated with some standard address
+    typically represented in an environment variable such as
+    ${kernel_addr_r}. The value is cached so that it only needs to be
+    actively read once.
+
+    Args:
+        u_boot_console: A console connection to U-Boot.
+
+    Returns:
+        The address of U-Boot's first RAM bank, as an integer.
+    '''
+
+    global ram_base
+    if u_boot_console.config.buildconfig.get('config_cmd_bdi', 'n') != 'y':
+        pytest.skip('bdinfo command not supported')
+    if ram_base == -1:
+        pytest.skip('Previously failed to find RAM bank start')
+    if ram_base is not None:
+        return ram_base
+
+    with u_boot_console.log.section('find_ram_base'):
+        response = u_boot_console.run_command('bdinfo')
+        for l in response.split('\n'):
+            if '-> start' in l:
+                ram_base = int(l.split('=')[1].strip(), 16)
+                break
+        if ram_base is None:
+            ram_base = -1
+            raise Exception('Failed to find RAM bank start in `bdinfo`')
+
+    return ram_base

Signature · 1 line

2.7.0

Stephen Warren

author

2016-01-21 23:05 UTC #1 lore raw

PATCH 2/2 test/py: add a networking test

From: Stephen Warren <[email protected]>

This tests:
- dhcp (if indicated by boardenv file).
- Static IP network setup (if provided by boardenv file).
- Ping.
- TFTP get.

Signed-off-by Stephen Warren <[email protected]>

---
 test/py/tests/test_net.py | 153 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)
 create mode 100644 test/py/tests/test_net.py

test/py/tests/test_net.py new file +153 -0

new file mode 100644
index 000000000000..c73854ea74e0
@@ -0,0 +1,153 @@
+# Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
+#
+# SPDX-License-Identifier: GPL-2.0
+
+# Test various network-related functionality, such as the dhcp, ping, and
+# tftpboot commands.
+
+import pytest
+import u_boot_utils
+
+'''
+Note: This test relies on boardenv_* containing configuration values to define
+which the network environment available for testing. Without this, this test
+will be automatically skipped.
+
+For example:
+
+# Any commands that need to be executed prior to testing,
+# to get the network hardware into an operational state.
+#
+# If no commands are required, this variable may be omitted, or set to an
+# empty list.
+env__net_pre_commands = [
+    "usb start",
+]
+
+# True if a DHCP server is attached to the network, and should be tested.
+# If DHCP testing is not possible or desired, this variable may be omitted or
+# set to False.
+env__net_dhcp_server = True
+
+# A list of environment variables that should be set in order to configure a
+# static IP. If solely relying on DHCP, this variable may be omitted or set to
+# an empty list.
+env__net_static_env_vars = [
+    ("ipaddr", "10.0.0.100"),
+    ("netmask", "255.255.255.0"),
+    ("serverip", "10.0.0.1"),
+]
+
+# Details regarding a file that may be read from a TFTP server. This variable
+# may be omitted or set to None if TFTP testing is not possible or desired.
+env__net_tftp_readable_file = {
+    "fn": "ubtest-readable.bin",
+    "size": 5058624,
+    "crc32": "c2244b26",
+}
+'''
+
+net_set_up = False
+
+def test_net_pre_commands(u_boot_console):
+    '''Execute any commands required to enable network hardware.
+
+    These commands are provided by the boardenv_* file; see the comment at the
+    beginning of this file.
+    '''
+
+    cmds = u_boot_console.config.env.get('env__net_pre_commands', None)
+    if not cmds:
+        pytest.skip('No network pre-commands defined')
+
+    for cmd in cmds:
+        u_boot_console.run_command(cmd)
+
+ at pytest.mark.buildconfigspec('cmd_dhcp')
+def test_net_dhcp(u_boot_console):
+    '''Test the dhcp command.
+
+    The boardenv_* file may be used to enable/disable this test; see the
+    comment at the beginning of this file.
+    '''
+
+    test_dhcp = u_boot_console.config.env.get('env__net_dhcp_server', False)
+    if not test_dhcp:
+        pytest.skip('No DHCP server available')
+
+    u_boot_console.run_command('setenv autoload no')
+    output = u_boot_console.run_command('dhcp')
+    assert 'DHCP client bound to address ' in output
+
+    global net_set_up
+    net_set_up = True
+
+ at pytest.mark.buildconfigspec('net')
+def test_net_setup_static(u_boot_console):
+    '''Set up a static IP configuration.
+
+    The configuration is provided by the boardenv_* file; see the comment at
+    the beginning of this file.
+    '''
+
+    env_vars = u_boot_console.config.env.get('env__net_static_env_vars', None)
+    if not env_vars:
+        pytest.skip('No static network configuration is defined')
+
+    for (var, val) in env_vars:
+        u_boot_console.run_command('setenv %s %s' % (var, val))
+
+    global net_set_up
+    net_set_up = True
+
+ at pytest.mark.buildconfigspec('cmd_ping')
+def test_net_ping(u_boot_console):
+    '''Test the ping command.
+
+    The $serverip (as set up by either test_net_dhcp or test_net_setup_static)
+    is pinged. The test validates that the host is alive, as reported by the
+    ping command's output.
+    '''
+
+    if not net_set_up:
+        pytest.skip("Network not initialized")
+
+    output = u_boot_console.run_command('ping $serverip')
+    assert 'is alive' in output
+
+ at pytest.mark.buildconfigspec('cmd_net')
+def test_net_tftpboot(u_boot_console):
+    '''Test the tftpboot command.
+
+    A file is downloaded from the TFTP server, its size and optionally its
+    CRC32 are validated.
+
+    The details of the file to download are provided by the boardenv_* file;
+    see the comment at the beginning of this file.
+    '''
+
+    if not net_set_up:
+        pytest.skip("Network not initialized")
+
+    f = u_boot_console.config.env.get('env__net_tftp_readable_file', None)
+    if not f:
+        pytest.skip('No TFTP readable file to read')
+
+    addr = u_boot_utils.find_ram_base(u_boot_console)
+    fn = f['fn']
+    output = u_boot_console.run_command('tftpboot %x %s' % (addr, fn))
+    expected_text = 'Bytes transferred = '
+    sz = f.get('size', None)
+    if sz:
+        expected_text += '%d' % sz
+    assert expected_text in output
+
+    expected_crc = f.get('crc32', None)
+    if not expected_crc:
+        return
+
+    if u_boot_console.config.buildconfig.get('config_cmd_crc32', 'n') != 'y':
+        return
+
+    output = u_boot_console.run_command('crc32 %x $filesize' % addr)
+    assert expected_crc in output

Signature · 1 line

2.7.0

Simon Glass

2016-01-26 01:15 UTC #3 lore raw

PATCH 2/2 test/py: add a networking test

Quoted 9 lines · Simon Glass

Quoted 8 lines · Stephen Warren
From: Stephen Warren <[email protected]>
This tests:
- dhcp (if indicated by boardenv file).
- Static IP network setup (if provided by boardenv file).
- Ping.
- TFTP get.

Quoted 7 lines

Quoted 5 lines
Signed-off-by Stephen Warren <[email protected]>
---
 test/py/tests/test_net.py | 153 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)
 create mode 100644 test/py/tests/test_net.py
Acked-by Simon Glass <[email protected]>

Applied to u-boot-dm, thanks!

Wolfgang Denk

2016-01-22 22:30 UTC #5 lore raw

PATCH 1/2 test/py: move find_ram_base() into u_boot_utils

In reply to Stephen Warren

Dear Stephen,

Quoted 3 lines · In message <[email protected]> you wrote:

find_ram_base() is a shared utility function, not a core part of the U-Boot console interaction.

On which boards did you test this feature? Eventually ARM only?

test/py/u_boot_utils.py def run_and_log(u_boot_console, cmd, ignore_errors=False): · line 204

Quoted 6 lines

test/py/u_boot_utils.py +6 -0

+    with u_boot_console.log.section('find_ram_base'):
+        response = u_boot_console.run_command('bdinfo')
+        for l in response.split('\n'):
+            if '-> start' in l:
+                ram_base = int(l.split('=')[1].strip(), 16)
+                break

Searching for "-> start" is probably not exactly portable. For example, on a PowerPC system the output of "bdi" might look like this:

=> bdi

memstart    = 0x00000000
memsize     = 0x04000000
flashstart  = 0xFC000000
flashsize   = 0x02000000
flashoffset = 0x0005D000
sramstart   = 0x00000000
sramsize    = 0x00000000
bootflags   = 0x00000000
intfreq     =    396 MHz
busfreq     =    132 MHz
ethaddr     = 00:D0:93:2A:C2:1A
IP addr     = 192.168.240.240
baudrate    = 115200 bps
relocaddr   = 0x03F47000

[example is from a TQM5200S, U-Boot 2016.01-00223-gb57843e]

Best regards,

Wolfgang Denk

Signature · 5 lines

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: wd at denx.de
When choosing between two evils, I always like to take the  one  I've
never tried before.                     -- Mae West, "Klondike Annie"

Stephen Warren

author

2016-01-25 16:50 UTC #6 lore raw

PATCH 1/2 test/py: move find_ram_base() into u_boot_utils

Quoted 8 lines · Wolfgang Denk

Dear Stephen,
Quoted 3 lines · In message <[email protected]> you wrote:
find_ram_base() is a shared utility function, not a core part of the U-Boot console interaction.
On which boards did you test this feature? Eventually ARM only?

It's been tested on a few ARM, sandbox, and at least one microblaze.

Quoted 13 lines

test/py/u_boot_utils.py def run_and_log(u_boot_console, cmd, ignore_errors=False): · line 204
Quoted 6 lines
test/py/u_boot_utils.py +6 -0
+    with u_boot_console.log.section('find_ram_base'):
+        response = u_boot_console.run_command('bdinfo')
+        for l in response.split('\n'):
+            if '-> start' in l:
+                ram_base = int(l.split('=')[1].strip(), 16)
+                break
Searching for "-> start" is probably not exactly portable. For example, on a PowerPC system the output of "bdi" might look like this:
=> bdi
memstart = 0x00000000
memsize = 0x04000000

...

Quoted 2 lines

[example is from a TQM5200S, U-Boot 2016.01-00223-gb57843e]

Good point. I think the best fix here is to modify all implementations of "bdinfo" to print the same information and in the same format as much as possible. Do you agree?

Simon Glass

2016-01-26 01:03 UTC #7 lore raw

PATCH 1/2 test/py: move find_ram_base() into u_boot_utils

Hi Stephen,

Quoted 35 lines · Stephen Warren

Quoted 7 lines · Wolfgang Denk
Dear Stephen,
In message <[email protected]> you
Quoted 2 lines · wrote:
Quoted 5 lines
Quoted 2 lines
find_ram_base() is a shared utility function, not a core part of the U-Boot console interaction.
On which boards did you test this feature? Eventually ARM only?
It's been tested on a few ARM, sandbox, and at least one microblaze.
Quoted 14 lines
test/py/u_boot_utils.py def run_and_log(u_boot_console, cmd, ignore_errors=False): · line 204
Quoted 6 lines
test/py/u_boot_utils.py +6 -0
+    with u_boot_console.log.section('find_ram_base'):
+        response = u_boot_console.run_command('bdinfo')
+        for l in response.split('\n'):
+            if '-> start' in l:
+                ram_base = int(l.split('=')[1].strip(), 16)
+                break
Searching for "-> start" is probably not exactly portable. For example, on a PowerPC system the output of "bdi" might look like this:
=> bdi
memstart = 0x00000000
memsize = 0x04000000
...
Quoted 2 lines

Quoted 6 lines

Quoted 1 line
[example is from a TQM5200S, U-Boot 2016.01-00223-gb57843e]
Good point. I think the best fix here is to modify all implementations of "bdinfo" to print the same information and in the same format as much as possible. Do you agree?

Yes - and the best way to do this is to use the same code for all boards if possible.

BTW I can't apply this patch as the u_boot_utils.py file is missing. Can you please rebase and resend?

Regards,
Simon

Stephen Warren

author

2016-01-26 01:09 UTC #8 lore raw

PATCH 1/2 test/py: move find_ram_base() into u_boot_utils

Quoted 45 lines · Simon Glass

Hi Stephen,
Quoted 35 lines · Stephen Warren
Quoted 7 lines · Wolfgang Denk
Dear Stephen,
In message <[email protected]> you
Quoted 2 lines · wrote:
Quoted 5 lines
Quoted 2 lines
find_ram_base() is a shared utility function, not a core part of the U-Boot console interaction.
On which boards did you test this feature? Eventually ARM only?
It's been tested on a few ARM, sandbox, and at least one microblaze.
Quoted 14 lines
test/py/u_boot_utils.py def run_and_log(u_boot_console, cmd, ignore_errors=False): · line 204
Quoted 6 lines
test/py/u_boot_utils.py +6 -0
+    with u_boot_console.log.section('find_ram_base'):
+        response = u_boot_console.run_command('bdinfo')
+        for l in response.split('\n'):
+            if '-> start' in l:
+                ram_base = int(l.split('=')[1].strip(), 16)
+                break
Searching for "-> start" is probably not exactly portable. For example, on a PowerPC system the output of "bdi" might look like this:
=> bdi
memstart = 0x00000000
memsize = 0x04000000
...
Quoted 2 lines
Quoted 6 lines
Quoted 1 line
[example is from a TQM5200S, U-Boot 2016.01-00223-gb57843e]
Good point. I think the best fix here is to modify all implementations of "bdinfo" to print the same information and in the same format as much as possible. Do you agree?

Quoted 5 lines

Yes - and the best way to do this is to use the same code for all boards if possible.
BTW I can't apply this patch as the u_boot_utils.py file is missing. Can you please rebase and resend?

Do you have "test/py: add various utility code" already applied? That creates u_boot_utils.py. As mentioned in the original patch email, this series depends on the series that contains that patch. You had replied earlier that you had applied that series in u-boot-dm.

Simon Glass

2016-01-26 01:15 UTC #9 lore raw

PATCH 1/2 test/py: move find_ram_base() into u_boot_utils

Hi Stephen,

Quoted 60 lines · Stephen Warren

Quoted 59 lines · Simon Glass
Hi Stephen,
Quoted 43 lines · Stephen Warren
Quoted 9 lines · Wolfgang Denk
Dear Stephen,
In message <[email protected]> you
Quoted 3 lines · wrote:
Quoted 6 lines
Quoted 2 lines
find_ram_base() is a shared utility function, not a core part of the U-Boot console interaction.
On which boards did you test this feature? Eventually ARM only?
It's been tested on a few ARM, sandbox, and at least one microblaze.
Quoted 15 lines
test/py/u_boot_utils.py def run_and_log(u_boot_console, cmd, ignore_errors=False): · line 204
Quoted 6 lines
test/py/u_boot_utils.py +6 -0
+    with u_boot_console.log.section('find_ram_base'):
+        response = u_boot_console.run_command('bdinfo')
+        for l in response.split('\n'):
+            if '-> start' in l:
+                ram_base = int(l.split('=')[1].strip(), 16)
+                break
Searching for "-> start" is probably not exactly portable. For example, on a PowerPC system the output of "bdi" might look like this:
=> bdi
memstart = 0x00000000
memsize = 0x04000000
...
Quoted 3 lines
Quoted 7 lines
Quoted 1 line
[example is from a TQM5200S, U-Boot 2016.01-00223-gb57843e]
Good point. I think the best fix here is to modify all implementations of "bdinfo" to print the same information and in the same format as much as possible. Do you agree?
Yes - and the best way to do this is to use the same code for all boards if possible.

Quoted 8 lines

Quoted 2 lines
BTW I can't apply this patch as the u_boot_utils.py file is missing. Can you please rebase and resend?
Do you have "test/py: add various utility code" already applied? That creates u_boot_utils.py. As mentioned in the original patch email, this series depends on the series that contains that patch. You had replied earlier that you had applied that series in u-boot-dm.

Ah yes, user error, sorry.

BTW re your question about """ for comments, please see PEP8 etc.:

http://legacy.python.org/dev/peps/pep-0008/#block-comments
https://www.python.org/dev/peps/pep-0257/

Applied to u-boot-dm, thanks!

Regards,
Simon

Stephen Warren

author

2016-01-26 18:13 UTC #10 lore raw

PATCH 1/2 test/py: move find_ram_base() into u_boot_utils

Quoted 72 lines · Simon Glass

Hi Stephen,
Quoted 60 lines · Stephen Warren
Quoted 59 lines · Simon Glass
Hi Stephen,
Quoted 43 lines · Stephen Warren
Quoted 9 lines · Wolfgang Denk
Dear Stephen,
In message <[email protected]> you
Quoted 3 lines · wrote:
Quoted 6 lines
Quoted 2 lines
find_ram_base() is a shared utility function, not a core part of the U-Boot console interaction.
On which boards did you test this feature? Eventually ARM only?
It's been tested on a few ARM, sandbox, and at least one microblaze.
Quoted 15 lines
test/py/u_boot_utils.py def run_and_log(u_boot_console, cmd, ignore_errors=False): · line 204
Quoted 6 lines
test/py/u_boot_utils.py +6 -0
+    with u_boot_console.log.section('find_ram_base'):
+        response = u_boot_console.run_command('bdinfo')
+        for l in response.split('\n'):
+            if '-> start' in l:
+                ram_base = int(l.split('=')[1].strip(), 16)
+                break
Searching for "-> start" is probably not exactly portable. For example, on a PowerPC system the output of "bdi" might look like this:
=> bdi
memstart = 0x00000000
memsize = 0x04000000
...
Quoted 3 lines
Quoted 7 lines
Quoted 1 line
[example is from a TQM5200S, U-Boot 2016.01-00223-gb57843e]
Good point. I think the best fix here is to modify all implementations of "bdinfo" to print the same information and in the same format as much as possible. Do you agree?
Yes - and the best way to do this is to use the same code for all boards if possible.
Quoted 8 lines
Quoted 2 lines
BTW I can't apply this patch as the u_boot_utils.py file is missing. Can you please rebase and resend?
Do you have "test/py: add various utility code" already applied? That creates u_boot_utils.py. As mentioned in the original patch email, this series depends on the series that contains that patch. You had replied earlier that you had applied that series in u-boot-dm.

Quoted 6 lines

Ah yes, user error, sorry.
BTW re your question about """ for comments, please see PEP8 etc.:
http://legacy.python.org/dev/peps/pep-0008/#block-comments
https://www.python.org/dev/peps/pep-0257/

OK, I see the recommendation to use """ for docstrings. Can we also use " rather than ' for regular string too please, to avoid mixing different quote characters?

Simon Glass

2016-01-26 19:59 UTC #11 lore raw

PATCH 1/2 test/py: move find_ram_base() into u_boot_utils

Hi Stephen,

Quoted 96 lines · Stephen Warren

Quoted 95 lines · Simon Glass
Hi Stephen,
Quoted 81 lines · Stephen Warren
Quoted 71 lines · Simon Glass
Hi Stephen,
Quoted 57 lines · Stephen Warren
Quoted 12 lines · Wolfgang Denk
Dear Stephen,
In message <[email protected]> you
Quoted 4 lines · wrote:
Quoted 7 lines
Quoted 2 lines
find_ram_base() is a shared utility function, not a core part of the U-Boot console interaction.
On which boards did you test this feature? Eventually ARM only?
It's been tested on a few ARM, sandbox, and at least one microblaze.
Quoted 16 lines
Review
Quoted 6 lines
(quoted diff) +6 -0
+    with u_boot_console.log.section('find_ram_base'):
+        response = u_boot_console.run_command('bdinfo')
+        for l in response.split('\n'):
+            if '-> start' in l:
+                ram_base = int(l.split('=')[1].strip(), 16)
+                break
Searching for "-> start" is probably not exactly portable. For example, on a PowerPC system the output of "bdi" might look like this:
=> bdi
memstart = 0x00000000
memsize = 0x04000000
...
Quoted 5 lines
[example is from a TQM5200S, U-Boot 2016.01-00223-gb57843e]
Quoted 5 lines
Good point. I think the best fix here is to modify all implementations of
"bdinfo" to print the same information and in the same format as much as
possible. Do you agree?
Quoted 5 lines
Yes - and the best way to do this is to use the same code for all boards if possible.
BTW I can't apply this patch as the u_boot_utils.py file is missing. Can you please rebase and resend?
Quoted 4 lines
Do you have "test/py: add various utility code" already applied? That creates u_boot_utils.py. As mentioned in the original patch email, this series depends on the series that contains that patch. You had replied earlier that you had applied that series in u-boot-dm.
Ah yes, user error, sorry.
BTW re your question about """ for comments, please see PEP8 etc.:

Quoted 7 lines

Quoted 2 lines
http://legacy.python.org/dev/peps/pep-0008/#block-comments
https://www.python.org/dev/peps/pep-0257/
OK, I see the recommendation to use """ for docstrings. Can we also use " rather than ' for regular string too please, to avoid mixing different quote characters?

That's the style used for patman/buildman. I think it's actually good to have them different. You will sometimes hit the case where you need a quoted double quote, like print 'This is a "test" of things'.

Regards,
Simon

Stephen Warren

author

2016-01-26 20:08 UTC #12 lore raw

PATCH 1/2 test/py: move find_ram_base() into u_boot_utils

Quoted 103 lines · Simon Glass

Hi Stephen,
Quoted 100 lines · Stephen Warren
Quoted 97 lines · Simon Glass
Hi Stephen,
Quoted 81 lines · Stephen Warren
Quoted 71 lines · Simon Glass
Hi Stephen,
Quoted 57 lines · Stephen Warren
Quoted 12 lines · Wolfgang Denk
Dear Stephen,
In message <[email protected]> you
Quoted 4 lines · wrote:
Quoted 7 lines
Quoted 2 lines
find_ram_base() is a shared utility function, not a core part of the U-Boot console interaction.
On which boards did you test this feature? Eventually ARM only?
It's been tested on a few ARM, sandbox, and at least one microblaze.
Quoted 16 lines
Review
Quoted 6 lines
(quoted diff) +6 -0
+    with u_boot_console.log.section('find_ram_base'):
+        response = u_boot_console.run_command('bdinfo')
+        for l in response.split('\n'):
+            if '-> start' in l:
+                ram_base = int(l.split('=')[1].strip(), 16)
+                break
Searching for "-> start" is probably not exactly portable. For example, on a PowerPC system the output of "bdi" might look like this:
=> bdi
memstart = 0x00000000
memsize = 0x04000000
...
Quoted 5 lines
[example is from a TQM5200S, U-Boot 2016.01-00223-gb57843e]
Quoted 5 lines
Good point. I think the best fix here is to modify all implementations of
"bdinfo" to print the same information and in the same format as much as
possible. Do you agree?
Quoted 5 lines
Yes - and the best way to do this is to use the same code for all boards if possible.
BTW I can't apply this patch as the u_boot_utils.py file is missing. Can you please rebase and resend?
Quoted 4 lines
Do you have "test/py: add various utility code" already applied? That creates u_boot_utils.py. As mentioned in the original patch email, this series depends on the series that contains that patch. You had replied earlier that you had applied that series in u-boot-dm.
Ah yes, user error, sorry.
BTW re your question about """ for comments, please see PEP8 etc.:
http://legacy.python.org/dev/peps/pep-0008/#block-comments
https://www.python.org/dev/peps/pep-0257/

Quoted 7 lines

Quoted 3 lines
OK, I see the recommendation to use """ for docstrings. Can we also use " rather than ' for regular string too please, to avoid mixing different quote characters?
That's the style used for patman/buildman. I think it's actually good to have them different. You will sometimes hit the case where you need a quoted double quote, like print 'This is a "test" of things'.

There are counter-cases where ' needs to be escaped in the current scheme too, e.g.:

Quoted 1 line

assert('Unknown command \'non_existent_cmd\' - try \'help\'' in response)

... although I haven't quantified which way around would lead to more escapes.

I'll just convert the docstrings for now. Since you've acked/reviewed everything I have sent so far, I'll build my patch on top of all those patches.