sysfs: Fix TxPackets() to behave as expected.

spi: improve documentation accordingly.

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

The next step is to fix sysfs SPI implementation to stop refusing large
transactions, and instead transparently split them up. Filed as issue #310.

Fixes #222
(again)

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
 }
 

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