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 9323DC61DA4 for ; Fri, 17 Feb 2023 02:57:02 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id AF10585BFF; Fri, 17 Feb 2023 03:56:40 +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="P8ZMdH6E"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id CE26F85BF0; Fri, 17 Feb 2023 03:55:47 +0100 (CET) Received: from mail-yw1-x1132.google.com (mail-yw1-x1132.google.com [IPv6:2607:f8b0:4864:20::1132]) (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 07B1485BF8 for ; Fri, 17 Feb 2023 03:55:43 +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-yw1-x1132.google.com with SMTP id 00721157ae682-52ecd867d89so53311087b3.8 for ; Thu, 16 Feb 2023 18:55:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=xrhcJlNufjnRCWCApRk0BvN6NR8Te5s+MlXgNykRIos=; b=P8ZMdH6EubZzJKVE/TMjjV12MoHfcNv5rRznqhdH0yMt8FO3eo/Ou4gjCTuLoyfplt 0jaSAgrCqDNvaWcdLHalm78/aLVsuY2PfDAjzIfU8AGrfCReAiclqqoe0l5JbeHq04Zd LS52A1Tajw4PELNnq0ObrQkYaIAVoT7aSFEu0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=xrhcJlNufjnRCWCApRk0BvN6NR8Te5s+MlXgNykRIos=; b=BM2kHC3eoKFkpG3PXvBtcy0mv/9px3R0WHoNDAAXR6LNDrf1lrKuYsAKTxnW98dhsy akKCUttfs3E468WYD5tsNsTMkrB+ZOyq69HW2UQhwVN/3gMWQrLNoX7U514vDb75ANI7 d5ps6pTauH/eaWj9UBDEyUvi6y0b/ErN1I/iyg6tm8vKaPpdBq1itztNf1+0Y8STE+uz 5TSmluVIZ1ZlLZOjm+0ixH6qv5iU3yFftUHiVVECauDPx3Gxf1Ee0nrF4cU9jck1s2jK cgu3yprcCua3Xzk+ieFK+/ODPZIT2tNStTmXA1nezMFhty2vZWH6U6t2MU3PCrVlGFFf JMYg== X-Gm-Message-State: AO0yUKUAojRzmOGjzod7Sb6GF6azdfJoAiKYutDkj5w/i9kYaG45SWHB 5zjWnO62qhHyaOX54FiSMEPSwnby5cznybiX1cwUKrQ0t9nSC0Tf X-Google-Smtp-Source: AK7set9WTfKwxKhptjHdgHj36o1Pwz2+H1U12Y9IzPkOoscG3ydSYOQRbJVaTZiW2Sns7WHhrg3PBq/QogyW4PUh/24= X-Received: by 2002:a81:6041:0:b0:52e:e6ed:30b0 with SMTP id u62-20020a816041000000b0052ee6ed30b0mr1034307ywb.560.1676602542278; Thu, 16 Feb 2023 18:55:42 -0800 (PST) MIME-Version: 1.0 References: <20230205202116.2891673-1-jonas@kwiboo.se> <20230214103300.690542-1-jonas@kwiboo.se> <20230214103300.690542-7-jonas@kwiboo.se> <3af1296f-b50b-7c51-a20c-a3c4815c6161@kwiboo.se> In-Reply-To: <3af1296f-b50b-7c51-a20c-a3c4815c6161@kwiboo.se> From: Simon Glass Date: Thu, 16 Feb 2023 19:55:03 -0700 Message-ID: Subject: Re: [PATCH v2 6/6] RFC: binman: Improve allow missing for mkimage entry To: Jonas Karlman Cc: Philipp Tomsich , Kever Yang , Joseph Chen , Alper Nebi Yasak , Quentin Schulz , Jagan Teki , Heinrich Schuchardt , u-boot@lists.denx.de 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.6 at phobos.denx.de X-Virus-Status: Clean Hi Jonas, On Wed, 15 Feb 2023 at 11:25, Jonas Karlman wrote: > > Hi Simon, > > On 2023-02-14 20:48, Simon Glass wrote: > > Hi Jonas, > > > > On Tue, 14 Feb 2023 at 03:34, Jonas Karlman wrote: > >> > >> Implement CheckMissing and CheckOptional methods that is adapted to > >> Entry_mkimage in order to improve support for allow missing flag. > >> > >> Use collect_contents_to_file in multiple-data-files handling to improve > >> support for allow missing and fake blobs handling. > >> > >> Signed-off-by: Jonas Karlman > >> --- > >> Building for RK3568 without ROCKCHIP_TPL will result in the following > >> error message, regardless if BINMAN_ALLOW_MISSING is used or not. > >> > >> binman: Filename 'rockchip-tpl' not found in input path (...) > >> > >> With this patch and using BINMAN_ALLOW_MISSING=1 a new external blob > >> with no content is created and then passed to mkimage, resulting in an > >> error from the mkimage command. > >> > >> binman: Error 255 running 'mkimage -d ./mkimage-0.simple-bin.mkimage:./mkimage-1.simple-bin.mkimage -n rk3568 -T rksd ./idbloader.img': mkimage: Can't read ./mkimage-0.simple-bin.mkimage: Invalid argument > >> > >> If the --fake-ext-blobs argument is also used I get the message I was > >> expecting to see from the beginning. > >> > >> Image 'main-section' is missing external blobs and is non-functional: rockchip-tpl > >> > >> /binman/simple-bin/mkimage/rockchip-tpl: > >> An external TPL is required to initialize DRAM. Get the external TPL > >> binary and build with ROCKCHIP_TPL=/path/to/ddr.bin. One possible source > >> for the external TPL binary is https://github.com/rockchip-linux/rkbin>>> Image 'main-section' has faked external blobs and is non-functional: rockchip-tpl > >> > >> Some images are invalid > >> > >> Not sure how this should work, but I was expecting to see the message > >> about the missing rockchip-tpl from the beginning instead of the generic > >> "not found in input path" message. At leas with BINMAN_ALLOW_MISSING=1. > >> > >> tools/binman/etype/mkimage.py | 37 +++++++++++++++++++++++++++++++---- > >> 1 file changed, 33 insertions(+), 4 deletions(-) > >> > >> diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py > >> index cb264c3cad0b..44510a8c40ba 100644 > >> --- a/tools/binman/etype/mkimage.py > >> +++ b/tools/binman/etype/mkimage.py > >> @@ -153,10 +153,12 @@ class Entry_mkimage(Entry): > >> if self._multiple_data_files: > >> fnames = [] > >> uniq = self.GetUniqueName() > >> - for entry in self._mkimage_entries.values(): > >> - if not entry.ObtainContents(fake_size=fake_size): > >> + for idx, entry in enumerate(self._mkimage_entries.values()): > >> + entry_data, entry_fname, _ = self.collect_contents_to_file( > >> + [entry], 'mkimage-%s' % idx, fake_size) > >> + if entry_data is None: > > > > This is OK, I suppose, but I'm not quite sure what actually changes > > here, other than writing each entry to a file? > > The collect_contents_to_file function seemed to handle the > external/missing/optional/faked entry flags. So I changed to use that > function in order to see if that would change anything, see below. > > Use of this function does make it possible to use entry type other > then external blobs, not sure if that would ever be needed/useful. > > > > > Also, if you do this, please add / extend a test that checks that the > > output files are written, since there is otherwise no coverage here. > > > > What test uses these optional mkimage pieces? > > Sorry, I was not clear enough about the reason for these changes in my > message above. > > Building with --rockchip-tpl-path=/path/to/existing/tpl works as > expected and generates a working image. > > I was expecting that the missing-blob-help message added in patch 1 > would be shown by binman when rockchip-tpl-path was empty/not-existing, > or at least together with the allow-missing flag. > > However, whatever I tested, empty/none-existing rockchip-tpl-path, > allow-missing, fake-ext-blobs, I was not able to see this message. > Instead, all I could get from binman was this "Filename 'rockchip-tpl' > not found in input path" message. > > Maybe my assumptions about when these missing messages should be shown > is wrong? > > Trying binman with the following two dts and --allow-missing gives > different results. First one shows the missing message, second one show > filename not found. > > binman { > rockchip-tpl { > }; > }; > > binman { > mkimage { > args = "-n rk3568 -T rksd"; > multiple-data-files; > > rockchip-tpl { > }; > }; > }; > > With the changes in this patch I instead get the missing message when I > also add the --fake-ext-blobs flag, so it clearly needs more work or > a completely different approach if we want to be able to see the missing > message added in patch 1. > > Adding a message that never will be displayed annoys me :-) > Maybe I should just drop this rfc/patch for a v3 of this series? > Does the mkimage etype need a CheckMissing() function? That is how binman knows whether something is missing. Regards, Simon [..]