[PATCH 2/8] spl: fit: Factor out FIT parsing and use a context struct

[Alex G.]

On 12/21/20 2:23 PM, Simon Glass wrote:
> Hi Alex,
> 
> On Mon, 21 Dec 2020 at 12:28, Alex G. <mr.nuke.me at gmail.com> wrote:
>>
>> On 12/18/20 8:28 PM, Simon Glass wrote:
>>> Hi Alexandru,
>>>
>>> On Tue, 15 Dec 2020 at 17:09, Alexandru Gagniuc <mr.nuke.me at gmail.com> wrote:
>>>>
>>>> The logical steps in spl_load_simple_fit() are difficult to follow.
>>>> I think the long comments, ifdefs, and ungodly number of variables
>>>> seriously affect the readability. In particular, it violates section 6
>>>> of the coding style, paragraphs (3), and (4).
>>>>
>>>> The purpose of this patch is to improve the situation by
>>>>     - Factoring out initialization and parsing to separate functions
>>>>     - Reduce the number of variables by using a context structure
>>>> This change introduces no functional changes.
>>>>
>>>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me at gmail.com>
>>>> ---
>>>>    common/spl/spl_fit.c | 87 ++++++++++++++++++++++++++++++--------------
>>>>    1 file changed, 60 insertions(+), 27 deletions(-)
>>>
>>> This certainly looks a lot better although your email address does not
>>> inspire confidence...
>>
>> Don't worry. It doesn't bite.
>>
>>> Do you think you could look at creating a sandbox SPL test for this?
>>> It should be possible to write it in C, set up a bit of data, call
>>> your function and check the results.
>>
>> I can look at it. I can't promise anything though, since this is the
>> first time I heard of the sandbox. Maybe doc knows more.
> 
> Yes, see doc/arch/sandbox.rst
> 
> test/dm/Makefile shows that only one test file is enabled for SPL, but
> you can add more. Let me know if you need pointers.
> 
> These aliases might help, if you build into /tmp/b/<board> :
> 
> # Run a pytest on sandbox
> # $1: Name of test to run (optional, else run all)
> 
> function pyt {
> test/py/test.py -B sandbox --build-dir /tmp/b/sandbox ${1:+"-k $1"}
> }
> 
> # Run a pytest on sandbox_spl
> # $1: Name of test to run  (optional, else run all SPL tests)
> function pytspl {
> local run=$1
> 
> [[ -z "$run" ]] && run=spl
> test/py/test.py -B sandbox_spl --build-dir /tmp/b/sandbox_spl -k $run
> }

You're thinking way ahead of where I am. I know how to build a board, 
but I've never used the test infrastructure. After some fumbling, I 
figured I'd try sandbox_spl:

	$ test/py/test.py -B sandbox_spl --bd sandbox_spl --build

As you can imagine, it kept complaining about SDL. I've never used 
environment variables with Kbuild, so using NO_SPL=1 seems unnatural. 
But then why would we need SDL for testing an SPL build anyway? 'swig' 
was missing too, but that was an easy fix.

Second try:

	$ NO_SDL=1 test/py/test.py -B sandbox_spl --bd sandbox_spl \
		--build

Went a bit better, but " 29 failed, 502 passed, 212 skipped". Is this 
normal?

What I seem to be missing is how to connect this python to calling 
spl_load_simple_fit(). In the real world, I'd build u-boot and feed it a 
FIT image -- boots, okay.

Alex

> Regards,
> Simon
> 
>>
>> Alex
>>
>>>>
>>>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>>>> index 1b4a7f6b15..a6f85b6f9d 100644
>>>> --- a/common/spl/spl_fit.c
>>>> +++ b/common/spl/spl_fit.c
>>>> @@ -26,6 +26,12 @@ DECLARE_GLOBAL_DATA_PTR;
>>>>    #define CONFIG_SYS_BOOTM_LEN   (64 << 20)
>>>>    #endif
>>>>
>>>> +struct spl_fit_info {
>>>> +       const void *fit;
>>>> +       size_t ext_data_offset;
>>>> +       int images_node;
>>>> +};
>>>
>>> struct comments
>>>
> 
> Regards,
> Simon
>