Re: [dunfell/master PATCH] ipc: ti-rpmsg-char: Add initial recipes

["Denys Dmytriyenko" <denys@ti.com>]

On Wed, Oct 21, 2020 at 05:09:38PM -0500, Suman Anna wrote:
> Hi Denys,
> 
> Thank you for the review comments.
> 
> On 10/21/20 3:49 PM, Denys Dmytriyenko wrote:
> > On Wed, Oct 21, 2020 at 08:56:21AM -0500, Suman Anna wrote:
> >> Add the initial recipes for building the ti-rpmsg-char package.
> >> The library and examples are built using two separate recipes
> >> in ti-rpmsg-char_git.bb and ti-rpmsg-char-examples_git.bb.
> >>
> >> Signed-off-by: Suman Anna <s-anna@ti.com>
> >> Signed-off-by: Gowtham Tammana <g-tammana@ti.com>
> >> ---
> >>  recipes-ti/ipc/ti-rpmsg-char-examples_git.bb | 12 ++++++++++++
> >>  recipes-ti/ipc/ti-rpmsg-char.inc             | 19 +++++++++++++++++++
> >>  recipes-ti/ipc/ti-rpmsg-char_git.bb          | 11 +++++++++++
> >>  3 files changed, 42 insertions(+)
> >>  create mode 100644 recipes-ti/ipc/ti-rpmsg-char-examples_git.bb
> >>  create mode 100644 recipes-ti/ipc/ti-rpmsg-char.inc
> >>  create mode 100644 recipes-ti/ipc/ti-rpmsg-char_git.bb
> >>
> >> diff --git a/recipes-ti/ipc/ti-rpmsg-char-examples_git.bb b/recipes-ti/ipc/ti-rpmsg-char-examples_git.bb
> >> new file mode 100644
> >> index 000000000000..1d86e95526cc
> >> --- /dev/null
> >> +++ b/recipes-ti/ipc/ti-rpmsg-char-examples_git.bb
> >> @@ -0,0 +1,12 @@
> >> +DESCRIPTION = "TI RPMsg Char Sample Applications and Tests"
> > 
> > SUMMARY is for short one-liner description of the package
> > DESCRIPTION is for longer, potentially multi-line detailed description
> > If only short one is used, then should be SUMMARY
> 
> OK, will update.
> 
> > 
> > 
> >> +require ti-rpmsg-char.inc
> >> +
> >> +DEPENDS = "ti-rpmsg-char"
> >> +RDEPENDS_${PN} = "ti-rpmsg-char"
> > 
> > This is redundant - build dependency automatically sets corresponding run-time 
> > dependency.
> 
> OK, will drop the RDEPENDS line.
> 
> > 
> > 
> >> +inherit autotools pkgconfig
> >> +
> >> +EXTRA_OEMAKE = "-C ${B}/examples"
> >> +
> >> +PR = "r0"
> >> diff --git a/recipes-ti/ipc/ti-rpmsg-char.inc b/recipes-ti/ipc/ti-rpmsg-char.inc
> >> new file mode 100644
> >> index 000000000000..454deea674b3
> >> --- /dev/null
> >> +++ b/recipes-ti/ipc/ti-rpmsg-char.inc
> >> @@ -0,0 +1,19 @@
> >> +DESCRIPTION = "TI RPMsg Char Utility Library"
> > 
> > Same comment.
> 
> OK, will fix this as well.
> 
> > 
> > 
> >> +LICENSE = "BSD-3-Clause"
> >> +LIC_FILES_CHKSUM = "file://${S}/src/rpmsg_char.c;beginline=1;endline=31;md5=71987db43a2159cff5ea00505c6fce58"
> >> +
> >> +TI_RPMSG_CHAR_GIT_URI = "git://git.ti.com/rpmsg/ti-rpmsg-char.git"
> >> +TI_RPMSG_CHAR_GIT_PROTOCOL = "git"
> >> +TI_RPMSG_CHAR_GIT_BRANCH = "master"
> > 
> > Any specific reason for these redirections? These were used by some recipes to 
> > be able to easily override these vars from outisde the recipe to point to some 
> > internal development tree... But even then there's another way to override 
> > those and don't use such indirect variables.
> 
> No specific reason, it was from an existing recipe I used as reference in
> creating this one. I can simplify by using the direct variables.
> 
> > 
> > E.g. instead of this in the local.conf:
> > 
> > TI_RPMSG_CHAR_GIT_BRANCH = "next"
> > 
> > One can do this:
> > 
> > BRANCH_pn-ti-rpmsg-char = "next"
> > BRANCH_pn-ti-rpmsg-char-examples = "next"
> 
> But do we even need to separate this out for the two packages? Is there an issue
> with just using,
> 
> BRANCH = "master"

In the recipe, yes. I was showing how you could override it from outside of 
the recipe to point it to "next" branch w/o using TI_RPMSG_CHAR_GIT_BRANCH.


> >> +# Initial 0.1.0 release
> >> +TI_RPMSG_CHAR_SRCREV = "9be402c2bf4053900b6b9a8092fbb42e7e303108"
> >> +
> >> +BRANCH = "${TI_RPMSG_CHAR_GIT_BRANCH}"
> >> +SRC_URI = "${TI_RPMSG_CHAR_GIT_URI};protocol=${TI_RPMSG_CHAR_GIT_PROTOCOL};branch=${BRANCH};name=ti-rpmsg-char"
> > 
> > The part at the end is unnecessary - ;name=ti-rpmsg-char
> 
> OK
> 
> > 
> > 
> >> +SRCREV = "${TI_RPMSG_CHAR_SRCREV}"
> >> +
> >> +PV = "0.1.0+git${SRCPV}"
> >> +
> >> +S = "${WORKDIR}/git"
> >> diff --git a/recipes-ti/ipc/ti-rpmsg-char_git.bb b/recipes-ti/ipc/ti-rpmsg-char_git.bb
> >> new file mode 100644
> >> index 000000000000..c132a390f301
> >> --- /dev/null
> >> +++ b/recipes-ti/ipc/ti-rpmsg-char_git.bb
> >> @@ -0,0 +1,11 @@
> >> +DESCRIPTION = "TI RPMsg Char Utility Library"
> >> +
> >> +require ti-rpmsg-char.inc
> >> +
> >> +DEPENDS += "virtual/kernel"
> >> +
> >> +inherit autotools pkgconfig
> >> +
> >> +do_compile[depends] += "virtual/kernel:do_shared_workdir"
> > 
> > How do you pass kernel location for the header file? Is it hardcoded in the 
> > Makefile?
> 
> The kernel header is from an already upstream driver, so is already exported and
> available on the host machine. I didn't have to pass any specific kernel path
> during the build, just the include line <linux/rpmsg.h> in the source code.

In that case you don't need extra dependencies - you can drop DEPENDS and 
do_compile[depends] lines. As linux/rpmsg.h file comes from the standard 
linux-libc-headers package, which is the default dependency.


> Granted, this might be needed on some older machines running an ancient kernel,
> that would need some additional conditionals to be added to the project build
> environment. I have added the do_compile for scalability if we end up adding
> functionality to the header from the kernel we are building.
> 
> regards
> Suman
> 
> > 
> > 
> >> +PR = "r0"
> >> -- 
> >> 2.28.0
> >>
>