On Fri, Feb 21, 2014 at 08:07:41PM -0500, Aravind Batni wrote:
> - TI Resource Manager Low Level Driver
>
> Signed-off-by: Aravind Batni <[email protected]>
>
> ---
> Changes from last patch:
> * Removed ti-rm.inc file
> * Added destsuffix in the SRCURI to clone RM git under git/ti/drv/rm folder
> * Removed the symbolic link workaround
> * Added PV
> * Added for loop for multiple devices and choices for the test binaries
Thanks! That's a good list of fixes and the patch now looks much better and
cleaner. Please see few minor comments inline.
> ---
> ---
> meta-arago-extras/recipes-bsp/ti-rm/ti-rm_git.bb | 53
> ++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
> create mode 100755 meta-arago-extras/recipes-bsp/ti-rm/ti-rm_git.bb
>
> diff --git a/meta-arago-extras/recipes-bsp/ti-rm/ti-rm_git.bb
> b/meta-arago-extras/recipes-bsp/ti-rm/ti-rm_git.bb
> new file mode 100755
> index 0000000..761a722
> --- /dev/null
> +++ b/meta-arago-extras/recipes-bsp/ti-rm/ti-rm_git.bb
> @@ -0,0 +1,53 @@
> +DESCRIPTION = "TI Resource Manager Low Level Driver"
> +LICENSE = "TI BSD"
> +LIC_FILES_CHKSUM =
> "file://${WORKDIR}/git/ti/drv/rm/COPYING.txt;md5=dc61631b65360e6beb73b6c337800afc"
> +
> +BRANCH="master"
> +SRC_URI =
> "git://git.ti.com/keystone-rtos/rm-lld.git;destsuffix=git/ti/drv/rm;protocol=git;branch=${BRANCH}"
> +# Below commit ID corresponds to DEV.RM_LLD.02.00.00.08
> +SRCREV = "3a73cfe015214ff0401639f85fa5e52ea610e59d"
> +PR = "r0"
> +PV = "02.00.00.08"
> +
> +COMPATIBLE_MACHINE = "keystone"
> +
> +DEPENDS="ti-ipc"
1. It depends on ti-ipc, so will have to wait for that one to get merged
first.
2. Not critical at all, but would be nice to have a single space before and
after = sign... :)
> +PACKAGES =+ "${PN}-test"
> +
> +FILES_${PN}-test = "${bindir}/rmDspClientTest_*.out \
> + ${bindir}/rmLinuxClientTest_*.out \
> + ${bindir}/ti/drv/rm/test/dts_files/*.dtb"
> +
> +DEVICELIST = " k2h \
> + k2k \
> +"
> +
> +CHOICELIST = " yes \
> + no \
> +"
Since you are using above 2 vars in a "for" loop, they can be simple one-line
strings like "k2h k2k" and "yes no". But it's Ok to split them in multiple
lines too - it's up to you.
> +BASEDIR = "${WORKDIR}/git"
> +S = "${BASEDIR}/ti/drv/rm"
> +
> +do_compile () {
> +# Now build the lld in the updated directory
> + make -f makefile_armv7 clean lib PDK_INSTALL_PATH=${STAGING_INCDIR}
> RM_SRC_DIR=${S}
> + for device in ${DEVICELIST}
> + do
> + for choice in ${CHOICELIST}
> + do
> + make -f makefile_armv7 tests
> IPC_DEVKIT_INSTALL_PATH=${STAGING_INCDIR} PDK_INSTALL_PATH=${BASEDIR}
> DEVICE="$device" USEDYNAMIC_LIB="$choice"
> + done
> + done
> +}
> +
> +do_install () {
> + install -d ${D}/${includedir}/ti/drv/rm
There seems to be an inconsistent indentation here - the above line uses tab,
while other lines use spaces. Either way is fine, as long as they are
consistent.
> + install -d ${D}/${libdir}
> + install -d ${D}/${bindir}
There shouldn't be / between ${D} and ${libdir}. All the ${*dir} variables are
defined from root, i.e. starting with / and hence you'd end up with double-/
in the code...
> + for device in ${DEVICELIST}
> + do
> + make -f makefile_armv7 install installbin installbin_test
> INSTALL_INC_BASE_DIR=${D}/${includedir} INSTALL_LIB_BASE_DIR=${D}/${libdir}
> INSTALL_BIN_BASE_DIR=${D}/${bindir} DEVICE="$device"
Same comment about ${D}/${*dir} as above.
> + done
> +}
> --
> 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
