Hi Marek, On Mon, 20 Jul 2026 at 20:10, Marek Vasut <[email protected]> wrote: > > On 7/17/26 4:05 PM, Simon Glass wrote: > > Hello Simon, > > >> diff --git a/tools/binman/etype/nxp_imx8mcst.py > >> b/tools/binman/etype/nxp_imx8mcst.py > >> @@ -114,6 +115,13 @@ class Entry_nxp_imx8mcst(Entry_mkimage): > >> + elif signtype == MAGIC_NXP_IMX_FCFB: # SPL/imx8mimage with FCFB > >> + # Sign the payload including FCFB and imx8mimage headers > >> + # (extra 0x1000 and 0x40 bytes before the payload) > >> + signbase -= 0x1040 > >> + signsize = struct.unpack('<I', data[4120:4124])[0] - signbase > > > > Please see below for test ideas. > > > > Also, 4120 is quite obscure as a decimal literal. Since this branch is > > the IVT one shifted by 0x1000, perhaps the two could be merged, e.g.: > > > > if signtype in (MAGIC_NXP_IMX_IVT, MAGIC_NXP_IMX_FCFB): > > off = 0x1000 if signtype == MAGIC_NXP_IMX_FCFB else 0 > > signbase -= off + 0x40 > > signsize = struct.unpack( > > '<I', data[off + 24:off + 28])[0] - signbase > > data = data[:signsize] > > > > That makes it clear the value being read is the csf pointer at IVT > > offset 24, same as the existing case. What do you think? > > Yuck, no, I would very much prefer the current readable form than this > conflated horribleness. The image assembly is convoluted enough as it is. > > >> diff --git a/tools/binman/etype/nxp_imx8mcst.py > >> b/tools/binman/etype/nxp_imx8mcst.py > >> @@ -114,6 +115,13 @@ class Entry_nxp_imx8mcst(Entry_mkimage): > >> + elif signtype == MAGIC_NXP_IMX_FCFB: # SPL/imx8mimage with FCFB > > > > Please can you also update the block comment above this code ("Parse > > the input data and figure out what it is that is being signed") to > > mention the FCFB case? > > Done in V2. > > > Here is my attempt at a test (it doesn't actually check the output though): > If you would like to write a proper test, I can help with providing > expected outputs.
Checking outputs is nice but (perhaps oddly!) not essential. Binman just requires that tests exercise the code. So the test I sent previously should be enough to get 'binman test -T' to pass. Regards, Simon
