Lines.request fail at runtime

[slyshykO]

I've tried to run readall.rs example on orange pi zero, but app panicked with the message "panicked at 'index out of bounds: the len is 64 but the index is 64', .\gpio-cdev-0.2.0\src\lib.rs:766:13"
The problem is that Lines struct in my case contains more than 64(GPIOHANDLES_MAX) lines.

[fpagliughi]

Wow. Interesting. The handle max for GPIO lines is straight out of the kernel ABI:
https://github.com/torvalds/linux/blob/f7c3bf8fa7e5a8e45f4a8e82be6466157854b59b/include/uapi/linux/gpio.h#L57

I had assumed that to mean that 64 was the most lines a GPIO chip was supposed to export. I guess that's not the case! I have a NanoPi R1 sitting on my desk at the moment reporting:

# gpiodetect 
gpiochip0 [1c20800.pinctrl] (224 lines)
gpiochip1 [1f02c00.pinctrl] (32 lines)

I'm a little overloaded at the moment, but will try to get a fix in ASAP.

[posborne]

@slyshykO @fpagliughi It's not good that we panic but I do expect the kernel to return -EINVAL on handle creation for anything > GPIOHANDLES_MAX lines. See https://elixir.bootlin.com/linux/v4.8/source/drivers/gpio/gpiolib.c#L419

You'll probably need to create more than one handle, partitioning the handles per chip into chunks of at most 64 lines.

[slyshykO]

@posborne
I am not familiar with code, but as I can see request panicked before calling kernel functions.

[fpagliughi]

OK. I was able to recreate the bug, but I don't have time to get a fix in and tested this morning. Give me a day, or so.

It appears that the Lines::request() function is preparing the request to send to the kernel, but trips over an array-out-of-bounds error.

I'm thinking the Lines::new() function should return an error if you give it a bad set of offsets, like:

if offsets.len() == 0 || offsets.len() > ffi::GPIOHANDLES_MAX {
    bail!(nix::Error::Sys(nix::errno::Errno::EINVAL));
}

[posborne]

I think returning EINVAL is the right behavior here. This could either be done by checking in userspace or by ensuring that we cap the iteration and pass the original desired n to the kernel. Since we already need to encode the GPIOHANDLES_MAX in our struct I think doing an explicit check in userspace, at least for the max case makes sense. The zero case could go either way but I think what you suggest is fine.

[posborne]

We should also consider updating the read_all example to be able to work with >64 lines/chip.

[fpagliughi]

I believe the Rust userspace code panicked when filling the struct to send to the kernel. So better to short circuit and just return the obvious error, I think.

I didn’t think of testing for the zero case. I just noticed the kernel code was doing that so I copied it.

I’m on the readall example. Should have a PR in a few hours...

[fpagliughi]

OK. A PR is up. I should note that I had some difficulty finding a board with a gpiochip that had all of its lines free. If not, the readall fails with EBUSY (as expected). But it shows the limited usefulness of this example, as is, but I guess it's still useful to show how the code can work. It was just a coincidence that the board I was using last year had a free chip (a UDOO Neo, I think)

[fpagliughi]

On a side note... When I run readall on gpiochip0 of a RaspberryPi 2, the board turns off. I confirmed this using the gpiod tools, as follows:

$ gpiodetect 
gpiochip0 [pinctrl-bcm2835] (54 lines)
$ gpioget /dev/gpiochip0 {0..53}
<board goes dead>

So I know it's not a bug in our library.

It seems to be on an attempt to access Pin 31. Does anyone know what/how/why this is?!?

[fpagliughi]

I assume that this can be closed.

[nastevens]

Closing as I'm pretty sure everything is done here - please feel free to reopen if I am mistaken.