Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1364710 > unrolled thread

[RFC 0/4] NFC: pn533: support for pn532 via I2C

Back to article view | Back to linux.kernel

Contents

  [RFC 0/4] NFC: pn533: support for pn532 via I2C Michael Thalmeier <michael.thalmeier@hale.at> - 2016-03-25 16:00 +0100
    [RFC 1/4] NFC: pn533: Send ATR_REQ only if NFC_PROTO_NFC_DEP bit is set in poll_protocols Michael Thalmeier <michael.thalmeier@hale.at> - 2016-03-25 16:00 +0100
    [RFC 4/4] NFC: pn533: add I2C phy driver Michael Thalmeier <michael.thalmeier@hale.at> - 2016-03-25 16:00 +0100
    [RFC 2/4] NFC: pn533: fix deadlock when socket is closed while processing command Michael Thalmeier <michael.thalmeier@hale.at> - 2016-03-25 16:00 +0100

#1364710 — [RFC 0/4] NFC: pn533: support for pn532 via I2C

This patchset is an attempt to implement support for NXP pn532 NFC chip
connected via I2C.

PN532 and PN533 are nearly identical despite the transport layer. So this
implementation seperates the phy layer of the current pn533 usb driver and
implements a new I2C phy layer for pn532.

While implementing and testing I also triggered a few bugs that are fixed with
the first two patches.

Michael Thalmeier (4):
  NFC: pn533: Send ATR_REQ only if NFC_PROTO_NFC_DEP bit is set in
    poll_protocols
  NFC: pn533: fix deadlock when socket is closed while processing
    command
  NFC: pn533: Separate pn533 driver in HW dependant and independant
    parts
  NFC: pn533: add I2C phy driver

 drivers/nfc/Kconfig        |   11 +-
 drivers/nfc/Makefile       |    2 +-
 drivers/nfc/pn533.c        | 3313 --------------------------------------------
 drivers/nfc/pn533/Kconfig  |   27 +
 drivers/nfc/pn533/Makefile |    9 +
 drivers/nfc/pn533/i2c.c    |  277 ++++
 drivers/nfc/pn533/pn533.c  | 2681 +++++++++++++++++++++++++++++++++++
 drivers/nfc/pn533/pn533.h  |  236 ++++
 drivers/nfc/pn533/usb.c    |  595 ++++++++
 9 files changed, 3827 insertions(+), 3324 deletions(-)
 delete mode 100644 drivers/nfc/pn533.c
 create mode 100644 drivers/nfc/pn533/Kconfig
 create mode 100644 drivers/nfc/pn533/Makefile
 create mode 100644 drivers/nfc/pn533/i2c.c
 create mode 100644 drivers/nfc/pn533/pn533.c
 create mode 100644 drivers/nfc/pn533/pn533.h
 create mode 100644 drivers/nfc/pn533/usb.c

-- 
2.5.5

[toc] | [next] | [standalone]

#1364713 — [RFC 1/4] NFC: pn533: Send ATR_REQ only if NFC_PROTO_NFC_DEP bit is set in poll_protocols

Currently it is not possible to only poll for passive targets with the pn533
driver. To change this ATR_REQ is only sent when NFC_PROTO_NFC_DEP is
explicitly requested in poll_protocols.
As most implementations (e.g. neard) poll for all protocols that are reported
to be supported by the adapter, this should not have much of an effect for
current implementations.

Signed-off-by: Michael Thalmeier <michael.thalmeier@hale.at>
---
 drivers/nfc/pn533.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/nfc/pn533.c b/drivers/nfc/pn533.c
index bb3d5ea..a85830f 100644
--- a/drivers/nfc/pn533.c
+++ b/drivers/nfc/pn533.c
@@ -1540,7 +1540,8 @@ static int pn533_start_poll_complete(struct pn533 *dev, struct sk_buff *resp)
 	int rc, tgdata_len;
 
 	/* Toggle the DEP polling */
-	dev->poll_dep = 1;
+	if (dev->poll_protocols & NFC_PROTO_NFC_DEP_MASK)
+		dev->poll_dep = 1;
 
 	nbtg = resp->data[0];
 	tg = resp->data[1];
@@ -2054,7 +2055,7 @@ static int pn533_send_poll_frame(struct pn533 *dev)
 	dev_dbg(&dev->interface->dev, "%s mod len %d\n",
 		__func__, mod->len);
 
-	if (dev->poll_dep)  {
+	if ((dev->poll_protocols & NFC_PROTO_NFC_DEP_MASK) && dev->poll_dep)  {
 		dev->poll_dep = 0;
 		return pn533_poll_dep(dev->nfc_dev);
 	}
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1364717 — [RFC 4/4] NFC: pn533: add I2C phy driver

This adds the I2C phy interface for the pn533 driver. This way the driver can
be used to interact with I2C connected pn532.

Signed-off-by: Michael Thalmeier <michael.thalmeier@hale.at>
---
 drivers/nfc/pn533/Kconfig  |  11 ++
 drivers/nfc/pn533/Makefile |   2 +
 drivers/nfc/pn533/i2c.c    | 277 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/nfc/pn533/pn533.c  |  29 +++++
 drivers/nfc/pn533/pn533.h  |   2 +
 5 files changed, 321 insertions(+)
 create mode 100644 drivers/nfc/pn533/i2c.c

diff --git a/drivers/nfc/pn533/Kconfig b/drivers/nfc/pn533/Kconfig
index b5a926e..d94122d 100644
--- a/drivers/nfc/pn533/Kconfig
+++ b/drivers/nfc/pn533/Kconfig
@@ -14,3 +14,14 @@ config NFC_PN533_USB
 
 	  If you choose to build a module, it'll be called pn533_usb.
 	  Say N if unsure.
+
+config NFC_PN533_I2C
+	tristate "NFC PN533 device support (I2C)"
+	depends on I2C
+	select NFC_PN533
+	---help---
+	  This module adds support for the NXP pn533 I2C interface.
+	  Select this if your platform is using the I2C bus.
+
+	  If you choose to build a module, it'll be called pn533_i2c.
+	  Say N if unsure.
diff --git a/drivers/nfc/pn533/Makefile b/drivers/nfc/pn533/Makefile
index 12c6be4..51d24c6 100644
--- a/drivers/nfc/pn533/Makefile
+++ b/drivers/nfc/pn533/Makefile
@@ -2,6 +2,8 @@
 # Makefile for PN533 NFC driver
 #
 pn533_usb-objs  = usb.o
+pn533_i2c-objs  = i2c.o
 
 obj-$(CONFIG_NFC_PN533)     += pn533.o
 obj-$(CONFIG_NFC_PN533_USB) += pn533_usb.o
+obj-$(CONFIG_NFC_PN533_I2C) += pn533_i2c.o
diff --git a/drivers/nfc/pn533/i2c.c b/drivers/nfc/pn533/i2c.c
new file mode 100644
index 0000000..35066b30
--- /dev/null
+++ b/drivers/nfc/pn533/i2c.c
@@ -0,0 +1,277 @@
+/*
+ * Driver for NXP PN533 NFC Chip - I2C transport layer
+ *
+ * Copyright (C) 2011 Instituto Nokia de Tecnologia
+ * Copyright (C) 2012-2013 Tieto Poland
+ * Copyright (C) 2016 HALE electronic
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/nfc.h>
+#include <linux/netdevice.h>
+#include <linux/interrupt.h>
+#include <net/nfc/nfc.h>
+#include "pn533.h"
+
+#define VERSION "0.1"
+
+#define PN533_I2C_DRIVER_NAME "pn533_i2c"
+
+struct pn533_i2c_phy {
+	struct i2c_client *i2c_dev;
+	struct pn533 *priv;
+
+	int hard_fault;		/*
+				 * < 0 if hardware error occured (e.g. i2c err)
+				 * and prevents normal operation.
+				 */
+};
+
+static int pn533_i2c_send_ack(struct pn533 *dev, gfp_t flags)
+{
+	struct pn533_i2c_phy *phy = dev->phy;
+	struct i2c_client *client = phy->i2c_dev;
+	u8 ack[6] = {0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
+	/* spec 6.2.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
+	int rc;
+
+	rc = i2c_master_send(client, ack, 6);
+
+	return rc;
+}
+
+static int pn533_i2c_send_frame(struct pn533 *dev,
+				struct sk_buff *out)
+{
+	struct pn533_i2c_phy *phy = dev->phy;
+	struct i2c_client *client = phy->i2c_dev;
+	int rc;
+
+	if (phy->hard_fault != 0)
+		return phy->hard_fault;
+
+	if (phy->priv == NULL)
+		phy->priv = dev;
+
+	print_hex_dump_debug("PN533_i2c TX: ", DUMP_PREFIX_NONE, 16, 1,
+			     out->data, out->len, false);
+
+	rc = i2c_master_send(client, out->data, out->len);
+
+	if (rc == -EREMOTEIO) { /* Retry, chip was in power down */
+		usleep_range(6000, 10000);
+		rc = i2c_master_send(client, out->data, out->len);
+	}
+
+	if (rc >= 0) {
+		if (rc != out->len)
+			rc = -EREMOTEIO;
+		else
+			rc = 0;
+	}
+
+	return rc;
+}
+
+static void pn533_i2c_abort_cmd(struct pn533 *dev, gfp_t flags)
+{
+	/* An ack will cancel the last issued command */
+	pn533_i2c_send_ack(dev, flags);
+
+	/* schedule cmd_complete_work to finish current command execution */
+	if (dev->cmd != NULL)
+		dev->cmd->status = -ENOENT;
+	queue_work(dev->wq, &dev->cmd_complete_work);
+}
+
+static int pn533_i2c_read(struct pn533_i2c_phy *phy, struct sk_buff **skb)
+{
+	struct i2c_client *client = phy->i2c_dev;
+	int len = PN533_EXT_FRAME_HEADER_LEN +
+		  PN533_STD_FRAME_MAX_PAYLOAD_LEN +
+		  PN533_STD_FRAME_TAIL_LEN + 1;
+	int r;
+
+	*skb = alloc_skb(len, GFP_KERNEL);
+	if (*skb == NULL)
+		return -ENOMEM;
+
+	r = i2c_master_recv(client, skb_put(*skb, len), len);
+	if (r != len) {
+		nfc_err(&client->dev, "cannot read. r=%d len=%d\n", r, len);
+		kfree_skb(*skb);
+		return -EREMOTEIO;
+	}
+
+	if (!((*skb)->data[0] & 0x01)) {
+		nfc_err(&client->dev, "READY flag not set");
+		kfree_skb(*skb);
+		return -EBUSY;
+	}
+
+	/* remove READY byte */
+	skb_pull(*skb, 1);
+	/* trim to frame size */
+	skb_trim(*skb, phy->priv->ops->rx_frame_size((*skb)->data));
+
+	return 0;
+}
+
+static irqreturn_t pn533_i2c_irq_thread_fn(int irq, void *data)
+{
+	struct pn533_i2c_phy *phy = data;
+	struct i2c_client *client;
+	struct sk_buff *skb = NULL;
+	int r;
+
+	if (!phy || irq != phy->i2c_dev->irq) {
+		WARN_ON_ONCE(1);
+		return IRQ_NONE;
+	}
+
+	client = phy->i2c_dev;
+	dev_dbg(&client->dev, "IRQ\n");
+
+	if (phy->hard_fault != 0)
+		return IRQ_HANDLED;
+
+	r = pn533_i2c_read(phy, &skb);
+	if (r == -EREMOTEIO) {
+		phy->hard_fault = r;
+
+		pn533_recv_frame(phy->priv, NULL, -EREMOTEIO);
+
+		return IRQ_HANDLED;
+	} else if ((r == -ENOMEM) || (r == -EBADMSG) || (r == -EBUSY)) {
+		return IRQ_HANDLED;
+	}
+
+	pn533_recv_frame(phy->priv, skb, 0);
+
+	return IRQ_HANDLED;
+
+#if 0
+#endif
+}
+
+static struct pn533_phy_ops i2c_phy_ops = {
+	.send_frame = pn533_i2c_send_frame,
+	.send_ack = pn533_i2c_send_ack,
+	.abort_cmd = pn533_i2c_abort_cmd,
+};
+
+
+static int pn533_i2c_probe(struct i2c_client *client,
+			       const struct i2c_device_id *id)
+{
+	struct pn533_i2c_phy *phy;
+	struct pn533 *priv;
+	int r = 0;
+
+	dev_dbg(&client->dev, "%s\n", __func__);
+	dev_dbg(&client->dev, "IRQ: %d\n", client->irq);
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		nfc_err(&client->dev, "Need I2C_FUNC_I2C\n");
+		return -ENODEV;
+	}
+
+	phy = devm_kzalloc(&client->dev, sizeof(struct pn533_i2c_phy),
+			   GFP_KERNEL);
+	if (!phy) {
+		nfc_err(&client->dev,
+			"Cannot allocate memory for pn533 i2c phy.\n");
+		return -ENOMEM;
+	}
+
+	phy->i2c_dev = client;
+	i2c_set_clientdata(client, phy);
+
+	r = request_threaded_irq(client->irq, NULL, pn533_i2c_irq_thread_fn,
+				 IRQF_TRIGGER_FALLING | IRQF_SHARED | IRQF_ONESHOT,
+				 PN533_I2C_DRIVER_NAME, phy);
+
+	if (r < 0) {
+		nfc_err(&client->dev, "Unable to register IRQ handler\n");
+	}
+
+	priv = pn533_register_device(PN533_DEVICE_PN532,
+				     PN533_NO_TYPE_B_PROTOCOLS,
+				     PN533_PROTO_REQ_ACK_RESP,
+				     phy, &i2c_phy_ops, NULL,
+				     &phy->i2c_dev->dev);
+
+	if (IS_ERR(priv)) {
+		r = PTR_ERR(priv);
+		goto err_register;
+	}
+
+	phy->priv = priv;
+
+	return 0;
+
+err_register:
+	free_irq(client->irq, phy);
+
+	return r;
+}
+
+static int pn533_i2c_remove(struct i2c_client *client)
+{
+	struct pn533_i2c_phy *phy = i2c_get_clientdata(client);
+
+	dev_dbg(&client->dev, "%s\n", __func__);
+
+	pn533_unregister_device(phy->priv);
+
+	return 0;
+}
+
+static const struct of_device_id of_pn533_i2c_match[] = {
+	{ .compatible = "nxp,pn533-i2c", },
+	{ .compatible = "nxp,pn532-i2c", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_pn533_i2c_match);
+
+static struct i2c_device_id pn533_i2c_id_table[] = {
+	{ PN533_I2C_DRIVER_NAME, 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, pn533_i2c_id_table);
+
+static struct i2c_driver pn533_i2c_driver = {
+	.driver = {
+		   .name = PN533_I2C_DRIVER_NAME,
+		   .owner = THIS_MODULE,
+		   .of_match_table = of_match_ptr(of_pn533_i2c_match),
+		  },
+	.probe = pn533_i2c_probe,
+	.id_table = pn533_i2c_id_table,
+	.remove = pn533_i2c_remove,
+};
+
+module_i2c_driver(pn533_i2c_driver);
+
+MODULE_AUTHOR("Michael Thalmeier <michael.thalmeier@hale.at>");
+MODULE_DESCRIPTION("PN533 I2C driver ver " VERSION);
+MODULE_VERSION(VERSION);
+MODULE_LICENSE("GPL");
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index 4468284..3d286a7 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -2435,8 +2435,35 @@ static int pn533_rf_field(struct nfc_dev *nfc_dev, u8 rf)
 	return rc;
 }
 
+static int pn532_sam_configuration(struct nfc_dev *nfc_dev)
+{
+	struct pn533 *dev = nfc_get_drvdata(nfc_dev);
+	struct sk_buff *skb;
+	struct sk_buff *resp;
+
+	skb = pn533_alloc_skb(dev, 1);
+	if (!skb)
+		return -ENOMEM;
+
+	*skb_put(skb, 1) = 0x01;
+
+	resp = pn533_send_cmd_sync(dev, PN533_CMD_SAM_CONFIGURATION, skb);
+	if (IS_ERR(resp))
+		return PTR_ERR(resp);
+
+	dev_kfree_skb(resp);
+	return 0;
+}
+
 static int pn533_dev_up(struct nfc_dev *nfc_dev)
 {
+	struct pn533 *dev = nfc_get_drvdata(nfc_dev);
+	if (dev->device_type == PN533_DEVICE_PN532) {
+		int rc = pn532_sam_configuration(nfc_dev);
+		if (rc)
+			return rc;
+	}
+
 	return pn533_rf_field(nfc_dev, 1);
 }
 
@@ -2469,6 +2496,7 @@ static int pn533_setup(struct pn533 *dev)
 	case PN533_DEVICE_STD:
 	case PN533_DEVICE_PASORI:
 	case PN533_DEVICE_ACR122U:
+	case PN533_DEVICE_PN532:
 		max_retries.mx_rty_atr = 0x2;
 		max_retries.mx_rty_psl = 0x1;
 		max_retries.mx_rty_passive_act =
@@ -2504,6 +2532,7 @@ static int pn533_setup(struct pn533 *dev)
 
 	switch (dev->device_type) {
 	case PN533_DEVICE_STD:
+	case PN533_DEVICE_PN532:
 		break;
 
 	case PN533_DEVICE_PASORI:
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 0958c653..61c5d6a 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -21,6 +21,7 @@
 #define PN533_DEVICE_STD     0x1
 #define PN533_DEVICE_PASORI  0x2
 #define PN533_DEVICE_ACR122U 0x3
+#define PN533_DEVICE_PN532   0x4
 
 #define PN533_ALL_PROTOCOLS (NFC_PROTO_JEWEL_MASK | NFC_PROTO_MIFARE_MASK |\
 			     NFC_PROTO_FELICA_MASK | NFC_PROTO_ISO14443_MASK |\
@@ -72,6 +73,7 @@
 #define PN533_FRAME_CMD(f) (f->data[1])
 
 #define PN533_CMD_GET_FIRMWARE_VERSION 0x02
+#define PN533_CMD_SAM_CONFIGURATION 0x14
 #define PN533_CMD_RF_CONFIGURATION 0x32
 #define PN533_CMD_IN_DATA_EXCHANGE 0x40
 #define PN533_CMD_IN_COMM_THRU     0x42
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1364719 — [RFC 2/4] NFC: pn533: fix deadlock when socket is closed while processing command

A deadlock can occur when the NFC raw socket is closed while the driver is
processing a command.

Following is the call graph of the affected situation:

send data via raw_sock:
-------------
rawsock_tx_work
  sock_hold                                             => socket refcnt++
  nfc_data_exchange                                     => cb = rawsock_data_exchange_complete
    ops->im_transceive = pn533_transceive               => arg->cb = db = rawsock_data_exchange_complete
      pn533_send_data_async                             => cb = pn533_data_exchange_complete
        __pn533_send_async                              => cmd->complete_cb = cb = pn533_data_exchange_complete
          if_ops->send_frame_async

response:
--------
pn533_recv_response
  queue_work(priv->wq, &priv->cmd_complete_work)

pn533_wq_cmd_complete
  pn533_send_async_complete
    cmd->complete_cb() = pn533_data_exchange_complete()
      arg->cb() = rawsock_data_exchange_complete()
        sock_put                                                         => socket refcnt-- => If the corresponding socket gets closed in the meantime socket will be destructed
          sk_free
            __sk_free
              sk->sk_destruct = rawsock_destruct
                nfc_deactivate_target
                  ops->deactivate_target = pn533_deactivate_target
                    pn533_send_cmd_sync
                      pn533_send_cmd_async
                        __pn533_send_async
                          list_add_tail(&cmd->queue, &dev->cmd_queue)    => add to command list because a command is currently processed
                        wait_for_completion                              => the workqueue thread waits here because it is the one processing the commands => deadlock

To fix the deadlock pn533_deactivate_target is changed to issue the
PN533_CMD_IN_RELEASE command in async mode. This way nothing blocks and the
release command is executed after the current command.

Signed-off-by: Michael Thalmeier <michael.thalmeier@hale.at>
---
 drivers/nfc/pn533.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/nfc/pn533.c b/drivers/nfc/pn533.c
index a85830f..074f1e4 100644
--- a/drivers/nfc/pn533.c
+++ b/drivers/nfc/pn533.c
@@ -2263,12 +2263,35 @@ static int pn533_activate_target(struct nfc_dev *nfc_dev,
 	return 0;
 }
 
+static int pn533_deactivate_target_complete(struct pn533 *dev, void *arg,
+			     struct sk_buff *resp)
+{
+	int rc = 0;
+
+	dev_dbg(&dev->interface->dev, "%s\n", __func__);
+
+	if (IS_ERR(resp)) {
+		rc = PTR_ERR(resp);
+
+		nfc_err(&dev->interface->dev, "Target release error %d\n", rc);
+
+		return rc;
+	}
+
+	rc = resp->data[0] & PN533_CMD_RET_MASK;
+	if (rc != PN533_CMD_RET_SUCCESS)
+		nfc_err(&dev->interface->dev,
+			"Error 0x%x when releasing the target\n", rc);
+
+	dev_kfree_skb(resp);
+	return rc;
+}
+
 static void pn533_deactivate_target(struct nfc_dev *nfc_dev,
 				    struct nfc_target *target, u8 mode)
 {
 	struct pn533 *dev = nfc_get_drvdata(nfc_dev);
 	struct sk_buff *skb;
-	struct sk_buff *resp;
 	int rc;
 
 	dev_dbg(&dev->interface->dev, "%s\n", __func__);
@@ -2287,16 +2310,13 @@ static void pn533_deactivate_target(struct nfc_dev *nfc_dev,
 
 	*skb_put(skb, 1) = 1; /* TG*/
 
-	resp = pn533_send_cmd_sync(dev, PN533_CMD_IN_RELEASE, skb);
-	if (IS_ERR(resp))
-		return;
-
-	rc = resp->data[0] & PN533_CMD_RET_MASK;
-	if (rc != PN533_CMD_RET_SUCCESS)
-		nfc_err(&dev->interface->dev,
-			"Error 0x%x when releasing the target\n", rc);
+	rc = pn533_send_cmd_async(dev, PN533_CMD_IN_RELEASE, skb,
+				  pn533_deactivate_target_complete, NULL);
+	if (rc < 0) {
+		dev_kfree_skb(skb);
+		nfc_err(&dev->interface->dev, "Target release error %d\n", rc);
+	}
 
-	dev_kfree_skb(resp);
 	return;
 }
 
-- 
2.5.5

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web