Thanks for the quick response.

-Best Regards,
Aravind


> -----Original Message-----
> From: Maupin, Chase
> Sent: Wednesday, February 05, 2014 12:50 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: 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.
> 
[Aravind Batni]  Yes, we will fix the 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?
> >>
> >[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?
> 
[Aravind Batni]  Yes, this is only headers and some C source package and will 
be part of DEVKIT only. (nothing goes into the root file system).
> >>
> >> >+
> >> >+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

Reply via email to