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

[Simon Glass]

Hi Alex,

On Mon, 21 Dec 2020 at 15:24, Alex G. <mr.nuke.me at gmail.com> wrote:
>
>
>
> 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.

Here's a suggestoin
- Write a function that calls the function to load a fit and does some
checks that it worked correct, e.g. by looking in memory
- put a call to that function in an SPL C test (as mentioned ealler)

I suppose you could also boot it, perhaps by switching sandbox to use
FIT to boot?

Regards,
Simon