Skip to contentSkip to conversation

NFC: The core part should generate the target index

RFC PATCH In review Samuel Ortiz 2 participants 3 messages Mar 22, 2012

Subsystems (2)
Networking NFC
lore
Share & tools
lore

Samuel Ortiz

author
#0 lore raw

RFC PATCH 1/1 NFC: The core part should generate the target index

The target index can be used by userspace to uniquely identify a target and thus should be kept unique, per NFC adapter. Moreover, some protocols do not provide a logical index when discovering new targets, so we have to generate one for them.
For NCI or pn533 to fetch their logical index, we added a logical_idx field to the target structure.

Signed-off-by Samuel Ortiz <[email protected]>

---

Ilan, could you please comment on this patch and also make sure that it doesn't break multiple targets support for NCI ?

---

 include/net/nfc/nfc.h |    2 ++
 net/nfc/core.c        |    5 +++++
 net/nfc/nci/core.c    |    2 +-
 net/nfc/nci/ntf.c     |   11 ++++++-----
 net/nfc/rawsock.c     |   12 ++++++++++++
 5 files changed, 26 insertions(+), 6 deletions(-)
include/net/nfc/nfc.h +2 -0
index 57ea095..9097093 100644
@@ -78,6 +78,7 @@ struct nfc_target {
 	u8 sensb_res[NFC_SENSB_RES_MAXSIZE];
 	u8 sensf_res_len;
 	u8 sensf_res[NFC_SENSF_RES_MAXSIZE];
+	u8 logical_idx;
 };
 
 struct nfc_genl_data {

@@ -87,6 +88,7 @@ struct nfc_genl_data {
 
 struct nfc_dev {
 	unsigned idx;
+	unsigned target_idx;
 	struct nfc_target *targets;
 	int n_targets;
 	int targets_generation;
net/nfc/core.c +5 -0
index 376f9ac..2818582 100644
@@ -428,10 +428,15 @@ EXPORT_SYMBOL(nfc_alloc_recv_skb);
 int nfc_targets_found(struct nfc_dev *dev,
 		      struct nfc_target *targets, int n_targets)
 {
+	int i;
+
 	pr_debug("dev_name=%s n_targets=%d\n", dev_name(&dev->dev), n_targets);
 
 	dev->polling = false;
 
+	for (i = 0; i < n_targets; i++)
+		targets[i].idx = dev->target_idx++;
+
 	spin_lock_bh(&dev->targets_lock);
 
 	dev->targets_generation++;
net/nfc/nci/core.c +1 -1
index 9ec065b..8737c20 100644
@@ -477,7 +477,7 @@ static int nci_activate_target(struct nfc_dev *nfc_dev, __u32 target_idx,
 	}
 
 	if (atomic_read(&ndev->state) == NCI_W4_HOST_SELECT) {
-		param.rf_discovery_id = target->idx;
+		param.rf_discovery_id = target->logical_idx;
 
 		if (protocol == NFC_PROTO_JEWEL)
 			param.rf_protocol = NCI_RF_PROTOCOL_T1T;
net/nfc/nci/ntf.c +6 -5
index 2e3dee4..99e1632 100644
@@ -227,7 +227,7 @@ static void nci_add_new_target(struct nci_dev *ndev,
 
 	for (i = 0; i < ndev->n_targets; i++) {
 		target = &ndev->targets[i];
-		if (target->idx == ntf->rf_discovery_id) {
+		if (target->logical_idx == ntf->rf_discovery_id) {
 			/* This target already exists, add the new protocol */
 			nci_add_new_protocol(ndev, target, ntf->rf_protocol,
 					     ntf->rf_tech_and_mode,

@@ -248,10 +248,10 @@ static void nci_add_new_target(struct nci_dev *ndev,
 				  ntf->rf_tech_and_mode,
 				  &ntf->rf_tech_specific_params);
 	if (!rc) {
-		target->idx = ntf->rf_discovery_id;
+		target->logical_idx = ntf->rf_discovery_id;
 		ndev->n_targets++;
 
-		pr_debug("target_idx %d, n_targets %d\n", target->idx,
+		pr_debug("logical idx %d, n_targets %d\n", target->logical_idx,
 			 ndev->n_targets);
 	}
 }

@@ -372,10 +372,11 @@ static void nci_target_auto_activated(struct nci_dev *ndev,
 	if (rc)
 		return;
 
-	target->idx = ntf->rf_discovery_id;
+	target->logical_idx = ntf->rf_discovery_id;
 	ndev->n_targets++;
 
-	pr_debug("target_idx %d, n_targets %d\n", target->idx, ndev->n_targets);
+	pr_debug("logical idx %d, n_targets %d\n",
+		 target->logical_idx, ndev->n_targets);
 
 	nfc_targets_found(ndev->nfc_dev, ndev->targets, ndev->n_targets);
 }
net/nfc/rawsock.c +12 -0
index 5a839ce..47fcf0d 100644
@@ -92,6 +92,18 @@ static int rawsock_connect(struct socket *sock, struct sockaddr *_addr,
 		goto error;
 	}
 
+	if (addr->target_idx > dev->target_idx - 1 ||
+	    addr->target_idx < dev->target_idx - dev->n_targets) {
+		rc = -EINVAL;
+		goto error;
+	}
+
+	if (addr->target_idx > dev->target_idx - 1 ||
+	    addr->target_idx < dev->target_idx - dev->n_targets) {
+		rc = -EINVAL;
+		goto error;
+	}
+
 	rc = nfc_activate_target(dev, addr->target_idx, addr->nfc_protocol);
 	if (rc)
 		goto put_dev;
Signature · 5 lines
1.7.9.1

-- 
Intel Open Source Technology Centre
http://oss.intel.com/
#1 lore raw

Hi Samuel,

Quoted 3 lines

Ilan, could you please comment on this patch and also make sure that it
doesn't break multiple targets support for NCI ?

It seems to me that this patch doesn't break multiple targets support for NCI.

Quoted 11 lines
net/nfc/rawsock.c +2 -0
index 5a839ce..47fcf0d 100644
@@ -92,6 +92,18 @@ static int rawsock_connect(struct socket 
 *sock, struct sockaddr *_addr,
 		goto error;
 	}
 
+	if (addr->target_idx > dev->target_idx - 1 ||
+	    addr->target_idx < dev->target_idx - dev->n_targets) {

net/nfc/rawsock.c static int rawsock_connect(struct socket · line 107

Quoted 10 lines
net/nfc/rawsock.c +10 -0
+		rc = -EINVAL;
+		goto error;
+	}
+
+	if (addr->target_idx > dev->target_idx - 1 ||
+	    addr->target_idx < dev->target_idx - dev->n_targets) {
+		rc = -EINVAL;
+		goto error;
+	}
+

It looks like the 'if' statement you added appears twice.

Thanks & BR,
Ilan

author
#2 lore raw

Hi Ilan,

Quoted 6 lines · Ilan

Hi Samuel,

Quoted 3 lines

Ilan, could you please comment on this patch and also make sure that it
doesn't break multiple targets support for NCI ?

It seems to me that this patch doesn't break multiple targets support for NCI.

Thanks for checking.

Quoted 9 lines
Quoted 9 lines
net/nfc/rawsock.c +0 -0
index 5a839ce..47fcf0d 100644
@@ -92,6 +92,18 @@ static int rawsock_connect(struct socket 
 *sock, struct sockaddr *_addr,
 		goto error;
 	}
Quoted 13 lines

net/nfc/rawsock.c static int rawsock_connect(struct socket *sock, struct sockaddr *_addr, · line 106

Quoted 12 lines
net/nfc/rawsock.c +12 -0
+	if (addr->target_idx > dev->target_idx - 1 ||
+	    addr->target_idx < dev->target_idx - dev->n_targets) {
+		rc = -EINVAL;
+		goto error;
+	}
+
+	if (addr->target_idx > dev->target_idx - 1 ||
+	    addr->target_idx < dev->target_idx - dev->n_targets) {
+		rc = -EINVAL;
+		goto error;
+	}
+

It looks like the 'if' statement you added appears twice.

Oops, thanks for that one too.

Cheers,
Samuel.

Signature · 2 lines
Intel Open Source Technology Centre
http://oss.intel.com/