[Buildroot] [PATCH 0/3] Support for the Freescale frame manager utility.

[Buildroot] [PATCH 0/3] Support for the Freescale frame manager utility.

[Matt Weber]

The fmc package builds the actual command line tool.  With the fmlib
package used for common functions/headers and the tclap package
used to aid parsing of command line arguments.

The initial version was selected as the latest stable FSL SDK release (v1.5).

The fmc tool does add a configuration folder (/etc/fmc/) but makes an assumption
that any configuration files are user provided as they change based on platform
use cases.

Matt Weber (3):
  fmlib: new package
  tclap: new package
  fmc: new package

 package/Config.in       |    3 +++
 package/fmc/Config.in   |   14 ++++++++++++++
 package/fmc/fmc.mk      |   32 ++++++++++++++++++++++++++++++++
 package/fmlib/Config.in |    9 +++++++++
 package/fmlib/fmlib.mk  |   34 ++++++++++++++++++++++++++++++++++
 package/tclap/Config.in |   12 ++++++++++++
 package/tclap/tclap.mk  |   16 ++++++++++++++++
 7 files changed, 120 insertions(+)
 create mode 100644 package/fmc/Config.in
 create mode 100644 package/fmc/fmc.mk
 create mode 100644 package/fmlib/Config.in
 create mode 100644 package/fmlib/fmlib.mk
 create mode 100644 package/tclap/Config.in
 create mode 100644 package/tclap/tclap.mk

-- 
1.7.9.5

[Buildroot] [PATCH 1/3] fmlib: new package

[Matt Weber]

Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
---
 package/Config.in       |    1 +
 package/fmlib/Config.in |    9 +++++++++
 package/fmlib/fmlib.mk  |   33 +++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+)
 create mode 100644 package/fmlib/Config.in
 create mode 100644 package/fmlib/fmlib.mk

diff --git a/package/Config.in b/package/Config.in
index 1706197..60118ea 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -715,6 +715,7 @@ source "package/c-ares/Config.in"
 source "package/cppzmq/Config.in"
 source "package/czmq/Config.in"
 source "package/filemq/Config.in"
+source "package/fmlib/Config.in"
 source "package/glib-networking/Config.in"
 source "package/libcgi/Config.in"
 source "package/libcgicc/Config.in"
diff --git a/package/fmlib/Config.in b/package/fmlib/Config.in
new file mode 100644
index 0000000..43f7aa3
--- /dev/null
+++ b/package/fmlib/Config.in
@@ -0,0 +1,9 @@
+config BR2_PACKAGE_FMLIB
+	bool "fmlib"
+	depends on BR2_powerpc_e500mc
+	help
+	  The Frame Manager library provides an API on top of the Frame Manager driver
+	  ioctl calls, that provides a user space application with a simple way
+	  to configure driver parameters and PCD (parse - classify - distribute) rules.
+
+	  http://git.freescale.com/git/cgit.cgi/ppc/sdk/fmlib.git/tree/README
diff --git a/package/fmlib/fmlib.mk b/package/fmlib/fmlib.mk
new file mode 100644
index 0000000..b2dedaa
--- /dev/null
+++ b/package/fmlib/fmlib.mk
@@ -0,0 +1,33 @@
+#############################################################
+#
+# fmlib
+#
+#############################################################
+
+FMLIB_VERSION = fsl-sdk-v1.5-rc3
+FMLIB_SITE = git://git.freescale.com/ppc/sdk/fmlib.git
+FMLIB_LICENSE = BSD-c3
+FMLIB_LICENSE_FILES = COPYING
+FMLIB_DEPENDENCIES = linux
+FMLIB_INSTALL_STAGING = YES
+FMLIB_INSTALL_TARGET = NO
+
+FMLIB_MAKE_OPTS = \
+	CC="$(TARGET_CC)" \
+	CROSS_COMPILE="$(TARGET_CROSS)" \
+	KERNEL_SRC="$(LINUX_DIR)" \
+	PREFIX="$(STAGING_DIR)/usr"
+
+ifeq ($(BR2_powerpc_e500mc), y)
+	FMLIB_ARCHTYPE = ppce500mc
+endif
+
+define FMLIB_BUILD_CMDS
+	$(TARGET_MAKE_ENV) $(MAKE) $(FMLIB_MAKE_OPTS) -C $(@D) libfm-$(FMLIB_ARCHTYPE).a
+endef
+
+define FMLIB_INSTALL_STAGING_CMDS
+	$(FMLIB_MAKE_ENV) $(MAKE) $(FMLIB_MAKE_OPTS) -C $(@D) install-libfm-$(FMLIB_ARCHTYPE)
+endef
+
+$(eval $(generic-package))
-- 
1.7.9.5

[Buildroot] [PATCH 2/3] tclap: new package

[Matt Weber]

Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
---
 package/Config.in       |    1 +
 package/tclap/Config.in |   12 ++++++++++++
 package/tclap/tclap.mk  |   16 ++++++++++++++++
 3 files changed, 29 insertions(+)
 create mode 100644 package/tclap/Config.in
 create mode 100644 package/tclap/tclap.mk

diff --git a/package/Config.in b/package/Config.in
index 60118ea..ac3af6b 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -856,6 +856,7 @@ source "package/pcre/Config.in"
 source "package/popt/Config.in"
 source "package/readline/Config.in"
 source "package/slang/Config.in"
+source "package/tclap/Config.in"
 endmenu
 
 endmenu
diff --git a/package/tclap/Config.in b/package/tclap/Config.in
new file mode 100644
index 0000000..e50704b
--- /dev/null
+++ b/package/tclap/Config.in
@@ -0,0 +1,12 @@
+config BR2_PACKAGE_TCLAP
+	bool "tclap"
+	help
+	  This is a small, flexible library that provides a simple interface for
+	  defining and accessing command line arguments. It was intially inspired
+	  by the user friendly CLAP libary. The difference is that this library is
+	  templatized, so the argument class is type independent. Type independence
+	  avoids identical-except-for-type objects, such as IntArg, FloatArg, and
+	  StringArg. While the library is not strictly compliant with the GNU or
+	  POSIX standards, it is close.
+
+	  http://tclap.sourceforge.net/
diff --git a/package/tclap/tclap.mk b/package/tclap/tclap.mk
new file mode 100644
index 0000000..268b90b
--- /dev/null
+++ b/package/tclap/tclap.mk
@@ -0,0 +1,16 @@
+################################################################################
+#
+# tclap
+#
+################################################################################
+
+TCLAP_VERSION = 1.2.1
+TCLAP_SITE = http://downloads.sourceforge.net/project/tclap
+TCLAP_SOURCE = tclap-$(TCLAP_VERSION).tar.gz
+TCLAP_LICENSE = MIT
+TCLAP_LICENSE_FILES = COPYING
+
+TCLAP_INSTALL_STAGING = YES
+TCLAP_INSTALL_TARGET = NO
+
+$(eval $(autotools-package))
-- 
1.7.9.5

[Buildroot] [PATCH 3/3] fmc: new package

[Matt Weber]

Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com>
---
 package/Config.in     |    1 +
 package/fmc/Config.in |   14 ++++++++++++++
 package/fmc/fmc.mk    |   31 +++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+)
 create mode 100644 package/fmc/Config.in
 create mode 100644 package/fmc/fmc.mk

diff --git a/package/Config.in b/package/Config.in
index ac3af6b..f874eaf 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -918,6 +918,7 @@ source "package/dnsmasq/Config.in"
 source "package/dropbear/Config.in"
 source "package/ebtables/Config.in"
 source "package/ethtool/Config.in"
+source "package/fmc/Config.in
 source "package/foomatic-filters/Config.in"
 source "package/fping/Config.in"
 source "package/gesftpserver/Config.in"
diff --git a/package/fmc/Config.in b/package/fmc/Config.in
new file mode 100644
index 0000000..9530756
--- /dev/null
+++ b/package/fmc/Config.in
@@ -0,0 +1,14 @@
+config BR2_PACKAGE_FMC
+	bool "fmc"
+	depends on BR2_powerpc_e500mc
+	select BR2_PACKAGE_TCLAP
+	select BR2_PACKAGE_LIBXML2
+	select BR2_PACKAGE_FMLIB
+	help
+	  The Frame Manager Configuration tool is a software package whose primary
+	  purpose is converting Parse-Classify-Police-Distribute (PCD) descriptions of
+	  network packets flow into hardware configuration. The tool provides an
+	  abstraction layer between the end customer's vision of the PCD task
+	  definition and real hardware implementation.
+
+	  http://cache.freescale.com/files/32bit/doc/app_note/AN4760.pdf
diff --git a/package/fmc/fmc.mk b/package/fmc/fmc.mk
new file mode 100644
index 0000000..513a8ef
--- /dev/null
+++ b/package/fmc/fmc.mk
@@ -0,0 +1,31 @@
+###############################################################################
+#
+# fmc
+#
+###############################################################################
+
+FMC_VERSION = fsl-sdk-v1.5-rc3
+FMC_SITE = git://git.freescale.com/ppc/sdk/fmc.git
+FMC_LICENSE = MIT
+FMC_LICENSE_FILES = COPYING
+FMC_DEPENDENCIES = libxml2 tclap fmlib
+
+FMC_MAKE_OPTS = \
+	CC="$(TARGET_CC)" \
+	CXX="$(TARGET_CXX)" \
+	FMD_USPACE_HEADER_PATH="$(STAGING_DIR)/usr/include/fmd" \
+	FMD_USPACE_LIB_PATH="$(STAGING_DIR)/usr/lib" \
+	LIBXML2_HEADER_PATH="$(STAGING_DIR)/usr/include/libxml2" \
+	TCLAP_HEADER_PATH="$(STAGING_DIR)/usr/include"
+
+define FMC_BUILD_CMDS
+	# The linking step has dependency issues so using MAKE1
+	$(TARGET_MAKE_ENV) $(MAKE1) $(FMC_MAKE_OPTS) -C $(@D)/source
+endef
+
+define FMC_INSTALL_TARGET_CMDS
+	$(INSTALL) -D -m 0755 $(@D)/source/fmc $(TARGET_DIR)/usr/sbin/fmc
+	cp -dr --no-preserve=ownership $(@D)/etc/fmc $(TARGET_DIR)/etc/
+endef
+
+$(eval $(generic-package))
-- 
1.7.9.5

[Buildroot] [PATCH 1/3] fmlib: new package

[Thomas Petazzoni]

Dear Matt Weber,

On Tue, 10 Jun 2014 08:02:04 -0700, Matt Weber wrote:

> diff --git a/package/fmlib/Config.in b/package/fmlib/Config.in
> new file mode 100644
> index 0000000..43f7aa3
> --- /dev/null
> +++ b/package/fmlib/Config.in
> @@ -0,0 +1,9 @@
> +config BR2_PACKAGE_FMLIB
> +	bool "fmlib"
> +	depends on BR2_powerpc_e500mc

If your package has 'linux' in its dependencies, then it should
'depends on BR2_LINUX_KERNEL' here, and have a comment about this. See
package/freescale-imx/imx-lib/Config.in for an example.

> +	help
> +	  The Frame Manager library provides an API on top of the Frame Manager driver
> +	  ioctl calls, that provides a user space application with a simple way
> +	  to configure driver parameters and PCD (parse - classify - distribute) rules.

I think the lines are too long here. Wrap to 72 max.

> +FMLIB_VERSION = fsl-sdk-v1.5-rc3
> +FMLIB_SITE = git://git.freescale.com/ppc/sdk/fmlib.git
> +FMLIB_LICENSE = BSD-c3

We actually use BSD-3c and not BSD-c3. Also, the license should
actually be:

	BSD-3c or GPLv2+

See the COPYING file of the project for details.

> +FMLIB_LICENSE_FILES = COPYING
> +FMLIB_DEPENDENCIES = linux
> +FMLIB_INSTALL_STAGING = YES
> +FMLIB_INSTALL_TARGET = NO

Please add a comment above this like:

# This package installs a static library only, so there's nothing to
# install to the target

> +FMLIB_MAKE_OPTS = \
> +	CC="$(TARGET_CC)" \
> +	CROSS_COMPILE="$(TARGET_CROSS)" \
> +	KERNEL_SRC="$(LINUX_DIR)" \
> +	PREFIX="$(STAGING_DIR)/usr"
> +
> +ifeq ($(BR2_powerpc_e500mc), y)

No space between , and y

> +	FMLIB_ARCHTYPE = ppce500mc
> +endif

That being said, since the package can only be enabled on ppce500mc, do
we really need this condition? Or maybe you're making the thing a bit
more future proof to support the other variants?

> +define FMLIB_BUILD_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE) $(FMLIB_MAKE_OPTS) -C $(@D) libfm-$(FMLIB_ARCHTYPE).a
> +endef
> +
> +define FMLIB_INSTALL_STAGING_CMDS
> +	$(FMLIB_MAKE_ENV) $(MAKE) $(FMLIB_MAKE_OPTS) -C $(@D) install-libfm-$(FMLIB_ARCHTYPE)
> +endef
> +
> +$(eval $(generic-package))

Other than that, looks good. Care to fix and resend?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/3] tclap: new package

[Thomas Petazzoni]

Dear Matt Weber,

On Tue, 10 Jun 2014 08:02:05 -0700, Matt Weber wrote:

> diff --git a/package/tclap/Config.in b/package/tclap/Config.in
> new file mode 100644
> index 0000000..e50704b
> --- /dev/null
> +++ b/package/tclap/Config.in
> @@ -0,0 +1,12 @@
> +config BR2_PACKAGE_TCLAP
> +	bool "tclap"

This package needs C++ support, so the appropriate 'depends on' +
comment is needed.

> +	help
> +	  This is a small, flexible library that provides a simple interface for
> +	  defining and accessing command line arguments. It was intially inspired

intially -> initially.

> +	  by the user friendly CLAP libary. The difference is that this library is
> +	  templatized, so the argument class is type independent. Type independence
> +	  avoids identical-except-for-type objects, such as IntArg, FloatArg, and
> +	  StringArg. While the library is not strictly compliant with the GNU or
> +	  POSIX standards, it is close.

Please check lines do not exceed 72 chars.

> +	  http://tclap.sourceforge.net/
> diff --git a/package/tclap/tclap.mk b/package/tclap/tclap.mk
> new file mode 100644
> index 0000000..268b90b
> --- /dev/null
> +++ b/package/tclap/tclap.mk
> @@ -0,0 +1,16 @@
> +################################################################################
> +#
> +# tclap
> +#
> +################################################################################
> +
> +TCLAP_VERSION = 1.2.1
> +TCLAP_SITE = http://downloads.sourceforge.net/project/tclap
> +TCLAP_SOURCE = tclap-$(TCLAP_VERSION).tar.gz

Not needed, that's the default.

> +TCLAP_LICENSE = MIT
> +TCLAP_LICENSE_FILES = COPYING
> +
> +TCLAP_INSTALL_STAGING = YES
> +TCLAP_INSTALL_TARGET = NO

A short comment that explains why INSTALL_TARGET = NO is useful. Just
say it's a pure C++ template library, only made of headers.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 3/3] fmc: new package

[Thomas Petazzoni]

Dear Matt Weber,

On Tue, 10 Jun 2014 08:02:06 -0700, Matt Weber wrote:

> diff --git a/package/fmc/Config.in b/package/fmc/Config.in
> new file mode 100644
> index 0000000..9530756
> --- /dev/null
> +++ b/package/fmc/Config.in
> @@ -0,0 +1,14 @@
> +config BR2_PACKAGE_FMC
> +	bool "fmc"
> +	depends on BR2_powerpc_e500mc
> +	select BR2_PACKAGE_TCLAP

Propagate the C++ dependency of tclap here.

> +	select BR2_PACKAGE_LIBXML2
> +	select BR2_PACKAGE_FMLIB
> +	help
> +	  The Frame Manager Configuration tool is a software package whose primary
> +	  purpose is converting Parse-Classify-Police-Distribute (PCD) descriptions of
> +	  network packets flow into hardware configuration. The tool provides an
> +	  abstraction layer between the end customer's vision of the PCD task
> +	  definition and real hardware implementation.

Length of lines too long. It would be good to mention that it's
specific to Freescale PowerPC machines at the beginning of the
description.

> +
> +	  http://cache.freescale.com/files/32bit/doc/app_note/AN4760.pdf
> diff --git a/package/fmc/fmc.mk b/package/fmc/fmc.mk
> new file mode 100644
> index 0000000..513a8ef
> --- /dev/null
> +++ b/package/fmc/fmc.mk
> @@ -0,0 +1,31 @@
> +###############################################################################
> +#
> +# fmc
> +#
> +###############################################################################
> +
> +FMC_VERSION = fsl-sdk-v1.5-rc3
> +FMC_SITE = git://git.freescale.com/ppc/sdk/fmc.git
> +FMC_LICENSE = MIT
> +FMC_LICENSE_FILES = COPYING
> +FMC_DEPENDENCIES = libxml2 tclap fmlib
> +
> +FMC_MAKE_OPTS = \
> +	CC="$(TARGET_CC)" \
> +	CXX="$(TARGET_CXX)" \
> +	FMD_USPACE_HEADER_PATH="$(STAGING_DIR)/usr/include/fmd" \
> +	FMD_USPACE_LIB_PATH="$(STAGING_DIR)/usr/lib" \
> +	LIBXML2_HEADER_PATH="$(STAGING_DIR)/usr/include/libxml2" \
> +	TCLAP_HEADER_PATH="$(STAGING_DIR)/usr/include"
> +
> +define FMC_BUILD_CMDS
> +	# The linking step has dependency issues so using MAKE1
> +	$(TARGET_MAKE_ENV) $(MAKE1) $(FMC_MAKE_OPTS) -C $(@D)/source
> +endef
> +
> +define FMC_INSTALL_TARGET_CMDS
> +	$(INSTALL) -D -m 0755 $(@D)/source/fmc $(TARGET_DIR)/usr/sbin/fmc
> +	cp -dr --no-preserve=ownership $(@D)/etc/fmc $(TARGET_DIR)/etc/

Do we need this --no-preserve=ownership here? We usually use:

	cp -dpfr

in Buildroot.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 3/3] fmc: new package

[Matthew Weber]

Thomas,

On Tue, Jun 10, 2014 at 3:21 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Matt Weber,
>
> On Tue, 10 Jun 2014 08:02:06 -0700, Matt Weber wrote:
>
>> diff --git a/package/fmc/Config.in b/package/fmc/Config.in
>> new file mode 100644
>> index 0000000..9530756
>> --- /dev/null
>> +++ b/package/fmc/Config.in
>> @@ -0,0 +1,14 @@
>> +config BR2_PACKAGE_FMC
>> +     bool "fmc"
>> +     depends on BR2_powerpc_e500mc
>> +     select BR2_PACKAGE_TCLAP
>
> Propagate the C++ dependency of tclap here.

Agreed.

>
>> +     select BR2_PACKAGE_LIBXML2
>> +     select BR2_PACKAGE_FMLIB
>> +     help
>> +       The Frame Manager Configuration tool is a software package whose primary
>> +       purpose is converting Parse-Classify-Police-Distribute (PCD) descriptions of
>> +       network packets flow into hardware configuration. The tool provides an
>> +       abstraction layer between the end customer's vision of the PCD task
>> +       definition and real hardware implementation.
>
> Length of lines too long. It would be good to mention that it's
> specific to Freescale PowerPC machines at the beginning of the
> description.

Agreed.

>
>> +
>> +       http://cache.freescale.com/files/32bit/doc/app_note/AN4760.pdf
>> diff --git a/package/fmc/fmc.mk b/package/fmc/fmc.mk
>> new file mode 100644
>> index 0000000..513a8ef
>> --- /dev/null
>> +++ b/package/fmc/fmc.mk
>> @@ -0,0 +1,31 @@
>> +###############################################################################
>> +#
>> +# fmc
>> +#
>> +###############################################################################
>> +
>> +FMC_VERSION = fsl-sdk-v1.5-rc3
>> +FMC_SITE = git://git.freescale.com/ppc/sdk/fmc.git
>> +FMC_LICENSE = MIT
>> +FMC_LICENSE_FILES = COPYING
>> +FMC_DEPENDENCIES = libxml2 tclap fmlib
>> +
>> +FMC_MAKE_OPTS = \
>> +     CC="$(TARGET_CC)" \
>> +     CXX="$(TARGET_CXX)" \
>> +     FMD_USPACE_HEADER_PATH="$(STAGING_DIR)/usr/include/fmd" \
>> +     FMD_USPACE_LIB_PATH="$(STAGING_DIR)/usr/lib" \
>> +     LIBXML2_HEADER_PATH="$(STAGING_DIR)/usr/include/libxml2" \
>> +     TCLAP_HEADER_PATH="$(STAGING_DIR)/usr/include"
>> +
>> +define FMC_BUILD_CMDS
>> +     # The linking step has dependency issues so using MAKE1
>> +     $(TARGET_MAKE_ENV) $(MAKE1) $(FMC_MAKE_OPTS) -C $(@D)/source
>> +endef
>> +
>> +define FMC_INSTALL_TARGET_CMDS
>> +     $(INSTALL) -D -m 0755 $(@D)/source/fmc $(TARGET_DIR)/usr/sbin/fmc
>> +     cp -dr --no-preserve=ownership $(@D)/etc/fmc $(TARGET_DIR)/etc/
>
> Do we need this --no-preserve=ownership here? We usually use:

No, will update to cp -dpfr.

<snip>

-- 
Matthew L Weber / Sr Software Engineer / Platform Software
matthew.weber at rockwellcollins.com
www.rockwellcollins.com

[Buildroot] [PATCH 2/3] tclap: new package

[Matthew Weber]

Thomas,

On Tue, Jun 10, 2014 at 3:11 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Matt Weber,
>
> On Tue, 10 Jun 2014 08:02:05 -0700, Matt Weber wrote:
>
>> diff --git a/package/tclap/Config.in b/package/tclap/Config.in
>> new file mode 100644
>> index 0000000..e50704b
>> --- /dev/null
>> +++ b/package/tclap/Config.in
>> @@ -0,0 +1,12 @@
>> +config BR2_PACKAGE_TCLAP
>> +     bool "tclap"
>
> This package needs C++ support, so the appropriate 'depends on' +
> comment is needed.
>
>> +     help
>> +       This is a small, flexible library that provides a simple interface for
>> +       defining and accessing command line arguments. It was intially inspired
>
> intially -> initially.
Agree.

>
>> +       by the user friendly CLAP libary. The difference is that this library is
>> +       templatized, so the argument class is type independent. Type independence
>> +       avoids identical-except-for-type objects, such as IntArg, FloatArg, and
>> +       StringArg. While the library is not strictly compliant with the GNU or
>> +       POSIX standards, it is close.
>
> Please check lines do not exceed 72 chars.
Agree.

>
>> +       http://tclap.sourceforge.net/
>> diff --git a/package/tclap/tclap.mk b/package/tclap/tclap.mk
>> new file mode 100644
>> index 0000000..268b90b
>> --- /dev/null
>> +++ b/package/tclap/tclap.mk
>> @@ -0,0 +1,16 @@
>> +################################################################################
>> +#
>> +# tclap
>> +#
>> +################################################################################
>> +
>> +TCLAP_VERSION = 1.2.1
>> +TCLAP_SITE = http://downloads.sourceforge.net/project/tclap
>> +TCLAP_SOURCE = tclap-$(TCLAP_VERSION).tar.gz
>
> Not needed, that's the default.
Sure.

>
>> +TCLAP_LICENSE = MIT
>> +TCLAP_LICENSE_FILES = COPYING
>> +
>> +TCLAP_INSTALL_STAGING = YES
>> +TCLAP_INSTALL_TARGET = NO
>
> A short comment that explains why INSTALL_TARGET = NO is useful. Just
> say it's a pure C++ template library, only made of headers.
Sure.

<snip>


-- 
Matthew L Weber / Sr Software Engineer / Platform Software
matthew.weber at rockwellcollins.com
www.rockwellcollins.com

[Buildroot] [PATCH 1/3] fmlib: new package

[Matthew Weber]

Thomas, thanks for the review.

On Tue, Jun 10, 2014 at 3:01 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Matt Weber,
>
> On Tue, 10 Jun 2014 08:02:04 -0700, Matt Weber wrote:
>
>> diff --git a/package/fmlib/Config.in b/package/fmlib/Config.in
>> new file mode 100644
>> index 0000000..43f7aa3
>> --- /dev/null
>> +++ b/package/fmlib/Config.in
>> @@ -0,0 +1,9 @@
>> +config BR2_PACKAGE_FMLIB
>> +     bool "fmlib"
>> +     depends on BR2_powerpc_e500mc
>
> If your package has 'linux' in its dependencies, then it should
> 'depends on BR2_LINUX_KERNEL' here, and have a comment about this. See
> package/freescale-imx/imx-lib/Config.in for an example.
Agree.

>
>> +     help
>> +       The Frame Manager library provides an API on top of the Frame Manager driver
>> +       ioctl calls, that provides a user space application with a simple way
>> +       to configure driver parameters and PCD (parse - classify - distribute) rules.
>
> I think the lines are too long here. Wrap to 72 max.
Agree.

>
>> +FMLIB_VERSION = fsl-sdk-v1.5-rc3
>> +FMLIB_SITE = git://git.freescale.com/ppc/sdk/fmlib.git
>> +FMLIB_LICENSE = BSD-c3
>
> We actually use BSD-3c and not BSD-c3. Also, the license should
> actually be:
>
>         BSD-3c or GPLv2+
Good catch.

>
> See the COPYING file of the project for details.
>
>> +FMLIB_LICENSE_FILES = COPYING
>> +FMLIB_DEPENDENCIES = linux
>> +FMLIB_INSTALL_STAGING = YES
>> +FMLIB_INSTALL_TARGET = NO
>
> Please add a comment above this like:
>
> # This package installs a static library only, so there's nothing to
> # install to the target
Sure.

>
>> +FMLIB_MAKE_OPTS = \
>> +     CC="$(TARGET_CC)" \
>> +     CROSS_COMPILE="$(TARGET_CROSS)" \
>> +     KERNEL_SRC="$(LINUX_DIR)" \
>> +     PREFIX="$(STAGING_DIR)/usr"
>> +
>> +ifeq ($(BR2_powerpc_e500mc), y)
>
> No space between , and y
Sure.

>
>> +     FMLIB_ARCHTYPE = ppce500mc
>> +endif
>
> That being said, since the package can only be enabled on ppce500mc, do
> we really need this condition? Or maybe you're making the thing a bit
> more future proof to support the other variants?
(future proof)  I don't have the necessary target configurations to
test more then the
ppce500mc configuration at this point.  However it is capable of
building a number
of ppc and ppc64 libs.

<snip>

-- 
Matthew L Weber / Sr Software Engineer / Platform Software
matthew.weber at rockwellcollins.com
www.rockwellcollins.com