[U-Boot] [PATCH 1/2] test/py: move find_ram_base() into u_boot_utils

[U-Boot] [PATCH 1/2] test/py: move find_ram_base() into u_boot_utils

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

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

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
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(-)

diff --git a/test/py/tests/test_md.py b/test/py/tests/test_md.py
index 94603c7df609..32cce4f15c53 100644
--- a/test/py/tests/test_md.py
+++ b/test/py/tests/test_md.py
@@ -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))
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index 06f61f987180..51163bc0db68 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -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
diff --git a/test/py/u_boot_utils.py b/test/py/u_boot_utils.py
index 539af618dbf2..522390a207ef 100644
--- a/test/py/u_boot_utils.py
+++ b/test/py/u_boot_utils.py
@@ -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
-- 
2.7.0

[U-Boot] [PATCH 2/2] test/py: add a networking test

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

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 <swarren@nvidia.com>
---
 test/py/tests/test_net.py | 153 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)
 create mode 100644 test/py/tests/test_net.py

diff --git a/test/py/tests/test_net.py b/test/py/tests/test_net.py
new file mode 100644
index 000000000000..c73854ea74e0
--- /dev/null
+++ b/test/py/tests/test_net.py
@@ -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
-- 
2.7.0

[U-Boot] [PATCH 1/2] test/py: move find_ram_base() into u_boot_utils

[Simon Glass]

On 21 January 2016 at 16:05, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> find_ram_base() is a shared utility function, not a core part of the
> U-Boot console interaction.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> 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(-)

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 2/2] test/py: add a networking test

[Simon Glass]

On 21 January 2016 at 16:05, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> 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 <swarren@nvidia.com>
> ---
>  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 <sjg@chromium.org>

[U-Boot] [PATCH 1/2] test/py: move find_ram_base() into u_boot_utils

[Wolfgang Denk]

Dear Stephen,

In message <1453417531-23669-1-git-send-email-swarren@wwwdotorg.org> 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?

> +    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

-- 
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"

[U-Boot] [PATCH 1/2] test/py: move find_ram_base() into u_boot_utils

[Stephen Warren]

On 01/22/2016 03:30 PM, Wolfgang Denk wrote:
> Dear Stephen,
>
> In message <1453417531-23669-1-git-send-email-swarren@wwwdotorg.org> 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.

>> +    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
...
>
> [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?

[U-Boot] [PATCH 1/2] test/py: move find_ram_base() into u_boot_utils

[Simon Glass]

Hi Stephen,

On 25 January 2016 at 09:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 01/22/2016 03:30 PM, Wolfgang Denk wrote:
>>
>> Dear Stephen,
>>
>> In message <1453417531-23669-1-git-send-email-swarren@wwwdotorg.org> 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.
>
>>> +    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
>
> ...
>>
>>
>> [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

[U-Boot] [PATCH 1/2] test/py: move find_ram_base() into u_boot_utils

[Stephen Warren]

On 01/25/2016 06:03 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 25 January 2016 at 09:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 01/22/2016 03:30 PM, Wolfgang Denk wrote:
>>>
>>> Dear Stephen,
>>>
>>> In message <1453417531-23669-1-git-send-email-swarren@wwwdotorg.org> 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.
>>
>>>> +    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
>>
>> ...
>>>
>>>
>>> [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?

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.

[U-Boot] [PATCH 1/2] test/py: move find_ram_base() into u_boot_utils

[Simon Glass]

Hi Stephen,

On 25 January 2016 at 18:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 01/25/2016 06:03 PM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 25 January 2016 at 09:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 01/22/2016 03:30 PM, Wolfgang Denk wrote:
>>>>
>>>>
>>>> Dear Stephen,
>>>>
>>>> In message <1453417531-23669-1-git-send-email-swarren@wwwdotorg.org> 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.
>>>
>>>>> +    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
>>>
>>>
>>> ...
>>>>
>>>>
>>>>
>>>> [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?
>
>
> 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

[U-Boot] [PATCH 2/2] test/py: add a networking test

[Simon Glass]

On 21 January 2016 at 20:36, Simon Glass <sjg@chromium.org> wrote:
> On 21 January 2016 at 16:05, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> 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 <swarren@nvidia.com>
>> ---
>>  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 <sjg@chromium.org>

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH 1/2] test/py: move find_ram_base() into u_boot_utils

[Stephen Warren]

On 01/25/2016 06:15 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 25 January 2016 at 18:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 01/25/2016 06:03 PM, Simon Glass wrote:
>>>
>>> Hi Stephen,
>>>
>>> On 25 January 2016 at 09:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>
>>>> On 01/22/2016 03:30 PM, Wolfgang Denk wrote:
>>>>>
>>>>>
>>>>> Dear Stephen,
>>>>>
>>>>> In message <1453417531-23669-1-git-send-email-swarren@wwwdotorg.org> 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.
>>>>
>>>>>> +    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
>>>>
>>>>
>>>> ...
>>>>>
>>>>>
>>>>>
>>>>> [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?
>>
>>
>> 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/

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?

[U-Boot] [PATCH 1/2] test/py: move find_ram_base() into u_boot_utils

[Simon Glass]

Hi Stephen,

On 26 January 2016 at 11:13, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 01/25/2016 06:15 PM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 25 January 2016 at 18:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 01/25/2016 06:03 PM, Simon Glass wrote:
>>>>
>>>>
>>>> Hi Stephen,
>>>>
>>>> On 25 January 2016 at 09:50, Stephen Warren <swarren@wwwdotorg.org>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 01/22/2016 03:30 PM, Wolfgang Denk wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Dear Stephen,
>>>>>>
>>>>>> In message <1453417531-23669-1-git-send-email-swarren@wwwdotorg.org>
>>>>>> 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.
>>>>>
>>>>>>> +    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
>>>>>
>>>>>
>>>>>
>>>>> ...
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> [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?
>>>
>>>
>>>
>>> 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/
>
>
> 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

[U-Boot] [PATCH 1/2] test/py: move find_ram_base() into u_boot_utils

[Stephen Warren]

On 01/26/2016 12:59 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 26 January 2016 at 11:13, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 01/25/2016 06:15 PM, Simon Glass wrote:
>>>
>>> Hi Stephen,
>>>
>>> On 25 January 2016 at 18:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>
>>>> On 01/25/2016 06:03 PM, Simon Glass wrote:
>>>>>
>>>>>
>>>>> Hi Stephen,
>>>>>
>>>>> On 25 January 2016 at 09:50, Stephen Warren <swarren@wwwdotorg.org>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> On 01/22/2016 03:30 PM, Wolfgang Denk wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Dear Stephen,
>>>>>>>
>>>>>>> In message <1453417531-23669-1-git-send-email-swarren@wwwdotorg.org>
>>>>>>> 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.
>>>>>>
>>>>>>>> +    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
>>>>>>
>>>>>>
>>>>>>
>>>>>> ...
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> [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?
>>>>
>>>>
>>>>
>>>> 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/
>>
>>
>> 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.:

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