Setting lsbfirst = True breaks access to GPIO on Raspberry Pi 1/2 with 3.18 kernel

[tdicola]

Apologies if this isn't the right repo but I can't open issues on gadgetoid's fork here (https://github.com/gadgetoid/py-spidev) that appears to be the new master for pyspidev on PyPi (according to http://www.raspberrypi.org/forums/viewtopic.php?f=32&t=98070&p=720659 and https://github.com/doceme/py-spidev/issues/15).

I'm using spidev 3.0 from PyPi and see that opening a spidev device and setting lsbfirst = True causes future access to GPIO (through the RPi.GPIO library) to fail. Here's the simplest example to repro the problem:

import time

import RPi.GPIO as GPIO
import spidev

spi = spidev.SpiDev()
spi.open(0, 0)
spi.lsbfirst = True

# Initialize GPIO23 as an output.
GPIO.setwarnings(False)
GPIO.setmode(GPIO.BCM)
GPIO.setup(23, GPIO.OUT)

# Set GPIO23 high and then low.
print 'High'
GPIO.output(23, GPIO.HIGH)
time.sleep(1)

print 'Low'
GPIO.output(23, GPIO.LOW)
time.sleep(1)

print 'Done!'

That should open /dev/spidev0.0 with LSB first mode and then blink GPIO23 high and low for a second. However when you run it immediately fails when trying to setup the GPIO:

Traceback (most recent call last):
  File "test2.py", line 13, in <module>
    GPIO.setup(23, GPIO.OUT)
IOError: [Errno 22] Invalid argument

If you comment out the line spi.lsbfirst = True everything works as expected and the GPIO23 line goes high and low without any errors.

This repros for me on a Raspberry Pi 1 or 2 using the latest February 16th, 2015, image which uses kernel 3.18.7.

I haven't really dug in to investigate further but it definitely seems like something is up with the post 3.15 kernel and setting lsbfirst = True.

[Gadgetoid]

I'm a little late to the party, but I've just duplicated your test script and am getting exactly the same results.

Suffice to say, I'm a little baffled- BCM 23 has nothing to do with SPI. However, lsbfirst is implemented with setters/getters so it has side-effects.

Here's the setter:

static int
SpiDev_set_lsbfirst(SpiDevObject *self, PyObject *val, void *closure)
{
    uint8_t tmp;

    if (val == NULL) {
        PyErr_SetString(PyExc_TypeError,
            "Cannot delete attribute");
        return -1;
    }
    else if (!PyBool_Check(val)) {
        PyErr_SetString(PyExc_TypeError,
            "The lsbfirst attribute must be boolean");
        return -1;
    }

    if (val == Py_True)
        tmp = self->mode | SPI_LSB_FIRST;
    else
        tmp = self->mode & ~SPI_LSB_FIRST;

    __spidev_set_mode(self->fd, tmp);

    self->mode = tmp;
    return 0;
}

which calls:

static int __spidev_set_mode( int fd, __u8 mode) {
    __u8 test;
    if (ioctl(fd, SPI_IOC_WR_MODE, &mode) == -1) {
        PyErr_SetFromErrno(PyExc_IOError);
        return -1;
    }
    if (ioctl(fd, SPI_IOC_RD_MODE, &test) == -1) {
        PyErr_SetFromErrno(PyExc_IOError);
        return -1;
    }
    if (test != mode) {
        return -1;
    }
    return 0;
}

But why the SPI_IOC_WR_MODE or SPI_IOC_RD_MODE ioctls called on the SPI device should have any effect on GPIO is beyond me. This might be an issue to raise against RPi.GPIO too.

[Gadgetoid]

Okay, this is a good one!

Here's a puzzler for you. Modify your original code example to:

import time

import RPi.GPIO as GPIO
import spidev

spi = spidev.SpiDev()
spi.open(0, 0)
spi.lsbfirst = True

# Initialize GPIO23 as an output.
GPIO.setwarnings(False)
GPIO.setmode(GPIO.BCM)

try:
    GPIO.setup(23, GPIO.OUT)
except IOError:
    pass

GPIO.setup(23, GPIO.OUT)

# Set GPIO23 high and then low.
print 'High'
GPIO.output(23, GPIO.HIGH)
time.sleep(1)

print 'Low'
GPIO.output(23, GPIO.LOW)
time.sleep(1)

print 'Done!'

And see if you can figure out why that works :D

[Gadgetoid]

Welp! Down the rabbit hole I went. The issue is this;

1. lsbfirst = True triggers set_lsbfirst
2. This calls __spidev_set_mode
3. __spidev_set_mode calls ioctl which fails with [Errno 22] Invalid Argument and passes this to Python with PyErr_SetFromErrno
4. Back to set_lsbfirst which somehow exits without the previously set PyErr being raised
5. Now we have an error buried in Python which hasn't been raised...
6. Along comes RPi.GPIO... for some reason GPIO.setwarnings and GPIO.setmode ignore this buried error
7. But now GPIO.setup(23, GPIO.OUT) digs up the error set by __spidev_set_mode and raises it... WHY!? I don't even...

Note about point 6. setwarnings bails with Py_RETURN_NONE so why it takes until setup for the error to be raised is anyone's guess. This stuff is bonkers!

I've rewritten a substantial portion of the code to check the return value of __spidev_set_mode and use Py_RETURN_NONE which seems to trigger error handling- whereas return NULL or return 0 do not.

This is odd, because according to https://docs.python.org/3/extending/extending.html#intermezzo-errors-and-exceptions return NULL should cause Python to choke and raise the exception, whereas Py_RETURN_NONE should do.. ?

With the setter and getter functions defined as static PyObject *. return NULL not only doesn't raise an exception set by PyErr_SetFromErrno but leaves that error latent in the interpreter:

Exceptions are stored in a static global variable inside the interpreter

For the next method that uses Py_RETURN_NONE to inadvertently raise.

D'oh =/

I've pushed these "fixes" (I'm not 100% sure what I'm doing is correct) into a new branch on my fork for some testing and peer overview before I submit a PR:

https://github.com/Gadgetoid/py-spidev/commit/53b96957d406cff2566803767a7e9a3b2b895719

[Gadgetoid]

After bashing my head against this problem for some time, I determined I'd made a mistake in changing the setters from static int to PyObject * in my original tweaks.

Despite the overwhelming majority of documentation pointing toward returning Py_NONE ( or a value ) upon success, and NULL upon error and using the PyObject type for functions that communicate with Python, a setter is a special little snowflake which, presumably, is called by some internal glue logic and must return 0 upon success, and -1 upon error.

This is alluded to only in its definition: typedef int (*setter)(PyObject *, PyObject *, void *); and I couldn't find any good examples or documentation for setters anywhere- it must be too generic a search term, and my search-fu has failed me.

Anyway, with my latest commit I have reverted my fudge and now only check __spidev_set_mode for a -1 return code, and bubble this up accordingly:

    if (__spidev_set_mode(self->fd, tmp) == -1){
        return -1;;
    }

As simple as that.

https://github.com/Gadgetoid/py-spidev/tree/ErrorHandling

[Gadgetoid]

And, finally, the reason why this throws an error at all is that lsbfirst = True may be unsupported on the Raspberry Pi. If you need to reverse your bit order, you'll have to do it in software.