From 68a0ca32d47a2e1539c3aef9ac89b0daf3953a35 Mon Sep 17 00:00:00 2001
From: Marc-Antoine Ruel <maruel@chromium.org>
Date: Sun, 4 Nov 2018 15:44:39 -0500
Subject: [PATCH] sysfs: Fix TxPackets() to behave as expected.

spi: improve documentation accordingly.

A 'fix' was implemented in a851c85cbf4c but it was not good for TxPackets(). Fix
the implementation and fix the documentation.

Fixes #222
(again)
---
 conn/spi/spi.go   | 21 +++++++++++----------
 host/sysfs/spi.go | 23 +++++++++++++++++------
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/conn/spi/spi.go b/conn/spi/spi.go
index 822251af3..b1a2159db 100644
--- a/conn/spi/spi.go
+++ b/conn/spi/spi.go
@@ -101,14 +101,9 @@ type Packet struct {
 	// completed. This can be leveraged to create long transaction as multiple
 	// packets like to use 9 bits commands then 8 bits data.
 	//
-	// Casual observation on a Rasberry Pi 3 is that two packets with
-	// KeepCS:false, there is a few µs with CS asserted after the clock stops,
-	// then 11.2µs with CS not asserted, then CS is asserted for (roughly) one
-	// clock cycle before the clock starts again for the next packet. This seems
-	// to be independent of the port clock speed but this wasn't fully verified.
-	//
-	// It cannot be expected that the driver will correctly keep CS asserted even
-	// if KeepCS:true on the last packet.
+	// Normally during a spi.Conn.TxPackets() call, KeepCS should be set to true
+	// for all packets except the last one. If the last one is set to true, the
+	// CS line stays asserted, leaving the transaction hanging on the bus.
 	//
 	// KeepCS is ignored when NoCS was specified to Connect.
 	KeepCS bool
@@ -125,8 +120,14 @@ type Conn interface {
 	// The maximum number of bytes can be limited depending on the driver. Query
 	// conn.Limits.MaxTxSize() can be used to determine the limit.
 	//
-	// If the last packet has KeepCS:true, the behavior is undefined. The CS line
-	// will likely not stay asserted. This is a driver limitation.
+	// If the last packet has KeepCS:true, the CS line stays asserted. This
+	// enables doing SPI transaction over multiple calls.
+	//
+	// Conversely, if any packet beside the last one has KeepCS:false, the CS
+	// line will blip for a short amount of time to force a new transaction.
+	//
+	// It was observed on RPi3 hardware to have a one clock delay between each
+	// packet.
 	TxPackets(p []Packet) error
 }
 
diff --git a/host/sysfs/spi.go b/host/sysfs/spi.go
index 676f8eba0..096220d1b 100644
--- a/host/sysfs/spi.go
+++ b/host/sysfs/spi.go
@@ -183,7 +183,6 @@ func newSPI(busNumber, chipSelect int) (*SPI, error) {
 			f:          f,
 			busNumber:  busNumber,
 			chipSelect: chipSelect,
-			p:          [2]spi.Packet{{KeepCS: true}, {KeepCS: true}},
 		},
 	}, nil
 }
@@ -289,9 +288,13 @@ func (s *spiConn) Tx(w, r []byte) error {
 	if s.halfDuplex && len(w) != 0 && len(r) != 0 {
 		// Create two packets for HalfDuplex operation: one write then one read.
 		s.p[0].R = nil
+		s.p[0].KeepCS = true
 		s.p[1].W = nil
 		s.p[1].R = r
+		s.p[1].KeepCS = false
 		p = s.p[:2]
+	} else {
+		s.p[0].KeepCS = false
 	}
 	if err := s.txPackets(p); err != nil {
 		return fmt.Errorf("sysfs-spi: Tx() failed: %v", err)
@@ -396,10 +399,14 @@ func (s *spiConn) txPackets(p []spi.Packet) error {
 		if bits == 0 {
 			bits = s.bitsPerWord
 		}
-		m[i].reset(p[i].W, p[i].R, f, bits)
-		if !s.noCS && !p[i].KeepCS {
-			m[i].csChange = 1
+		csInvert := false
+		if !s.noCS {
+			// Invert CS behavior when a packet has KeepCS false, except for the last
+			// packet when KeepCS is true.
+			last := i == len(p)-1
+			csInvert = p[i].KeepCS == last
 		}
+		m[i].reset(p[i].W, p[i].R, f, bits, csInvert)
 	}
 	return s.f.Ioctl(spiIOCTx(len(m)), uintptr(unsafe.Pointer(&m[0])))
 }
@@ -517,7 +524,7 @@ type spiIOCTransfer struct {
 	pad         uint16
 }
 
-func (s *spiIOCTransfer) reset(w, r []byte, f physic.Frequency, bitsPerWord uint8) {
+func (s *spiIOCTransfer) reset(w, r []byte, f physic.Frequency, bitsPerWord uint8, csInvert bool) {
 	s.tx = 0
 	s.rx = 0
 	s.length = 0
@@ -533,7 +540,11 @@ func (s *spiIOCTransfer) reset(w, r []byte, f physic.Frequency, bitsPerWord uint
 	s.speedHz = uint32((f + 500*physic.MilliHertz) / physic.Hertz)
 	s.delayUsecs = 0
 	s.bitsPerWord = bitsPerWord
-	s.csChange = 0
+	if csInvert {
+		s.csChange = 1
+	} else {
+		s.csChange = 0
+	}
 	s.txNBits = 0
 	s.rxNBits = 0
 	s.pad = 0