diff mbox series

[net-next,v2,1/2] nfc: Add a virtual nci device driver

Message ID 20210120115645.32798-2-bongsu.jeon@samsung.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add nci suit and virtual nci device driver | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 2 of 2 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 1 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Bongsu Jeon Jan. 20, 2021, 11:56 a.m. UTC 
From: Bongsu Jeon <bongsu.jeon@samsung.com>

A NCI virtual device can be made to simulate a NCI device in user space.
Using the virtual NCI device, The NCI module and application can be
validated. This driver supports to communicate between the virtual NCI
device and NCI module.

Signed-off-by: Bongsu Jeon <bongsu.jeon@samsung.com>
---
 drivers/nfc/Kconfig          |  11 ++
 drivers/nfc/Makefile         |   1 +
 drivers/nfc/virtual_ncidev.c | 235 +++++++++++++++++++++++++++++++++++
 3 files changed, 247 insertions(+)
 create mode 100644 drivers/nfc/virtual_ncidev.c

Comments

Jakub Kicinski Jan. 23, 2021, 2:49 a.m. UTC | #1 
On Wed, 20 Jan 2021 20:56:44 +0900 Bongsu Jeon wrote:
> From: Bongsu Jeon <bongsu.jeon@samsung.com>
> 
> A NCI virtual device can be made to simulate a NCI device in user space.
> Using the virtual NCI device, The NCI module and application can be
> validated. This driver supports to communicate between the virtual NCI
> device and NCI module.
> 
> Signed-off-by: Bongsu Jeon <bongsu.jeon@samsung.com>

Please CC Krzysztof on next version, maybe we'll be lucky and he finds
time to look at this :)

> diff --git a/drivers/nfc/Kconfig b/drivers/nfc/Kconfig
> index 75c65d339018..d32c3a8937ed 100644
> --- a/drivers/nfc/Kconfig
> +++ b/drivers/nfc/Kconfig
> @@ -49,6 +49,17 @@ config NFC_PORT100
>  
>  	  If unsure, say N.
>  
> +config NFC_VIRTUAL_NCI
> +	tristate "NCI device simulator driver"
> +	depends on NFC_NCI
> +	help
> +	  A NCI virtual device can be made to simulate a NCI device in user
> +	  level. Using the virtual NCI device, The NCI module and application
> +	  can be validated. This driver supports to communicate between the
> +	  virtual NCI device and NCI module.

How about:

  NCI virtual device simulates a NCI device to the user. 
  It can be used to validate the NCI module and applications. 
  This driver supports communication between the virtual NCI 
  device and NCI module.

Just trying to improve the grammar.


> +#define IOCTL_GET_NCIDEV_IDX    0
> +#define VIRTUAL_NFC_PROTOCOLS	(NFC_PROTO_JEWEL_MASK | \
> +				 NFC_PROTO_MIFARE_MASK | \
> +				 NFC_PROTO_FELICA_MASK | \
> +				 NFC_PROTO_ISO14443_MASK | \
> +				 NFC_PROTO_ISO14443_B_MASK | \
> +				 NFC_PROTO_ISO15693_MASK)
> +
> +static enum virtual_ncidev_mode state;
> +static struct mutex nci_send_mutex;
> +static struct miscdevice miscdev;
> +static struct sk_buff *send_buff;
> +static struct mutex nci_mutex;

I think if you use:

static DEFINE_MUTEX(...);

you won't have to init them in the code

> +static struct nci_dev *ndev;
> +static bool full_txbuff;
> +
> +static bool virtual_ncidev_check_enabled(void)
> +{
> +	bool ret = true;
> +
> +	mutex_lock(&nci_mutex);
> +	if (state != virtual_ncidev_enabled)
> +		ret = false;
> +	mutex_unlock(&nci_mutex);
> +
> +	return ret;
> +}
> +
> +static int virtual_nci_open(struct nci_dev *ndev)
> +{
> +	return 0;
> +}
> +
> +static int virtual_nci_close(struct nci_dev *ndev)
> +{
> +	mutex_lock(&nci_send_mutex);
> +	if (full_txbuff)
> +		kfree_skb(send_buff);
> +	full_txbuff = false;
> +	mutex_unlock(&nci_send_mutex);
> +
> +	return 0;
> +}
> +
> +static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
> +{
> +	if (virtual_ncidev_check_enabled() == false)
> +		return 0;
> +
> +	mutex_lock(&nci_send_mutex);
> +	if (full_txbuff) {
> +		mutex_unlock(&nci_send_mutex);
> +		return -1;
> +	}
> +	send_buff = skb_copy(skb, GFP_KERNEL);
> +	full_txbuff = true;

Do you need this variable? looks like you can just set send_buff to NULL

> +	mutex_unlock(&nci_send_mutex);
> +
> +	return 0;
> +}
> +
> +static struct nci_ops virtual_nci_ops = {
> +	.open = virtual_nci_open,
> +	.close = virtual_nci_close,
> +	.send = virtual_nci_send
> +};
> +
> +static ssize_t virtual_ncidev_read(struct file *file, char __user *buf,
> +				   size_t count, loff_t *ppos)
> +{
> +	size_t actual_len;
> +
> +	mutex_lock(&nci_send_mutex);
> +	if (!full_txbuff) {
> +		mutex_unlock(&nci_send_mutex);
> +		return 0;
> +	}
> +
> +	actual_len = count > send_buff->len ? send_buff->len : count;

min_t()

> +	if (copy_to_user(buf, send_buff->data, actual_len)) {
> +		mutex_unlock(&nci_send_mutex);
> +		return -EFAULT;
> +	}
> +
> +	skb_pull(send_buff, actual_len);
> +	if (send_buff->len == 0) {
> +		kfree_skb(send_buff);

consume_skb()

> +		full_txbuff = false;
> +	}
> +	mutex_unlock(&nci_send_mutex);
> +
> +	return actual_len;
> +}
> +
> +static ssize_t virtual_ncidev_write(struct file *file,
> +				    const char __user *buf,
> +				    size_t count, loff_t *ppos)
> +{
> +	struct sk_buff *skb;
> +
> +	skb = alloc_skb(count, GFP_KERNEL);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	if (copy_from_user(skb_put(skb, count), buf, count))

leaks skb

> +		return -EFAULT;
> +
> +	nci_recv_frame(ndev, skb);
> +	return count;
> +}

> +static long virtual_ncidev_ioctl(struct file *flip, unsigned int cmd,
> +				 unsigned long arg)
> +{
> +	long res = -ENOTTY;
> +
> +	if (cmd == IOCTL_GET_NCIDEV_IDX) {
> +		struct nfc_dev *nfc_dev = ndev->nfc_dev;
> +		void __user *p = (void __user *)arg;
> +
> +		if (copy_to_user(p, &nfc_dev->idx, sizeof(nfc_dev->idx)))
> +			return -EFAULT;
> +		res = 0;
> +	}
> +
> +	return res;

The condition can be flipped to save the indentation and local variable:

if (cmd != ...)
	return -ENOTTY;

....
return 0;

> +}
> +
> +static const struct file_operations virtual_ncidev_fops = {
> +	.owner = THIS_MODULE,
> +	.read = virtual_ncidev_read,
> +	.write = virtual_ncidev_write,
> +	.open = virtual_ncidev_open,
> +	.release = virtual_ncidev_close,
> +	.unlocked_ioctl = virtual_ncidev_ioctl
> +};
> +
> +static int __init virtual_ncidev_init(void)
> +{
> +	int ret;
> +
> +	mutex_init(&nci_mutex);
> +	state = virtual_ncidev_disabled;
> +	miscdev.minor = MISC_DYNAMIC_MINOR;
> +	miscdev.name = "virtual_nci";
> +	miscdev.fops = &virtual_ncidev_fops;
> +	miscdev.mode = S_IALLUGO;
> +	ret = misc_register(&miscdev);
> +
> +	return ret;

no need for the local variable here, just 

	return misc_register()

> +}
> +
> +static void __exit virtual_ncidev_exit(void)
> +{
> +	misc_deregister(&miscdev);
> +}
> +
> +module_init(virtual_ncidev_init);
> +module_exit(virtual_ncidev_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Virtual NCI device simulation driver");
> +MODULE_AUTHOR("Bongsu Jeon <bongsu.jeon@samsung.com>");
diff mbox series

Patch

diff --git a/drivers/nfc/Kconfig b/drivers/nfc/Kconfig
index 75c65d339018..d32c3a8937ed 100644
--- a/drivers/nfc/Kconfig
+++ b/drivers/nfc/Kconfig
@@ -49,6 +49,17 @@  config NFC_PORT100
 
 	  If unsure, say N.
 
+config NFC_VIRTUAL_NCI
+	tristate "NCI device simulator driver"
+	depends on NFC_NCI
+	help
+	  A NCI virtual device can be made to simulate a NCI device in user
+	  level. Using the virtual NCI device, The NCI module and application
+	  can be validated. This driver supports to communicate between the
+	  virtual NCI device and NCI module.
+
+	  If unsure, say N.
+
 source "drivers/nfc/fdp/Kconfig"
 source "drivers/nfc/pn544/Kconfig"
 source "drivers/nfc/pn533/Kconfig"
diff --git a/drivers/nfc/Makefile b/drivers/nfc/Makefile
index 5393ba59b17d..7b1bfde1d971 100644
--- a/drivers/nfc/Makefile
+++ b/drivers/nfc/Makefile
@@ -17,3 +17,4 @@  obj-$(CONFIG_NFC_ST_NCI)	+= st-nci/
 obj-$(CONFIG_NFC_NXP_NCI)	+= nxp-nci/
 obj-$(CONFIG_NFC_S3FWRN5)	+= s3fwrn5/
 obj-$(CONFIG_NFC_ST95HF)	+= st95hf/
+obj-$(CONFIG_NFC_VIRTUAL_NCI)	+= virtual_ncidev.o
diff --git a/drivers/nfc/virtual_ncidev.c b/drivers/nfc/virtual_ncidev.c
new file mode 100644
index 000000000000..0e327c08327c
--- /dev/null
+++ b/drivers/nfc/virtual_ncidev.c
@@ -0,0 +1,235 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Virtual NCI device simulation driver
+ *
+ * Copyright (C) 2020 Samsung Electrnoics
+ * Bongsu Jeon <bongsu.jeon@samsung.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/miscdevice.h>
+#include <linux/mutex.h>
+#include <net/nfc/nci_core.h>
+
+enum virtual_ncidev_mode {
+	virtual_ncidev_enabled,
+	virtual_ncidev_disabled,
+	virtual_ncidev_disabling,
+};
+
+#define IOCTL_GET_NCIDEV_IDX    0
+#define VIRTUAL_NFC_PROTOCOLS	(NFC_PROTO_JEWEL_MASK | \
+				 NFC_PROTO_MIFARE_MASK | \
+				 NFC_PROTO_FELICA_MASK | \
+				 NFC_PROTO_ISO14443_MASK | \
+				 NFC_PROTO_ISO14443_B_MASK | \
+				 NFC_PROTO_ISO15693_MASK)
+
+static enum virtual_ncidev_mode state;
+static struct mutex nci_send_mutex;
+static struct miscdevice miscdev;
+static struct sk_buff *send_buff;
+static struct mutex nci_mutex;
+static struct nci_dev *ndev;
+static bool full_txbuff;
+
+static bool virtual_ncidev_check_enabled(void)
+{
+	bool ret = true;
+
+	mutex_lock(&nci_mutex);
+	if (state != virtual_ncidev_enabled)
+		ret = false;
+	mutex_unlock(&nci_mutex);
+
+	return ret;
+}
+
+static int virtual_nci_open(struct nci_dev *ndev)
+{
+	return 0;
+}
+
+static int virtual_nci_close(struct nci_dev *ndev)
+{
+	mutex_lock(&nci_send_mutex);
+	if (full_txbuff)
+		kfree_skb(send_buff);
+	full_txbuff = false;
+	mutex_unlock(&nci_send_mutex);
+
+	return 0;
+}
+
+static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
+{
+	if (virtual_ncidev_check_enabled() == false)
+		return 0;
+
+	mutex_lock(&nci_send_mutex);
+	if (full_txbuff) {
+		mutex_unlock(&nci_send_mutex);
+		return -1;
+	}
+	send_buff = skb_copy(skb, GFP_KERNEL);
+	full_txbuff = true;
+	mutex_unlock(&nci_send_mutex);
+
+	return 0;
+}
+
+static struct nci_ops virtual_nci_ops = {
+	.open = virtual_nci_open,
+	.close = virtual_nci_close,
+	.send = virtual_nci_send
+};
+
+static ssize_t virtual_ncidev_read(struct file *file, char __user *buf,
+				   size_t count, loff_t *ppos)
+{
+	size_t actual_len;
+
+	mutex_lock(&nci_send_mutex);
+	if (!full_txbuff) {
+		mutex_unlock(&nci_send_mutex);
+		return 0;
+	}
+
+	actual_len = count > send_buff->len ? send_buff->len : count;
+
+	if (copy_to_user(buf, send_buff->data, actual_len)) {
+		mutex_unlock(&nci_send_mutex);
+		return -EFAULT;
+	}
+
+	skb_pull(send_buff, actual_len);
+	if (send_buff->len == 0) {
+		kfree_skb(send_buff);
+		full_txbuff = false;
+	}
+	mutex_unlock(&nci_send_mutex);
+
+	return actual_len;
+}
+
+static ssize_t virtual_ncidev_write(struct file *file,
+				    const char __user *buf,
+				    size_t count, loff_t *ppos)
+{
+	struct sk_buff *skb;
+
+	skb = alloc_skb(count, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	if (copy_from_user(skb_put(skb, count), buf, count))
+		return -EFAULT;
+
+	nci_recv_frame(ndev, skb);
+	return count;
+}
+
+static int virtual_ncidev_open(struct inode *inode, struct file *file)
+{
+	int ret = 0;
+
+	mutex_lock(&nci_mutex);
+	if (state != virtual_ncidev_disabled) {
+		mutex_unlock(&nci_mutex);
+		return -EBUSY;
+	}
+
+	mutex_init(&nci_send_mutex);
+
+	ndev = nci_allocate_device(&virtual_nci_ops, VIRTUAL_NFC_PROTOCOLS,
+				   0, 0);
+	if (!ndev) {
+		mutex_unlock(&nci_mutex);
+		return -ENOMEM;
+	}
+
+	ret = nci_register_device(ndev);
+	if (ret < 0) {
+		nci_free_device(ndev);
+		mutex_unlock(&nci_mutex);
+		return ret;
+	}
+	state = virtual_ncidev_enabled;
+	mutex_unlock(&nci_mutex);
+
+	return 0;
+}
+
+static int virtual_ncidev_close(struct inode *inode, struct file *file)
+{
+	mutex_lock(&nci_mutex);
+
+	if (state == virtual_ncidev_enabled) {
+		state = virtual_ncidev_disabling;
+		mutex_unlock(&nci_mutex);
+
+		nci_unregister_device(ndev);
+		nci_free_device(ndev);
+
+		mutex_lock(&nci_mutex);
+	}
+
+	state = virtual_ncidev_disabled;
+	mutex_unlock(&nci_mutex);
+
+	return 0;
+}
+
+static long virtual_ncidev_ioctl(struct file *flip, unsigned int cmd,
+				 unsigned long arg)
+{
+	long res = -ENOTTY;
+
+	if (cmd == IOCTL_GET_NCIDEV_IDX) {
+		struct nfc_dev *nfc_dev = ndev->nfc_dev;
+		void __user *p = (void __user *)arg;
+
+		if (copy_to_user(p, &nfc_dev->idx, sizeof(nfc_dev->idx)))
+			return -EFAULT;
+		res = 0;
+	}
+
+	return res;
+}
+
+static const struct file_operations virtual_ncidev_fops = {
+	.owner = THIS_MODULE,
+	.read = virtual_ncidev_read,
+	.write = virtual_ncidev_write,
+	.open = virtual_ncidev_open,
+	.release = virtual_ncidev_close,
+	.unlocked_ioctl = virtual_ncidev_ioctl
+};
+
+static int __init virtual_ncidev_init(void)
+{
+	int ret;
+
+	mutex_init(&nci_mutex);
+	state = virtual_ncidev_disabled;
+	miscdev.minor = MISC_DYNAMIC_MINOR;
+	miscdev.name = "virtual_nci";
+	miscdev.fops = &virtual_ncidev_fops;
+	miscdev.mode = S_IALLUGO;
+	ret = misc_register(&miscdev);
+
+	return ret;
+}
+
+static void __exit virtual_ncidev_exit(void)
+{
+	misc_deregister(&miscdev);
+}
+
+module_init(virtual_ncidev_init);
+module_exit(virtual_ncidev_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Virtual NCI device simulation driver");
+MODULE_AUTHOR("Bongsu Jeon <bongsu.jeon@samsung.com>");