I have tested this patch locally and in our nightly builds. 

See my response to your comments below.

-----Original Message-----
From: Cooper Jr., Franklin 
Sent: Wednesday, March 25, 2015 5:02 PM
To: Stiffler, Jacob
Cc: [email protected]
Subject: Re: [meta-arago] [PATCH 1/2] omap3-sgx-modules: Add source to SDK.




> On Mar 25, 2015, at 3:58 PM, Cooper Jr., Franklin <[email protected]> wrote:
> 
> Overall this needs more testing. additional comments below
> 
>> On Mar 25, 2015, at 2:28 PM, Stiffler, Jacob <[email protected]> wrote:
>> 
>> * omap3-sgx-modules provides out-of-tree drivers which are required 
>> for graphics.
>> * These sources need to be provided so that whenever there is a 
>> kernel modification, the user can easily rebuild these moduels to reenable 
>> graphics.
>> 
>> Signed-off-by: Jacob Stiffler <[email protected]>
>> ---
>> .../conf/distro/arago-source-ipk.conf              |    3 ++
>> .../ti-tisdk-makefile/Makefile_omap3-sgx-modules   |   29 
>> ++++++++++++++++++++
>> .../ti-tisdk-makefile/ti-tisdk-makefile_1.0.bb     |   11 +++++++-
>> 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 
>> meta-arago-distro/recipes-tisdk/ti-tisdk-makefile/ti-tisdk-makefile/M
>> akefile_omap3-sgx-modules
>> 
>> diff --git a/meta-arago-distro/conf/distro/arago-source-ipk.conf 
>> b/meta-arago-distro/conf/distro/arago-source-ipk.conf
>> index b457504..40c7058 100644
>> --- a/meta-arago-distro/conf/distro/arago-source-ipk.conf
>> +++ b/meta-arago-distro/conf/distro/arago-source-ipk.conf
>> @@ -94,6 +94,9 @@ SRCIPK_INSTALL_DIR_pn-ti-pru-sw-examples = 
>> "example-applications/pru-${PV}"
>> CREATE_SRCIPK_pn-ti-ocf-crypto-module = "1"
>> SRCIPK_INSTALL_DIR_pn-ti-ocf-crypto-module = 
>> "board-support/extra-drivers/${PN}-${PV}"
>> 
>> +CREATE_SRCIPK_pn-omap3-sgx-modules = "1"
>> +SRCIPK_INSTALL_DIR_pn-omap3-sgx-modules = 
>> "board-support/extra-drivers/${PN}-${PV}"
>> +
> We shouldn't call it omap3-sgx-modules since that name has no meaning within 
> the Graphics SDK nor anywhere outside of OE. Plus omap3 part is confusing 
> from an SDK perspective.
What would you suggest?
> 
> Now when you do sourceipk is it only including a certain portion of the 
> graphics SdK or is it packaging the entire contents of the graphics Sdk.
> 
> You may need to be careful but I though when you installed the graphics SDK 
> that it puts installation paths inside its various make files. This can cause 
> a problem since these paths will be from your build machine.
This is only the module source, as specified in the recipe: 
TI_BIN_UNPK_WDEXT="/Graphics_SDK_${SGXPV}"
S = "${WORKDIR}${TI_BIN_UNPK_WDEXT}/GFX_Linux_KM"
> 
>> CREATE_SRCIPK_pn-ti-compat-wireless-wl18xx = "1"
>> SRCIPK_INSTALL_DIR_pn-ti-compat-wireless-wl18xx = 
>> "board-support/extra-drivers/${PN}-${PV}"
>> 
>> diff --git 
>> a/meta-arago-distro/recipes-tisdk/ti-tisdk-makefile/ti-tisdk-makefile
>> /Makefile_omap3-sgx-modules 
>> b/meta-arago-distro/recipes-tisdk/ti-tisdk-makefile/ti-tisdk-makefile
>> /Makefile_omap3-sgx-modules
>> new file mode 100644
>> index 0000000..2bc0ba5
>> --- /dev/null
>> +++ b/meta-arago-distro/recipes-tisdk/ti-tisdk-makefile/ti-tisdk-make
>> +++ file/Makefile_omap3-sgx-modules
>> @@ -0,0 +1,29 @@
>> +omap3-sgx-modules: linux
>> +    @echo =====================================
>> +    @echo      Building omap3-sgx-modules
>> +    @echo =====================================
>> +    @cd board-support/extra-drivers; \
>> +    cd `find . -maxdepth 1 -name "omap3-sgx-modules*"`; \
>> +    make ARCH=arm KERNELDIR=$(LINUXKERNEL_INSTALL_DIR) BUILD=release 
>> +TI_PLATFORM=__PLATFORM_SGX__ SUPPORT_XORG=0
> Make sure you test this and compare against how we build the recipe in OE. I 
> remembered in the past you needed a special flag passed in to insure PM 
> runtime worked.
I got the make options directly from the recipe.
> 
> So really you need to test this by removing all the sgx modules from the SDK 
> and let the make install run on ur filesystem and see if everything  still 
> works. This includes suspend and resume while an sgx demo is running.
How do you  test this?
>> +
>> +omap3-sgx-modules_clean:
>> +    @echo =====================================
>> +    @echo      Cleaning omap3-sgx-modules
>> +    @echo =====================================
>> +    @cd board-support/extra-drivers; \
>> +    cd `find . -maxdepth 1 -name "omap3-sgx-modules*"`; \
>> +    make ARCH=arm KERNELDIR=$(LINUXKERNEL_INSTALL_DIR) clean
>> +
>> +omap3-sgx-modules_install:
>> +    @echo =====================================
>> +    @echo      Installing omap3-sgx-modules
>> +    @echo =====================================
>> +    @if [ ! -d $(DESTDIR) ] ; then \
>> +        echo "The extracted target filesystem directory doesn't exist."; \
>> +        echo "Please run setup.sh in the SDK's root directory and then try 
>> again."; \
>> +        exit 1; \
>> +    fi
>> +    @cd board-support/extra-drivers; \
>> +    cd `find . -maxdepth 1 -name "omap3-sgx-modules*"`; \
>> +    make -C $(LINUXKERNEL_INSTALL_DIR) SUBDIRS=`pwd` 
>> +INSTALL_MOD_PATH=$(DESTDIR) PREFIX=$(SDK_PATH_TARGET) 
>> +modules_install
> This will cause problems as is. The graphics recipe for reasons I never 
> installs the graphics modules differently then the graphics SDK.
> Typing from my phone. *reasons I never liked 
> So you will end with two versions of the graphics SDK in ur fs and both will 
> try to load and maybe possibly fail.
> 
> So either update the OE recipe to use the makefile install or update this 
> part to mimic the OE way it built things.
> *built and installs things
It seems that the recipe tries to fake that these modules are not out-of-tree, 
and I guess yocto can hide this by updating the necessary 
/lib/modules/<k_ver>/modules.* files. Without installing it like this, then the 
modules cannot be found by "modprobe".
>> +
>> diff --git 
>> a/meta-arago-distro/recipes-tisdk/ti-tisdk-makefile/ti-tisdk-makefile
>> _1.0.bb 
>> b/meta-arago-distro/recipes-tisdk/ti-tisdk-makefile/ti-tisdk-makefile
>> _1.0.bb
>> index ffedd4c..a31bb69 100644
>> --- 
>> a/meta-arago-distro/recipes-tisdk/ti-tisdk-makefile/ti-tisdk-makefile
>> _1.0.bb
>> +++ b/meta-arago-distro/recipes-tisdk/ti-tisdk-makefile/ti-tisdk-make
>> +++ file_1.0.bb
>> @@ -34,9 +34,10 @@ SRC_URI = "\
>>    file://Makefile_dual-camera-demo \
>>    file://Makefile_image-gallery \
>>    file://Makefile_cryptodev \
>> +    file://Makefile_omap3-sgx-modules \
>> "
>> 
>> -PR = "r41"
>> +PR = "r42"
>> 
>> MAKEFILES_COMMON = "linux \
>>                    matrix-gui \
>> @@ -69,6 +70,7 @@ MAKEFILES_append_ti33x = " u-boot-spl \
>>                           linux-dtbs \
>>                           wireless \
>>                           cryptodev \
>> +                           omap3-sgx-modules \
>> "
>> MAKEFILES_append_ti43x = " u-boot-spl \
>>                           ${QUICK_PLAYGROUND} \ @@ -76,6 +78,7 @@ 
>> MAKEFILES_append_ti43x = " u-boot-spl \
>>                           linux-dtbs \
>>                           wireless \
>>                           cryptodev \
>> +                           omap3-sgx-modules \
>>                           dual-camera-demo \
>>                           image-gallery \ "
>> @@ -95,6 +98,10 @@ MAKEFILES_append_am180x-evm = " pru \ 
>> PLATFORM_ARCH = "armv7-a"
>> PLATFORM_ARCH_omapl138 = "armv5te"
>> 
>> +PLATFORM_SGX = ""
>> +PLATFORM_SGX_ti33x = "ti335x"
>> +PLATFORM_SGX_ti43x = "ti43xx"
>> +
>> KERNEL_BUILD_CMDS = 
>> "${@base_conditional('KERNEL_IMAGETYPE','uImage','LOADADDR=${UBOOT_LOADADDRESS}
>>  uImage','zImage',d)}"
>> 
>> KERNEL_DEVICETREE_ti33x = "am335x-evm.dtb am335x-evmsk.dtb am335x-bone.dtb 
>> am335x-boneblack.dtb"
>> @@ -150,6 +157,8 @@ do_install () {
>> 
>>    sed -i -e "s/__KERNEL_BUILD_CMDS__/${KERNEL_BUILD_CMDS}/" 
>> ${D}/Makefile
>> 
>> +    sed -i -e "s/__PLATFORM_SGX__/${PLATFORM_SGX}/" ${D}/Makefile
>> +
>>    cat ${D}/Makefile | grep "__DTB_DEPEND__" > /dev/null
>> 
>>    if [ "$?" == "0" ]
>> --
>> 1.7.9.5
>> 
>> _______________________________________________
>> meta-arago mailing list
>> [email protected]
>> http://arago-project.org/cgi-bin/mailman/listinfo/meta-arago
> _______________________________________________
> meta-arago mailing list
> [email protected]
> http://arago-project.org/cgi-bin/mailman/listinfo/meta-arago
_______________________________________________
meta-arago mailing list
[email protected]
http://arago-project.org/cgi-bin/mailman/listinfo/meta-arago

Reply via email to