fs/sysfs: MIPS: Don't translate all IOCTLs (#402)

Some IOCTLs are fixed, e.g. i2c-dev. There, we must not translate the
IOCTLs. Instead, the IOCTLs are generated arch-dependent in the fs
package.

- fs: add IO() IOR() IOW() IORW() functions for the equivalent linux macros.
- videocore: Replace ioctl magic with derived value.
  Beside being clearer, the comment in the code was incorrect. The message
  box magic is in fact 'd' = 100 = 0x64, not 0x100.
- videocore: Use dynamic pointer size for ioctl.

cmd/periph-smoketest/spismoketest/spismoketest.go

@@ -23,6 +23,14 @@ import (
 	"periph.io/x/periph/conn/physic"
 	"periph.io/x/periph/conn/spi"
 	"periph.io/x/periph/conn/spi/spireg"
+	"periph.io/x/periph/host/fs"
+)
+
+var (
+	iorMode        = fs.IOR('k', 1, 1)
+	iorLSBFirst    = fs.IOR('k', 2, 1)
+	iorBitsPerWord = fs.IOR('k', 3, 1)
+	iorMaxSpeedHz  = fs.IOR('k', 4, 4)
 )
 
 // SmokeTest is imported by periph-smoketest.
@@ -69,6 +77,15 @@ func (s *SmokeTest) Run(f *flag.FlagSet, args []string) error {
 		return fmt.Errorf("error setting SPI parameters: %v", err)
 	}
 
+	type flagGetter interface{ GetFlag(uint) (uint64, error) }
+	mode, err := c.(flagGetter).GetFlag(iorMode)
+	if err != nil {
+		return fmt.Errorf("failed to read back mode: %v", err)
+	}
+	if spi.Mode(mode) != spi.Mode0 {
+		return fmt.Errorf("read back mode doesn't match spi.Mode0: %v", spi.Mode(mode))
+	}
+
 	// Open the WC pin.
 	var wpPin gpio.PinIO
 	if *wp != "" {

host/fs/fs.go

@@ -62,12 +62,6 @@ type File struct {
 
 // Ioctl sends an ioctl to the file handle.
 func (f *File) Ioctl(op uint, data uintptr) error {
-	if isMIPS {
-		var err error
-		if op, err = translateOpMIPS(op); err != nil {
-			return err
-		}
-	}
 	return ioctl(f.Fd(), op, data)
 }
 
@@ -104,27 +98,3 @@ var (
 	inhibited bool
 	used      bool
 )
-
-func translateOpMIPS(op uint) (uint, error) {
-	// Decode the arm/x64 encoding and reencode as MIPS specific linux ioctl.
-	// arm/x64: DIR(2), SIZE(14), TYPE(8), NR(8)
-	// mips:    DIR(3), SIZE(13), TYPE(8), NR(8)
-	// Check for size overflow.
-	if (op & (1 << (13 + 8 + 8))) != 0 {
-		return 0, errors.New("fs: op code size is too large")
-	}
-	const mask = (1 << (13 + 8 + 8)) - 1
-	out := op & mask
-	// Convert dir.
-	switch op >> (14 + 8 + 8) {
-	case 0: // none
-		out |= 1 << (13 + 8 + 8)
-	case 1: // write
-		out |= 4 << (13 + 8 + 8)
-	case 2: // read
-		out |= 2 << (13 + 8 + 8)
-	default:
-		return 0, errors.New("fs: op code dir is invalid")
-	}
-	return out, nil
-}

host/fs/ioctl.go

@@ -0,0 +1,51 @@
+// Copyright 2019 The Periph Authors. All rights reserved.
+// Use of this source code is governed under the Apache License, Version 2.0
+// that can be found in the LICENSE file.
+
+package fs
+
+// These constants, variables and functions are ported from the Linux userland
+// API header ioctl.h (commonly packaged at /usr/include/linux/ioctl.h which
+// includes /usr/include/asm-generic/ioctl.h).
+
+const (
+	iocNrbits   uint = 8
+	iocTypebits uint = 8
+
+	iocNrshift uint = 0
+
+	iocTypeshift = iocNrshift + iocNrbits
+	iocSizeshift = iocTypeshift + iocTypebits
+	iocDirshift  = iocSizeshift + iocSizebits
+)
+
+func ioc(dir, typ, nr, size uint) uint {
+	return (dir << iocDirshift) |
+		(typ << iocTypeshift) |
+		(nr << iocNrshift) |
+		(size << iocSizeshift)
+}
+
+// IO defines an ioctl with no parameters. It corresponds to _IO in the Linux
+// userland API.
+func IO(typ, nr uint) uint {
+	return ioc(iocNone, typ, nr, 0)
+}
+
+// IOR defines an ioctl with read (userland perspective) parameters. It
+// corresponds to _IOR in the Linux userland API.
+func IOR(typ, nr, size uint) uint {
+	return ioc(iocRead, typ, nr, size)
+}
+
+// IOW defines an ioctl with write (userland perspective) parameters. It
+// corresponds to _IOW in the Linux userland API.
+func IOW(typ, nr, size uint) uint {
+	return ioc(iocWrite, typ, nr, size)
+}
+
+// IOWR defines an ioctl with both read and write parameters. It corresponds to
+// _IOWR in the Linux userland API.
+func IOWR(typ, nr, size uint) uint {
+	return ioc(iocRead|iocWrite, typ, nr, size)
+}

host/fs/ioctl_mips_like.go

@@ -6,4 +6,11 @@
 
 package fs
 
-const isMIPS = true
+const (
+	iocNone  uint = 1
+	iocRead  uint = 2
+	iocWrite uint = 4
+
+	iocSizebits uint = 13
+	iocDirbits  uint = 3
+)

host/fs/ioctl_other.go

@@ -6,4 +6,11 @@
 
 package fs
 
-const isMIPS = false
+const (
+	iocNone  uint = 0
+	iocWrite uint = 1
+	iocRead  uint = 2
+
+	iocSizebits uint = 14
+	iocDirbits  uint = 2
+)

host/sysfs/spi.go

@@ -412,21 +412,14 @@ func (s *spiConn) txPackets(p []spi.Packet) error {
 }
 
 func (s *spiConn) setFlag(op uint, arg uint64) error {
-	if err := s.f.Ioctl(op|0x40000000, uintptr(unsafe.Pointer(&arg))); err != nil {
-		return err
-	}
-	if false {
-		// Verification.
-		actual := uint64(0)
-		// getFlag() equivalent.
-		if err := s.f.Ioctl(op|0x80000000, uintptr(unsafe.Pointer(&actual))); err != nil {
-			return err
-		}
-		if actual != arg {
-			return fmt.Errorf("sysfs-spi: op 0x%x: set 0x%x, read 0x%x", op, arg, actual)
-		}
-	}
-	return nil
+	return s.f.Ioctl(op, uintptr(unsafe.Pointer(&arg)))
+}
+
+// GetFlag allows to read back flags set via a ioctl, i.e. setFlag. It is
+// exported to allow calling it from the smoke test.
+func (s *spiConn) GetFlag(op uint) (arg uint64, err error) {
+	err = s.f.Ioctl(op, uintptr(unsafe.Pointer(&arg)))
+	return
 }
 
 func (s *spiConn) initPins() {
@@ -469,42 +462,20 @@ const (
 //
 // Constants and structure definition can be found at
 // /usr/include/linux/spi/spidev.h.
-const (
-	spiIOCMode        = 0x16B01 // SPI_IOC_WR_MODE (8 bits)
-	spiIOLSBFirst     = 0x16B02 // SPI_IOC_WR_LSB_FIRST
-	spiIOCBitsPerWord = 0x16B03 // SPI_IOC_WR_BITS_PER_WORD
-	spiIOCMaxSpeedHz  = 0x46B04 // SPI_IOC_WR_MAX_SPEED_HZ
-	spiIOCMode32      = 0x46B05 // SPI_IOC_WR_MODE32 (32 bits)
+const spiIOCMagic uint = 'k'
+
+var (
+	spiIOCMode        = fs.IOW(spiIOCMagic, 1, 1) // SPI_IOC_WR_MODE (8 bits)
+	spiIOLSBFirst     = fs.IOW(spiIOCMagic, 2, 1) // SPI_IOC_WR_LSB_FIRST
+	spiIOCBitsPerWord = fs.IOW(spiIOCMagic, 3, 1) // SPI_IOC_WR_BITS_PER_WORD
+	spiIOCMaxSpeedHz  = fs.IOW(spiIOCMagic, 4, 4) // SPI_IOC_WR_MAX_SPEED_HZ
+	spiIOCMode32      = fs.IOW(spiIOCMagic, 5, 4) // SPI_IOC_WR_MODE32 (32 bits)
 )
 
 // spiIOCTx(l) calculates the equivalent of SPI_IOC_MESSAGE(l) to execute a
 // transaction.
-//
-// The IOCTL for TX was deduced from this C code:
-//
-//   #include "linux/spi/spidev.h"
-//   #include "sys/ioctl.h"
-//   #include <stdio.h>
-//   int main() {
-//     for (int i = 1; i < 10; i++) {
-//       printf("len(%d) = 0x%08X\n", i, SPI_IOC_MESSAGE(i));
-//     }
-//     return 0;
-//   }
-//
-//   $ gcc a.cc && ./a.out
-//   len(1) = 0x40206B00
-//   len(2) = 0x40406B00
-//   len(3) = 0x40606B00
-//   len(4) = 0x40806B00
-//   len(5) = 0x40A06B00
-//   len(6) = 0x40C06B00
-//   len(7) = 0x40E06B00
-//   len(8) = 0x41006B00
-//   len(9) = 0x41206B00
 func spiIOCTx(l int) uint {
-	op := uint(0x40006B00)
-	return op | uint(0x200000)*uint(l)
+	return fs.IOW(spiIOCMagic, 0, uint(l)*32)
 }
 
 // spiIOCTransfer is spi_ioc_transfer in linux/spi/spidev.h.

host/sysfs/spi_test.go

@@ -269,15 +269,6 @@ func TestSPI_Connect_Half(t *testing.T) {
 	}
 }
 
-func TestSPIIOCTX(t *testing.T) {
-	if v := spiIOCTx(1); v != 0x40206B00 {
-		t.Fatalf("Expected 0x40206B00, got 0x%08X", v)
-	}
-	if v := spiIOCTx(9); v != 0x41206B00 {
-		t.Fatalf("Expected 0x41206B00, got 0x%08X", v)
-	}
-}
-
 func TestSPIDriver(t *testing.T) {
 	if len((&driverSPI{}).Prerequisites()) != 0 {
 		t.Fatal("unexpected SPI prerequisites")

host/videocore/videocore.go

@@ -94,10 +94,11 @@ var (
 	mu         sync.Mutex
 	mailbox    messager
 	mailboxErr error
+
+	mbIoctl = fs.IOWR('d', 0, uint(unsafe.Sizeof(new(byte))))
 )
 
 const (
-	mbIoctl = 0xc0046400 //_IOWR(0x100, 0, char *)
 	// All of these return anything but zero (‽)
 	mbFirmwareVersion = 0x1     // 0, 4
 	mbBoardModel      = 0x10001 // 0, 4