From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Fri, 8 Apr 2016 10:28:06 -0600
Subject: [U-Boot] [PATCH 2/3] tests: py: dfu: Add functionality to set
 different u-boot's dfu env variable
In-Reply-To: <1460130291-24223-2-git-send-email-l.majewski@samsung.com>
References: <1460130291-24223-1-git-send-email-l.majewski@samsung.com>
 <1460130291-24223-2-git-send-email-l.majewski@samsung.com>
Message-ID: <5707DC16.1090009@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 04/08/2016 09:44 AM, Lukasz Majewski wrote:
> By default (on almost all systems) the dfu env variable, which defines
> available alt settings, is named as "dfu_alt_info".
>
> However on some platforms (i.e. Odroid XU3), the 'dfu_alt_info' is concatenated
> from other variables - namely 'dfu_alt_boot' and 'dfu_alt_system' at run time
> (when one types 'dfu 0 mmc 0' for first time).
>
> 'dfu_alt_boot' describes alt settings which depend on boot medium - for example
> boot loader's LBA sectors which are different on eMMC and SD card because of e.g.
> MBR/GPT.
>
> 'dfu_alt_system' describes board agnostic alt settings - like rootfs, kernel.
> On such system we can only append/modify this env variable.
>
> Because of the above, we must have way to modify other than "dfu_ale_info"
> variable to perform tests.

Uggh. That's a pretty nasty user-experience since it prevents the user 
from controlling what happens. U-Boot should never over-ride what the 
user requests for alt_info, but rather simply provide help to the user 
allowing them to make use of the system-supplied information if they 
want. I.e. dfu_alt_boot should be set at boot time based on code. The 
user should then be free to define dfu_alt_info as they see fit; either 
hard-coded, or including the value of $dfu_alt_boot /if/ they want.

Anyway, I suppose since this feature exists, we have no choice but to 
support it.

> diff --git a/test/py/tests/test_dfu.py b/test/py/tests/test_dfu.py
> index df3b561..1ed6020 100644
> --- a/test/py/tests/test_dfu.py
> +++ b/test/py/tests/test_dfu.py
> @@ -44,6 +44,11 @@ env__dfu_configs = (
>           # configurations, but don't want to test every single transfer size
>           # on each, to avoid bloating the overall time taken by testing.
>           "test_sizes": (63, 64, 65),
> +        # Optional values.
> +        # Those values are necessary on boards like Odroid XU3, where
> +        # - dfu alt info env variable is concatenated from boot medium dependent
> +        #   (dfu_alt_boot) and user defined (dfu_alt_system) variables
> +        "alt_info_env_name": "dfu_alt_system",

I think the "- " and leading space should be dropped; this isn't a list 
of items. I'd suggest re-phrasing this as:

# The name of the environment variable that the the dfu command reads
# alt info from. If unspecified, this defaults to dfu_alt_info, which is
# valid for most systems. Some systems use a different variable name.
# One example is the Odroid XU3,  which automatically generates
# $dfu_alt_info, each time the dfu command is run, by concatenating
# $dfu_alt_boot and $dfu_alt_system.

>       },
>   )
>
> @@ -124,7 +129,12 @@ def test_dfu(u_boot_console, env__usb_dev_port, env__dfu_config):
>           u_boot_console.log.action(
>               'Starting long-running U-Boot dfu shell command')
>
> -        cmd = 'setenv dfu_alt_info "%s"' % env__dfu_config['alt_info']
> +        dfu_alt_info_env = "dfu_alt_info"
> +        if "alt_info_env_name" in env__dfu_config:
> +	        dfu_alt_info_env = env__dfu_config['alt_info_env_name']

That can be just:
dfu_alt_info_env = env__dfu_config.get('alt_info_env_name', \ 
'dfu_alt_info')

> +
> +        cmd = 'setenv "%s" "%s"' % (dfu_alt_info_env,
> +                                    env__dfu_config['alt_info'])
>           u_boot_console.run_command(cmd)
>
>           cmd = 'dfu 0 ' + env__dfu_config['cmd_params']