linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
@ 2018-05-29 18:50 Marcus Folkesson
  2018-05-29 18:50 ` [PATCH v3 2/3] Documentation: usb: add documentation " Marcus Folkesson
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Marcus Folkesson @ 2018-05-29 18:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jonathan Corbet, Felipe Balbi, davem,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart
  Cc: linux-usb, linux-doc, linux-kernel, Marcus Folkesson

Chip Card Interface Device (CCID) protocol is a USB protocol that
allows a smartcard device to be connected to a computer via a card
reader using a standard USB interface, without the need for each manufacturer
of smartcards to provide its own reader or protocol.

This gadget driver makes Linux show up as a CCID device to the host and let a
userspace daemon act as the smartcard.

This is useful when the Linux gadget itself should act as a cryptographic
device or forward APDUs to an embedded smartcard device.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---

Notes:
    v3:
    	- fix sparse warnings reported by kbuild test robot
    v2:
    	- add the missing changelog text

 drivers/usb/gadget/Kconfig           |  17 +
 drivers/usb/gadget/function/Makefile |   1 +
 drivers/usb/gadget/function/f_ccid.c | 993 +++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/function/f_ccid.h |  91 ++++
 include/uapi/linux/usb/ccid.h        |  93 ++++
 5 files changed, 1195 insertions(+)
 create mode 100644 drivers/usb/gadget/function/f_ccid.c
 create mode 100644 drivers/usb/gadget/function/f_ccid.h
 create mode 100644 include/uapi/linux/usb/ccid.h

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 31cce7805eb2..bdebdf1ffa2b 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -149,6 +149,9 @@ config USB_LIBCOMPOSITE
 config USB_F_ACM
 	tristate
 
+config USB_F_CCID
+	tristate
+
 config USB_F_SS_LB
 	tristate
 
@@ -248,6 +251,20 @@ config USB_CONFIGFS_ACM
 	  ACM serial link.  This function can be used to interoperate with
 	  MS-Windows hosts or with the Linux-USB "cdc-acm" driver.
 
+config USB_CONFIGFS_CCID
+	bool "Chip Card Interface Device (CCID)"
+	depends on USB_CONFIGFS
+	select USB_F_CCID
+	help
+	  The CCID function driver provides generic emulation of a
+	  Chip Card Interface Device (CCID).
+
+	  You will need a user space server talking to /dev/ccidg*,
+	  since the kernel itself does not implement CCID/TPDU/APDU
+	  protocol.
+
+	  For more information, see Documentation/usb/gadget_ccid.rst.
+
 config USB_CONFIGFS_OBEX
 	bool "Object Exchange Model (CDC OBEX)"
 	depends on USB_CONFIGFS
diff --git a/drivers/usb/gadget/function/Makefile b/drivers/usb/gadget/function/Makefile
index 5d3a6cf02218..629851009e1a 100644
--- a/drivers/usb/gadget/function/Makefile
+++ b/drivers/usb/gadget/function/Makefile
@@ -9,6 +9,7 @@ ccflags-y			+= -I$(srctree)/drivers/usb/gadget/udc/
 # USB Functions
 usb_f_acm-y			:= f_acm.o
 obj-$(CONFIG_USB_F_ACM)		+= usb_f_acm.o
+obj-$(CONFIG_USB_F_CCID)	+= f_ccid.o
 usb_f_ss_lb-y			:= f_loopback.o f_sourcesink.o
 obj-$(CONFIG_USB_F_SS_LB)	+= usb_f_ss_lb.o
 obj-$(CONFIG_USB_U_SERIAL)	+= u_serial.o
diff --git a/drivers/usb/gadget/function/f_ccid.c b/drivers/usb/gadget/function/f_ccid.c
new file mode 100644
index 000000000000..47fb229a06db
--- /dev/null
+++ b/drivers/usb/gadget/function/f_ccid.c
@@ -0,0 +1,993 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * f_ccid.c -- Chip Card Interface Device (CCID) function Driver
+ *
+ * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@gmail.com>
+ *
+ */
+#include <linux/cdev.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/poll.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/usb/composite.h>
+#include <uapi/linux/usb/ccid.h>
+
+#include "f_ccid.h"
+#include "u_f.h"
+
+/* Number of tx requests to allocate */
+#define N_TX_REQS 4
+
+/* Maximum number of devices */
+#define CCID_MINORS 4
+
+struct ccidg_bulk_dev {
+	atomic_t is_open;
+	atomic_t rx_req_busy;
+	wait_queue_head_t read_wq;
+	wait_queue_head_t write_wq;
+	struct usb_request *rx_req;
+	atomic_t rx_done;
+	struct list_head tx_idle;
+};
+
+struct f_ccidg {
+	struct usb_function_instance	func_inst;
+	struct usb_function function;
+	spinlock_t lock;
+	atomic_t online;
+
+	/* Character device */
+	struct cdev cdev;
+	int minor;
+
+	/* Dynamic attributes */
+	u32 features;
+	u32 protocols;
+	u8 pinsupport;
+	u8 nslots;
+	u8 lcdlayout;
+
+	/* Endpoints */
+	struct usb_ep *in;
+	struct usb_ep *out;
+	struct ccidg_bulk_dev bulk_dev;
+};
+
+/* Interface Descriptor: */
+static struct usb_interface_descriptor ccid_interface_desc = {
+	.bLength =		USB_DT_INTERFACE_SIZE,
+	.bDescriptorType =	USB_DT_INTERFACE,
+	.bNumEndpoints =	2,
+	.bInterfaceClass =	USB_CLASS_CSCID,
+	.bInterfaceSubClass =	0,
+	.bInterfaceProtocol =	0,
+};
+
+/* CCID Class Descriptor */
+static struct ccid_class_descriptor ccid_class_desc = {
+	.bLength =		sizeof(ccid_class_desc),
+	.bDescriptorType =	CCID_DECRIPTOR_TYPE,
+	.bcdCCID =		cpu_to_le16(CCID1_10),
+	/* .bMaxSlotIndex =	DYNAMIC */
+	.bVoltageSupport =	CCID_VOLTS_3_0,
+	/* .dwProtocols =	DYNAMIC */
+	.dwDefaultClock =	cpu_to_le32(3580),
+	.dwMaximumClock =	cpu_to_le32(3580),
+	.bNumClockSupported =	0,
+	.dwDataRate =		cpu_to_le32(9600),
+	.dwMaxDataRate =	cpu_to_le32(9600),
+	.bNumDataRatesSupported = 0,
+	.dwMaxIFSD =		0,
+	.dwSynchProtocols =	0,
+	.dwMechanical =		0,
+	/* .dwFeatures =	DYNAMIC */
+
+	/* extended APDU level Message Length */
+	.dwMaxCCIDMessageLength = cpu_to_le32(0x200),
+	.bClassGetResponse =	0x0,
+	.bClassEnvelope =	0x0,
+	/* .wLcdLayout =	DYNAMIC */
+	/* .bPINSupport =	DYNAMIC */
+	.bMaxCCIDBusySlots =	1
+};
+
+/* Full speed support: */
+static struct usb_endpoint_descriptor ccid_fs_in_desc = {
+	.bLength =		USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType =	USB_DT_ENDPOINT,
+	.bEndpointAddress =	USB_DIR_IN,
+	.bmAttributes =		USB_ENDPOINT_XFER_BULK,
+	.wMaxPacketSize   =	cpu_to_le16(64),
+};
+
+static struct usb_endpoint_descriptor ccid_fs_out_desc = {
+	.bLength =		USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType =	USB_DT_ENDPOINT,
+	.bEndpointAddress =	USB_DIR_OUT,
+	.bmAttributes =		USB_ENDPOINT_XFER_BULK,
+	.wMaxPacketSize   =	 cpu_to_le16(64),
+};
+
+static struct usb_descriptor_header *ccid_fs_descs[] = {
+	(struct usb_descriptor_header *) &ccid_interface_desc,
+	(struct usb_descriptor_header *) &ccid_class_desc,
+	(struct usb_descriptor_header *) &ccid_fs_in_desc,
+	(struct usb_descriptor_header *) &ccid_fs_out_desc,
+	NULL,
+};
+
+/* High speed support: */
+static struct usb_endpoint_descriptor ccid_hs_in_desc = {
+	.bLength =		USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType =	USB_DT_ENDPOINT,
+	.bEndpointAddress =	USB_DIR_IN,
+	.bmAttributes =		USB_ENDPOINT_XFER_BULK,
+	.wMaxPacketSize =	cpu_to_le16(512),
+};
+
+static struct usb_endpoint_descriptor ccid_hs_out_desc = {
+	.bLength =		USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType =	USB_DT_ENDPOINT,
+	.bEndpointAddress =	USB_DIR_OUT,
+	.bmAttributes =		USB_ENDPOINT_XFER_BULK,
+	.wMaxPacketSize =	cpu_to_le16(512),
+};
+
+static struct usb_descriptor_header *ccid_hs_descs[] = {
+	(struct usb_descriptor_header *) &ccid_interface_desc,
+	(struct usb_descriptor_header *) &ccid_class_desc,
+	(struct usb_descriptor_header *) &ccid_hs_in_desc,
+	(struct usb_descriptor_header *) &ccid_hs_out_desc,
+	NULL,
+};
+
+static DEFINE_IDA(ccidg_ida);
+static int major;
+static DEFINE_MUTEX(ccidg_ida_lock); /* protects access to ccidg_ida */
+static struct class *ccidg_class;
+
+static inline struct f_ccidg_opts *to_f_ccidg_opts(struct config_item *item)
+{
+	return container_of(to_config_group(item), struct f_ccidg_opts,
+			    func_inst.group);
+}
+
+static inline struct f_ccidg *func_to_ccidg(struct usb_function *f)
+{
+	return container_of(f, struct f_ccidg, function);
+}
+
+static inline int ccidg_get_minor(void)
+{
+	int ret;
+
+	ret = ida_simple_get(&ccidg_ida, 0, 0, GFP_KERNEL);
+	if (ret >= CCID_MINORS) {
+		ida_simple_remove(&ccidg_ida, ret);
+		ret = -ENODEV;
+	}
+
+	return ret;
+}
+
+static inline void ccidg_put_minor(int minor)
+{
+	ida_simple_remove(&ccidg_ida, minor);
+}
+
+static int ccidg_setup(void)
+{
+	int ret;
+	dev_t dev;
+
+	ccidg_class = class_create(THIS_MODULE, "ccidg");
+	if (IS_ERR(ccidg_class)) {
+		ccidg_class = NULL;
+		return PTR_ERR(ccidg_class);
+	}
+
+	ret = alloc_chrdev_region(&dev, 0, CCID_MINORS, "ccidg");
+	if (ret) {
+		class_destroy(ccidg_class);
+		ccidg_class = NULL;
+		return ret;
+	}
+
+	major = MAJOR(dev);
+
+	return 0;
+}
+
+static void ccidg_cleanup(void)
+{
+	if (major) {
+		unregister_chrdev_region(MKDEV(major, 0), CCID_MINORS);
+		major = 0;
+	}
+
+	class_destroy(ccidg_class);
+	ccidg_class = NULL;
+}
+
+static void ccidg_attr_release(struct config_item *item)
+{
+	struct f_ccidg_opts *opts = to_f_ccidg_opts(item);
+
+	usb_put_function_instance(&opts->func_inst);
+}
+
+static struct configfs_item_operations ccidg_item_ops = {
+	.release	= ccidg_attr_release,
+};
+
+#define F_CCIDG_OPT(name, prec, limit)					\
+static ssize_t f_ccidg_opts_##name##_show(struct config_item *item, char *page)\
+{									\
+	struct f_ccidg_opts *opts = to_f_ccidg_opts(item);		\
+	int result;							\
+									\
+	mutex_lock(&opts->lock);					\
+	result = sprintf(page, "%x\n", opts->name);			\
+	mutex_unlock(&opts->lock);					\
+									\
+	return result;							\
+}									\
+									\
+static ssize_t f_ccidg_opts_##name##_store(struct config_item *item,	\
+					 const char *page, size_t len)	\
+{									\
+	struct f_ccidg_opts *opts = to_f_ccidg_opts(item);		\
+	int ret;							\
+	u##prec num;							\
+									\
+	mutex_lock(&opts->lock);					\
+	if (opts->refcnt) {						\
+		ret = -EBUSY;						\
+		goto end;						\
+	}								\
+									\
+	ret = kstrtou##prec(page, 0, &num);				\
+	if (ret)							\
+		goto end;						\
+									\
+	if (num > limit) {						\
+		ret = -EINVAL;						\
+		goto end;						\
+	}								\
+	opts->name = num;						\
+	ret = len;							\
+									\
+end:									\
+	mutex_unlock(&opts->lock);					\
+	return ret;							\
+}									\
+									\
+CONFIGFS_ATTR(f_ccidg_opts_, name)
+
+F_CCIDG_OPT(features, 32, 0xffffffff);
+F_CCIDG_OPT(protocols, 32, 0x03);
+F_CCIDG_OPT(pinsupport, 8, 0x03);
+F_CCIDG_OPT(lcdlayout, 16, 0xffff);
+F_CCIDG_OPT(nslots, 8, 0xff);
+
+static struct configfs_attribute *ccidg_attrs[] = {
+	&f_ccidg_opts_attr_features,
+	&f_ccidg_opts_attr_protocols,
+	&f_ccidg_opts_attr_pinsupport,
+	&f_ccidg_opts_attr_lcdlayout,
+	&f_ccidg_opts_attr_nslots,
+	NULL,
+};
+
+static struct config_item_type ccidg_func_type = {
+	.ct_item_ops	= &ccidg_item_ops,
+	.ct_attrs	= ccidg_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+static void ccidg_req_put(struct f_ccidg *ccidg, struct list_head *head,
+		struct usb_request *req)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ccidg->lock, flags);
+	list_add_tail(&req->list, head);
+	spin_unlock_irqrestore(&ccidg->lock, flags);
+}
+
+static struct usb_request *ccidg_req_get(struct f_ccidg *ccidg,
+					struct list_head *head)
+{
+	unsigned long flags;
+	struct usb_request *req = NULL;
+
+	spin_lock_irqsave(&ccidg->lock, flags);
+	if (!list_empty(head)) {
+		req = list_first_entry(head, struct usb_request, list);
+		list_del(&req->list);
+	}
+	spin_unlock_irqrestore(&ccidg->lock, flags);
+
+	return req;
+}
+
+static void ccidg_bulk_complete_tx(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_ccidg *ccidg = (struct f_ccidg *)ep->driver_data;
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+	struct usb_composite_dev *cdev	= ccidg->function.config->cdev;
+
+	switch (req->status) {
+	default:
+		VDBG(cdev, "ccid: tx err %d\n", req->status);
+		/* FALLTHROUGH */
+	case -ECONNRESET:		/* unlink */
+	case -ESHUTDOWN:		/* disconnect etc */
+		break;
+	case 0:
+		break;
+	}
+
+	ccidg_req_put(ccidg, &bulk_dev->tx_idle, req);
+	wake_up(&bulk_dev->write_wq);
+}
+
+static void ccidg_bulk_complete_rx(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_ccidg *ccidg = (struct f_ccidg *)ep->driver_data;
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+	struct usb_composite_dev *cdev	= ccidg->function.config->cdev;
+
+	switch (req->status) {
+
+	/* normal completion */
+	case 0:
+		/* We only cares about packets with nonzero length */
+		if (req->actual > 0)
+			atomic_set(&bulk_dev->rx_done, 1);
+		break;
+
+	/* software-driven interface shutdown */
+	case -ECONNRESET:		/* unlink */
+	case -ESHUTDOWN:		/* disconnect etc */
+		VDBG(cdev, "ccid: rx shutdown, code %d\n", req->status);
+		break;
+
+	/* for hardware automagic (such as pxa) */
+	case -ECONNABORTED:		/* endpoint reset */
+		DBG(cdev, "ccid: rx %s reset\n", ep->name);
+		break;
+
+	/* data overrun */
+	case -EOVERFLOW:
+		/* FALLTHROUGH */
+	default:
+		DBG(cdev, "ccid: rx status %d\n", req->status);
+		break;
+	}
+
+	wake_up(&bulk_dev->read_wq);
+}
+
+static struct usb_request *
+ccidg_request_alloc(struct usb_ep *ep, unsigned int len)
+{
+	struct usb_request *req;
+
+	req = usb_ep_alloc_request(ep, GFP_ATOMIC);
+	if (!req)
+		return ERR_PTR(-ENOMEM);
+
+	req->length = len;
+	req->buf = kmalloc(len, GFP_ATOMIC);
+	if (req->buf == NULL) {
+		usb_ep_free_request(ep, req);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	return req;
+}
+
+static void ccidg_request_free(struct usb_request *req, struct usb_ep *ep)
+{
+	if (req) {
+		kfree(req->buf);
+		usb_ep_free_request(ep, req);
+	}
+}
+
+static int ccidg_function_setup(struct usb_function *f,
+		const struct usb_ctrlrequest *ctrl)
+{
+	struct f_ccidg *ccidg = container_of(f, struct f_ccidg, function);
+	struct usb_composite_dev *cdev	= f->config->cdev;
+	struct usb_request *req		= cdev->req;
+	int ret				= -EOPNOTSUPP;
+	u16 w_index			= le16_to_cpu(ctrl->wIndex);
+	u16 w_value			= le16_to_cpu(ctrl->wValue);
+	u16 w_length			= le16_to_cpu(ctrl->wLength);
+
+	if (!atomic_read(&ccidg->online))
+		return -ENOTCONN;
+
+	switch (ctrl->bRequestType & USB_TYPE_MASK) {
+	case USB_TYPE_CLASS:
+		{
+		switch (ctrl->bRequest) {
+		case CCIDGENERICREQ_GET_CLOCK_FREQUENCIES:
+			if (w_length > sizeof(ccid_class_desc.dwDefaultClock))
+				break;
+
+			*(__le32 *) req->buf = ccid_class_desc.dwDefaultClock;
+			ret = sizeof(ccid_class_desc.dwDefaultClock);
+			break;
+
+		case CCIDGENERICREQ_GET_DATA_RATES:
+			if (w_length > sizeof(ccid_class_desc.dwDataRate))
+				break;
+
+			*(__le32 *) req->buf = ccid_class_desc.dwDataRate;
+			ret = sizeof(ccid_class_desc.dwDataRate);
+			break;
+
+		default:
+			VDBG(f->config->cdev,
+				"ccid: invalid control req%02x.%02x v%04x i%04x l%d\n",
+				ctrl->bRequestType, ctrl->bRequest,
+				w_value, w_index, w_length);
+		}
+		}
+	}
+
+	/* responded with data transfer or status phase? */
+	if (ret >= 0) {
+		VDBG(f->config->cdev, "ccid: req%02x.%02x v%04x i%04x l%d\n",
+			ctrl->bRequestType, ctrl->bRequest,
+			w_value, w_index, w_length);
+
+		req->length = ret;
+		ret = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
+		if (ret < 0)
+			ERROR(f->config->cdev,
+				"ccid: ep0 enqueue err %d\n", ret);
+	}
+
+	return ret;
+}
+
+static void ccidg_function_disable(struct usb_function *f)
+{
+	struct f_ccidg *ccidg = func_to_ccidg(f);
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+	struct usb_request *req;
+
+	/* Disable endpoints */
+	usb_ep_disable(ccidg->in);
+	usb_ep_disable(ccidg->out);
+
+	/* Free endpoint related requests */
+	if (!atomic_read(&bulk_dev->rx_req_busy))
+		ccidg_request_free(bulk_dev->rx_req, ccidg->out);
+	while ((req = ccidg_req_get(ccidg, &bulk_dev->tx_idle)))
+		ccidg_request_free(req, ccidg->in);
+
+	atomic_set(&ccidg->online, 0);
+
+	/* Wake up threads */
+	wake_up(&bulk_dev->write_wq);
+	wake_up(&bulk_dev->read_wq);
+}
+
+static int ccidg_start_ep(struct f_ccidg *ccidg, struct usb_function *f,
+			struct usb_ep *ep)
+{
+	struct usb_composite_dev *cdev = f->config->cdev;
+	int ret;
+
+	usb_ep_disable(ep);
+
+	ret = config_ep_by_speed(cdev->gadget, f, ep);
+	if (ret) {
+		ERROR(cdev, "ccid: can't configure %s: %d\n", ep->name, ret);
+		return ret;
+	}
+
+	ret = usb_ep_enable(ep);
+	if (ret) {
+		ERROR(cdev, "ccid: can't start %s: %d\n", ep->name, ret);
+		return ret;
+	}
+
+	ep->driver_data = ccidg;
+
+	return ret;
+}
+
+static int ccidg_function_set_alt(struct usb_function *f,
+		unsigned int intf, unsigned int alt)
+{
+	struct f_ccidg *ccidg		= func_to_ccidg(f);
+	struct usb_composite_dev *cdev	= f->config->cdev;
+	struct ccidg_bulk_dev *bulk_dev	= &ccidg->bulk_dev;
+	struct usb_request *req;
+	int ret;
+	int i;
+
+	/* Allocate requests for our endpoints */
+	req = ccidg_request_alloc(ccidg->out,
+			sizeof(struct ccidg_bulk_out_header));
+	if (IS_ERR(req)) {
+		ERROR(cdev, "ccid: uname to allocate memory for out req\n");
+		return PTR_ERR(req);
+	}
+	req->complete = ccidg_bulk_complete_rx;
+	req->context = ccidg;
+	bulk_dev->rx_req = req;
+
+	/* Allocate bunch of in requests */
+	for (i = 0; i < N_TX_REQS; i++) {
+		req = ccidg_request_alloc(ccidg->in,
+				sizeof(struct ccidg_bulk_in_header));
+
+		if (IS_ERR(req)) {
+			ret = PTR_ERR(req);
+			ERROR(cdev,
+				"ccid: uname to allocate memory for in req\n");
+			goto free_bulk_out;
+		}
+		req->complete = ccidg_bulk_complete_tx;
+		req->context = ccidg;
+		ccidg_req_put(ccidg, &bulk_dev->tx_idle, req);
+	}
+
+	/* choose the descriptors and enable endpoints */
+	ret = ccidg_start_ep(ccidg, f, ccidg->in);
+	if (ret)
+		goto free_bulk_in;
+
+	ret = ccidg_start_ep(ccidg, f, ccidg->out);
+	if (ret)
+		goto disable_ep_in;
+
+	atomic_set(&ccidg->online, 1);
+	return ret;
+
+disable_ep_in:
+	usb_ep_disable(ccidg->in);
+free_bulk_in:
+	while ((req = ccidg_req_get(ccidg, &bulk_dev->tx_idle)))
+		ccidg_request_free(req, ccidg->in);
+free_bulk_out:
+	ccidg_request_free(bulk_dev->rx_req, ccidg->out);
+	return ret;
+}
+
+static int ccidg_bulk_open(struct inode *inode, struct file *file)
+{
+	struct f_ccidg *ccidg;
+	struct ccidg_bulk_dev *bulk_dev;
+
+	ccidg = container_of(inode->i_cdev, struct f_ccidg, cdev);
+	bulk_dev = &ccidg->bulk_dev;
+
+	if (!atomic_read(&ccidg->online)) {
+		DBG(ccidg->function.config->cdev, "ccid: device not online\n");
+		return -ENODEV;
+	}
+
+	if (atomic_read(&bulk_dev->is_open)) {
+		DBG(ccidg->function.config->cdev,
+				"ccid: device already opened\n");
+		return -EBUSY;
+	}
+
+	atomic_set(&bulk_dev->is_open, 1);
+
+	file->private_data = ccidg;
+
+	return 0;
+}
+
+static int ccidg_bulk_release(struct inode *inode, struct file *file)
+{
+	struct f_ccidg *ccidg =  file->private_data;
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+
+	atomic_set(&bulk_dev->is_open, 0);
+	return 0;
+}
+
+static ssize_t ccidg_bulk_read(struct file *file, char __user *buf,
+				size_t count, loff_t *pos)
+{
+	struct f_ccidg *ccidg =  file->private_data;
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+	struct usb_request *req;
+	int r = count, xfer;
+	int ret;
+
+	/* Make sure we have enough space for a whole package */
+	if (count < sizeof(struct ccidg_bulk_out_header)) {
+		DBG(ccidg->function.config->cdev,
+				"ccid: too small buffer size. %zu provided, need at least %zu\n",
+				count, sizeof(struct ccidg_bulk_out_header));
+		return -ENOMEM;
+	}
+
+	if (!atomic_read(&ccidg->online))
+		return -ENODEV;
+
+	/* queue a request */
+	req = bulk_dev->rx_req;
+	req->length = count;
+	atomic_set(&bulk_dev->rx_done, 0);
+
+	ret = usb_ep_queue(ccidg->out, req, GFP_KERNEL);
+	if (ret < 0) {
+		ERROR(ccidg->function.config->cdev,
+				"ccid: usb ep queue failed\n");
+		return -EIO;
+	}
+
+	if (!atomic_read(&bulk_dev->rx_done) &&
+			file->f_flags & (O_NONBLOCK | O_NDELAY))
+		return -EAGAIN;
+
+	/* wait for a request to complete */
+	ret = wait_event_interruptible(bulk_dev->read_wq,
+			atomic_read(&bulk_dev->rx_done) ||
+			!atomic_read(&ccidg->online));
+	if (ret < 0) {
+		usb_ep_dequeue(ccidg->out, req);
+		return -ERESTARTSYS;
+	}
+
+	/* Still online? */
+	if (!atomic_read(&ccidg->online))
+		return -ENODEV;
+
+	atomic_set(&bulk_dev->rx_req_busy, 1);
+	xfer = (req->actual < count) ? req->actual : count;
+
+	if (copy_to_user(buf, req->buf, xfer))
+		r = -EFAULT;
+
+	atomic_set(&bulk_dev->rx_req_busy, 0);
+	if (!atomic_read(&ccidg->online)) {
+		ccidg_request_free(bulk_dev->rx_req, ccidg->out);
+		return -ENODEV;
+	}
+
+	return xfer;
+}
+
+static ssize_t ccidg_bulk_write(struct file *file, const char __user *buf,
+				 size_t count, loff_t *pos)
+{
+	struct f_ccidg *ccidg =  file->private_data;
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+	struct usb_request *req = NULL;
+	int ret;
+
+	/* Are we online? */
+	if (!atomic_read(&ccidg->online))
+		return -ENODEV;
+
+	/* Avoid Zero Length Packets (ZLP) */
+	if (!count)
+		return 0;
+
+	/* Make sure we have enough space for a whole package */
+	if (count > sizeof(struct ccidg_bulk_out_header)) {
+		DBG(ccidg->function.config->cdev,
+				"ccid: too much data. %zu provided, but we can only handle %zu\n",
+				count, sizeof(struct ccidg_bulk_out_header));
+		return -ENOMEM;
+	}
+
+	if (list_empty(&bulk_dev->tx_idle) &&
+			file->f_flags & (O_NONBLOCK | O_NDELAY))
+		return -EAGAIN;
+
+	/* get an idle tx request to use */
+	ret = wait_event_interruptible(bulk_dev->write_wq,
+		((req = ccidg_req_get(ccidg, &bulk_dev->tx_idle))));
+
+	if (ret < 0)
+		return -ERESTARTSYS;
+
+	if (copy_from_user(req->buf, buf, count)) {
+		if (!atomic_read(&ccidg->online)) {
+			ccidg_request_free(req, ccidg->in);
+			return -ENODEV;
+		} else {
+			ccidg_req_put(ccidg, &bulk_dev->tx_idle, req);
+			return -EFAULT;
+		}
+	}
+
+	req->length = count;
+	ret = usb_ep_queue(ccidg->in, req, GFP_KERNEL);
+	if (ret < 0) {
+		ccidg_req_put(ccidg, &bulk_dev->tx_idle, req);
+
+		if (!atomic_read(&ccidg->online)) {
+			/* Free up all requests if we are not online */
+			while ((req = ccidg_req_get(ccidg, &bulk_dev->tx_idle)))
+				ccidg_request_free(req, ccidg->in);
+
+			return -ENODEV;
+		}
+		return -EIO;
+	}
+
+	return count;
+}
+
+static __poll_t ccidg_bulk_poll(struct file *file, poll_table * wait)
+{
+	struct f_ccidg *ccidg =  file->private_data;
+	struct ccidg_bulk_dev *bulk_dev = &ccidg->bulk_dev;
+	__poll_t	ret = 0;
+
+	poll_wait(file, &bulk_dev->read_wq, wait);
+	poll_wait(file, &bulk_dev->write_wq, wait);
+
+	if (list_empty(&bulk_dev->tx_idle))
+		ret |= EPOLLOUT | EPOLLWRNORM;
+
+	if (atomic_read(&bulk_dev->rx_done))
+		ret |= EPOLLIN | EPOLLRDNORM;
+
+	return ret;
+}
+
+static const struct file_operations f_ccidg_fops = {
+	.owner = THIS_MODULE,
+	.read = ccidg_bulk_read,
+	.write = ccidg_bulk_write,
+	.open = ccidg_bulk_open,
+	.poll = ccidg_bulk_poll,
+	.release = ccidg_bulk_release,
+};
+
+static int ccidg_bulk_device_init(struct f_ccidg *dev)
+{
+	struct ccidg_bulk_dev *bulk_dev = &dev->bulk_dev;
+
+	init_waitqueue_head(&bulk_dev->read_wq);
+	init_waitqueue_head(&bulk_dev->write_wq);
+	INIT_LIST_HEAD(&bulk_dev->tx_idle);
+
+	return 0;
+}
+
+static void ccidg_function_free(struct usb_function *f)
+{
+	struct f_ccidg *ccidg;
+	struct f_ccidg_opts *opts;
+
+	ccidg = func_to_ccidg(f);
+	opts = container_of(f->fi, struct f_ccidg_opts, func_inst);
+
+	kfree(ccidg);
+	mutex_lock(&opts->lock);
+	--opts->refcnt;
+	mutex_unlock(&opts->lock);
+}
+
+static void ccidg_function_unbind(struct usb_configuration *c,
+					struct usb_function *f)
+{
+	struct f_ccidg *ccidg = func_to_ccidg(f);
+
+	device_destroy(ccidg_class, MKDEV(major, ccidg->minor));
+	cdev_del(&ccidg->cdev);
+
+	/* disable/free request and end point */
+	usb_free_all_descriptors(f);
+}
+
+static int ccidg_function_bind(struct usb_configuration *c,
+					struct usb_function *f)
+{
+	struct f_ccidg *ccidg = func_to_ccidg(f);
+	struct usb_ep *ep;
+	struct usb_composite_dev *cdev = c->cdev;
+	struct device *device;
+	dev_t dev;
+	int ifc_id;
+	int ret;
+
+	/* allocate instance-specific interface IDs, and patch descriptors */
+	ifc_id = usb_interface_id(c, f);
+	if (ifc_id < 0) {
+		ERROR(cdev, "ccid: unable to allocate ifc id, err:%d\n",
+				ifc_id);
+		return ifc_id;
+	}
+	ccid_interface_desc.bInterfaceNumber = ifc_id;
+
+	/* allocate instance-specific endpoints */
+	ep = usb_ep_autoconfig(cdev->gadget, &ccid_fs_in_desc);
+	if (!ep) {
+		ERROR(cdev, "ccid: usb epin autoconfig failed\n");
+		ret = -ENODEV;
+		goto ep_auto_in_fail;
+	}
+	ccidg->in = ep;
+	ep->driver_data = ccidg;
+
+	ep = usb_ep_autoconfig(cdev->gadget, &ccid_fs_out_desc);
+	if (!ep) {
+		ERROR(cdev, "ccid: usb epout autoconfig failed\n");
+		ret = -ENODEV;
+		goto ep_auto_out_fail;
+	}
+	ccidg->out = ep;
+	ep->driver_data = ccidg;
+
+	/* set descriptor dynamic values */
+	ccid_class_desc.dwFeatures	= cpu_to_le32(ccidg->features);
+	ccid_class_desc.bPINSupport	= ccidg->pinsupport;
+	ccid_class_desc.wLcdLayout	= cpu_to_le16(ccidg->lcdlayout);
+	ccid_class_desc.bMaxSlotIndex	= ccidg->nslots;
+	ccid_class_desc.dwProtocols	= cpu_to_le32(ccidg->protocols);
+
+	if (ccidg->protocols == CCID_PROTOCOL_NOT_SEL) {
+		ccidg->protocols = CCID_PROTOCOL_T0 | CCID_PROTOCOL_T1;
+		INFO(ccidg->function.config->cdev,
+			"ccid: No protocol selected. Support both T0 and T1.\n");
+	}
+
+
+	ccid_hs_in_desc.bEndpointAddress =
+			ccid_fs_in_desc.bEndpointAddress;
+	ccid_hs_out_desc.bEndpointAddress =
+			ccid_fs_out_desc.bEndpointAddress;
+
+	ret  = usb_assign_descriptors(f, ccid_fs_descs,
+			ccid_hs_descs, NULL, NULL);
+	if (ret)
+		goto ep_auto_out_fail;
+
+	/* create char device */
+	cdev_init(&ccidg->cdev, &f_ccidg_fops);
+	dev = MKDEV(major, ccidg->minor);
+	ret = cdev_add(&ccidg->cdev, dev, 1);
+	if (ret)
+		goto fail_free_descs;
+
+	device = device_create(ccidg_class, NULL, dev, NULL,
+			       "%s%d", "ccidg", ccidg->minor);
+	if (IS_ERR(device)) {
+		ret = PTR_ERR(device);
+		goto del;
+	}
+
+	return 0;
+
+del:
+	cdev_del(&ccidg->cdev);
+fail_free_descs:
+	usb_free_all_descriptors(f);
+ep_auto_out_fail:
+	ccidg->out->driver_data = NULL;
+	ccidg->out = NULL;
+ep_auto_in_fail:
+	ccidg->in->driver_data = NULL;
+	ccidg->in = NULL;
+	ERROR(f->config->cdev, "ccidg_bind FAILED\n");
+
+	return ret;
+}
+
+static struct usb_function *ccidg_alloc(struct usb_function_instance *fi)
+{
+	struct f_ccidg *ccidg;
+	struct f_ccidg_opts *opts;
+	int ret;
+
+	ccidg = kzalloc(sizeof(*ccidg), GFP_KERNEL);
+	if (!ccidg)
+		return ERR_PTR(-ENOMEM);
+
+	spin_lock_init(&ccidg->lock);
+
+	ret = ccidg_bulk_device_init(ccidg);
+	if (ret) {
+		kfree(ccidg);
+		return ERR_PTR(ret);
+	}
+
+	opts = container_of(fi, struct f_ccidg_opts, func_inst);
+
+	mutex_lock(&opts->lock);
+	++opts->refcnt;
+
+	ccidg->minor = opts->minor;
+	ccidg->features = opts->features;
+	ccidg->protocols = opts->protocols;
+	ccidg->pinsupport = opts->pinsupport;
+	ccidg->nslots = opts->nslots;
+	mutex_unlock(&opts->lock);
+
+	ccidg->function.name	= "ccid";
+	ccidg->function.bind	= ccidg_function_bind;
+	ccidg->function.unbind	= ccidg_function_unbind;
+	ccidg->function.set_alt	= ccidg_function_set_alt;
+	ccidg->function.disable	= ccidg_function_disable;
+	ccidg->function.setup	= ccidg_function_setup;
+	ccidg->function.free_func = ccidg_function_free;
+
+	return &ccidg->function;
+}
+
+static void ccidg_free_inst(struct usb_function_instance *f)
+{
+	struct f_ccidg_opts *opts;
+
+	opts = container_of(f, struct f_ccidg_opts, func_inst);
+	mutex_lock(&ccidg_ida_lock);
+
+	ccidg_put_minor(opts->minor);
+	if (ida_is_empty(&ccidg_ida))
+		ccidg_cleanup();
+
+	mutex_unlock(&ccidg_ida_lock);
+
+	kfree(opts);
+}
+
+static struct usb_function_instance *ccidg_alloc_inst(void)
+{
+	struct f_ccidg_opts *opts;
+	struct usb_function_instance *ret;
+	int status = 0;
+
+	opts = kzalloc(sizeof(*opts), GFP_KERNEL);
+	if (!opts)
+		return ERR_PTR(-ENOMEM);
+
+	mutex_init(&opts->lock);
+	opts->func_inst.free_func_inst = ccidg_free_inst;
+	ret = &opts->func_inst;
+
+	mutex_lock(&ccidg_ida_lock);
+
+	if (ida_is_empty(&ccidg_ida)) {
+		status = ccidg_setup();
+		if (status)  {
+			ret = ERR_PTR(status);
+			kfree(opts);
+			goto unlock;
+		}
+	}
+
+	opts->minor = ccidg_get_minor();
+	if (opts->minor < 0) {
+		ret = ERR_PTR(opts->minor);
+		kfree(opts);
+		if (ida_is_empty(&ccidg_ida))
+			ccidg_cleanup();
+		goto unlock;
+	}
+
+	config_group_init_type_name(&opts->func_inst.group,
+			"", &ccidg_func_type);
+
+unlock:
+	mutex_unlock(&ccidg_ida_lock);
+	return ret;
+}
+
+DECLARE_USB_FUNCTION_INIT(ccid, ccidg_alloc_inst, ccidg_alloc);
+
+MODULE_DESCRIPTION("USB CCID Gadget driver");
+MODULE_AUTHOR("Marcus Folkesson <marcus.folkesson@gmail.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/usb/gadget/function/f_ccid.h b/drivers/usb/gadget/function/f_ccid.h
new file mode 100644
index 000000000000..f1053ec5c4d9
--- /dev/null
+++ b/drivers/usb/gadget/function/f_ccid.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@gmail.com>
+ */
+
+#ifndef F_CCID_H
+#define F_CCID_H
+
+#define CCID1_10                0x0110
+#define CCID_DECRIPTOR_TYPE     0x21
+#define ABDATA_SIZE		512
+#define SMART_CARD_DEVICE_CLASS	0x0B
+
+/* CCID Class Specific Request */
+#define CCIDGENERICREQ_ABORT                    0x01
+#define CCIDGENERICREQ_GET_CLOCK_FREQUENCIES    0x02
+#define CCIDGENERICREQ_GET_DATA_RATES           0x03
+
+/* Supported voltages */
+#define CCID_VOLTS_AUTO                             0x00
+#define CCID_VOLTS_5_0                              0x01
+#define CCID_VOLTS_3_0                              0x02
+#define CCID_VOLTS_1_8                              0x03
+
+struct f_ccidg_opts {
+	struct usb_function_instance func_inst;
+	int	minor;
+	__u32	features;
+	__u32	protocols;
+	__u8	pinsupport;
+	__u8	nslots;
+	__u8	lcdlayout;
+
+	/*
+	 * Protect the data form concurrent access by read/write
+	 * and create symlink/remove symlink.
+	 */
+	struct mutex	lock;
+	int		refcnt;
+};
+
+struct ccidg_bulk_in_header {
+	__u8	bMessageType;
+	__le32	wLength;
+	__u8	bSlot;
+	__u8	bSeq;
+	__u8	bStatus;
+	__u8	bError;
+	__u8	bSpecific;
+	__u8	abData[ABDATA_SIZE];
+	__u8	bSizeToSend;
+} __packed;
+
+struct ccidg_bulk_out_header {
+	__u8	 bMessageType;
+	__le32	 wLength;
+	__u8	 bSlot;
+	__u8	 bSeq;
+	__u8	 bSpecific_0;
+	__u8	 bSpecific_1;
+	__u8	 bSpecific_2;
+	__u8	 APDU[ABDATA_SIZE];
+} __packed;
+
+struct ccid_class_descriptor {
+	__u8	bLength;
+	__u8	bDescriptorType;
+	__le16	bcdCCID;
+	__u8	bMaxSlotIndex;
+	__u8	bVoltageSupport;
+	__le32	dwProtocols;
+	__le32	dwDefaultClock;
+	__le32	dwMaximumClock;
+	__u8	bNumClockSupported;
+	__le32	dwDataRate;
+	__le32	dwMaxDataRate;
+	__u8	bNumDataRatesSupported;
+	__le32	dwMaxIFSD;
+	__le32	dwSynchProtocols;
+	__le32	dwMechanical;
+	__le32	dwFeatures;
+	__le32	dwMaxCCIDMessageLength;
+	__u8	bClassGetResponse;
+	__u8	bClassEnvelope;
+	__le16	wLcdLayout;
+	__u8	bPINSupport;
+	__u8	bMaxCCIDBusySlots;
+} __packed;
+
+
+#endif
diff --git a/include/uapi/linux/usb/ccid.h b/include/uapi/linux/usb/ccid.h
new file mode 100644
index 000000000000..517897201563
--- /dev/null
+++ b/include/uapi/linux/usb/ccid.h
@@ -0,0 +1,93 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@gmail.com>
+ *
+ * This file holds USB constants defined by the CCID Specification.
+ */
+
+#ifndef CCID_H
+#define CCID_H
+
+/* Slot error register when bmCommandStatus = 1 */
+#define CCID_CMD_ABORTED                            0xFF
+#define CCID_ICC_MUTE                               0xFE
+#define CCID_XFR_PARITY_ERROR                       0xFD
+#define CCID_XFR_OVERRUN                            0xFC
+#define CCID_HW_ERROR                               0xFB
+#define CCID_BAD_ATR_TS                             0xF8
+#define CCID_BAD_ATR_TCK                            0xF7
+#define CCID_ICC_PROTOCOL_NOT_SUPPORTED             0xF6
+#define CCID_ICC_CLASS_NOT_SUPPORTED                0xF5
+#define CCID_PROCEDURE_BYTE_CONFLICT                0xF4
+#define CCID_DEACTIVATED_PROTOCOL                   0xF3
+#define CCID_BUSY_WITH_AUTO_SEQUENCE                0xF2
+#define CCID_PIN_TIMEOUT                            0xF0
+#define CCID_PIN_CANCELLED                          0xEF
+#define CCID_CMD_SLOT_BUSY                          0xE0
+
+/* PC to RDR messages (bulk out) */
+#define CCID_PC_TO_RDR_ICCPOWERON                   0x62
+#define CCID_PC_TO_RDR_ICCPOWEROFF                  0x63
+#define CCID_PC_TO_RDR_GETSLOTSTATUS                0x65
+#define CCID_PC_TO_RDR_XFRBLOCK                     0x6F
+#define CCID_PC_TO_RDR_GETPARAMETERS                0x6C
+#define CCID_PC_TO_RDR_RESETPARAMETERS              0x6D
+#define CCID_PC_TO_RDR_SETPARAMETERS                0x61
+#define CCID_PC_TO_RDR_ESCAPE                       0x6B
+#define CCID_PC_TO_RDR_ICCCLOCK                     0x6E
+#define CCID_PC_TO_RDR_T0APDU                       0x6A
+#define CCID_PC_TO_RDR_SECURE                       0x69
+#define CCID_PC_TO_RDR_MECHANICAL                   0x71
+#define CCID_PC_TO_RDR_ABORT                        0x72
+#define CCID_PC_TO_RDR_SETDATARATEANDCLOCKFREQUENCY 0x73
+
+/* RDR to PC messages (bulk in) */
+#define CCID_RDR_TO_PC_DATABLOCK                    0x80
+#define CCID_RDR_TO_PC_SLOTSTATUS                   0x81
+#define CCID_RDR_TO_PC_PARAMETERS                   0x82
+#define CCID_RDR_TO_PC_ESCAPE                       0x83
+#define CCID_RDR_TO_PC_DATARATEANDCLOCKFREQUENCY    0x84
+
+/* Class Features */
+
+/* No special characteristics */
+#define CCID_FEATURES_NADA       0x00000000
+/* Automatic parameter configuration based on ATR data */
+#define CCID_FEATURES_AUTO_PCONF 0x00000002
+/* Automatic activation of ICC on inserting */
+#define CCID_FEATURES_AUTO_ACTIV 0x00000004
+/* Automatic ICC voltage selection */
+#define CCID_FEATURES_AUTO_VOLT  0x00000008
+/* Automatic ICC clock frequency change */
+#define CCID_FEATURES_AUTO_CLOCK 0x00000010
+/* Automatic baud rate change */
+#define CCID_FEATURES_AUTO_BAUD  0x00000020
+/*Automatic parameters negotiation made by the CCID */
+#define CCID_FEATURES_AUTO_PNEGO 0x00000040
+/* Automatic PPS made by the CCID according to the active parameters */
+#define CCID_FEATURES_AUTO_PPS   0x00000080
+/* CCID can set ICC in clock stop mode */
+#define CCID_FEATURES_ICCSTOP    0x00000100
+/* NAD value other than 00 accepted (T=1 protocol in use) */
+#define CCID_FEATURES_NAD        0x00000200
+/* Automatic IFSD exchange as first exchange (T=1 protocol in use) */
+#define CCID_FEATURES_AUTO_IFSD  0x00000400
+/* TPDU level exchanges with CCID */
+#define CCID_FEATURES_EXC_TPDU   0x00010000
+/* Short APDU level exchange with CCID */
+#define CCID_FEATURES_EXC_SAPDU  0x00020000
+/* Short and Extended APDU level exchange with CCID */
+#define CCID_FEATURES_EXC_APDU   0x00040000
+/* USB Wake up signaling supported on card insertion and removal */
+#define CCID_FEATURES_WAKEUP	0x00100000
+
+/* Supported protocols */
+#define CCID_PROTOCOL_NOT_SEL	0x00
+#define CCID_PROTOCOL_T0	0x01
+#define CCID_PROTOCOL_T1	0x02
+
+#define CCID_PINSUPOORT_NONE		0x00
+#define CCID_PINSUPOORT_VERIFICATION	(1 << 1)
+#define CCID_PINSUPOORT_MODIFICATION	(1 << 2)
+
+#endif
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/3] Documentation: usb: add documentation for USB CCID Gadget Device
  2018-05-29 18:50 [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device Marcus Folkesson
@ 2018-05-29 18:50 ` Marcus Folkesson
  2018-05-29 20:27   ` Randy Dunlap
  2018-05-29 18:50 ` [PATCH v3 3/3] MAINTAINERS: add " Marcus Folkesson
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Marcus Folkesson @ 2018-05-29 18:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jonathan Corbet, Felipe Balbi, davem,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart
  Cc: linux-usb, linux-doc, linux-kernel, Marcus Folkesson

Add documentation to give a brief description on how to use the
CCID Gadget Device.
This includes a description for all attributes followed by an example on
how to setup the device with ConfigFS.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---

Notes:
    v3:
    	- correct the grammer (thanks Randy)
    v2:
    	- add the missing changelog text

 Documentation/usb/gadget_ccid.rst | 267 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 267 insertions(+)
 create mode 100644 Documentation/usb/gadget_ccid.rst

diff --git a/Documentation/usb/gadget_ccid.rst b/Documentation/usb/gadget_ccid.rst
new file mode 100644
index 000000000000..524fe9e6ac19
--- /dev/null
+++ b/Documentation/usb/gadget_ccid.rst
@@ -0,0 +1,267 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============
+CCID Gadget
+============
+
+:Author: Marcus Folkesson <marcus.folkesson@gmail.com>
+
+Introduction
+============
+
+The CCID Gadget will present itself as a CCID device to the host system.
+The device supports two endpoints for now; BULK IN and BULK OUT.
+These endpoints are exposed to userspace via /dev/ccidg*.
+
+All CCID commands are sent on the BULK-OUT endpoint. Each command sent to the CCID
+has an associated ending response. Some commands can also have intermediate
+responses. The response is sent on the BULK-IN endpoint.
+See Figure 3-3 in the CCID Specification [1]_ for more details.
+
+The CCID commands must be handled in userspace since the driver is only working
+as a transport layer for the TPDUs.
+
+
+CCID Commands
+--------------
+
+All CCID commands begins with a 10-byte header followed by an optional
+data field depending on message type.
+
++--------+--------------+-------+----------------------------------+
+| Offset | Field        | Size  | Description                      |
++========+==============+=======+==================================+
+| 0      | bMessageType | 1     | Type of message                  |
++--------+--------------+-------+----------------------------------+
+| 1      | dwLength     | 4     | Message specific data length     |
+|        |              |       |                                  |
++--------+--------------+-------+----------------------------------+
+| 5      | bSlot        | 1     | Identifies the slot number       |
+|        |              |       | for this command                 |
++--------+--------------+-------+----------------------------------+
+| 6      | bSeq         | 1     | Sequence number for command      |
++--------+--------------+-------+----------------------------------+
+| 7      | ...          | 3     | Fields depends on message type   |
++--------+--------------+-------+----------------------------------+
+| 10     | abData       | array | Message specific data (OPTIONAL) |
++--------+--------------+-------+----------------------------------+
+
+
+Multiple CCID gadgets
+----------------------
+
+It is possible to create multiple instances of the CCID gadget, however,
+a much more flexible way is to create one gadget and set the `nslots` attribute
+to the number of desired CCID devices.
+
+All CCID commands specify which slot is the receiver in the `bSlot` field
+of the CCID header.
+
+Usage
+=====
+
+Access from userspace
+----------------------
+All communication is by read(2) and write(2) to the corresponding /dev/ccidg* device.
+Only one file descriptor is allowed to be open to the device at a time.
+
+The buffer size provided to read(2) **must be at least** 522 (10 bytes header + 512 bytes payload)
+bytes as we are working with whole commands.
+
+The buffer size provided to write(2) **may not exceed** 522 (10 bytes header + 512 bytes payload)
+bytes as we are working with whole commands.
+
+
+Configuration with configfs
+----------------------------
+
+ConfigFS is used to create and configure the CCID gadget.
+In order to get a device to work as intended, a few attributes must
+be considered.
+
+The attributes are described below followed by an example.
+
+features
+~~~~~~~~~
+
+The `feature` attribute writes to the dwFeatures field in the class descriptor.
+See Table 5.1-1 Smart Card Device Descriptors in the CCID Specification [1]_.
+
+The value indicates what intelligent features the CCID has.
+These values are available to user application as defined in ccid.h [2]_.
+The default value is 0x00000000.
+
+The value is a bitwise OR operation performed on the following values:
+
++------------+----------------------------------------------------------------+
+| Value      | Description                                                    |
++============+================================================================+
+| 0x00000000 | No special characteristics                                     |
++------------+----------------------------------------------------------------+
+| 0x00000002 | Automatic parameter configuration based on ATR data            |
++------------+----------------------------------------------------------------+
+| 0x00000004 | Automatic activation of ICC on inserting                       |
++------------+----------------------------------------------------------------+
+| 0x00000008 | Automatic ICC voltage selection                                |
++------------+----------------------------------------------------------------+
+| 0x00000010 | Automatic ICC clock frequency change according to active       |
+|            | parameters provided by the Host or self determined             |
++------------+----------------------------------------------------------------+
+| 0x00000020 | Automatic baud rate change according to active                 |
+|            | parameters provided by the Host or self determined             |
++------------+----------------------------------------------------------------+
+| 0x00000040 | Automatic parameters negotiation made by the CCID              |
++------------+----------------------------------------------------------------+
+| 0x00000080 | Automatic PPS made by the CCID according to the                |
+|            | active parameters                                              |
++------------+----------------------------------------------------------------+
+| 0x00000100 | CCID can set ICC in clock stop mode                            |
++------------+----------------------------------------------------------------+
+| 0x00000200 | NAD value other than 00 accepted (T=1 protocol in use)         |
++------------+----------------------------------------------------------------+
+| 0x00000400 | Automatic IFSD exchange as first exchange                      |
++------------+----------------------------------------------------------------+
+
+
+Only one of the following values may be present to select a level of exchange:
+
++------------+--------------------------------------------------+
+| Value      | Description                                      |
++============+==================================================+
+| 0x00010000 | TPDU level exchanges with CCID                   |
++------------+--------------------------------------------------+
+| 0x00020000 | Short APDU level exchange with CCID              |
++------------+--------------------------------------------------+
+| 0x00040000 | Short and Extended APDU level exchange with CCID |
++------------+--------------------------------------------------+
+
+If none of those values is indicated the level of exchange is
+character.
+
+
+protocols
+~~~~~~~~~~
+The `protocols` attribute writes to the dwProtocols field in the class descriptor.
+See Table 5.1-1 Smart Card Device Descriptors in the CCID Specification [1]_.
+
+The value is a bitwise OR operation performed on the following values:
+
++--------+--------------+
+| Value  | Description  |
++========+==============+
+| 0x0001 | Protocol T=0 |
++--------+--------------+
+| 0x0002 | Protocol T=1 |
++--------+--------------+
+
+If no protocol is selected both T=0 and T=1 will be supported (`protocols` = 0x0003).
+
+nslots
+~~~~~~
+
+The `nslots` attribute writes to the bMaxSlotIndex field in the class descriptor.
+See Table 5.1-1 Smart Card Device Descriptors in the CCID Specification [1]_.
+
+This is the index of the highest available slot on this device. All slots are consecutive starting at 00h.
+i.e. 0Fh = 16 slots on this device numbered 00h to 0Fh.
+
+The default value is 0, which means one slot.
+
+
+pinsupport
+~~~~~~~~~~~~
+
+This value indicates what PIN support features the CCID has.
+
+The `pinsupport` attribute writes to the dwPINSupport field in the class descriptor.
+See Table 5.1-1 Smart Card Device Descriptors in the CCID Specification [1]_.
+
+
+The value is a bitwise OR operation performed on the following values:
+
++--------+----------------------------+
+| Value  | Description                |
++========+============================+
+| 0x00   | No PIN support             |
++--------+----------------------------+
+| 0x01   | PIN Verification supported |
++--------+----------------------------+
+| 0x02   | PIN Modification supported |
++--------+----------------------------+
+
+The default value is set to 0x00.
+
+
+lcdlayout
+~~~~~~~~~~
+
+Number of lines and characters for the LCD display used to send messages for PIN entry.
+
+The `lcdLayout` attribute writes to the wLcdLayout field in the class descriptor.
+See Table 5.1-1 Smart Card Device Descriptors in the CCID Specification [1]_.
+
+
+The value is set as follows:
+
++--------+------------------------------------+
+| Value  | Description                        |
++========+====================================+
+| 0x0000 | No LCD                             |
++--------+------------------------------------+
+| 0xXXYY | XX: number of lines                |
+|        | YY: number of characters per line. |
++--------+------------------------------------+
+
+The default value is set to 0x0000.
+
+
+Example
+-------
+
+Here is an example on how to setup a CCID gadget with configfs ::
+
+    #!/bin/sh
+
+    CONFIGDIR=/sys/kernel/config
+    GADGET=$CONFIGDIR/usb_gadget/g0
+    FUNCTION=$GADGET/functions/ccid.sc0
+
+    VID=YOUR_VENDOR_ID_HERE
+    PID=YOUR_PRODUCT_ID_HERE
+    UDC=YOUR_UDC_HERE
+
+    #Mount filesystem
+    mount none -t configfs $CONFIGDIR
+
+    #Populate ID:s
+    echo $VID > $GADGET/idVendor
+    echo $PID > $GADGET/idProduct
+
+    #Create and configure the gadget
+    mkdir $FUNCTION
+    echo 0x000407B8 > $FUNCTION/features
+    echo 0x02 > $FUNCTION/protocols
+
+    #Create our english strings
+    mkdir  $GADGET/strings/0x409
+    echo 556677 > $GADGET/strings/0x409/serialnumber
+    echo "Hungry Penguins" > $GADGET/strings/0x409/manufacturer
+    echo "Harpoon With SmartCard"  > $GADGET/strings/0x409/product
+
+    #Create configuration
+    mkdir  $GADGET/configs/c.1
+    mkdir  $GADGET/configs/c.1/strings/0x409
+    echo Config1 > $GADGET/configs/c.1/strings/0x409/configuration
+
+    #Use `Config1` for our CCID gadget
+    ln -s $FUNCTION $GADGET/configs/c.1
+
+    #Execute
+    echo $UDC > $GADGET/UDC
+
+
+References
+==========
+
+.. [1] http://www.usb.org/developers/docs/devclass_docs/DWG_Smart-Card_CCID_Rev110.pdf
+.. [2] include/uapi/linux/usb/ccid.h
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/3] MAINTAINERS: add USB CCID Gadget Device
  2018-05-29 18:50 [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device Marcus Folkesson
  2018-05-29 18:50 ` [PATCH v3 2/3] Documentation: usb: add documentation " Marcus Folkesson
@ 2018-05-29 18:50 ` Marcus Folkesson
  2018-05-30  0:55 ` [PATCH v3 1/3] usb: gadget: ccid: add support for " Andy Shevchenko
  2018-05-30 12:28 ` Felipe Balbi
  3 siblings, 0 replies; 16+ messages in thread
From: Marcus Folkesson @ 2018-05-29 18:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jonathan Corbet, Felipe Balbi, davem,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart
  Cc: linux-usb, linux-doc, linux-kernel, Marcus Folkesson

Add MAINTAINERS entry for USB CCID Gadget Device

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---

Notes:
    v3:
    	- No changes
    v2:
    	- No changes

 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 078fd80f664f..e77c3d2bec89 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14541,6 +14541,14 @@ L:	linux-scsi@vger.kernel.org
 S:	Maintained
 F:	drivers/usb/storage/uas.c
 
+USB CCID GADGET
+M:	Marcus Folkesson <marcus.folkesson@gmail.com>
+L:	linux-usb@vger.kernel.org
+S:	Maintained
+F:	drivers/usb/gadget/function/f_ccid.*
+F:	include/uapi/linux/usb/ccid.h
+F:	Documentation/usb/gadget_ccid.rst
+
 USB CDC ETHERNET DRIVER
 M:	Oliver Neukum <oliver@neukum.org>
 L:	linux-usb@vger.kernel.org
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/3] Documentation: usb: add documentation for USB CCID Gadget Device
  2018-05-29 18:50 ` [PATCH v3 2/3] Documentation: usb: add documentation " Marcus Folkesson
@ 2018-05-29 20:27   ` Randy Dunlap
  2018-05-30 11:34     ` Marcus Folkesson
  0 siblings, 1 reply; 16+ messages in thread
From: Randy Dunlap @ 2018-05-29 20:27 UTC (permalink / raw)
  To: Marcus Folkesson, Greg Kroah-Hartman, Jonathan Corbet,
	Felipe Balbi, davem, Mauro Carvalho Chehab, Andrew Morton,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart
  Cc: linux-usb, linux-doc, linux-kernel

On 05/29/2018 11:50 AM, Marcus Folkesson wrote:
> Add documentation to give a brief description on how to use the
> CCID Gadget Device.
> This includes a description for all attributes followed by an example on
> how to setup the device with ConfigFS.
> 
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
> 
> Notes:
>     v3:
>     	- correct the grammer (thanks Randy)
>     v2:
>     	- add the missing changelog text
> 
>  Documentation/usb/gadget_ccid.rst | 267 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 267 insertions(+)
>  create mode 100644 Documentation/usb/gadget_ccid.rst
> 
> diff --git a/Documentation/usb/gadget_ccid.rst b/Documentation/usb/gadget_ccid.rst
> new file mode 100644
> index 000000000000..524fe9e6ac19
> --- /dev/null
> +++ b/Documentation/usb/gadget_ccid.rst
> @@ -0,0 +1,267 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +============
> +CCID Gadget
> +============
> +
> +:Author: Marcus Folkesson <marcus.folkesson@gmail.com>
> +
> +Introduction
> +============
> +
> +The CCID Gadget will present itself as a CCID device to the host system.
> +The device supports two endpoints for now; BULK IN and BULK OUT.
> +These endpoints are exposed to userspace via /dev/ccidg*.
> +
> +All CCID commands are sent on the BULK-OUT endpoint. Each command sent to the CCID
> +has an associated ending response. Some commands can also have intermediate
> +responses. The response is sent on the BULK-IN endpoint.
> +See Figure 3-3 in the CCID Specification [1]_ for more details.
> +
> +The CCID commands must be handled in userspace since the driver is only working
> +as a transport layer for the TPDUs.

I think that it would be helpful to tell us what the naming of the /dev/ccidg*
endpoints looks like.  Also, how to distinguish the BULK-IN from the BULK-OUT
endpoint.

> +
> +
> +CCID Commands
> +--------------
> +
> +All CCID commands begins with a 10-byte header followed by an optional
> +data field depending on message type.
> +
> ++--------+--------------+-------+----------------------------------+
> +| Offset | Field        | Size  | Description                      |
> ++========+==============+=======+==================================+
> +| 0      | bMessageType | 1     | Type of message                  |
> ++--------+--------------+-------+----------------------------------+
> +| 1      | dwLength     | 4     | Message specific data length     |
> +|        |              |       |                                  |
> ++--------+--------------+-------+----------------------------------+
> +| 5      | bSlot        | 1     | Identifies the slot number       |
> +|        |              |       | for this command                 |
> ++--------+--------------+-------+----------------------------------+
> +| 6      | bSeq         | 1     | Sequence number for command      |
> ++--------+--------------+-------+----------------------------------+
> +| 7      | ...          | 3     | Fields depends on message type   |
> ++--------+--------------+-------+----------------------------------+
> +| 10     | abData       | array | Message specific data (OPTIONAL) |
> ++--------+--------------+-------+----------------------------------+
> +
> +
> +Multiple CCID gadgets
> +----------------------
> +
> +It is possible to create multiple instances of the CCID gadget, however,
> +a much more flexible way is to create one gadget and set the `nslots` attribute
> +to the number of desired CCID devices.
> +
> +All CCID commands specify which slot is the receiver in the `bSlot` field
> +of the CCID header.
> +
> +Usage
> +=====
> +
> +Access from userspace
> +----------------------
> +All communication is by read(2) and write(2) to the corresponding /dev/ccidg* device.
> +Only one file descriptor is allowed to be open to the device at a time.


Reviewed-by: Randy Dunlap <rdunlap@infradead.org>


thanks,
-- 
~Randy

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
  2018-05-29 18:50 [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device Marcus Folkesson
  2018-05-29 18:50 ` [PATCH v3 2/3] Documentation: usb: add documentation " Marcus Folkesson
  2018-05-29 18:50 ` [PATCH v3 3/3] MAINTAINERS: add " Marcus Folkesson
@ 2018-05-30  0:55 ` Andy Shevchenko
  2018-05-30 11:24   ` Marcus Folkesson
  2018-05-30 12:28 ` Felipe Balbi
  3 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2018-05-30  0:55 UTC (permalink / raw)
  To: Marcus Folkesson
  Cc: Greg Kroah-Hartman, Jonathan Corbet, Felipe Balbi,
	David S. Miller, Mauro Carvalho Chehab, Andrew Morton,
	Randy Dunlap, Ruslan Bilovol, Thomas Gleixner, Kate Stewart, USB,
	Linux Documentation List, Linux Kernel Mailing List

On Tue, May 29, 2018 at 9:50 PM, Marcus Folkesson
<marcus.folkesson@gmail.com> wrote:
> Chip Card Interface Device (CCID) protocol is a USB protocol that
> allows a smartcard device to be connected to a computer via a card
> reader using a standard USB interface, without the need for each manufacturer
> of smartcards to provide its own reader or protocol.
>
> This gadget driver makes Linux show up as a CCID device to the host and let a
> userspace daemon act as the smartcard.
>
> This is useful when the Linux gadget itself should act as a cryptographic
> device or forward APDUs to an embedded smartcard device.

> + * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@gmail.com>

> + *

Redundant line

> +static DEFINE_IDA(ccidg_ida);

Where is it destroyed?

> +               ccidg_class = NULL;
> +               return PTR_ERR(ccidg_class);

Are you sure?

> +       if (!list_empty(head)) {
> +               req = list_first_entry(head, struct usb_request, list);

list_first_entry_or_null()

> +       req->length = len;

Perhaps assign this obly if malloc successedeed ?

> +       req->buf = kmalloc(len, GFP_ATOMIC);

> +       if (req->buf == NULL) {

if (!req->buf) ?

> +               usb_ep_free_request(ep, req);
> +               return ERR_PTR(-ENOMEM);
> +       }

> +static void ccidg_request_free(struct usb_request *req, struct usb_ep *ep)
> +{

> +       if (req) {

Is it even possible?

What about

if (!req)
 return;

?

> +               kfree(req->buf);
> +               usb_ep_free_request(ep, req);
> +       }
> +}

> +                       *(__le32 *) req->buf = ccid_class_desc.dwDefaultClock;

Hmm... put_unaligned()? cpu_to_le32()? cpu_to_le32p()?

> +                       *(__le32 *) req->buf = ccid_class_desc.dwDataRate;

Ditto.

> +               }
> +               }

Indentation.

> +       /* responded with data transfer or status phase? */
> +       if (ret >= 0) {

Why not

if (ret < 0)
 return ret;

?

> +       }
> +
> +       return ret;
> +}

> +       atomic_set(&ccidg->online, 1);
> +       return ret;

return 0; ?

> +       struct f_ccidg *ccidg;

> +       ccidg = container_of(inode->i_cdev, struct f_ccidg, cdev);

One line ?

> +       xfer = (req->actual < count) ? req->actual : count;

min_t()

> +       ret = wait_event_interruptible(bulk_dev->write_wq,
> +               ((req = ccidg_req_get(ccidg, &bulk_dev->tx_idle))));
> +
> +       if (ret < 0)
> +               return -ERESTARTSYS;

Redundant blank line above.

> +static void ccidg_function_free(struct usb_function *f)
> +{

> +       struct f_ccidg_opts *opts;

> +       opts = container_of(f->fi, struct f_ccidg_opts, func_inst);

One line.


> +       mutex_lock(&opts->lock);
> +       --opts->refcnt;

-- will work

> +       mutex_unlock(&opts->lock);
> +}

> +       struct f_ccidg_opts *opts;

> +       opts = container_of(fi, struct f_ccidg_opts, func_inst);

Perhaps one line ?
> +       ++opts->refcnt;
X++ would work as well.
> +       struct f_ccidg_opts *opts;
> +
> +       opts = container_of(f, struct f_ccidg_opts, func_inst);

Perhaps one line?

> +#define CCID_PINSUPOORT_NONE           0x00

(0 << 0)

 ?

for sake of consistency

> +#define CCID_PINSUPOORT_VERIFICATION   (1 << 1)
> +#define CCID_PINSUPOORT_MODIFICATION   (1 << 2)

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
  2018-05-30  0:55 ` [PATCH v3 1/3] usb: gadget: ccid: add support for " Andy Shevchenko
@ 2018-05-30 11:24   ` Marcus Folkesson
  2018-05-30 11:30     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 16+ messages in thread
From: Marcus Folkesson @ 2018-05-30 11:24 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Jonathan Corbet, Felipe Balbi,
	David S. Miller, Mauro Carvalho Chehab, Andrew Morton,
	Randy Dunlap, Ruslan Bilovol, Thomas Gleixner, Kate Stewart, USB,
	Linux Documentation List, Linux Kernel Mailing List

Hi Andy,

Thank you for your comments!
Many good catches here!

On Wed, May 30, 2018 at 03:55:39AM +0300, Andy Shevchenko wrote:
> On Tue, May 29, 2018 at 9:50 PM, Marcus Folkesson
> <marcus.folkesson@gmail.com> wrote:
> > Chip Card Interface Device (CCID) protocol is a USB protocol that
> > allows a smartcard device to be connected to a computer via a card
> > reader using a standard USB interface, without the need for each manufacturer
> > of smartcards to provide its own reader or protocol.
> >
> > This gadget driver makes Linux show up as a CCID device to the host and let a
> > userspace daemon act as the smartcard.
> >
> > This is useful when the Linux gadget itself should act as a cryptographic
> > device or forward APDUs to an embedded smartcard device.
> 
> > + * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@gmail.com>
> 
> > + *
> 
> Redundant line
> 

Yep

> > +static DEFINE_IDA(ccidg_ida);
> 
> Where is it destroyed?

Hm, I'm not sure it needs to be destroyed. From lib/idr.c:

 * You can also use ida_get_new_above() if you need an ID to be allocated
 * above a particular number.  ida_destroy() can be used to dispose of an
 * IDA without needing to free the individual IDs in it.  You can use
 * ida_is_empty() to find out whether the IDA has any IDs currently allocated.


An empty ccidg_ida is the indication that we should clean up our
mess:

static void ccidg_free_inst(struct usb_function_instance *f)
...
	if (ida_is_empty(&ccidg_ida))
		ccidg_cleanup();

If the IDA is empty, should I call ida_destroy() anyway?
Other similiar drivers does not seems to do that.

I must say that I'm not very familiar with the IDA API.

> 
> > +               ccidg_class = NULL;
> > +               return PTR_ERR(ccidg_class);
> 
> Are you sure?

Heh, :-)

> 
> > +       if (!list_empty(head)) {
> > +               req = list_first_entry(head, struct usb_request, list);
> 
> list_first_entry_or_null()

Will do, thanks.

> 
> > +       req->length = len;
> 
> Perhaps assign this obly if malloc successedeed ?

Will do.

> 
> > +       req->buf = kmalloc(len, GFP_ATOMIC);
> 
> > +       if (req->buf == NULL) {
> 
> if (!req->buf) ?

Will do

> 
> > +               usb_ep_free_request(ep, req);
> > +               return ERR_PTR(-ENOMEM);
> > +       }
> 
> > +static void ccidg_request_free(struct usb_request *req, struct usb_ep *ep)
> > +{
> 
> > +       if (req) {
> 
> Is it even possible?
> 
> What about
> 
> if (!req)
>  return;
> 
> ?

Hmm, maybe it is not.
I think I remove this check.

> 
> > +               kfree(req->buf);
> > +               usb_ep_free_request(ep, req);
> > +       }
> > +}
> 
> > +                       *(__le32 *) req->buf = ccid_class_desc.dwDefaultClock;
> 
> Hmm... put_unaligned()? cpu_to_le32()? cpu_to_le32p()?
> 

Hmm, dwDefaultClock is already represented in little endian format. If I
use any of those functions, would not the byte order be swaped on a big
endian system?

dwDefaultClock is set as:
	.dwDefaultClock =	cpu_to_le32(3580),

I'm not sure what the best practice is here.

> > +                       *(__le32 *) req->buf = ccid_class_desc.dwDataRate;
> 
> Ditto.
> 
> > +               }
> > +               }
> 
> Indentation.

I remove the extra brackets from the case instead.
> 
> > +       /* responded with data transfer or status phase? */
> > +       if (ret >= 0) {
> 
> Why not
> 
> if (ret < 0)
>  return ret;
> 
> ?
> 

Will do

> > +       }
> > +
> > +       return ret;
> > +}
> 
> > +       atomic_set(&ccidg->online, 1);
> > +       return ret;
> 
> return 0; ?

Will do

> 
> > +       struct f_ccidg *ccidg;
> 
> > +       ccidg = container_of(inode->i_cdev, struct f_ccidg, cdev);
> 
> One line ?

The line exceeds 80 characters then, but I will put it like this:

	struct f_ccidg *ccidg = container_of(inode->i_cdev,
								struct f_ccidg, cdev);

> 
> > +       xfer = (req->actual < count) ? req->actual : count;
> 
> min_t()
> 

Thanks, will do

> > +       ret = wait_event_interruptible(bulk_dev->write_wq,
> > +               ((req = ccidg_req_get(ccidg, &bulk_dev->tx_idle))));
> > +
> > +       if (ret < 0)
> > +               return -ERESTARTSYS;
> 
> Redundant blank line above.
> 

I remove the extra blank line


> > +static void ccidg_function_free(struct usb_function *f)
> > +{
> 
> > +       struct f_ccidg_opts *opts;
> 
> > +       opts = container_of(f->fi, struct f_ccidg_opts, func_inst);
> 
> One line.

See above

> 
> 
> > +       mutex_lock(&opts->lock);
> > +       --opts->refcnt;
> 
> -- will work

Will do

> 
> > +       mutex_unlock(&opts->lock);
> > +}
> 
> > +       struct f_ccidg_opts *opts;
> 
> > +       opts = container_of(fi, struct f_ccidg_opts, func_inst);
> 
> Perhaps one line ?

See above


> > +       ++opts->refcnt;
> X++ would work as well.

Will do

> > +       struct f_ccidg_opts *opts;
> > +
> > +       opts = container_of(f, struct f_ccidg_opts, func_inst);
> 
> Perhaps one line?
> 

See above


> > +#define CCID_PINSUPOORT_NONE           0x00
> 
> (0 << 0)
> 
>  ?
> 
> for sake of consistency

Yep, will change

> 
> > +#define CCID_PINSUPOORT_VERIFICATION   (1 << 1)
> > +#define CCID_PINSUPOORT_MODIFICATION   (1 << 2)
> 
> -- 
> With Best Regards,
> Andy Shevchenko

Best regards,
Marcus Folkesson

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
  2018-05-30 11:24   ` Marcus Folkesson
@ 2018-05-30 11:30     ` Greg Kroah-Hartman
  2018-05-30 12:13       ` Marcus Folkesson
  0 siblings, 1 reply; 16+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-30 11:30 UTC (permalink / raw)
  To: Marcus Folkesson
  Cc: Andy Shevchenko, Jonathan Corbet, Felipe Balbi, David S. Miller,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart, USB,
	Linux Documentation List, Linux Kernel Mailing List

On Wed, May 30, 2018 at 01:24:59PM +0200, Marcus Folkesson wrote:
> Hi Andy,
> 
> Thank you for your comments!
> Many good catches here!
> 
> On Wed, May 30, 2018 at 03:55:39AM +0300, Andy Shevchenko wrote:
> > On Tue, May 29, 2018 at 9:50 PM, Marcus Folkesson
> > <marcus.folkesson@gmail.com> wrote:
> > > Chip Card Interface Device (CCID) protocol is a USB protocol that
> > > allows a smartcard device to be connected to a computer via a card
> > > reader using a standard USB interface, without the need for each manufacturer
> > > of smartcards to provide its own reader or protocol.
> > >
> > > This gadget driver makes Linux show up as a CCID device to the host and let a
> > > userspace daemon act as the smartcard.
> > >
> > > This is useful when the Linux gadget itself should act as a cryptographic
> > > device or forward APDUs to an embedded smartcard device.
> > 
> > > + * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@gmail.com>
> > 
> > > + *
> > 
> > Redundant line
> > 
> 
> Yep
> 
> > > +static DEFINE_IDA(ccidg_ida);
> > 
> > Where is it destroyed?
> 
> Hm, I'm not sure it needs to be destroyed. From lib/idr.c:
> 
>  * You can also use ida_get_new_above() if you need an ID to be allocated
>  * above a particular number.  ida_destroy() can be used to dispose of an
>  * IDA without needing to free the individual IDs in it.  You can use
>  * ida_is_empty() to find out whether the IDA has any IDs currently allocated.
> 
> 
> An empty ccidg_ida is the indication that we should clean up our
> mess:
> 
> static void ccidg_free_inst(struct usb_function_instance *f)
> ...
> 	if (ida_is_empty(&ccidg_ida))
> 		ccidg_cleanup();
> 
> If the IDA is empty, should I call ida_destroy() anyway?
> Other similiar drivers does not seems to do that.
> 
> I must say that I'm not very familiar with the IDA API.

When your module is removed, you need to clean up any remaining memory
that the ida used.  It's not obvious at all, and is a pain as you would
think that if you statically allocate one, like you have here, it would
not be needed.  You need to just call:
	ida_destroy(&ccidg_ida);
in your module exit function.

Hope this helps,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/3] Documentation: usb: add documentation for USB CCID Gadget Device
  2018-05-29 20:27   ` Randy Dunlap
@ 2018-05-30 11:34     ` Marcus Folkesson
  0 siblings, 0 replies; 16+ messages in thread
From: Marcus Folkesson @ 2018-05-30 11:34 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Greg Kroah-Hartman, Jonathan Corbet, Felipe Balbi, davem,
	Mauro Carvalho Chehab, Andrew Morton, Ruslan Bilovol,
	Thomas Gleixner, Kate Stewart, linux-usb, linux-doc,
	linux-kernel

Hi Randy,

On Tue, May 29, 2018 at 01:27:23PM -0700, Randy Dunlap wrote:
> On 05/29/2018 11:50 AM, Marcus Folkesson wrote:
> > Add documentation to give a brief description on how to use the
> > CCID Gadget Device.
> > This includes a description for all attributes followed by an example on
> > how to setup the device with ConfigFS.
> > 
> > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > ---
> > 
> > Notes:
> >     v3:
> >     	- correct the grammer (thanks Randy)
> >     v2:
> >     	- add the missing changelog text
> > 
> >  Documentation/usb/gadget_ccid.rst | 267 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 267 insertions(+)
> >  create mode 100644 Documentation/usb/gadget_ccid.rst
> > 
> > diff --git a/Documentation/usb/gadget_ccid.rst b/Documentation/usb/gadget_ccid.rst
> > new file mode 100644
> > index 000000000000..524fe9e6ac19
> > --- /dev/null
> > +++ b/Documentation/usb/gadget_ccid.rst
> > @@ -0,0 +1,267 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +============
> > +CCID Gadget
> > +============
> > +
> > +:Author: Marcus Folkesson <marcus.folkesson@gmail.com>
> > +
> > +Introduction
> > +============
> > +
> > +The CCID Gadget will present itself as a CCID device to the host system.
> > +The device supports two endpoints for now; BULK IN and BULK OUT.
> > +These endpoints are exposed to userspace via /dev/ccidg*.
> > +
> > +All CCID commands are sent on the BULK-OUT endpoint. Each command sent to the CCID
> > +has an associated ending response. Some commands can also have intermediate
> > +responses. The response is sent on the BULK-IN endpoint.
> > +See Figure 3-3 in the CCID Specification [1]_ for more details.
> > +
> > +The CCID commands must be handled in userspace since the driver is only working
> > +as a transport layer for the TPDUs.
> 
> I think that it would be helpful to tell us what the naming of the /dev/ccidg*
> endpoints looks like.  Also, how to distinguish the BULK-IN from the BULK-OUT
> endpoint.

You are right, it is not clear.

The devices are named as /dev/ccidg[0-4] and is per device.
Writing/reading to the device is using BULK-IN/BULK-OUT, the /dev/ccidg*
device is not bound to a particular endpoint.

I will come up with something that make it more clear.

Thank you!


> 
> > +
> > +
> > +CCID Commands
> > +--------------
> > +
> > +All CCID commands begins with a 10-byte header followed by an optional
> > +data field depending on message type.
> > +
> > ++--------+--------------+-------+----------------------------------+
> > +| Offset | Field        | Size  | Description                      |
> > ++========+==============+=======+==================================+
> > +| 0      | bMessageType | 1     | Type of message                  |
> > ++--------+--------------+-------+----------------------------------+
> > +| 1      | dwLength     | 4     | Message specific data length     |
> > +|        |              |       |                                  |
> > ++--------+--------------+-------+----------------------------------+
> > +| 5      | bSlot        | 1     | Identifies the slot number       |
> > +|        |              |       | for this command                 |
> > ++--------+--------------+-------+----------------------------------+
> > +| 6      | bSeq         | 1     | Sequence number for command      |
> > ++--------+--------------+-------+----------------------------------+
> > +| 7      | ...          | 3     | Fields depends on message type   |
> > ++--------+--------------+-------+----------------------------------+
> > +| 10     | abData       | array | Message specific data (OPTIONAL) |
> > ++--------+--------------+-------+----------------------------------+
> > +
> > +
> > +Multiple CCID gadgets
> > +----------------------
> > +
> > +It is possible to create multiple instances of the CCID gadget, however,
> > +a much more flexible way is to create one gadget and set the `nslots` attribute
> > +to the number of desired CCID devices.
> > +
> > +All CCID commands specify which slot is the receiver in the `bSlot` field
> > +of the CCID header.
> > +
> > +Usage
> > +=====
> > +
> > +Access from userspace
> > +----------------------
> > +All communication is by read(2) and write(2) to the corresponding /dev/ccidg* device.
> > +Only one file descriptor is allowed to be open to the device at a time.
> 
> 
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
> 
> 
> thanks,
> -- 
> ~Randy

Best regards,
Marcus Folkesson

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
  2018-05-30 11:30     ` Greg Kroah-Hartman
@ 2018-05-30 12:13       ` Marcus Folkesson
  2018-05-30 12:20         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 16+ messages in thread
From: Marcus Folkesson @ 2018-05-30 12:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Andy Shevchenko, Jonathan Corbet, Felipe Balbi, David S. Miller,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart, USB,
	Linux Documentation List, Linux Kernel Mailing List

Hi Greg,

On Wed, May 30, 2018 at 01:30:26PM +0200, Greg Kroah-Hartman wrote:
> On Wed, May 30, 2018 at 01:24:59PM +0200, Marcus Folkesson wrote:
> > Hi Andy,
> > 
> > Thank you for your comments!
> > Many good catches here!
> > 
> > On Wed, May 30, 2018 at 03:55:39AM +0300, Andy Shevchenko wrote:
> > > On Tue, May 29, 2018 at 9:50 PM, Marcus Folkesson
> > > <marcus.folkesson@gmail.com> wrote:
> > > > Chip Card Interface Device (CCID) protocol is a USB protocol that
> > > > allows a smartcard device to be connected to a computer via a card
> > > > reader using a standard USB interface, without the need for each manufacturer
> > > > of smartcards to provide its own reader or protocol.
> > > >
> > > > This gadget driver makes Linux show up as a CCID device to the host and let a
> > > > userspace daemon act as the smartcard.
> > > >
> > > > This is useful when the Linux gadget itself should act as a cryptographic
> > > > device or forward APDUs to an embedded smartcard device.
> > > 
> > > > + * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@gmail.com>
> > > 
> > > > + *
> > > 
> > > Redundant line
> > > 
> > 
> > Yep
> > 
> > > > +static DEFINE_IDA(ccidg_ida);
> > > 
> > > Where is it destroyed?
> > 
> > Hm, I'm not sure it needs to be destroyed. From lib/idr.c:
> > 
> >  * You can also use ida_get_new_above() if you need an ID to be allocated
> >  * above a particular number.  ida_destroy() can be used to dispose of an
> >  * IDA without needing to free the individual IDs in it.  You can use
> >  * ida_is_empty() to find out whether the IDA has any IDs currently allocated.
> > 
> > 
> > An empty ccidg_ida is the indication that we should clean up our
> > mess:
> > 
> > static void ccidg_free_inst(struct usb_function_instance *f)
> > ...
> > 	if (ida_is_empty(&ccidg_ida))
> > 		ccidg_cleanup();
> > 
> > If the IDA is empty, should I call ida_destroy() anyway?
> > Other similiar drivers does not seems to do that.
> > 
> > I must say that I'm not very familiar with the IDA API.
> 
> When your module is removed, you need to clean up any remaining memory
> that the ida used.  It's not obvious at all, and is a pain as you would
> think that if you statically allocate one, like you have here, it would
> not be needed.  You need to just call:
> 	ida_destroy(&ccidg_ida);
> in your module exit function.
> 
> Hope this helps,

Thank you for making it clear.

Maybe I should use
#define DECLARE_USB_FUNCTION(_name, _inst_alloc, _func_alloc)		\

instead of 
#define DECLARE_USB_FUNCTION_INIT(_name, _inst_alloc, _func_alloc)	\

and provide my own module_init/module_exit functions then?

Just give me a hint and I will do the same for
f_printer.c and
f_hid.c

as those use IDAs in a similiar way.

> 
> greg k-h

Best regards,
Marcus Folkesson

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
  2018-05-30 12:13       ` Marcus Folkesson
@ 2018-05-30 12:20         ` Greg Kroah-Hartman
  0 siblings, 0 replies; 16+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-30 12:20 UTC (permalink / raw)
  To: Marcus Folkesson
  Cc: Andy Shevchenko, Jonathan Corbet, Felipe Balbi, David S. Miller,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart, USB,
	Linux Documentation List, Linux Kernel Mailing List

On Wed, May 30, 2018 at 02:13:57PM +0200, Marcus Folkesson wrote:
> Hi Greg,
> 
> On Wed, May 30, 2018 at 01:30:26PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, May 30, 2018 at 01:24:59PM +0200, Marcus Folkesson wrote:
> > > Hi Andy,
> > > 
> > > Thank you for your comments!
> > > Many good catches here!
> > > 
> > > On Wed, May 30, 2018 at 03:55:39AM +0300, Andy Shevchenko wrote:
> > > > On Tue, May 29, 2018 at 9:50 PM, Marcus Folkesson
> > > > <marcus.folkesson@gmail.com> wrote:
> > > > > Chip Card Interface Device (CCID) protocol is a USB protocol that
> > > > > allows a smartcard device to be connected to a computer via a card
> > > > > reader using a standard USB interface, without the need for each manufacturer
> > > > > of smartcards to provide its own reader or protocol.
> > > > >
> > > > > This gadget driver makes Linux show up as a CCID device to the host and let a
> > > > > userspace daemon act as the smartcard.
> > > > >
> > > > > This is useful when the Linux gadget itself should act as a cryptographic
> > > > > device or forward APDUs to an embedded smartcard device.
> > > > 
> > > > > + * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@gmail.com>
> > > > 
> > > > > + *
> > > > 
> > > > Redundant line
> > > > 
> > > 
> > > Yep
> > > 
> > > > > +static DEFINE_IDA(ccidg_ida);
> > > > 
> > > > Where is it destroyed?
> > > 
> > > Hm, I'm not sure it needs to be destroyed. From lib/idr.c:
> > > 
> > >  * You can also use ida_get_new_above() if you need an ID to be allocated
> > >  * above a particular number.  ida_destroy() can be used to dispose of an
> > >  * IDA without needing to free the individual IDs in it.  You can use
> > >  * ida_is_empty() to find out whether the IDA has any IDs currently allocated.
> > > 
> > > 
> > > An empty ccidg_ida is the indication that we should clean up our
> > > mess:
> > > 
> > > static void ccidg_free_inst(struct usb_function_instance *f)
> > > ...
> > > 	if (ida_is_empty(&ccidg_ida))
> > > 		ccidg_cleanup();
> > > 
> > > If the IDA is empty, should I call ida_destroy() anyway?
> > > Other similiar drivers does not seems to do that.
> > > 
> > > I must say that I'm not very familiar with the IDA API.
> > 
> > When your module is removed, you need to clean up any remaining memory
> > that the ida used.  It's not obvious at all, and is a pain as you would
> > think that if you statically allocate one, like you have here, it would
> > not be needed.  You need to just call:
> > 	ida_destroy(&ccidg_ida);
> > in your module exit function.
> > 
> > Hope this helps,
> 
> Thank you for making it clear.
> 
> Maybe I should use
> #define DECLARE_USB_FUNCTION(_name, _inst_alloc, _func_alloc)		\
> 
> instead of 
> #define DECLARE_USB_FUNCTION_INIT(_name, _inst_alloc, _func_alloc)	\
> 
> and provide my own module_init/module_exit functions then?

Probably, yes.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
  2018-05-29 18:50 [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device Marcus Folkesson
                   ` (2 preceding siblings ...)
  2018-05-30  0:55 ` [PATCH v3 1/3] usb: gadget: ccid: add support for " Andy Shevchenko
@ 2018-05-30 12:28 ` Felipe Balbi
  2018-05-30 14:04   ` Marcus Folkesson
  3 siblings, 1 reply; 16+ messages in thread
From: Felipe Balbi @ 2018-05-30 12:28 UTC (permalink / raw)
  To: Marcus Folkesson, Greg Kroah-Hartman, Jonathan Corbet, davem,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart
  Cc: linux-usb, linux-doc, linux-kernel, Marcus Folkesson

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

Marcus Folkesson <marcus.folkesson@gmail.com> writes:

> Chip Card Interface Device (CCID) protocol is a USB protocol that
> allows a smartcard device to be connected to a computer via a card
> reader using a standard USB interface, without the need for each manufacturer
> of smartcards to provide its own reader or protocol.
>
> This gadget driver makes Linux show up as a CCID device to the host and let a
> userspace daemon act as the smartcard.
>
> This is useful when the Linux gadget itself should act as a cryptographic
> device or forward APDUs to an embedded smartcard device.
>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>

this could be done entirely in userspace with functionfs, why do we need
this part in the kernel? It does very little.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
  2018-05-30 12:28 ` Felipe Balbi
@ 2018-05-30 14:04   ` Marcus Folkesson
  2018-06-08 18:54     ` Marcus Folkesson
  0 siblings, 1 reply; 16+ messages in thread
From: Marcus Folkesson @ 2018-05-30 14:04 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Greg Kroah-Hartman, Jonathan Corbet, davem,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart, linux-usb,
	linux-doc, linux-kernel

Hi Filipe,

On Wed, May 30, 2018 at 03:28:18PM +0300, Felipe Balbi wrote:
> Marcus Folkesson <marcus.folkesson@gmail.com> writes:
> 
> > Chip Card Interface Device (CCID) protocol is a USB protocol that
> > allows a smartcard device to be connected to a computer via a card
> > reader using a standard USB interface, without the need for each manufacturer
> > of smartcards to provide its own reader or protocol.
> >
> > This gadget driver makes Linux show up as a CCID device to the host and let a
> > userspace daemon act as the smartcard.
> >
> > This is useful when the Linux gadget itself should act as a cryptographic
> > device or forward APDUs to an embedded smartcard device.
> >
> > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> 
> this could be done entirely in userspace with functionfs, why do we need
> this part in the kernel? It does very little.

Andrzej pointed this out, and I actually do not have any good answer
more than that the userspace application could be kept small and the
important configuration of the CCID device is done with well (I hope)
documented configfs attributes.

> 
> -- 
> balbi

Best regards,
Marcus Folkesson

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
  2018-05-30 14:04   ` Marcus Folkesson
@ 2018-06-08 18:54     ` Marcus Folkesson
  2018-06-18  8:22       ` Felipe Balbi
  0 siblings, 1 reply; 16+ messages in thread
From: Marcus Folkesson @ 2018-06-08 18:54 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Greg Kroah-Hartman, Jonathan Corbet, davem,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart, linux-usb,
	linux-doc, linux-kernel

Hi Felipe,

Should I send out v4 or what do you think?

On Wed, May 30, 2018 at 04:04:15PM +0200, Marcus Folkesson wrote:
> Hi Filipe,
> 
> On Wed, May 30, 2018 at 03:28:18PM +0300, Felipe Balbi wrote:
> > Marcus Folkesson <marcus.folkesson@gmail.com> writes:
> > 
> > > Chip Card Interface Device (CCID) protocol is a USB protocol that
> > > allows a smartcard device to be connected to a computer via a card
> > > reader using a standard USB interface, without the need for each manufacturer
> > > of smartcards to provide its own reader or protocol.
> > >
> > > This gadget driver makes Linux show up as a CCID device to the host and let a
> > > userspace daemon act as the smartcard.
> > >
> > > This is useful when the Linux gadget itself should act as a cryptographic
> > > device or forward APDUs to an embedded smartcard device.
> > >
> > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > 
> > this could be done entirely in userspace with functionfs, why do we need
> > this part in the kernel? It does very little.
> 
> Andrzej pointed this out, and I actually do not have any good answer
> more than that the userspace application could be kept small and the
> important configuration of the CCID device is done with well (I hope)
> documented configfs attributes.
> 
> > 
> > -- 
> > balbi
> 
> Best regards,
> Marcus Folkesson
> 
> 

Thank you,
Marcus Folkesson

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
  2018-06-08 18:54     ` Marcus Folkesson
@ 2018-06-18  8:22       ` Felipe Balbi
  2018-06-22 16:48         ` Marcus Folkesson
  0 siblings, 1 reply; 16+ messages in thread
From: Felipe Balbi @ 2018-06-18  8:22 UTC (permalink / raw)
  To: Marcus Folkesson
  Cc: Greg Kroah-Hartman, Jonathan Corbet, davem,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart, linux-usb,
	linux-doc, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]


Hi,

Marcus Folkesson <marcus.folkesson@gmail.com> writes:
> Hi Felipe,
>
> Should I send out v4 or what do you think?

sorry for the delay, have been busy with other tasks.

> On Wed, May 30, 2018 at 04:04:15PM +0200, Marcus Folkesson wrote:
>> Hi Filipe,
>> 
>> On Wed, May 30, 2018 at 03:28:18PM +0300, Felipe Balbi wrote:
>> > Marcus Folkesson <marcus.folkesson@gmail.com> writes:
>> > 
>> > > Chip Card Interface Device (CCID) protocol is a USB protocol that
>> > > allows a smartcard device to be connected to a computer via a card
>> > > reader using a standard USB interface, without the need for each manufacturer
>> > > of smartcards to provide its own reader or protocol.
>> > >
>> > > This gadget driver makes Linux show up as a CCID device to the host and let a
>> > > userspace daemon act as the smartcard.
>> > >
>> > > This is useful when the Linux gadget itself should act as a cryptographic
>> > > device or forward APDUs to an embedded smartcard device.
>> > >
>> > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
>> > 
>> > this could be done entirely in userspace with functionfs, why do we need
>> > this part in the kernel? It does very little.
>> 
>> Andrzej pointed this out, and I actually do not have any good answer
>> more than that the userspace application could be kept small and the
>> important configuration of the CCID device is done with well (I hope)
>> documented configfs attributes.

can we use existing open source applications without modification by
accepting this glue layer?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
  2018-06-18  8:22       ` Felipe Balbi
@ 2018-06-22 16:48         ` Marcus Folkesson
  2018-07-10 18:46           ` Marcus Folkesson
  0 siblings, 1 reply; 16+ messages in thread
From: Marcus Folkesson @ 2018-06-22 16:48 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Greg Kroah-Hartman, Jonathan Corbet, davem,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart, linux-usb,
	linux-doc, linux-kernel

Hi Filipe,

On Mon, Jun 18, 2018 at 11:22:04AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Marcus Folkesson <marcus.folkesson@gmail.com> writes:
> > Hi Felipe,
> >
> > Should I send out v4 or what do you think?
> 
> sorry for the delay, have been busy with other tasks.
> 

It is no hurry :-)

> > On Wed, May 30, 2018 at 04:04:15PM +0200, Marcus Folkesson wrote:
> >> Hi Filipe,
> >> 
> >> On Wed, May 30, 2018 at 03:28:18PM +0300, Felipe Balbi wrote:
> >> > Marcus Folkesson <marcus.folkesson@gmail.com> writes:
> >> > 
> >> > > Chip Card Interface Device (CCID) protocol is a USB protocol that
> >> > > allows a smartcard device to be connected to a computer via a card
> >> > > reader using a standard USB interface, without the need for each manufacturer
> >> > > of smartcards to provide its own reader or protocol.
> >> > >
> >> > > This gadget driver makes Linux show up as a CCID device to the host and let a
> >> > > userspace daemon act as the smartcard.
> >> > >
> >> > > This is useful when the Linux gadget itself should act as a cryptographic
> >> > > device or forward APDUs to an embedded smartcard device.
> >> > >
> >> > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> >> > 
> >> > this could be done entirely in userspace with functionfs, why do we need
> >> > this part in the kernel? It does very little.
> >> 
> >> Andrzej pointed this out, and I actually do not have any good answer
> >> more than that the userspace application could be kept small and the
> >> important configuration of the CCID device is done with well (I hope)
> >> documented configfs attributes.
> 
> can we use existing open source applications without modification by
> accepting this glue layer?


If you mean existing open source application to talk to the "daemon", the answer is
yes. I have been using PCSC-lite and OpenSC on Linux, and PCSC on
Windows to communicate with the smartcard.

If you mean existing application as the "daemon", the answer is probably
no.

> 
> -- 
> balbi

Thanks,

Best regards
Marcus Folkesson



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
  2018-06-22 16:48         ` Marcus Folkesson
@ 2018-07-10 18:46           ` Marcus Folkesson
  0 siblings, 0 replies; 16+ messages in thread
From: Marcus Folkesson @ 2018-07-10 18:46 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Greg Kroah-Hartman, Jonathan Corbet, davem,
	Mauro Carvalho Chehab, Andrew Morton, Randy Dunlap,
	Ruslan Bilovol, Thomas Gleixner, Kate Stewart, linux-usb,
	linux-doc, linux-kernel

Hi Filipe,

On Fri, Jun 22, 2018 at 06:48:23PM +0200, Marcus Folkesson wrote:
> Hi Filipe,
> 
> On Mon, Jun 18, 2018 at 11:22:04AM +0300, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > Marcus Folkesson <marcus.folkesson@gmail.com> writes:
> > > Hi Felipe,
> > >
> > > Should I send out v4 or what do you think?
> > 
> > sorry for the delay, have been busy with other tasks.
> > 
> 
> It is no hurry :-)
> 
> > > On Wed, May 30, 2018 at 04:04:15PM +0200, Marcus Folkesson wrote:
> > >> Hi Filipe,
> > >> 
> > >> On Wed, May 30, 2018 at 03:28:18PM +0300, Felipe Balbi wrote:
> > >> > Marcus Folkesson <marcus.folkesson@gmail.com> writes:
> > >> > 
> > >> > > Chip Card Interface Device (CCID) protocol is a USB protocol that
> > >> > > allows a smartcard device to be connected to a computer via a card
> > >> > > reader using a standard USB interface, without the need for each manufacturer
> > >> > > of smartcards to provide its own reader or protocol.
> > >> > >
> > >> > > This gadget driver makes Linux show up as a CCID device to the host and let a
> > >> > > userspace daemon act as the smartcard.
> > >> > >
> > >> > > This is useful when the Linux gadget itself should act as a cryptographic
> > >> > > device or forward APDUs to an embedded smartcard device.
> > >> > >
> > >> > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > >> > 
> > >> > this could be done entirely in userspace with functionfs, why do we need
> > >> > this part in the kernel? It does very little.
> > >> 
> > >> Andrzej pointed this out, and I actually do not have any good answer
> > >> more than that the userspace application could be kept small and the
> > >> important configuration of the CCID device is done with well (I hope)
> > >> documented configfs attributes.
> > 
> > can we use existing open source applications without modification by
> > accepting this glue layer?
> 
> 
> If you mean existing open source application to talk to the "daemon", the answer is
> yes. I have been using PCSC-lite and OpenSC on Linux, and PCSC on
> Windows to communicate with the smartcard.
> 
> If you mean existing application as the "daemon", the answer is probably
> no.


Just sending a friendly ping on this one.
I've sent out v4 as I had it prepared for a long time, just to catch
comments in the meanwhile, but none so far.

> 
> > 
> > -- 
> > balbi
> 
> Thanks,
> 
> Best regards
> Marcus Folkesson
> 

Thanks,

Best regards
Marcus Folkesson
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-07-10 18:46 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-29 18:50 [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device Marcus Folkesson
2018-05-29 18:50 ` [PATCH v3 2/3] Documentation: usb: add documentation " Marcus Folkesson
2018-05-29 20:27   ` Randy Dunlap
2018-05-30 11:34     ` Marcus Folkesson
2018-05-29 18:50 ` [PATCH v3 3/3] MAINTAINERS: add " Marcus Folkesson
2018-05-30  0:55 ` [PATCH v3 1/3] usb: gadget: ccid: add support for " Andy Shevchenko
2018-05-30 11:24   ` Marcus Folkesson
2018-05-30 11:30     ` Greg Kroah-Hartman
2018-05-30 12:13       ` Marcus Folkesson
2018-05-30 12:20         ` Greg Kroah-Hartman
2018-05-30 12:28 ` Felipe Balbi
2018-05-30 14:04   ` Marcus Folkesson
2018-06-08 18:54     ` Marcus Folkesson
2018-06-18  8:22       ` Felipe Balbi
2018-06-22 16:48         ` Marcus Folkesson
2018-07-10 18:46           ` Marcus Folkesson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).