diff mbox

[1/1] ACS CCID PC/SC Driver added.

Message ID 1451475203-23925-2-git-send-email-juha@codercoded.com
State Superseded
Headers show

Commit Message

Juha Rantanen Dec. 30, 2015, 11:33 a.m. UTC
Signed-off-by: Juha Rantanen <juha@codercoded.com>
---
 package/Config.in          |  1 +
 package/acsccid/Config.in  | 11 +++++++++++
 package/acsccid/acsccid.mk | 14 ++++++++++++++
 3 files changed, 26 insertions(+)
 create mode 100644 package/acsccid/Config.in
 create mode 100644 package/acsccid/acsccid.mk

Comments

Thomas Petazzoni Dec. 30, 2015, 1:19 p.m. UTC | #1
Dear Juha Rantanen,

Thanks for this contribution. See some comments below.

On Wed, 30 Dec 2015 13:33:23 +0200, Juha Rantanen wrote:

> diff --git a/package/acsccid/Config.in b/package/acsccid/Config.in
> new file mode 100644
> index 0000000..9635a99
> --- /dev/null
> +++ b/package/acsccid/Config.in
> @@ -0,0 +1,11 @@
> +config BR2_PACKAGE_ACSCCID
> +    bool "acsccid"

Indentation should be done with one tab.

> +    depends on BR2_TOOLCHAIN_HAS_THREADS # libusb
> +    select BR2_PACKAGE_PCSC_LITE
> +    select BR2_PACKAGE_LIBUSB

libusb is not mandatory, you can disable using --disable-libusb.

> +    select BR2_PACKAGE_FLEX
> +    select BR2_PACKAGE_PERL

What makes you think Perl and Flex are needed?

For Flex, I see a .l file in the source, so you quite probably needs
Flex on your build machine (by adding host-flex in <pkg>_DEPENDENCIES),
but I don't see why you would need Flex on the target.

For Perl, I see it being used at build time, but not at configure time.
And since we guarantee that a Perl interpreter is available on the
build machine as hard dependency of Buildroot, you don't need anything.

> +    help
> +      acsccid is a PC/SC driver for Linux/Mac OS X and it supports ACS CCID smart card readers.

Indentation for help text is one tab + two spaces. Also, it should be
wrapped so that the lines do not have more than 72 columns.

Since your package has a dependency on threads, you need to add a
Config.in comment like this:

comment "acsccid needs a toolchain w/ threads"
	depends on !BR2_TOOLCHAIN_HAS_THREADS

See the Buildroot manual for details about this (or the numerous
examples in the Buildroot tree).

> diff --git a/package/acsccid/acsccid.mk b/package/acsccid/acsccid.mk
> new file mode 100644
> index 0000000..1325e73
> --- /dev/null
> +++ b/package/acsccid/acsccid.mk
> @@ -0,0 +1,14 @@
> +#############################################################
> +#
> +# acsccid
> +#
> +#############################################################

80 # signs are needed, and an empty new line after this header.

> +ACSCCID_VERSION = 1.1.1
> +ACSCCID_SOURCE = acsccid-$(ACSCCID_VERSION).tar.bz2
> +ACSCCID_SITE = http://downloads.sourceforge.net/acsccid
> +ACSCCID_INSTALL_STAGING = YES
> +ACSCCID_INSTALL_TARGET = YES

Not needed, this is the default.

> +ACSCCID_CONF_OPTS = --enable-shared

Not needed, and potentially wrong. --{enable,disable}-{shared,static}
is automatically passed by the package infrastructure, depending on the
Buildroot configuration in terms of static/shared libraries.

> +ACSCCID_DEPENDENCIES = pcsc-lite libusb flex perl host-pkgconf

As indicated above, I believe the libusb dependency is optional, and
there is no flex and perl dependency, but instead just a host-flex
dependency.

So most likely, something like this:

ACSCCID_DEPENDENCIES = pcsc-lite host-flex host-pkgconf

ifeq ($(BR2_PACKAGE_LIBUSB),y)
ACSCCID_DEPENDENCIES += libusb
ACSCCID_CONF_OPTS += --enable-libusb
else
ACSCCID_CONF_OPTS += --disable-libusb
endif

Also, you forgot add the <pkg>_LICENSE and <pkg>_LICENSE_FILES
variables.

Could you rework your patch to take into account those comments, and
send an updated version ?

Thanks!

Thomas
diff mbox

Patch

diff --git a/package/Config.in b/package/Config.in
index bc6099d..c35cf17 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -906,6 +906,7 @@  menu "Graphics"
 endmenu
 
 menu "Hardware handling"
+	source "package/acsccid/Config.in"
 	source "package/bcm2835/Config.in"
 	source "package/c-periphery/Config.in"
 	source "package/ccid/Config.in"
diff --git a/package/acsccid/Config.in b/package/acsccid/Config.in
new file mode 100644
index 0000000..9635a99
--- /dev/null
+++ b/package/acsccid/Config.in
@@ -0,0 +1,11 @@ 
+config BR2_PACKAGE_ACSCCID
+    bool "acsccid"
+    depends on BR2_TOOLCHAIN_HAS_THREADS # libusb
+    select BR2_PACKAGE_PCSC_LITE
+    select BR2_PACKAGE_LIBUSB
+    select BR2_PACKAGE_FLEX
+    select BR2_PACKAGE_PERL
+    help
+      acsccid is a PC/SC driver for Linux/Mac OS X and it supports ACS CCID smart card readers.
+
+      http://acsccid.sourceforge.net/
diff --git a/package/acsccid/acsccid.mk b/package/acsccid/acsccid.mk
new file mode 100644
index 0000000..1325e73
--- /dev/null
+++ b/package/acsccid/acsccid.mk
@@ -0,0 +1,14 @@ 
+#############################################################
+#
+# acsccid
+#
+#############################################################
+ACSCCID_VERSION = 1.1.1
+ACSCCID_SOURCE = acsccid-$(ACSCCID_VERSION).tar.bz2
+ACSCCID_SITE = http://downloads.sourceforge.net/acsccid
+ACSCCID_INSTALL_STAGING = YES
+ACSCCID_INSTALL_TARGET = YES
+ACSCCID_CONF_OPTS = --enable-shared
+ACSCCID_DEPENDENCIES = pcsc-lite libusb flex perl host-pkgconf
+
+$(eval $(autotools-package))