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

[Simon Glass]

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
}

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