Re: [PATCH 1/5] tools: binman: add nxp_imx93cst etype for i.MX93 flash.bin signing
Simon Glass <[email protected]>
| Newsgroups | org.u-boot-project.lists.u-boot |
|---|---|
| Message-ID | <CAFLszTh3EaFTMusTDiPZZWvXZ4PvyXXNLKGORc=ag+4sDF5SFA@mail.gmail.com> |
Hi Jérémie, On 2026-08-14T17:07:09, Jérémie Dautheribes <[email protected]> wrote: > tools: binman: add nxp_imx93cst etype for i.MX93 flash.bin signing > > Add a new binman etype which allows signing both the SPL and U-Boot proper > sections of i.MX93 flash.bin using CST. > The implementation is largely derived from the nxp_imx8mcst etype, as the > signing procedures for both platforms are quite similar. > > Signed-off-by: Jérémie Dautheribes (Schneider Electric) <[email protected]> > > .gitignore | 2 + > tools/binman/etype/nxp_imx93cst.py | 143 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 145 insertions(+) > +class Entry_nxp_imx93cst(Entry_mkimage): > + """NXP i.MX93 CST .cfg file generator and cst invoker""" > + > + def __init__(self, section, etype, node): > + super().__init__(section, etype, node) Please expand the class docstring to document the supported properties (nxp,srk-table and nxp,srk-crt) and the SRK_TABLE / SRK_KEY environment overrides, in the same style as Entry_nxp_imx8mcst. Without that, users have no way to discover what to put in the DT node. Also, __init__() does nothing beyond calling super().__init__(), so drop it. > diff --git a/tools/binman/etype/nxp_imx93cst.py b/tools/binman/etype/nxp_imx93cst.py > @@ -0,0 +1,143 @@ > + if struct.unpack("<B", data[3:4])[0] != CONTAINER_HDR_TAG: > + # Unknown section type, pass input data through. > + return data struct.unpack() for a single byte is overkill - 'if data[3] != CONTAINER_HDR_TAG:' is clearer and does the same thing. Also, there is no length check on data before this slice or the flags_offset read below; a short input silently produces an empty slice and the unpack raises. A quick 'if len(data) < flags_offset + 4' guard (returning data unchanged) would fail more gracefully. > diff --git a/tools/binman/etype/nxp_imx93cst.py b/tools/binman/etype/nxp_imx93cst.py > @@ -0,0 +1,143 @@ > +KEY_NAME = "sha384_secp384r1_v3_usr_crt" > + > +CSF_CONFIG_TEMPLATE = f""" > +[Header] > + Target = AHAB > + Version = 1.0 > + > +[Install SRK] > + File = "SRK_1_2_3_4_table.bin" > + Source = "SRK1_{KEY_NAME}.pem" > + Source index = 0 > + Source set = OEM > + Revocations = 0x0 > + > +[Authenticate Data] > + File = "data.bin" > + Offsets = 0x400 0x490 > + The Offsets line embeds a literal tab and hard-coded values that are always overwritten before the config is written. Please use plain placeholders (e.g. '0x0 0x0') with a single space, so the template does not mislead the reader into thinking those numbers matter. Please follow the prevailing single-quote convention in binman. > diff --git a/tools/binman/etype/nxp_imx93cst.py b/tools/binman/etype/nxp_imx93cst.py > @@ -0,0 +1,143 @@ > + def BuildSectionData(self, required): > + data, input_fname, uniq = self.collect_contents_to_file( > + self._entries.values(), "input" > + ) input_fname is unused; assign it to _ to make that explicit. Regards, Simon