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

[Tom Rini <trini@ti.com>]

On 07/02/2014 03:57 PM, Denys Dmytriyenko wrote:
> 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.

Sounds good to me, thanks!

-- 
Tom