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