gpioioctl: Improve Registration (#64)

* Improve registration to handle vagaries of how the Linux kernel handles
GPIO Chips, particularly on the Pi 5. Improved handling of duplicate
pin names. Code now prefixes the pin name with the chip name if it's
duplicated.

* Fix lint issue with variable shadowing.

* Fix possible out of bounds error.

Author: gsexton

gpioioctl/basic_test.go

@@ -9,41 +9,16 @@
 package gpioioctl
 
 import (
-	"log"
 	"testing"
 
-	"periph.io/x/conn/v3/gpio"
 	"periph.io/x/conn/v3/gpio/gpioreg"
 )
 
 var testLine *GPIOLine
 
 func init() {
-	var err error
-
 	if len(Chips) == 0 {
 		makeDummyChip()
-		line := GPIOLine{
-			number:    0,
-			name:      "DummyGPIOLine",
-			consumer:  "",
-			edge:      gpio.NoEdge,
-			pull:      gpio.PullNoChange,
-			direction: LineDirNotSet,
-		}
-
-		chip := GPIOChip{name: "DummyGPIOChip",
-			path:      "/dev/gpiochipdummy",
-			label:     "Dummy GPIOChip for Testing Purposes",
-			lineCount: 1,
-			lines:     []*GPIOLine{&line},
-		}
-		Chips = append(Chips, &chip)
-		if err = gpioreg.Register(&line); err != nil {
-			nameStr := chip.Name()
-			lineStr := line.String()
-			log.Println("chip", nameStr, " gpioreg.Register(line) ", lineStr, " returned ", err)
-		}
 	}
 }
 

gpioioctl/dummy.go

@@ -36,7 +36,6 @@ func makeDummyChip() {
 		lines:     []*GPIOLine{&line},
 	}
 	Chips = append(Chips, &chip)
-	Chips = append(Chips, &chip)
 	if err := gpioreg.Register(&line); err != nil {
 		nameStr := chip.Name()
 		lineStr := line.String()

gpioioctl/gpio.go

@@ -14,6 +14,7 @@ import (
 	"path"
 	"path/filepath"
 	"runtime"
+	"sort"
 	"strings"
 	"sync"
 	"time"
@@ -402,6 +403,9 @@ func newGPIOChip(path string) (*GPIOChip, error) {
 
 	chip.name = strings.Trim(string(info.name[:]), "\x00")
 	chip.label = strings.Trim(string(info.label[:]), "\x00")
+	if len(chip.label) == 0 {
+		chip.label = chip.name
+	}
 	chip.lineCount = int(info.lines)
 	var line_info gpio_v2_line_info
 	for line := 0; line < int(info.lines); line++ {
@@ -591,32 +595,82 @@ func (d *driverGPIO) After() []string {
 //
 // https://docs.kernel.org/userspace-api/gpio/chardev.html
 func (d *driverGPIO) Init() (bool, error) {
-	if runtime.GOOS == "linux" {
-		items, err := filepath.Glob("/dev/gpiochip*")
-		if err != nil {
-			return true, err
-		}
-		if len(items) == 0 {
-			return false, errors.New("no GPIO chips found")
+	if runtime.GOOS != "linux" {
+		return true, nil
+	}
+	items, err := filepath.Glob("/dev/gpiochip*")
+	if err != nil {
+		return true, fmt.Errorf("gpioioctl: %w", err)
+	}
+	if len(items) == 0 {
+		return false, errors.New("no GPIO chips found")
+	}
+	// First, get all of the chips on the system.
+	var chips []*GPIOChip
+	var chip *GPIOChip
+	for _, item := range items {
+		chip, err = newGPIOChip(item)
+		if err == nil {
+			chips = append(chips, chip)
+		} else {
+			log.Println("gpioioctl.driverGPIO.Init() Error", err)
 		}
-		Chips = make([]*GPIOChip, 0)
-		for _, item := range items {
-			chip, err := newGPIOChip(item)
-			if err != nil {
-				log.Println("gpioioctl.driverGPIO.Init() Error", err)
-				return false, err
+	}
+	// Now, sort the chips so that those labeled with pinctrl- ( a Pi kernel standard)
+	// come first. Otherwise, sort them by label. This _should_ protect us from any
+	// random changes in chip naming/ordering.
+	sort.Slice(chips, func(i, j int) bool {
+		I := chips[i]
+		J := chips[j]
+		if strings.HasPrefix(I.Label(), "pinctrl-") {
+			if strings.HasPrefix(J.Label(), "pinctrl-") {
+				return I.Label() < J.Label()
 			}
+			return true
+		} else if strings.HasPrefix(J.Label(), "pinctrl-") {
+			return false
+		}
+		return I.Label() < J.Label()
+	})
+
+	mName := make(map[string]struct{})
+	// Get a list of already registered GPIO Line names.
+	registeredPins := make(map[string]struct{})
+	for _, pin := range gpioreg.All() {
+		registeredPins[pin.Name()] = struct{}{}
+	}
+
+	// Now, iterate over the chips we found and add their lines to conn/gpio/gpioreg
+	for _, chip := range chips {
+		// On a pi, gpiochip0 is also symlinked to gpiochip4, checking the map
+		// ensures we don't duplicate the chip.
+		if _, found := mName[chip.Name()]; !found {
 			Chips = append(Chips, chip)
+			mName[chip.Name()] = struct{}{}
+			// Now, iterate over the lines on this chip.
 			for _, line := range chip.lines {
+				// If the line has some sort of reasonable name...
 				if len(line.name) > 0 && line.name != "_" && line.name != "-" {
+					// See if the name is already registered. On the Pi5, there are at
+					// least two chips that export "2712_WAKE" as the line name.
+					if _, ok := registeredPins[line.Name()]; ok {
+						// This is a duplicate name. Prefix the line name with the
+						// chip name.
+						line.name = chip.Name() + "-" + line.Name()
+						if _, found := registeredPins[line.Name()]; found {
+							// It's still not unique. Skip it.
+							continue
+						}
+					}
+					registeredPins[line.Name()] = struct{}{}
 					if err = gpioreg.Register(line); err != nil {
 						log.Println("chip", chip.Name(), " gpioreg.Register(line) ", line, " returned ", err)
 					}
 				}
 			}
 		}
 	}
-	return true, nil
+	return len(Chips) > 0, nil
 }
 
 var drvGPIO driverGPIO