On 12/21/20 2:23 PM, Simon Glass wrote:
Hi Alex,

On Mon, 21 Dec 2020 at 12:28, Alex G. <mr.nuke...@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...@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...@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

Reply via email to