From 63e6b8c416d2468e1845df832597deaf3a2ff9cb Mon Sep 17 00:00:00 2001 From: gsexton Date: Sun, 24 Nov 2024 16:31:14 -0700 Subject: [PATCH] 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. --- gpioioctl/basic_test.go | 25 ------------- gpioioctl/dummy.go | 1 - gpioioctl/gpio.go | 82 ++++++++++++++++++++++++++++++++++------- 3 files changed, 68 insertions(+), 40 deletions(-) diff --git a/gpioioctl/basic_test.go b/gpioioctl/basic_test.go index 875ca29..90585be 100644 --- a/gpioioctl/basic_test.go +++ b/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) - } } } diff --git a/gpioioctl/dummy.go b/gpioioctl/dummy.go index 7f3c3ea..0b86a5f 100644 --- a/gpioioctl/dummy.go +++ b/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() diff --git a/gpioioctl/gpio.go b/gpioioctl/gpio.go index 3407e6f..fd2fb0d 100644 --- a/gpioioctl/gpio.go +++ b/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,24 +595,74 @@ 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) } @@ -616,7 +670,7 @@ func (d *driverGPIO) Init() (bool, error) { } } } - return true, nil + return len(Chips) > 0, nil } var drvGPIO driverGPIO