[U-Boot] [PATCH v4] arm: ep9315: Return back Cirrus Logic EDB9315A board support

[sergey kostanbaev]

Hi Albert,
See answers below


On Thu, Apr 17, 2014 at 10:12 AM, Albert ARIBAUD
<albert.u.boot at aribaud.net>wrote:

> Hi Sergey,
>
> On Thu,  6 Mar 2014 21:20:20 +0400, Sergey Kostanbaev
> <sergey.kostanbaev at gmail.com> wrote:
>
> > diff --git a/arch/arm/cpu/arm920t/ep93xx/lowlevel_init.S
> b/arch/arm/cpu/arm920t/ep93xx/lowlevel_init.S
> > index bf2fa2a..3ac0f88 100644
> > --- a/arch/arm/cpu/arm920t/ep93xx/lowlevel_init.S
> > +++ b/arch/arm/cpu/arm920t/ep93xx/lowlevel_init.S
> > @@ -1,49 +1,456 @@
> >  /*
> >   * Low-level initialization for EP93xx
> >   *
> > - * Copyright (C) 2009 Matthias Kaehlcke <matthias at kaehlcke.net>
> > + * Copyright (C) 2013
> > + * Sergey Kostanabev <sergey.kostanbaev <at> fairwaves.ru>
> >   *
> > - * Copyright (C) 2006 Dominic Rath <Dominic.Rath at gmx.de>
> > + * Copyright (C) 2006 Cirrus Logic Inc.
>
> Why do you remove existing copyright attributions?
>
Actually I used a completely different solution. I can add old ones.


>
> > [...]
>
> > + * Determine the size of the SDRAM. Use data=address for the scan.
> > +
> > + * Input:    r0 - Start SDRAM address
> > + * Return:   r0 - Single block size
> > + *           r1 - Valid block mask
> > + *           r2 - Total block count
> > + * Modifies: r0-r5
> > + */
> > +ep93xx_sdram_size:
>
> Is this needed? Can't the general get_ram_size() function be used?
>
This is REALLY needed. It's a key feature it calculates RAM size at runtime
otherwise you have to put constant of data-bus width, actual memory
installed. And this procedure does it and you don't have to recompile it
for the very specific board with similar processor family.



>
> > diff --git a/arch/arm/include/asm/arch-ep93xx/ep93xx.h
> b/arch/arm/include/asm/arch-ep93xx/ep93xx.h
> > index 9e7f2f3..71aa601 100644
> > --- a/arch/arm/include/asm/arch-ep93xx/ep93xx.h
> > +++ b/arch/arm/include/asm/arch-ep93xx/ep93xx.h
>
> > @@ -580,3 +658,11 @@ struct syscon_regs {
> >  /*
> >   * 0x80950000 - 0x9000FFFF: Reserved
> >   */
> > +
> > +/*
> > + * During low_level init we store memory layout in memory at specific
> location
> > + */
> > +#define UBOOT_MEMORYCNF_BANK_SIZE            0x2000
> > +#define UBOOT_MEMORYCNF_BANK_MASK            0x2004
> > +#define UBOOT_MEMORYCNF_BANK_COUNT           0x2008
> > +
>
> Remove blank line at EOF.
>
Ok


>
> > diff --git a/board/cirrus/edb93xx/u-boot.lds
> b/board/cirrus/edb93xx/u-boot.lds
>
> Does this board need a specific .lds file?
>
Yes. It must have special magic header to start booting otherwise it will
be considered as invalid image and internal boot ROOM ignore it.


>
> > [...]
>
> > +     /DISCARD/ : { *(.dynsym) }
> > +     /DISCARD/ : { *(.dynstr*) }
> > +     /DISCARD/ : { *(.dynamic*) }
> > +     /DISCARD/ : { *(.plt*) }
> > +     /DISCARD/ : { *(.interp*) }
> > +     /DISCARD/ : { *(.gnu*) }
>
> We don't /DISCARD/ these sections any more. If for some reason the
> generic .lds fils cannot be used, then adapt this one to current
> practice.
>
Ok I can remove these DISCARD.



>
> Amicalement,
> --
> Albert.
>