Hi Tom, On Thu, 9 Jan 2025 at 11:08, Tom Rini <tr...@konsulko.com> 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 <tr...@konsulko.com> 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 <tr...@konsulko.com> 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. > > 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. Regards, SImon
