[U-Boot] [PATCH v4 0/4] Factorize ARM relocate_code instances

[Simon Glass]

Hi Albert,

On Mon, May 27, 2013 at 9:16 AM, Albert ARIBAUD
<albert.u.boot at aribaud.net>wrote:

> Hi Simon,
>
> On Mon, 27 May 2013 07:56:48 -0700, Simon Glass <sjg at chromium.org>
> wrote:
>
> > Hi Albert,
> >
> > On Mon, May 20, 2013 at 2:26 AM, Albert ARIBAUD
> > <albert.u.boot at aribaud.net>wrote:
> >
> > > Hi Benoît,
> > >
> > > On Sun, 19 May 2013 17:57:59 +0200 (CEST), Benoît Thébaudeau
> > > <benoit.thebaudeau at advansee.com> wrote:
> > >
> > > > Hi Albert,
> > > >
> > > > On Sunday, May 19, 2013 1:48:11 PM, Albert ARIBAUD wrote:
> > > > > This series replaces all instances of relocate_code in
> > > > > various start.S files from the ARM architecture with a
> > > > > single instance in arch/arm/lib/relocate.S.
> > > > >
> > > > > This is done in steps, each step keeping the whole of ARM
> > > > > U-Boot buildable and runnable and touching as little code
> > > > > as necessary.
> > > > >
> > > > > 1. Drop mx31pdk SPL call to relocate_code
> > > > >
> > > > > In SPL of mx31pdk, board_init_f calls relocate_code to
> > > > > move SPL from the NAND read buffer to SRAM. This move
> > > > > is not a relocation and is thus better implemented as
> > > > > a simple ad hoc copy loop.
> > > > >
> > > > > 2. Drop tx25 SPL call to relocate_code
> > > > >
> > > > > Same as mx31pdk.
> > > > >
> > > > > 3. Prevent all SPLs from assembling relocate_code
> > > > >
> > > > > Now that no SPL uses relocate_code any more, remove
> > > > > it completely from SPL builds by putting it entirely
> > > > > between #ifndef CONFIG_SPL_BUILD and #endif preprocessor
> > > > > conditional.
> > > > >
> > > > > 4. Actual factorization of relocate_code
> > > > >
> > > > > Remove all instances of relocate_code from start.S and
> > > > > create a new file, arch/arm/lib/relocate.S, to provide
> > > > > the new single instance of relocate_code.
> > > > >
> > > > > The only non-trivial change is that for PXA25X, a call
> > > > > to unlock (actually disable) dcache is moved from before
> > > > > relocating to after relocating. As this cache is either
> > > > > locked-as-RAM or disabled, but never used as actual DDR
> > > > > cache, this move has no performance effect on relocation
> > > > > and no functional impact either.
> > > > >
> > > > > The relocation routine was step-tested on versatileqemu
> > > > > under gdb. Especially, both relocation offset (from
> > > > > _start to addr_moni and local offsets (from relocate_code
> > > > > to several linker script symbols) have been verified.
> > > > >
> > > > > Changes in v4:
> > > > > - added blank line after PXA25X conditional code
> > > > > - fixed typos in relocate_code comments
> > > > > - brought back 'mov pc,lr' instead of 'bx lr' for ARMv4
> > > >
> > > > [...]
> > > >
> > > > Perfect!
> > > >
> > > > For this v4 series:
> > > > Reviewed-by: Benoît Thébaudeau <benoit.thebaudeau at advansee.com>
> > >
> > > Thanks Benoît!
> > >
> > > Now I need some people to regression-test it on their
> > > HW, both ARM and non-ARM just in case.
> > >
> >
> > Tested on snow (ARMv7) and link (x86), seems fine.
> >
> > The series:
> >
> > Tested-by: Simon Glass <sjg at chromium.org>
>
> thanks a lot!
>
> > Re your second series containing 'arm: optimize relocate_code
> > routine<http://patchwork.ozlabs.org/patch/243817/>',
> > I ended up with conflicts here - perhaps because the first series was
> > updated. I would like to test that also, as it should make it possible to
> > remove CONFIG_SYS_TEXT_BASE which is currently of interest.
>
> Thanks Simon. I have to rebase and repost the second series indeed;
> will do this tonight. Note that it was first posted after merge window
> closure, so if you need it for things planned in 2013.07, then I'll
> have to ask for Tom's agreement to put the second series in 2013.07.
>

Sounds good. I think it would be really nice to clean all this up, not sure
about timeframe though. I don't have anything yet posted that depends on
it, but I will once I get it running (reprise the generic relocation idea).

Regards,
Simon


>
> > Regards,
> > Simon
>
> Amicalement,
> --
> Albert.
>