adafruit / Adafruit_Blinka

Add CircuitPython hardware API and libraries to MicroPython & CPython devices
https://learn.adafruit.com/circuitpython-on-raspberrypi-linux
MIT License
448 stars 335 forks source link

SPI regression on BeagleBone (AM3358) #104

Closed pdp7 closed 5 years ago

pdp7 commented 5 years ago

@s-light is adding support for PocketBeagle (#100) but could not get SPI to work. The issue is not specific to the PocketBeagle. SPI support is also broken on BeagleBone Black. Both boards have the TI AM3358 processor.

PR #46 added SPI support for the BeagleBone Black (which has a TI AM3358 processor). At that time, I was able to successfully test the BME280 in SPI mode.

However, it now encounters error OSError: [Errno 22] Invalid argument in microcontroller/generic_linux/spi.py:

From Adafruit_Python_BME280, I am testing with bme280_simpletest.py :

import time

import board
import busio
import adafruit_bme280
import digitalio
from adafruit_bus_device import i2c_device, spi_device

# create library object using our Bus SPI port
spi = busio.SPI(board.SCK, board.MOSI, board.MISO)
bme_cs = digitalio.DigitalInOut(board.P9_17)
bme280 = adafruit_bme280.Adafruit_BME280_SPI(spi, bme_cs)

# change this to match the location's pressure (hPa) at sea level
bme280.sea_level_pressure = 1013.25

while True:
    print("\nTemperature: %0.1f C" % bme280.temperature)
    print("Humidity: %0.1f %%" % bme280.humidity)
    print("Pressure: %0.1f hPa" % bme280.pressure)
    print("Altitude = %0.2f meters" % bme280.altitude)
    time.sleep(2)

which results in:

debian@beaglebone:~/Adafruit_CircuitPython_BME280$ sudo python3 examples/bme280_simpletest.py 
Traceback (most recent call last):
  File "examples/bme280_simpletest.py", line 17, in <module>
    bme280 = adafruit_bme280.Adafruit_BME280_SPI(spi, bme_cs)
  File "/usr/local/lib/python3.5/dist-packages/adafruit_bme280.py", line 469, in __init__
    super().__init__()
  File "/usr/local/lib/python3.5/dist-packages/adafruit_bme280.py", line 120, in __init__
    chip_id = self._read_byte(_BME280_REGISTER_CHIPID)
  File "/usr/local/lib/python3.5/dist-packages/adafruit_bme280.py", line 428, in _read_byte
    return self._read_register(register, 1)[0]
  File "/usr/local/lib/python3.5/dist-packages/adafruit_bme280.py", line 474, in _read_register
    spi.write(bytearray([register]))  #pylint: disable=no-member
  File "/usr/local/lib/python3.5/dist-packages/Adafruit_Blinka-1.2.9.dev23+g1d32b1f.d20190410-py3.5.egg/busio.py", line 129, in write
    return self._spi.write(buf, start, end)
  File "/usr/local/lib/python3.5/dist-packages/Adafruit_Blinka-1.2.9.dev23+g1d32b1f.d20190410-py3.5.egg/adafruit_blinka/microcontroller/generic_linux/spi.py", line 44, in write
    self._spi.no_cs = True  # this doesn't work but try anyways
OSError: [Errno 22] Invalid argument
pdp7 commented 5 years ago

I have added debug output to Adafruit_Blinka, Adafruit_CircuitPython_BusDevice, and py_spidev:

debian@beaglebone:~/Adafruit_CircuitPython_BME280$ sudo strace -f -o /tmp/strace.out python3 examples/bme280_simpletest.py 
[sudo] password for debian: 
spidev_module: SpiDev_init()
Adafruit_CircuitPython_BusDevice.__init__
Adafruit_CircuitPython_BusDevice.__init__: set self.chip_select = chip_select: P9_17
Adafruit_CircuitPython_BusDevice.__init__: set self.chip_select.value = True
Adafruit_CircuitPython_BusDevice.__init__: return
Adafruit_CircuitPython_BusDevice.__enter__
Adafruit_CircuitPython_BusDevice.__enter__: self.spi.configure()
Adafruit_CircuitPython_BusDevice.__enter__: set self.chip_select.value = False
Adafruit_CircuitPython_BusDevice.__enter__: return self.spi
generic_linux/spi.py: call self._spi.open(self._port, 0)
spidev_module: SpiDev_open(): enter
spidev_module: SpiDev_open(): call open(path, O_RDWR, 0)
spidev_module: SpiDev_open(): ioctl(self->fd, SPI_IOC_RD_MODE, &tmp8)
spidev_module: SpiDev_open(): ioctl(self->fd, SPI_IOC_RD_BITS_PER_WORD, &tmp8)
spidev_module: SpiDev_open(): ioctl(self->fd, SPI_IOC_RD_MAX_SPEED_HZ, &tmp32)
spidev_module: SpiDev_open(): return Py_None
generic_linux/spi.py: return from self._spi.open(self._port, 0)
generic_linux/spi.py: try: self._spi.no_cs
spidev_module: SpiDev_set_no_cs(): self->mode=0x1
spidev_module: SpiDev_set_no_cs(): SPI_NO_CS=0x40
spidev_module: SpiDev_set_no_cs(): set SPI_NO_CS bit
spidev_module: SpiDev_set_no_cs(): tmp=0x41
spidev_module: SpiDev_set_no_cs(): call __spidev_set_mode()
spidev_module: __spidev_set_mode(): fd=4
spidev_module: __spidev_set_mode(): mode=0x41
spidev_module: __spidev_set_mode(): call ioctl(fd, SPI_IOC_WR_MODE=0x40016b01, &mode)
spidev_module: __spidev_set_mode(): return -1
spidev_module: SpiDev_set_no_cs(): ret=-1
Traceback (most recent call last):
  File "examples/bme280_simpletest.py", line 17, in <module>
    bme280 = adafruit_bme280.Adafruit_BME280_SPI(spi, bme_cs)
  File "/usr/local/lib/python3.5/dist-packages/adafruit_bme280.py", line 469, in __init__
    super().__init__()
  File "/usr/local/lib/python3.5/dist-packages/adafruit_bme280.py", line 120, in __init__
    chip_id = self._read_byte(_BME280_REGISTER_CHIPID)
  File "/usr/local/lib/python3.5/dist-packages/adafruit_bme280.py", line 428, in _read_byte
    return self._read_register(register, 1)[0]
  File "/usr/local/lib/python3.5/dist-packages/adafruit_bme280.py", line 474, in _read_register
    spi.write(bytearray([register]))  #pylint: disable=no-member
  File "/usr/local/lib/python3.5/dist-packages/Adafruit_Blinka-1.2.9.dev23+g1d32b1f.d20190410-py3.5.egg/busio.py", line 129, in write
    return self._spi.write(buf, start, end)
  File "/usr/local/lib/python3.5/dist-packages/Adafruit_Blinka-1.2.9.dev23+g1d32b1f.d20190410-py3.5.egg/adafruit_blinka/microcontroller/generic_linux/spi.py", line 44, in write
    self._spi.no_cs = True  # this doesn't work but try anyways
OSError: [Errno 22] Invalid argument

The strace output shows that ioctl() call fails:

ioctl(4, SPI_IOC_WR_MODE, 0xbe81bb97) = -1 EINVAL (Invalid argument)
pdp7 commented 5 years ago

How does Adafruit_CircuitPython_BME280 expect chip select (CS) to be handled? https://github.com/adafruit/Adafruit_CircuitPython_BME280/blob/master/adafruit_bme280.py#L468

class Adafruit_BME280_SPI(Adafruit_BME280):
    """Driver for BME280 connected over SPI"""
    def __init__(self, spi, cs, baudrate=100000):
        import adafruit_bus_device.spi_device as spi_device
        self._spi = spi_device.SPIDevice(spi, cs, baudrate=baudrate)
        super().__init__()

adafruit_bus_device handles setting the CS pin: https://github.com/adafruit/Adafruit_CircuitPython_BusDevice/blob/master/adafruit_bus_device/spi_device.py#L85

The SPI implementation in Adafruit_Blinka does NOT handle CS https://github.com/adafruit/Adafruit_Blinka/blob/master/src/adafruit_blinka/microcontroller/generic_linux/spi.py

    def write(self, buf, start=0, end=None):
        if not buf:
            return
        if end is None:
            end = len(buf)
        try:
            self._spi.open(self._port, 0)
            try:
              self._spi.no_cs = True  # this doesn't work but try anyways
            except AttributeError:
              pass
            self._spi.max_speed_hz = self.baudrate
            self._spi.mode = self.mode
            self._spi.bits_per_word = self.bits
            self._spi.writebytes([x for x in buf[start:end]])
            self._spi.close()
        except FileNotFoundError as not_found:
            print("Could not open SPI device - check if SPI is enabled in kernel!")
            raise
pdp7 commented 5 years ago

This line in adafruit_blinka/microcontroller/generic_linux/spi.py:

self._spi.no_cs = True  # this doesn't work but try anyways

causes the error:

  File "/usr/local/lib/python3.5/dist-packages/Adafruit_Blinka-1.2.9.dev23+g1d32b1f.d20190410-py3.5.egg/adafruit_blinka/microcontroller/generic_linux/spi.py", line 44, in write
    self._spi.no_cs = True  # this doesn't work but try anyways
OSError: [Errno 22] Invalid argument

debug output:

generic_linux/spi.py: return from self._spi.open(self._port, 0)
generic_linux/spi.py: try: self._spi.no_cs
spidev_module: SpiDev_set_no_cs(): self->mode=0x1
spidev_module: SpiDev_set_no_cs(): SPI_NO_CS=0x40
spidev_module: SpiDev_set_no_cs(): set SPI_NO_CS bit
spidev_module: SpiDev_set_no_cs(): tmp=0x41
spidev_module: SpiDev_set_no_cs(): call __spidev_set_mode()
spidev_module: __spidev_set_mode(): fd=4
spidev_module: __spidev_set_mode(): mode=0x41
spidev_module: __spidev_set_mode(): call ioctl(fd, SPI_IOC_WR_MODE=0x40016b01, &mode)
spidev_module: __spidev_set_mode(): return -1
spidev_module: SpiDev_set_no_cs(): ret=-1

What is happening in self._spi.no_cs = True?

It invokes this code in spidev_module.c:

    {"no_cs", (getter)SpiDev_get_no_cs, (setter)SpiDev_set_no_cs,
            "disable chip select\n"},

which calls:

SpiDev_set_no_cs(SpiDevObject *self, PyObject *val, void *closure)

which calls:

static int __spidev_set_mode( int fd, __u8 mode)

this is where the ioctl() that fails is called:

    if (ioctl(fd, SPI_IOC_WR_MODE, &mode) == -1) {

This ioctl() is trying to set the SPI mode with the NO_CS bit set.

pdp7 commented 5 years ago

How does the Linux kernel handle SPI_NO_CS?

Here are the references to SPI_NO_CS in Linux kernel source code: https://elixir.bootlin.com/linux/v4.19/ident/SPI_NO_CS

It is defined in: https://elixir.bootlin.com/linux/v4.19/source/include/linux/spi/spi.h#L160

#define SPI_NO_CS   0x40            /* 1 dev/bus, no chipselect */

The Raspberry Pi (BCM2835 processor) seems to support SPI_NO_CS: https://elixir.bootlin.com/linux/v4.19/source/drivers/spi/spi-bcm2835.c#L79

#define BCM2835_SPI_MODE_BITS   (SPI_CPOL | SPI_CPHA | SPI_CS_HIGH \
                | SPI_NO_CS | SPI_3WIRE)
pdp7 commented 5 years ago

SPI driver for BeagleBone does not support for SPI_NO_CS The BeagleBone has TI Sitara AM3358 SoC. The support in the Linux kernel is often referred to as OMAP given the previous TI SoC family.

SPI on the AM3358 is called McSPI and is implemented in: https://elixir.bootlin.com/linux/v4.14/source/drivers/spi/spi-omap2-mcspi.c

The allowed mode bits are: https://elixir.bootlin.com/linux/v4.14/source/drivers/spi/spi-omap2-mcspi.c#L1351

master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;

The allowed mode bits for the SPI controller (spi->controller->mode_bits) are checked in: https://elixir.bootlin.com/linux/v4.19/source/drivers/spi/spi.c#L2787

int spi_setup(struct spi_device *spi)
{
       /*snip*/
    bad_bits = spi->mode & ~spi->controller->mode_bits;
    ugly_bits = bad_bits &
            (SPI_TX_DUAL | SPI_TX_QUAD | SPI_RX_DUAL | SPI_RX_QUAD);
    if (ugly_bits) {
        dev_warn(&spi->dev,
             "setup: ignoring unsupported mode bits %x\n",
             ugly_bits);
        spi->mode &= ~ugly_bits;
        bad_bits &= ~ugly_bits;
    }
    if (bad_bits) {
        dev_err(&spi->dev, "setup: unsupported mode bits %x\n",
            bad_bits);
        return -EINVAL;
    }

In the case of the BeagleBone, the omap2-mcspi driver does not support SPI_NO_CS so spi_setup() returns -EINVAL which translates to -22. This is why Python has the exception: OSError: [Errno 22] Invalid argument

I recompiled the kernel on the BeagleBone with some print statements which confirms that setting SPI_NO_CS (0x40) causes bad_bits to be non-zero and spi_setup() to return -EINVAL:

[  129.472021] DEBUG: spidev.c: SPI_IOC_WR_MODE
[  129.476873] DEBUG: spidev.c: SPI_IOC_WR_MODE: save=0x1
[  129.484114] DEBUG: spidev.c: SPI_IOC_WR_MODE: tmp=0x41
[  129.490318] DEBUG: spidev.c: SPI_IOC_WR_MODE: SPI_MODE_MASK=0xfff
[  129.497454] DEBUG: spidev.c: SPI_IOC_WR_MODE: ~SPI_MODE_MASK=0xfffff000
[  129.505120] DEBUG: spidev.c: SPI_IOC_WR_MODE: (tmp&~SPI_MODE_MASK)=0x0
[  129.512642] DEBUG: spi.c: spi_setup(): spi->mode=0x41
[  129.518549] DEBUG: spi.c: spi_setup(): spi->controller->mode_bits=0x7
[  129.526020] DEBUG: spi.c: spi_setup(): ~spi->controller->mode_bits=0xfffffff8
[  129.534085] DEBUG: spi.c: spi_setup(): bad_bits=0x40
[  129.539908] DEBUG: spi.c: spi_setup(): ugly_bits=0x0
[  129.545715] DEBUG: spi.c: spi_setup(): unsupported mode bits, return -EINVAL
ladyada commented 5 years ago

hihi yeah so there are situations where you want to use the SPI bus w/o a CS pin (dotstars for example) so what we do is we have one 'dummy' CS pin that we always toggle but never connect to. Then we use any plain gpio pin for CS. this lets us have any CS pin, and any config of SPI, at the loss of a GPIO

pdp7 commented 5 years ago

@ladyada thanks, yes, I can see why it is useful to decouple CS from the hardware SPI peripheral.

As a test, I just commented out all instances of self._spi.no_cs = True in src/adafruit_blinka/microcontroller/generic_linux/spi.py and the BME280 example runs OK on the BeagleBone:

debian@beaglebone:~/Adafruit_Blinka$ sudo python3 ~/Adafruit_CircuitPython_BME280/examples/bme280_simpletest.py 2>/dev/null |grep -v generic_linux |grep -v Adafruit_CircuitPython

Temperature: 23.7 C
Humidity: 39.7 %
Pressure: 1020.0 hPa
Altitude = -56.05 meters

This is because spidev no longer tries to set the SPI_NO_CS mode bit which is unsupported by the Linux driver (omap2_mcspi) for BeagleBone's SPI controller (AM3358 McSPI).

I think a work around maybe to add logic to skip setting no CS if AM3358 based board. I'll try that out and followup.

ladyada commented 5 years ago

please do a try-except

ladyada commented 5 years ago

fyi dont commit directly to this repo, have PRs only that i'll review. thanks :)

pdp7 commented 5 years ago

OK, no problem. I will create PR when I have a working code.

For src/adafruit_blinka/microcontroller/generic_linux/spi.py, I am thinking of replacing instances of:

            try:
              self._spi.no_cs = True  # this doesn't work but try anyways
            except AttributeError:
              pass

with a call to to a new method set_no_cs():

    def set_no_cs(self):
        print("generic_linux/spi.py: set_no_cs(): enter")
        if detector.chip.AM33XX:
            print("generic_linux/spi.py: set_no_cs(): detected AM33XX, SKIP setting no_cs")
        else:
            try:
                print("generic_linux/spi.py: set_no_cs(): self._spi.no_cs = True")
                self._spi.no_cs = True  # this doesn't work but try anyways
            except AttributeError:
                pass

I just tested this out and the BME280 works OK on the BeagleBone. However, I think that calling detector.chip.AM33XX in set_no_cs() may be inefficient, so it might need to move to init().

ladyada commented 5 years ago

why not add an except for OSError?

pdp7 commented 5 years ago

OSError can occur for any failed ioctl(). I think a better approach is avoid trying to set SPI_NO_CS mode bit when the chip is AM3358 (as this will always fail due to the Linux driver not supporting it).

ladyada commented 5 years ago

ok got it

pdp7 commented 5 years ago

Tested OK with BME280 on PocketBeagle.

Make sure pins are configured for SPI0 plus P1.6 as GPIO for CS:

config-pin p1.6  gpio     # CS
config-pin p1.8  spi_sclk # SPI0 CLK
config-pin p1.10 spi     # SPI0 MISO
config-pin p1.12 spi     # SPI0 MOSI

Adafruit_CircuitPython_BME280/examples/bme280_simpletest.py:

import board
import busio
import adafruit_bme280
import digitalio

spi = busio.SPI(board.SCK, board.MOSI, board.MISO)
bme_cs = digitalio.DigitalInOut(board.P1_6)
bme280 = adafruit_bme280.Adafruit_BME280_SPI(spi, bme_cs)

print("\nTemperature: %0.1f C" % bme280.temperature)
debian@beaglebone:~$ python3 bme280_simpletest.py

Temperature: 22.0 C

system version info:

debian@beaglebone:~$ cat /etc/dogtag 
BeagleBoard.org Debian Image 2019-03-03
debian@beaglebone:~$ uname -r
4.14.78-bone17
debian@beaglebone:~$ cat /etc/debian_version 
9.8
debian@beaglebone:~$ ls -ltar /dev/spidev*
crw-rw---- 1 root spi 153, 0 Apr 12 17:41 /dev/spidev0.0
crw-rw---- 1 root spi 153, 1 Apr 12 17:42 /dev/spidev1.0
crw-rw---- 1 root spi 153, 2 Apr 12 17:42 /dev/spidev1.1