[Buildroot] [PATCH 0/1] ACS CCID PC/SC Driver

[Buildroot] [PATCH 0/1] ACS CCID PC/SC Driver

[Juha Rantanen]

Hi,

This is my first patch, hope I did everything right. My device I tested with
is ACR122U USB NFC Reader on RPi 2 but the driver should support a lot of ACS
readers.

I wasn't sure whether this belongs in "Hardware handling" or
"Libraries/Hardware handling" and also not sure if flex and perl are needed
only on build.

Source: https://github.com/acshk/acsccid

Juha Rantanen (1):
  ACS CCID PC/SC Driver added.

 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

-- 
2.1.4

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

[Juha Rantanen]

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

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))
-- 
2.1.4

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

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v2] ACS CCID PC/SC Driver added.

[Juha Rantanen]

Fixes for acsccid.

Signed-off-by: Juha Rantanen <juha@codercoded.com>
---
Changes v1 -> v2:
  - syntax fixes
  - removed flex and perl deps
  - libusb optional
  - added license

 package/Config.in          |  1 +
 package/acsccid/Config.in  | 12 ++++++++++++
 package/acsccid/acsccid.mk | 22 ++++++++++++++++++++++
 3 files changed, 35 insertions(+)
 create mode 100644 package/acsccid/Config.in
 create mode 100644 package/acsccid/acsccid.mk

diff --git a/package/Config.in b/package/Config.in
index ef03632..25e574f 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -907,6 +907,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..b5af1d0
--- /dev/null
+++ b/package/acsccid/Config.in
@@ -0,0 +1,12 @@
+config BR2_PACKAGE_ACSCCID
+	bool "acsccid"
+	depends on BR2_TOOLCHAIN_HAS_THREADS
+	select BR2_PACKAGE_PCSC_LITE
+	help
+	  acsccid is a PC/SC driver for Linux/Mac OS X and it supports ACS CCID
+	  smart card readers.
+
+	  http://acsccid.sourceforge.net/
+
+comment "acsccid needs a toolchain w/ threads"
+	depends on !BR2_TOOLCHAIN_HAS_THREADS
diff --git a/package/acsccid/acsccid.mk b/package/acsccid/acsccid.mk
new file mode 100644
index 0000000..3d24414
--- /dev/null
+++ b/package/acsccid/acsccid.mk
@@ -0,0 +1,22 @@
+################################################################################
+#
+# acsccid
+#
+################################################################################
+
+ACSCCID_VERSION = 1.1.1
+ACSCCID_SOURCE = acsccid-$(ACSCCID_VERSION).tar.bz2
+ACSCCID_SITE = http://downloads.sourceforge.net/acsccid
+ACSCCID_LICENSE = LGPLv2.1+
+ACSCCID_LICENSE_FILES = COPYING
+ACSCCID_INSTALL_STAGING = YES
+ACSCCID_DEPENDENCIES = pcsc-lite host-perl host-flex host-pkgconf
+
+ifeq ($(BR2_PACKAGE_LIBUSB),y)
+ACSCCID_DEPENDENCIES += libusb
+ACSCCID_CONF_OPTS += --enable-libusb
+else
+ACSCCID_CONF_OPTS += --disable-libusb
+endif
+
+$(eval $(autotools-package))
-- 
2.1.4

[Buildroot] [PATCH v2] ACS CCID PC/SC Driver added.

[Arnout Vandecappelle]

 Hi Juha,

 Still a few more things that Thomas missed.

On 30-12-15 16:16, Juha Rantanen wrote:
> Fixes for acsccid.
> 
> Signed-off-by: Juha Rantanen <juha@codercoded.com>
> ---
> Changes v1 -> v2:
>   - syntax fixes
>   - removed flex and perl deps
>   - libusb optional
>   - added license
> 
>  package/Config.in          |  1 +
>  package/acsccid/Config.in  | 12 ++++++++++++
>  package/acsccid/acsccid.mk | 22 ++++++++++++++++++++++

 You should also add a hash file, see
http://nightly.buildroot.org/#adding-packages-hash

>  3 files changed, 35 insertions(+)
>  create mode 100644 package/acsccid/Config.in
>  create mode 100644 package/acsccid/acsccid.mk
> 
> diff --git a/package/Config.in b/package/Config.in
> index ef03632..25e574f 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -907,6 +907,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..b5af1d0
> --- /dev/null
> +++ b/package/acsccid/Config.in
> @@ -0,0 +1,12 @@
> +config BR2_PACKAGE_ACSCCID
> +	bool "acsccid"
> +	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	select BR2_PACKAGE_PCSC_LITE
> +	help
> +	  acsccid is a PC/SC driver for Linux/Mac OS X and it supports ACS CCID

 Still a bit too long, the tab counts as 8 characters so there should be just 62
"real" characters on the line.

> +	  smart card readers.
> +
> +	  http://acsccid.sourceforge.net/
> +
> +comment "acsccid needs a toolchain w/ threads"
> +	depends on !BR2_TOOLCHAIN_HAS_THREADS
> diff --git a/package/acsccid/acsccid.mk b/package/acsccid/acsccid.mk
> new file mode 100644
> index 0000000..3d24414
> --- /dev/null
> +++ b/package/acsccid/acsccid.mk
> @@ -0,0 +1,22 @@
> +################################################################################
> +#
> +# acsccid
> +#
> +################################################################################
> +
> +ACSCCID_VERSION = 1.1.1
> +ACSCCID_SOURCE = acsccid-$(ACSCCID_VERSION).tar.bz2
> +ACSCCID_SITE = http://downloads.sourceforge.net/acsccid
> +ACSCCID_LICENSE = LGPLv2.1+
> +ACSCCID_LICENSE_FILES = COPYING
> +ACSCCID_INSTALL_STAGING = YES
> +ACSCCID_DEPENDENCIES = pcsc-lite host-perl host-flex host-pkgconf

 I would be surprised if host-perl is really needed, normally the system perl
should be sufficient.

 Regards,
 Arnout

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


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2] ACS CCID PC/SC Driver added.

[Thomas Petazzoni]

Juha,

On Wed, 30 Dec 2015 17:16:23 +0200, Juha Rantanen wrote:
> Fixes for acsccid.
> 
> Signed-off-by: Juha Rantanen <juha@codercoded.com>
> ---
> Changes v1 -> v2:
>   - syntax fixes
>   - removed flex and perl deps
>   - libusb optional
>   - added license

I've applied after doing the following changes:

    [Thomas:
     - fix commit title
     - change libusb back to a mandatory dependency, since even though
       there is a --disable-libusb option, it has no effect, and the code
       fails to build due to missing libusb.h
     - add hash file, noticed by Arnout
     - remove host-perl dependency, noticed by Arnout
     - rewrap Config.in help text, noticed by Arnout.]

Thanks for your contribution!

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