From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7505DC433EF for ; Sun, 6 Mar 2022 03:09:58 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 54B6083CD9; Sun, 6 Mar 2022 04:08:58 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="CDhkG8Ob"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 215E083B7C; Sun, 6 Mar 2022 04:08:41 +0100 (CET) Received: from mail-oo1-xc36.google.com (mail-oo1-xc36.google.com [IPv6:2607:f8b0:4864:20::c36]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 1C1F883B7C for ; Sun, 6 Mar 2022 04:08:31 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-oo1-xc36.google.com with SMTP id s203-20020a4a3bd4000000b003191c2dcbe8so13855779oos.9 for ; Sat, 05 Mar 2022 19:08:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=pkSGMkornti61TR6ikOKoeUwlmQFLRh2N4ZmcJ5NdfA=; b=CDhkG8ObR+5OF1PsMXDKOr+3xQaJY/PPc55ADwNpcFqTW+sSjs7BB1e/gsYJvnEuKG YlfXpoTp/qaXUI2iNcpjgqcjOz6rnjGzbgwxhc5lj2QZPknGTAHjgDFSUb1el9iwLVd2 hTgRR1trz25yi6hurpVcua53Wr+Q+ec8wEgLc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=pkSGMkornti61TR6ikOKoeUwlmQFLRh2N4ZmcJ5NdfA=; b=ALjmJwGPhpeHa5W34flHu4hfo9doeZXjhD3wETtU26koCGP6N6sV0VnBbOdkeKvh6D WOaaQcMDUCuzxPruU/RsA0ujZo1w9yY8qsUFn2fJfVTQ2OGwmK0LhJ2Biw+IytWDOpom XdFPRZM0Dua4hUpmjMfFS1jrsWyTmQ6CcWdBMWY6MO38r5aa4ssHx7EiT1Ja/gZGkZwy SjfUM3dd/zgD2LMuhLCG7uM7Q+uHLvbGPVrq4BfvWJ/dT/ZJBYcYXBEtWrQPcSL8rcDP aOoJSDgQAUxuVxehkKps2xy6kGGDKeh/yC0zNl2WmyS7r7Nb0D+gPxcMCDcmGiALyFpe +Y3Q== X-Gm-Message-State: AOAM531VscPQi7IJckw+blAezBW2xwy4d4fsnQPsQnF5RoWeoPWTu3WJ KhJTtdUzYrtAWJegR30p8sTtC1154IINZEg6enULKg== X-Google-Smtp-Source: ABdhPJyZEHR6aUAg04IgvYWSD+0Y9GlJhNFldOZDrbUtpUb7jiX5xlfAHvPM21WLXLAhDpOXzqQIy/HVTpgYiWQK51E= X-Received: by 2002:a4a:d62b:0:b0:2e6:de0c:c737 with SMTP id n11-20020a4ad62b000000b002e6de0cc737mr2254464oon.39.1646536109526; Sat, 05 Mar 2022 19:08:29 -0800 (PST) MIME-Version: 1.0 References: <20220304195641.1993688-1-pgwipeout@gmail.com> In-Reply-To: <20220304195641.1993688-1-pgwipeout@gmail.com> From: Simon Glass Date: Sat, 5 Mar 2022 20:08:17 -0700 Message-ID: Subject: Re: [PATCH v2] binman: support mkimage separate files To: Peter Geis Cc: Alper Nebi Yasak , U-Boot Mailing List Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean Hi Peter, On Fri, 4 Mar 2022 at 12:56, Peter Geis wrote: > > mkimage has the ability to process two files at the same time. > This is necessary for rk356x support as both TPL and SPL need to be > hashed individually in the resulting header. > It also eases support for rkspi as mkimage handles everything for making > the requisite file for maskrom loading. This makes me wonder if we should move that functoinality out of mkimage and into binman? > > Add a new flag "separate_files" for mkimage handling to gather the files > separately rather than combining them before passing them to mkimage. > > Signed-off-by: Peter Geis > --- > Changelog: > v2: > I've managed to move this all into mkimage.py as per Alper's suggestion. > I've added an example to the readme portion of the function. > mkimage,separate_files is now separate-files. > > tools/binman/etype/mkimage.py | 41 ++++++++++++++++++++++++++++++++--- > 1 file changed, 38 insertions(+), 3 deletions(-) > > diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py > index 5f6def2287f6..0a86f904a2b7 100644 > --- a/tools/binman/etype/mkimage.py > +++ b/tools/binman/etype/mkimage.py > @@ -17,6 +17,7 @@ class Entry_mkimage(Entry): > Properties / Entry arguments: > - datafile: Filename for -d argument > - args: Other arguments to pass > + - separate-files: Boolean flag for passing files individually. > > The data passed to mkimage is collected from subnodes of the mkimage node, > e.g.:: > @@ -42,20 +43,54 @@ class Entry_mkimage(Entry): > }; > }; > > + This calls mkimage to create a rksd image with a standalone ram init > + binary file and u-boot-spl.bin as individual input files. The output from > + mkimage then becomes part of the image produced by binman. > + > + mkimage { > + args = "-n", CONFIG_SYS_SOC, "-T", "rksd"; > + separate-files; > + > + ddrl { > + type = "blob-ext"; > + filename = "rk3568_ddr_1560MHz_v1.12.bin"; > + }; > + > + u-boot-spl { > + }; > + }; > + > """ > def __init__(self, section, etype, node): > super().__init__(section, etype, node) > self._args = fdt_util.GetArgs(self._node, 'args') > + self._separate = fdt_util.GetBool(self._node, 'separate-files') > self._mkimage_entries = OrderedDict() > self.align_default = None > self.ReadEntries() > + self.index = 0 > + self.fname_tmp = str() > > def ObtainContents(self): > # Use a non-zero size for any fake files to keep mkimage happy > - data, input_fname, uniq = self.collect_contents_to_file( > - self._mkimage_entries.values(), 'mkimage', 1024) > - if data is None: > + if self._separate is False: > + data, input_fname, uniq = self.collect_contents_to_file( > + self._mkimage_entries.values(), 'mkimage', 1024) > + if data is None: > return False > + else: > + for entry in self._mkimage_entries.values(): We can do: for index, entry in enumerate(self._mkimage_entries.values()): then you don't need self.index > + self.index = (self.index + 1) > + if (self.index > 2): > + print('BINMAN Warn: mkimage only supports a maximum of two separate files') Can we use self.Raise() here instead? It seems like a fatal error. Also this check should go in ReadNode() since we don't want to die this late in the picture if we know it is wrong upfront. (BTW I am moving the node-reading code to ReadNode() in my v3 series as suggested by Alper). > + return False > + input_fname = ['mkimage', str(self.index)] > + data, input_fname, uniq = self.collect_contents_to_file( > + [entry], ".".join(input_fname), 1024) I suppose we can just use data = entry.GetData() here? > + if data is None: > + return False > + self.fname_tmp = [''.join(self.fname_tmp),input_fname] > + input_fname = ":".join(self.fname_tmp) > output_fname = tools.get_output_filename('mkimage-out.%s' % uniq) > if self.mkimage.run_cmd('-d', input_fname, *self._args, > output_fname) is not None: > -- > 2.25.1 > Looks OK to me, needs a test or two. Regards, Simon