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