[PATCH 3/3] dm: core: refactor functions reading an u32 from dt

[sjg at google.com]

Hi Dario,

On Sat, 4 Apr 2020 at 06:49, <dariobin at libero.it> wrote:
>
> > Il 2 aprile 2020 alle 20.54 Simon Glass <sjg at chromium.org> ha scritto:
> >
> >
> > Hi Dario,
> >
> > On Wed, 1 Apr 2020 at 13:34, <dariobin at libero.it> wrote:
> > >
> > >
> > > > Il 31 marzo 2020 alle 1.57 Simon Glass <sjg at chromium.org> ha scritto:
> > > >
> > > >
> > > > On Sun, 29 Mar 2020 at 10:05, Dario Binacchi <dariobin at libero.it> wrote:
> > > > >
> > > > > Now reading a 32 bit value from a device-tree property can be expressed
> > > > > as reading the first element of an array with a single value.
> > > > >
> > > > > Signed-off-by: Dario Binacchi <dariobin at libero.it>
> > > > >
> > > > > ---
> > > > >
> > > > >  drivers/core/of_access.c | 16 +---------------
> > > > >  drivers/core/ofnode.c    | 23 ++---------------------
> > > > >  2 files changed, 3 insertions(+), 36 deletions(-)
> > > >
> > > > Reviewed-by: Simon Glass <sjg at chromium.org>
> > > >
> > > > Can you please check the code-size delta in SPL on a suitable board?
> > >
> > > I have a black beaglebone available (am335x_evm_defconfig).
> > >
> > > u-boot-spl.map generated without applying the refactoring patch
> > > ....
> > >  .text.ofnode_read_u32
> > >                 0x0000000000000000       0x2e drivers/built-in.o
> > >  .text.ofnode_read_u32_index
> > >                 0x0000000000000000       0x38 drivers/built-in.o
> > >  .text.ofnode_read_u32_default
> > >                 0x0000000000000000       0x12 drivers/built-in.o
> > >
> > > u-boot-spl.map genarated with the refactoring patch applied
> > > ....
> > >  .text.ofnode_read_u32_index
> > >                 0x0000000000000000       0x38 drivers/built-in.o
> > >  .text.ofnode_read_u32
> > >                 0x0000000000000000        0x8 drivers/built-in.o
> > >  .text.ofnode_read_u32_default
> > >                 0x0000000000000000       0x14 drivers/built-in.o
> >
> > Possibly, but a better test is to build your branch with the patch in:
> >
> > buildman -b <branch> <board>
> >
> > Then check the size:
> >
> > buildman -b <branch> -sS
> >
> > or function detail:
> >
> > buildman -b <branch> -sSB
> >
> > That will give us a true picture for SPL. It will show incremental
> > size increase with your patch.
>
> Hi Simon,
> this is the buildman response:
> ...
> 03: dm: core: support reading a single indexed u32 value
> 04: dm: core: refactor functions reading an u32 from dt
>        arm: (for 4/708 boards) all +11.5 bss -10.0 text +21.5
>             am335x_hs_evm  : all +24 text +24
>                u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26)
>                  function                                   old     new   delta
>                  ofnode_read_u32_index                        -      62     +62
>                  ofnode_read_u32_default                     18      20      +2
>                  ofnode_read_u32                             46       8     -38
>             am335x_hs_evm_uart: all +24 text +24
>                u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26)
>                  function                                   old     new   delta
>                  ofnode_read_u32_index                        -      62     +62
>                  ofnode_read_u32_default                     18      20      +2
>                  ofnode_read_u32                             46       8     -38
>             am335x_evm     : bss -24 text +24
>                u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26)
>                  function                                   old     new   delta
>                  ofnode_read_u32_index                        -      62     +62
>                  ofnode_read_u32_default                     18      20      +2
>                  ofnode_read_u32                             46       8     -38
>             am335x_boneblack_vboot: all -2 bss -16 text +14
>                u-boot: add: 1/0, grow: 1/-1 bytes: 64/-38 (26)
>                  function                                   old     new   delta
>                  ofnode_read_u32_index                        -      62     +62
>                  ofnode_read_u32_default                     18      20      +2
>                  ofnode_read_u32                             46       8     -38

This does not actually look like SPL though, only U-Boot. I hope that
means that SPL doesn't increase in size.

Anyway for U-Boot proper it looks like it adds about 24 bytes of code.
I think it is worth it.

Regards,
Simon

Applied to u-boot-dm/next, thanks!