Re: [PATCH v2] u-boot: switch from SPL/MLO to "ais" image for omapl138 SOCs

[Denys Dmytriyenko <denys@ti.com>]

On Thu, Jun 26, 2014 at 04:16:09PM -0400, Tom Rini wrote:
> On Thu, Jun 26, 2014 at 03:51:50PM -0400, Denys Dmytriyenko wrote:
> > On Thu, Jun 26, 2014 at 03:33:20PM -0400, Tom Rini wrote:
> > > On Thu, Jun 26, 2014 at 02:18:15PM -0400, Denys Dmytriyenko wrote:
> > > > From: Denys Dmytriyenko <denys@ti.com>
> > > > 
> > > > Signed-off-by: Denys Dmytriyenko <denys@ti.com>
> > > > Cc: Tom Rini <trini@ti.com>
> > > > ---
> > > >  conf/machine/am180x-evm.conf         | 4 +++-
> > > >  recipes-bsp/u-boot/u-boot-ti.inc     | 4 ++--
> > > >  recipes-bsp/u-boot/u-boot_2014.07.bb | 3 +++
> > > >  3 files changed, 8 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/conf/machine/am180x-evm.conf b/conf/machine/am180x-evm.conf
> > > > index 3182e8a..f83814e 100644
> > > > --- a/conf/machine/am180x-evm.conf
> > > > +++ b/conf/machine/am180x-evm.conf
> > > > @@ -5,7 +5,9 @@
> > > >  require conf/machine/include/davinci.inc
> > > >  require conf/machine/include/omapl138.inc
> > > >  
> > > > -UBOOT_MACHINE = "da850evm_config"
> > > > +UBOOT_MACHINE = "da850_am18xxevm_config"
> > > > +UBOOT_SUFFIX = "ais"
> > > > +
> > > >  UBOOT_ENTRYPOINT = "0xc0008000"
> > > >  UBOOT_LOADADDRESS = "0xc0008000"
> > > 
> > > Since we've got ENTRYPOINT/LOADADDRESS in here which also are omapl138
> > > rather than am180x specific, that's OK with me.
> > > 
> > > > diff --git a/recipes-bsp/u-boot/u-boot-ti.inc b/recipes-bsp/u-boot/u-boot-ti.inc
> > > > index c024e96..0cb53ea 100644
> > > > --- a/recipes-bsp/u-boot/u-boot-ti.inc
> > > > +++ b/recipes-bsp/u-boot/u-boot-ti.inc
> > > > @@ -1,3 +1,5 @@
> > > > +UBOOT_SUFFIX ?= "img"
> > > > +
> > > >  require ${COREBASE}/meta/recipes-bsp/u-boot/u-boot.inc
> > > 
> > > This isn't completely safe, is it?  The COREBASE version also has ?= for
> > > UBOOT_SUFFIX too.  We need to either convince oe-core to use ??= or just
> > > populate our machine.conf/soc.inc files with UBOOT_SUFFIX.
> > 
> > That's why I moved it before require COREBASE, as otherwise it will be 
> > set there...
> > 
> > So, if machine defines own UBOOT_SUFFIX, our .inc won't overwrite it and 
> > COREBASE version won't either. If machine doesn't define one, we set it to 
> > "img" here. And for recipes not using our version of .inc, it will be set to 
> > "bin" in COREBASE .inc
> 
> I would have sworn the reason ??= was added was that what you describe
> isn't always safe even if it looks like it should be, in all cases.  If
> you're sure it's fine, OK.

Well, I think it may be unsafe when it cannot immediately evaluate the 
variable, which is not the case here. I did some more testing and seems to 
work. I guess long-term plan should be to populate all our machines with 
UBOOT_SUFFIX, then.

-- 
Denys