>-----Original Message-----
>From: [email protected] [mailto:meta-arago-
>[email protected]] On Behalf Of Aravind Batni
>Sent: Tuesday, February 04, 2014 4:40 PM
>To: [email protected]; Nelson, Sam
>Cc: Aravind Batni
>Subject: [meta-arago] [PATCH] ti-csl: new recipe providing CSL for
>TI-KeyStone2 devices -This package provides chip support library
>interface files

Thanks for the submission.  Some review comments below.

This looks like a malformed commit message

>
>Signed-off-by: Aravind Batni <[email protected]>
>---
> meta-arago-extras/recipes-bsp/ti-csl/ti-csl_git.bb |   27
>++++++++++++++++++++
> 1 file changed, 27 insertions(+)
> create mode 100644 meta-arago-extras/recipes-bsp/ti-csl/ti-
>csl_git.bb
>
>diff --git a/meta-arago-extras/recipes-bsp/ti-csl/ti-csl_git.bb
>b/meta-arago-extras/recipes-bsp/ti-csl/ti-csl_git.bb
>new file mode 100644
>index 0000000..2896506
>--- /dev/null
>+++ b/meta-arago-extras/recipes-bsp/ti-csl/ti-csl_git.bb
>@@ -0,0 +1,27 @@
>+DESCRIPTION = "TI CSL"
>+LICENSE = "TI BSD"

Any reason this can't just be BSD?

>+LIC_FILES_CHKSUM =
>"file://COPYING.txt;md5=5857833e20836213677fac33f9aded21"
>+
>+COMPATIBLE_MACHINE = "keystone-evm"
>+ALLOW_EMPTY_${PN} = "1"

Why do you want an empty package?  Judging from below this is because you are 
only trying to install headers right and those will be part of the dev package? 
 Just want to make sure I understand your purpose here.

>+
>+PR = "r1"

Start with r0

>+BRANCH="master"
>+LLD-NAME="csl"

Unused variable?

>+SRC_URI = "git://git.ti.com/keystone-rtos/common-csl-
>ip.git;protocol=git;branch=${BRANCH}"
>+
>+S = "${WORKDIR}/git"
>+# commit ID corresponds to DEV.CSL_KEYSTONE2.02.00.00.17
>+SRCREV = "f6f90144c14e1ee783c4b893b52e54830be8166e"

Just a nitpick but this would make more sense being declared before the SRC_URI 
line.  Next to the BRANCH setting.

>+
>+do_install () {
>+    install -d ${D}/${includedir}/ti/csl
>+    cp -r ${S}/* ${D}/${includedir}/ti/csl

Use install command.  Also, no need for the / between ${D} and ${includedir}

>+#   Retain only header files
>+    cd ${D}/${includedir}/ti/csl
>+    find . \( -name "*.xdt" -o -name "*.bld" -o -name "*.bat" -o
>-name "*.c" -o -name "*.xdc" -o -name "*.asm" -o -name "*.xs" \) |
>xargs rm -rf

This seems kludgy.  Why not just find all the .h files and then install those 
with a for loop?

>+    rm -rf build docs
>+    mkdir -p src/ip/serdes_sb

Implicit assumption that you are in a certain directory.  Please reference 
everything from ${D}

>+    cp -r ${S}/src/ip/serdes_sb/V0 src/ip/serdes_sb

Same comment about the assumption of directory location.

>+}
>+
>--
>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

Reply via email to