[OE-core] [PATCH v3 2/2] u-boot.inc : add compile multiple u-boot feature

[B40290 at freescale.com]

Hello Otavio,
Sorry for my late reply.
Thanks  for  your review .
I have submitted  v4  of  the  patchest.


Thanks,
Chunrong



-----Original Message-----
From: otavio.salvador at gmail.com [mailto:otavio.salvador at gmail.com] On Behalf Of Otavio Salvador
Sent: Monday, December 08, 2014 3:32 AM
To: Guo Chunrong-B40290
Cc: Patches and discussions about the oe-core layer
Subject: Re: [OE-core] [PATCH v3 2/2] u-boot.inc : add compile multiple u-boot feature

Hello Chunrong,

On Thu, Dec 4, 2014 at 3:49 AM, Chunrong Guo <B40290 at freescale.com> wrote:
> Signed-off-by: Chunrong Guo <B40290 at freescale.com>
> ---
>  meta/recipes-bsp/u-boot/u-boot.inc | 72 
> +++++++++++++++++++++++++-------------
>  1 file changed, 47 insertions(+), 25 deletions(-)
>
> diff --git a/meta/recipes-bsp/u-boot/u-boot.inc 
> b/meta/recipes-bsp/u-boot/u-boot.inc
> index c695b73..9b93946 100644
> --- a/meta/recipes-bsp/u-boot/u-boot.inc
> +++ b/meta/recipes-bsp/u-boot/u-boot.inc
> @@ -63,25 +63,32 @@ do_compile () {
>                 echo ${UBOOT_LOCALVERSION} > ${S}/.scmversion
>         fi
>
> -       oe_runmake ${UBOOT_MACHINE}
> -       oe_runmake ${UBOOT_MAKE_TARGET}
> +    for type in ${UBOOT_MACHINE}; do
> +        oe_runmake O=${type} ${type}
> +        oe_runmake O=${type} ${UBOOT_MAKE_TARGET}
> +        cp  ${S}/${type}/${UBOOT_BINARY}  ${S}/${type}/u-boot-${type}.${UBOOT_SUFFIX}
> +    done
> +
>  }

I don't see how you map the type to the UBOOT_MACHINE here. If I am reading the code correctly the for would be:

for config in ${UBOOT_MACHINE); do ...

and it is technically correct. The problem I see here is that we lose the 'type' conception. So the 'sd', 'flash', ... configuration type is lost and not propagated to the binary filename which I think we should try to improve.

[accept] 

>  do_install () {
> -    install -d ${D}/boot
> -    install ${S}/${UBOOT_BINARY} ${D}/boot/${UBOOT_IMAGE}
> -    ln -sf ${UBOOT_IMAGE} ${D}/boot/${UBOOT_BINARY}
> +    for type in ${UBOOT_MACHINE}; do
> +        install -d ${D}/boot
> +        install ${S}/${type}/u-boot-${type}.${UBOOT_SUFFIX} ${D}/boot/u-boot-${type}.${UBOOT_SUFFIX}
> +    done

Same here.

>      if [ -e ${WORKDIR}/fw_env.config ] ; then
>          install -d ${D}${sysconfdir}
>          install -m 644 ${WORKDIR}/fw_env.config ${D}${sysconfdir}/fw_env.config
>      fi
>
> -    if [ "x${SPL_BINARY}" != "x" ]
> -    then
> -        install ${S}/${SPL_BINARY} ${D}/boot/${SPL_IMAGE}
> -        ln -sf ${SPL_IMAGE} ${D}/boot/${SPL_BINARY}
> -    fi
> +    for type in ${UBOOT_MACHINE}; do
> +        if [ "x${SPL_BINARY}" != "x" ] && [ -d "${S}/${type}/${SPL_BINARY}"]
> +        then
> +            install ${S}/${type}/${SPL_BINARY} ${D}/boot/${SPL_IMAGE}
> +            ln -sf ${SPL_IMAGE} ${D}/boot/${SPL_BINARY}
> +        fi
> +    done

We ought to have one SPL binary with each 'type' assigned; here you are overriding it with the last SPL binary built.

[  accept ] 
>      if [ "x${UBOOT_ENV}" != "x" ]
>      then
> @@ -93,21 +100,36 @@ do_install () {
>  FILES_${PN} = "/boot ${sysconfdir}"
>
>  do_deploy () {
> -    install -d ${DEPLOYDIR}
> -    install ${S}/${UBOOT_BINARY} ${DEPLOYDIR}/${UBOOT_IMAGE}
> -
> -    cd ${DEPLOYDIR}
> -    rm -f ${UBOOT_BINARY} ${UBOOT_SYMLINK}
> -    ln -sf ${UBOOT_IMAGE} ${UBOOT_SYMLINK}
> -    ln -sf ${UBOOT_IMAGE} ${UBOOT_BINARY}
> -
> -    if [ "x${SPL_BINARY}" != "x" ]
> -    then
> -        install ${S}/${SPL_BINARY} ${DEPLOYDIR}/${SPL_IMAGE}
> -        rm -f ${DEPLOYDIR}/${SPL_BINARY} ${DEPLOYDIR}/${SPL_SYMLINK}
> -        ln -sf ${SPL_IMAGE} ${DEPLOYDIR}/${SPL_BINARY}
> -        ln -sf ${SPL_IMAGE} ${DEPLOYDIR}/${SPL_SYMLINK}
> -    fi
> +    for type in ${UBOOT_MACHINE}; do
> +        for imagetype in in ${UBOOT_CONFIG}; do
> +            if [ "${imagetype}"x = "in"x ]
> +            then
> +                continue
> +            fi
> +            if [ -d "${DEPLOYDIR}/u-boot-${imagetype}.${UBOOT_SUFFIX}"]
> +            then
> +                break
> +            else
> +                install -d ${DEPLOYDIR}
> +                install ${S}/${type}/u-boot-${type}.${UBOOT_SUFFIX} ${DEPLOYDIR}/u-boot-${imagetype}.${UBOOT_SUFFIX}
> +                if [ "${imagetype}"x = "sd"x ] || [ "${imagetype}"x = "mfgtool"x ]
> +                then
> +                    cp  ${DEPLOYDIR}/u-boot-${imagetype}.${UBOOT_SUFFIX}  ${DEPLOYDIR}/u-boot-${MACHINE}.${UBOOT_SUFFIX}
> +                fi
> +            fi

Way nicer (and should have some similar code on the install and compile tasks) however I didn't get why you're adding the 'sd' or 'mfgtool' specific case here. This is a FSL specific thing and shouldn't be part of OE-Core.

[ I use default link of last type uboot  of "UBOOT_CONFIG"   to fix the following error:

* difficulty to figure the binary when generating the rootfs: in meta-fsl-arm, for example, we generate the .sdcard rootfs and using the patch we now have...

ERROR: Error: The image creation script
'.../tmp/work/imx6qsabresd-oel-linux-gnueabi/core-image-minimal/1.0-r0/temp/create_image.sdcard'

]




> +        done
> +    done
> +
> +
> +    for type in ${UBOOT_MACHINE}; do
> +        if [ "x${SPL_BINARY}" != "x" ] && [ -d "${S}/${type}/${SPL_BINARY}"]
> +        then
> +            install ${S}/${type}/${SPL_BINARY} ${DEPLOYDIR}/${SPL_IMAGE}
> +            rm -f ${DEPLOYDIR}/${SPL_BINARY} ${DEPLOYDIR}/${SPL_SYMLINK}
> +            ln -sf ${SPL_IMAGE} ${DEPLOYDIR}/${SPL_BINARY}
> +            ln -sf ${SPL_IMAGE} ${DEPLOYDIR}/${SPL_SYMLINK}
> +        fi
> +    done

Same comment as the SPL binary above.

[accept] 
>      if [ "x${UBOOT_ENV}" != "x" ]
>      then

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750