[U-Boot] [PATCH 10/14] sunxi: Move await_completion dram helper to dram.h

[Siarhei Siamashka]

On Fri, 19 Dec 2014 17:42:58 +0100
Hans de Goede <hdegoede at redhat.com> wrote:

> Hi,
> 
> On 19-12-14 11:06, Siarhei Siamashka wrote:
> > On Tue, 16 Dec 2014 21:31:35 +0100
> > Hans de Goede <hdegoede at redhat.com> wrote:
> >
> >> The await_completion helper is already copy pasted between the sun4i and sun6i
> >> dram code, and we need it for sun8i too, so lets make it an inline helper in
> >> dram.h, rather then adding yet another copy.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> >
> > Making this function "static inline" and placing it into a header file
> > encourages the compiler to actually inline it. Which is not great for
> > code size:
> >
> >
> > == Before the patch and using gcc version 4.8.3 ==
> >
> > $ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun4i.o
> >   text    data     bss     dec     hex filename
> >   1731       0       0    1731     6c3
> >
> > $ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o
> >   text    data     bss     dec     hex filename
> >   1841       0       0    1841     731
> >
> > == After the patch and using gcc version 4.8.3 ==
> >
> > $ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun4i.o
> >   text    data     bss     dec     hex filename
> >   1763       0       0    1763     6e3
> >
> > $ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o
> >   text    data     bss     dec     hex filename
> >   1983       0       0    1983     7bf
> 
> Ah, thanks for catching that, the size increase does not seem to be a problem
> right now, but it certainly is something to keep in mind.
> 
> > Could we perhaps just introduce something like a new source file
> > "dram_common.c" or even "dram.c"?
> 
> Sounds like a good idea, patches welcome.

I would definitely appreciate if we could have avoided having this
unnecessary code size increase in the first place.

But if we run out of the code size space, then the oversized static
inline functions in the headers are always relatively easy to identify
and eliminate in one go. That's only a minor annoyance and I'm not
going to waste time on pointless debates.

> > As an additional observation, moving await_bits_clear/await_bits_set
> > functions into a common header file and using them in the sun6i dram
> > code might improve readability.
> 
> Sounds like another good idea, patches welcome.


-- 
Best regards,
Siarhei Siamashka