[U-Boot] [PATCH 2/3] tests: py: dfu: Add functionality to set different u-boot's dfu env variable

[Lukasz Majewski]

On Fri, 8 Apr 2016 10:28:06 -0600
Stephen Warren <swarren at wwwdotorg.org> wrote:

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

This indeed could be done a bit differently. 

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

+1

> 
> >       },
> >   )
> >
> > @@ -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')

This is the idiom which I was looking for :-). Thanks for hint.
I will rewrite it in such way.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160408/6a88f32c/attachment.sig>