>-----Original Message-----
>From: Aravind Batni
>Sent: Wednesday, February 05, 2014 12:33 PM
>To: Maupin, Chase; [email protected]; Nelson, Sam
>Subject: RE: [meta-arago] [PATCH] ti-csl: new recipe providing CSL
>for TI-KeyStone2 devices -This package provides chip support
>library interface files
>
>Thanks for the feedback. See response in-lined.
>
>-Best Regards,
>Aravind
>
>
>> -----Original Message-----
>> From: Maupin, Chase
>> Sent: Tuesday, February 04, 2014 5:37 PM
>> To: Aravind Batni; [email protected]; Nelson, Sam
>> Subject: RE: [meta-arago] [PATCH] ti-csl: new recipe providing
>CSL for TI-
>> KeyStone2 devices -This package provides chip support library
>interface files
>>
>> >-----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
Any comment here? Also, when you submit v2 be sure to add something like the
below after your signed-off-by line:
---
* Updated in version 2
* change 1
* change 2
The --- above is important.
>>
>> >
>> >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?
>>
>[Aravind Batni] The components were approved by TI OSRB for TI BSD
>(has TI copyright statement on the top and BSD license text
>below). Not sure if we can change this to BSD
OK
>> >+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.
Any comment here?
>>
>> >+
>> >+PR = "r1"
>>
>> Start with r0
>>
>[Aravind Batni] Fixed
>> >+BRANCH="master"
>> >+LLD-NAME="csl"
>>
>> Unused variable?
>>
>[Aravind Batni] Removed the 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.
>>
>[Aravind Batni] Fixed
>> >+
>> >+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
>>
>[Aravind Batni] How about something like this: find ${S} -name
>"*.h" -type f | xargs -I {} cp --parents {}
>${D}${includedir}/ti/csl
Seems OK to me
>> 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.
>>
>[Aravind Batni] same as before: How about this: find
>${S}/src/ip/serdes_sb/V0 -name "*.c" -type f | xargs -I {} cp --
>parents {} ${D}${includedir}/ti/csl/src/ip/serdes_sb/V0
>> >+}
>> >+
>> >--
>> >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
