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 E5E4CE77188 for ; Fri, 10 Jan 2025 19:23:59 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 1D64C800D0; Fri, 10 Jan 2025 20:23: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="be/FscvB"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 4818A801F5; Fri, 10 Jan 2025 20:23:57 +0100 (CET) Received: from mail-ej1-x62f.google.com (mail-ej1-x62f.google.com [IPv6:2a00:1450:4864:20::62f]) (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 F169B80050 for ; Fri, 10 Jan 2025 20:23:54 +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@chromium.org Received: by mail-ej1-x62f.google.com with SMTP id a640c23a62f3a-aa684b6d9c7so433404766b.2 for ; Fri, 10 Jan 2025 11:23:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1736537034; x=1737141834; darn=lists.denx.de; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=VdOxvJHcxITRJsXbQ/fwx86lF4KI3p5wjxIGGFDs8Jg=; b=be/FscvBRLEJD8sxe18lB/agQs+yNxzXyRiGN86IpQ3Ly1VI4BXlBIQDA2DDf5jX4z HXgxtebfKmRS0zq317TO0HuhH/g+JKRoTYa3sdBZsntOh6hf17r3h5jrfeMSfEpjxMq8 EKOZUHb0HkRfj1eJ8eG1DSIqjOYorVeBop0XM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736537034; x=1737141834; 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=VdOxvJHcxITRJsXbQ/fwx86lF4KI3p5wjxIGGFDs8Jg=; b=l+ZkXoj4Xwvab0Vae5QEFbkLmkQXxHa0Fn48Xb3p0/VAB68H33nDoUP646Ue01XOko i5ZetYRiLwKSqwylh04v3f70wtxtnVDmt4mg3Q8l9Axcbn6nD7JXm/povk9RJP9FcRYi NM0he326SEGQNyCrmITFxdMN5fWJ5lq8H2ci3hWbKYV+CXyiPWECjuiiHTYdOIuRfK0r 8WQlGB1GujId2p5/QysWJgf0gKnBnyAADM7wgrZ4CJipd6ZetIL+RPEaN3f8CGuZNBXP ehpwNLxlgUpZ2/6BpeQAfcMziiVBvC9PGa+M/F1zOpzPz2gmCCoVvtEKf6IxhaySKn9K Xbpw== X-Gm-Message-State: AOJu0YwaOpBkwtLTUp+X6hgPne/KOZTGI6vMUl3e3C4uQbemdKCXo92l QBUFWllAy+flDzrskFWKnenWpoj7HXKhQ0hQftGsvngfOqJfkah+h8kkACKLdh9Xl2wkjQ1OW7q que8YTzLpgOQom86PAPJJ3di++f691IR58De7 X-Gm-Gg: ASbGnculfcSn+j4gxSp+07hBpFgoFTXqPs9fZl+jzCkzMsEYyFixfS+CYCZ3wpQhxPX EpoBtaTRaWQgmu8FHBINGe/348AnG16KcTR3KFp0= X-Google-Smtp-Source: AGHT+IFqBKsDDo+U6eGZ6x2pbzgIxeKejlvvb2vQU+iG9/niKoKcdLvKuZiuqsx/NDGwMKFuE1ZxZMgTFUoxwwxuKHk= X-Received: by 2002:a17:907:8a8f:b0:ab2:d721:ed92 with SMTP id a640c23a62f3a-ab2d721f27dmr502269366b.45.1736537034225; Fri, 10 Jan 2025 11:23:54 -0800 (PST) MIME-Version: 1.0 References: <20250106143229.3023771-1-sjg@chromium.org> <20250106143229.3023771-18-sjg@chromium.org> <20250108182528.GN3476@bill-the-cat> <20250109150954.GU3476@bill-the-cat> <20250109180817.GC3476@bill-the-cat> <20250110161732.GI3476@bill-the-cat> In-Reply-To: <20250110161732.GI3476@bill-the-cat> From: Simon Glass Date: Fri, 10 Jan 2025 12:23:42 -0700 X-Gm-Features: AbW1kvaOr8-OAmEXIHXvu3dfaxEhPo6hf1lvczrcEmT51xBUdqi5SG10KFuu6Cc Message-ID: Subject: Re: [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf To: Tom Rini Cc: U-Boot Mailing List , Alexander Dahl , Alexander Kochetkov , Alexander Sverdlin , Bin Meng , Caleb Connolly , Devarsh Thakkar , Heinrich Schuchardt , Hugo Cornelis , Igor Opaniuk , Ilias Apalodimas , Janne Grunau , Julien Masson , Laurent Pinchart , Leo Yu-Chi Liang , Marek Vasut , Matthew Garrett , Mattijs Korpershoek , Maxim Moskalets , Nikhil M Jain , Paul-Erwan Rio , Raymond Mao , Roman Stratiienko , Sughosh Ganu 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.8 at phobos.denx.de X-Virus-Status: Clean Hi Tom, On Fri, 10 Jan 2025 at 09:17, Tom Rini wrote: > > On Fri, Jan 10, 2025 at 06:39:11AM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Thu, 9 Jan 2025 at 11:08, Tom Rini wrote: > > > > > > On Thu, Jan 09, 2025 at 08:14:53AM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Thu, 9 Jan 2025 at 08:10, Tom Rini wrote: > > > > > > > > > > On Thu, Jan 09, 2025 at 05:36:03AM -0700, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Wed, 8 Jan 2025 at 11:25, Tom Rini wrote: > > > > > > > > > > > > > > On Mon, Jan 06, 2025 at 07:32:13AM -0700, Simon Glass wrote: > > > > > > > > > > > > > > > This function uses separate arguments for data and size. Use the new > > > > > > > > abuf instead, so that they are paired and in one place. In some cases it > > > > > > > > also saves an argument, thus potentially reducing code size. > > > > > > > > > > > > > > This is one of the commits that globally increases size in both full > > > > > > > U-Boot and SPL/etc. > > > > > > > > > > > > > > Is all of the "abuf" changes just a "tidy up" that increases the code a > > > > > > > bit? > > > > > > > > > > > > Yes, a tidy-up which I hope will help overall. I have been thinking > > > > > > for a while of how to avoid having addr/size and ptr/size passed > > > > > > everywhere. For now abuf seems to provide some sort of solution. > > > > > > > > > > > > I see this: > > > > > > > > > > > > 18: boot: Update fit_image_get_emb_data to use abuf > > > > > > aarch64: (for 1/1 boards) all +4.0 bss -24.0 spl/u-boot-spl:all > > > > > > +16.0 spl/u-boot-spl:text +16.0 text +28.0 > > > > > > > > > > > > so growth on firefly-rk3399 but not with rk3288. I am not sure if the > > > > > > growth will tail off as there are more users, though. We might even be > > > > > > able to be more clever with static inlines. > > > > > > > > > > Yeah, lets not do this now then and worry about some "clean up" later > > > > > when we can show that it does, or does not, improve size. > > > > > > > > Oh. > > > > > > > > > And there's > > > > > something wrong with your numbers: > > > > > 01: Fix neighbor discovery ethernet address saving > > > > > aarch64: w+ firefly-rk3399 > > > > > +(firefly-rk3399) Image 'simple-bin' is missing external blobs and is non-functional: atf-bl31 > > > > > +(firefly-rk3399) > > > > > +(firefly-rk3399) /binman/simple-bin/fit/images/@atf-SEQ/atf-bl31 (atf-bl31): > > > > > +(firefly-rk3399) See the documentation for your board. You may need to build ARM Trusted > > > > > +(firefly-rk3399) Firmware and build with BL31=/path/to/bl31.bin > > > > > +(firefly-rk3399) Image 'simple-bin' is missing optional external blobs but is still functional: tee-os > > > > > +(firefly-rk3399) /binman/simple-bin/fit/images/@tee-SEQ/tee-os (tee-os): > > > > > +(firefly-rk3399) See the documentation for your board. You may need to build Open Portable > > > > > +(firefly-rk3399) Trusted Execution Environment (OP-TEE) and build with TEE=/path/to/tee.bin > > > > > +(firefly-rk3399) Some images are invalid > > > > > 37: dm: core: Provide ofnode_find_subnode_unit() > > > > > aarch64: (for 1/1 boards) all +324.0 bss +32.0 spl/u-boot-spl:all +16.0 spl/u-boot-spl:text +16.0 text +292.0 > > > > > firefly-rk3399 : all +324 bss +32 spl/u-boot-spl:all +16 spl/u-boot-spl:text +16 text +292 > > > > > u-boot: add: 6/-1, grow: 4/-4 bytes: 516/-224 (292) > > > > > function old new delta > > > > > ofnode_name_eq_unit - 160 +160 > > > > > ofnode_find_subnode_unit - 116 +116 > > > > > fit_image_get_data 80 176 +96 > > > > > fit_image_get_emb_data - 84 +84 > > > > > ofnode_write_prop 224 236 +12 > > > > > ofnode_add_subnode 232 244 +12 > > > > > abuf_init_const - 12 +12 > > > > > abuf_init - 12 +12 > > > > > abuf_addr - 8 +8 > > > > > fit_image_print 780 784 +4 > > > > > image_locate_script 696 692 -4 > > > > > fit_image_load 1584 1580 -4 > > > > > fit_image_verify 176 164 -12 > > > > > ofnode_find_subnode 140 116 -24 > > > > > fit_image_get_data_and_size 180 - -180 > > > > > spl-u-boot-spl: add: 3/-1, grow: 0/-1 bytes: 108/-92 (16) > > > > > function old new delta > > > > > fit_image_get_emb_data - 84 +84 > > > > > abuf_init_const - 12 +12 > > > > > abuf_init - 12 +12 > > > > > load_simple_fit 580 568 -12 > > > > > fit_image_get_data 80 - -80 > > > > > > > > Yes, that's the whole series, so not related to this change. > > > > > > Yes, that's the whole series including this change, so it's related to > > > this change. > > > > Right, but it is due to ofnode_find_subnode(), etc. > > > > > > > > > I elected to have two versions of ofnode_find_subnode() to avoid the > > > > size growth in the previous version. But the cost is larger size > > > > growth when OF_LIVE is used. > > > > > > > > Without OF_LIVE, the size growth is tiny. > > > > > > And even worse in SPL, somehow. But you want more OF_LIVE users, not > > > less, yes? > > > > Well, OF_LIVE is always quite a bit larger, at least at the moment. It > > has both Linux's of_access stuff and libfdt. It's not OF_LIVE I am > > bothered about, but I do want people using ofnode. Unfortunately > > people still send patches which use libfdt directly. > > > > > > > > > So...what to do? > > > > > > Well, if you drop the abuf changes for now, SPL won't change at all for > > > most platforms and that'll be an improvement. > > > > Yes, I'll look at that. This is one of many examples where I have a > > problem and realise that we need a nicer way of dealing with it, then > > implement it in the series, but then the series loses focus. So then I > > take it out again, then forget about it until next time, but I never > > actually make the change. > > I hate to start to derail this, but refactor for "nicer code" is very > much subjective. Especially when it also grows the code (and it's not > clear that wider usage would result in shrinkage). So yes, this really > needs to be put aside and also part of why I keep asking for one thing > at a time. Yes I very much agree with this. > > > > And I'm going to keep complaining about size growth here because a > > > non-trivial subset of users just wants things to boot quickly and be > > > small. > > > > Yes, you won't get any complaints from me on that. I did propose some > > automated checking a few years back, but it never went anywhere. > > It be great if buildman size comparison had some way to csv the output. > That's what's missing imo from being able to have some automation or > even just nicer tooling. What sort of tooling could we have? I would like something in CI which reports code-size changes in a useful way, perhaps failing if the delta is too large for more than x boards. WDYT? Regards, Simon