[U-Boot] [PATCH 6/7] arm: ls102xa: Add SD boot support for LS1021AQDS board

[Huan Wang]

Hi, Albert,

> On Wed, 8 Oct 2014 09:53:03 +0000, Huan Wang <alison.wang at freescale.com>
> wrote:
> 
> > Hi, Albert,
> >
> > > On Thu, 25 Sep 2014 06:45:00 +0000, Huan Wang
> > > <alison.wang at freescale.com> wrote:
> > >
> > > > Hi, Albert,
> > > >
> > > > > On Mon, 22 Sep 2014 06:46:20 +0000, Huan Wang
> > > > > <alison.wang at freescale.com> wrote:
> > > > >
> > > > > > Hi, Albert,
> > > > > >
> > > > > > > > On Thu, 18 Sep 2014 13:47:18 +0800, Alison Wang
> > > > > > > > <b18965 at freescale.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > +
> > > > > > > > > +     . = ALIGN(4);
> > > > > > > > > +     .u_boot_list : {
> > > > > > > > > +             KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > > > > > > > > +     }
> > > > > > > >
> > > > > > > > IS this required? And if it is, could it not be added to
> > > > > > > > the arch/arm/cpu/u-boot-spl.lds file? This way you would
> > > > > > > > not need an .lds file at all.
> > > > > > > >
> > > > > > > > [Alison Wang] Yes, it is required.
> > > > > > >
> > > > > > > Ok -- what for? :)
> > > > > > [Alison Wang] In SPL part, DDR is initialized by reading SPD
> > > > > > through
> > > > > I2C interface.
> > > > > > For I2C, ll_entry_count() is called, and it returns the
> number
> > > > > > of elements of a linker-generated array placed into
> subsection
> > > > > > of .u_boot_list section specified by _list argument. So I
> need
> > > > > > to
> > > add
> > > > > this to make I2C work in SPL.
> > > > >
> > > > > Understood. So your SPL code uses I2C, and for I2C, you need a
> > > > > linker list. But then:
> > > > >
> > > > > > > > I would like to add it in arch/arm/cpu/u-boot-spl.lds. I
> > > > > > > > was not sure adding it in arch/arm/cpu/u-boot-spl.lds is
> > > > > > > > acceptable or
> > > > > not.
> > > > > > >
> > > > > > > (assuming the reason why it is needed is valid) If it
> causes
> > > > > > > no change to boards which do not use it right now (and I
> > > > > > > mean 'no change' ad 'binary identical') then this is
> > > > > > > acceptable. Make sure you check the binary invariance and
> > > > > > > that you mention it in the
> > > > > commit.
> > > > > > >
> > > > > > [Alison Wang] It will cause the binary is not identical for
> > > > > > other
> > > > > board.
> > > > >
> > > > > Is this a prediction or an actual observation of compared
> builds
> > > > > with and without the I2C linker liste addition to the generic
> > > SPL .lds?
> > > >
> > > > [Alison Wang] I use mx31pdk as example. I compared the binaries
> > > > with and Without the I2C linker list in
> > > > arch/arm/cpu/u-boot-spl.lds. The binaries are not identical.
> > >
> > > I have just checked mx31pdk: the u-boot binaries (u-boot,
> > > u-boot.bin, u-boot-with-spl.bin, u-boot.map, u-boot.srec) are
> indeed
> > > different, but that's just normal considering the repository state
> > > and build date and time are included in the binaries [1].
> > >
> > > OTOH, I see that the u-boot-spl.bin files are identical.
> > >
> > > The only change I made between the two builds was inserting
> > >
> > > 	>         . = ALIGN(4);
> > > 	> +       .u_boot_list : {
> > > 	> +               KEEP(*(SORT(.u_boot_list*_i2c_*)));
> > > 	> +       }
> > >
> > > in arch/arm/cpu/u-boot-spl.lds at line 34.
> > >
> > > Can you re-check?
> >
> > [Alison Wang] Yes, you are right. u-boot-spl.bin files are identical.
> > The u-boot binaries (u-boot, u-boot.bin, u-boot-with-spl.bin, u-
> boot.map, u-boot.srec) are different only in build date and time.
> > >
> > > [1] BTW, how do you folks out here proceed when trying to compare
> u-
> > > boot.bin files from different builds of the same target without the
> > > repo state or build date and time affecting the comparison? I use a
> > > patch to Makefile that fakes the commit and repo state, and I also
> > > use fakelib to force timestamps, but there might be a simpler way.
> > >
> > [Alison Wang] Oh, your way is very good. I just used vimdiff.
> 
> I'll run a larger-scale check today, to see which SPLs are affected by
> adding u-boot_list*, not just u_boot_list*_i2c_*, i.e., maybe we can
> make this identical in u-boot.lds and u-boot-spl.lds for ARM.
> 
[Alison Wang] I can't agree with you more on making this identical in u-boot.lds
and u-boot-spl.lds for ARM. Anyway, I need to confirm it through your larger-scale
check result.


Best Regards,
Alison Wang