NFC: NCI: Modify NCI SPI to implement CS/INT handshake per the spec

The NFC Forum NCI specification defines both a hardware and software
protocol when using a SPI physical transport to connect an NFC NCI
Chipset. The hardware requirement is that, after having raised the chip
select line, the SPI driver must wait for an INT line from the NFC
chipset to raise before it sends the data. The chip select must be
raised first though, because this is the signal that the NFC chipset
will detect to wake up and then raise its INT line. If the INT line
doesn't raise in a timely fashion, the SPI driver should abort
operation.

When data is transferred from Device host (DH) to NFC Controller (NFCC),
the signaling sequence is the following:

Data Transfer from DH to NFCC
• 1-Master asserts SPI_CSN
• 2-Slave asserts SPI_INT
• 3-Master sends NCI-over-SPI protocol header and payload data
• 4-Slave deasserts SPI_INT
• 5-Master deasserts SPI_CSN

When data must be transferred from NFCC to DH, things are a little bit
different.

Data Transfer from NFCC to DH
• 1-Slave asserts SPI_INT -> NFC chipset irq handler called -> process
reading from SPI
• 2-Master asserts SPI_CSN
• 3-Master send 2-octet NCI-over-SPI protocol header
• 4-Slave sends 2-octet NCI-over-SPI protocol payload length
• 5-Slave sends NCI-over-SPI protocol payload
• 6-Master deasserts SPI_CSN

In this case, SPI driver should function normally as it does today. Note
that the INT line can and will be lowered anytime between beginning of
step 3 and end of step 5. A low INT is therefore valid after chip select
has been raised.

This would be easily implemented in a single driver. Unfortunately, we
don't write the SPI driver and I had to imagine some workaround trick to
get the SPI and NFC drivers to work in a synchronized fashion. The trick
is the following:

- send an empty spi message: this will raise the chip select line, and
send nothing. We expect the /CS line will stay arisen because we asked
for it in the spi_transfer cs_change field
- wait for a completion, that will be completed by the NFC driver IRQ
handler when it knows we are in the process of sending data (NFC spec
says that we use SPI in a half duplex mode, so we are either sending or
receiving).
- when completed, proceed with the normal data send.

This has been tested and verified to work very consistently on a Nexus
10 (spi-s3c64xx driver). It may not work the same with other spi
drivers.

The previously defined nci_spi_ops{} whose intended purpose were to
address this problem are not used anymore and therefore totally removed.

The nci_spi_send() takes a new optional write_handshake_completion
completion pointer. If non NULL, the nci spi layer will run the above
trick when sending data to the NFC Chip. If NULL, the data is sent
normally all at once and it is then the NFC driver responsibility to
know what it's doing.

Signed-off-by: Eric Lapuyade <eric.lapuyade@intel.com>
Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>

2 files changed, 34 insertions, 32 deletions

diff --git a/include/net/nfc/nci_core.h b/include/net/nfc/nci_core.h
index 1d505317dc67..6126f1f992b4 100644
--- a/include/net/nfc/nci_core.h
+++ b/include/net/nfc/nci_core.h
@@ -207,17 +207,9 @@ int nci_to_errno(__u8 code);
 #define NCI_SPI_CRC_ENABLED	0x01
 
 /* ----- NCI SPI structures ----- */
-struct nci_spi;
-
-struct nci_spi_ops {
-	void (*assert_int)(struct nci_spi *nspi);
-	void (*deassert_int)(struct nci_spi *nspi);
-};
-
 struct nci_spi {
 	struct nci_dev		*ndev;
 	struct spi_device	*spi;
-	struct nci_spi_ops	*ops;
 
 	unsigned int		xfer_udelay;	/* microseconds delay between
 						  transactions */
@@ -229,10 +221,11 @@ struct nci_spi {
 
 /* ----- NCI SPI ----- */
 struct nci_spi *nci_spi_allocate_spi(struct spi_device *spi,
-				     struct nci_spi_ops *ops,
 				     u8 acknowledge_mode, unsigned int delay,
 				     struct nci_dev *ndev);
-int nci_spi_send(struct nci_spi *nspi, struct sk_buff *skb);
+int nci_spi_send(struct nci_spi *nspi,
+		 struct completion *write_handshake_completion,
+		 struct sk_buff *skb);
 struct sk_buff *nci_spi_read(struct nci_spi *nspi);
 
 #endif /* __NCI_CORE_H */
diff --git a/net/nfc/nci/spi.c b/net/nfc/nci/spi.c
index 734c6dde7751..f1d426f10cce 100644
--- a/net/nfc/nci/spi.c
+++ b/net/nfc/nci/spi.c
@@ -38,15 +38,23 @@
 
 #define CRC_INIT		0xFFFF
 
-static int __nci_spi_send(struct nci_spi *nspi, struct sk_buff *skb)
+static int __nci_spi_send(struct nci_spi *nspi, struct sk_buff *skb,
+			  int cs_change)
 {
 	struct spi_message m;
 	struct spi_transfer t;
 
 	memset(&t, 0, sizeof(struct spi_transfer));
-	t.tx_buf = skb->data;
-	t.len = skb->len;
-	t.cs_change = 0;
+	/* a NULL skb means we just want the SPI chip select line to raise */
+	if (skb) {
+		t.tx_buf = skb->data;
+		t.len = skb->len;
+	} else {
+		/* still set tx_buf non NULL to make the driver happy */
+		t.tx_buf = &t;
+		t.len = 0;
+	}
+	t.cs_change = cs_change;
 	t.delay_usecs = nspi->xfer_udelay;
 
 	spi_message_init(&m);
@@ -55,15 +63,15 @@ static int __nci_spi_send(struct nci_spi *nspi, struct sk_buff *skb)
 	return spi_sync(nspi->spi, &m);
 }
 
-int nci_spi_send(struct nci_spi *nspi, struct sk_buff *skb)
+int nci_spi_send(struct nci_spi *nspi,
+		 struct completion *write_handshake_completion,
+		 struct sk_buff *skb)
 {
 	unsigned int payload_len = skb->len;
 	unsigned char *hdr;
 	int ret;
 	long completion_rc;
 
-	nspi->ops->deassert_int(nspi);
-
 	/* add the NCI SPI header to the start of the buffer */
 	hdr = skb_push(skb, NCI_SPI_HDR_LEN);
 	hdr[0] = NCI_SPI_DIRECT_WRITE;
@@ -79,11 +87,21 @@ int nci_spi_send(struct nci_spi *nspi, struct sk_buff *skb)
 		*skb_put(skb, 1) = crc & 0xFF;
 	}
 
-	ret = __nci_spi_send(nspi, skb);
+	if (write_handshake_completion)	{
+		/* Trick SPI driver to raise chip select */
+		ret = __nci_spi_send(nspi, NULL, 1);
+		if (ret)
+			goto done;
 
-	kfree_skb(skb);
-	nspi->ops->assert_int(nspi);
+		/* wait for NFC chip hardware handshake to complete */
+		if (wait_for_completion_timeout(write_handshake_completion,
+						msecs_to_jiffies(1000)) == 0) {
+			ret = -ETIME;
+			goto done;
+		}
+	}
 
+	ret = __nci_spi_send(nspi, skb, 0);
 	if (ret != 0 || nspi->acknowledge_mode == NCI_SPI_CRC_DISABLED)
 		goto done;
 
@@ -96,6 +114,8 @@ int nci_spi_send(struct nci_spi *nspi, struct sk_buff *skb)
 		ret = -EIO;
 
 done:
+	kfree_skb(skb);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(nci_spi_send);
@@ -106,26 +126,20 @@ EXPORT_SYMBOL_GPL(nci_spi_send);
  * nci_spi_allocate_spi - allocate a new nci spi
  *
  * @spi: SPI device
- * @ops: device operations
  * @acknowledge_mode: Acknowledge mode used by the NFC device
  * @delay: delay between transactions in us
  * @ndev: nci dev to send incoming nci frames to
  */
 struct nci_spi *nci_spi_allocate_spi(struct spi_device *spi,
-				     struct nci_spi_ops *ops,
 				     u8 acknowledge_mode, unsigned int delay,
 				     struct nci_dev *ndev)
 {
 	struct nci_spi *nspi;
 
-	if (!ops->assert_int || !ops->deassert_int)
-		return NULL;
-
 	nspi = devm_kzalloc(&spi->dev, sizeof(struct nci_spi), GFP_KERNEL);
 	if (!nspi)
 		return NULL;
 
-	nspi->ops = ops;
 	nspi->acknowledge_mode = acknowledge_mode;
 	nspi->xfer_udelay = delay;
 
@@ -156,7 +170,7 @@ static int send_acknowledge(struct nci_spi *nspi, u8 acknowledge)
 	*skb_put(skb, 1) = crc >> 8;
 	*skb_put(skb, 1) = crc & 0xFF;
 
-	ret = __nci_spi_send(nspi, skb);
+	ret = __nci_spi_send(nspi, skb, 0);
 
 	kfree_skb(skb);
 
@@ -189,7 +203,6 @@ static struct sk_buff *__nci_spi_read(struct nci_spi *nspi)
 	spi_message_add_tail(&rx, &m);
 
 	ret = spi_sync(nspi->spi, &m);
-
 	if (ret)
 		return NULL;
 
@@ -213,7 +226,6 @@ static struct sk_buff *__nci_spi_read(struct nci_spi *nspi)
 	spi_message_add_tail(&rx, &m);
 
 	ret = spi_sync(nspi->spi, &m);
-
 	if (ret)
 		goto receive_error;
 
@@ -271,8 +283,6 @@ struct sk_buff *nci_spi_read(struct nci_spi *nspi)
 {
 	struct sk_buff *skb;
 
-	nspi->ops->deassert_int(nspi);
-
 	/* Retrieve frame from SPI */
 	skb = __nci_spi_read(nspi);
 	if (!skb)
@@ -305,7 +315,6 @@ struct sk_buff *nci_spi_read(struct nci_spi *nspi)
 		send_acknowledge(nspi, ACKNOWLEDGE_ACK);
 
 done:
-	nspi->ops->assert_int(nspi);
 
 	return skb;
 }